libmount: cleanup locking in table update code

* use reference counting for libmnt_lock

* remove lock use from mnt_update_already_done(), it's read-only
  operation and utab update is atomic (based on rename(2))

* keep the lock instance mandatory for all low-level update_*
  functions. The lock is always initialized, so all the 'if(lc)'
  are unnecessary.

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/context.c b/libmount/src/context.c
index ed05e7ca3925939b33869ab8bd83beeee6f48b8f..62cd34948186fe5ace71d6fff6445c7c8ecc9037 100644 (file)
--- a/libmount/src/context.c
+++ b/libmount/src/context.c
@@ -2207,7 +2207,7 @@ int mnt_context_update_tabs(struct libmnt_context *cxt)
            && mnt_context_get_helper_status(cxt) == 0
            && mnt_context_utab_writable(cxt)) {
 
-               if (mnt_update_already_done(cxt->update, cxt->lock)) {
+               if (mnt_update_already_done(cxt->update)) {
                        DBG(CXT, ul_debugobj(cxt, "don't update: error evaluate or already updated"));
                        goto emit;
                }

diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h
index 704e940827fd2042fe21722bc30abeb690265b0e..08d08caa14a4a76e95478d7e4e5380dbcf5d9a92 100644 (file)
--- a/libmount/src/mountP.h
+++ b/libmount/src/mountP.h
@@ -665,8 +665,7 @@ extern struct libmnt_optlist *mnt_context_get_optlist(struct libmnt_context *cxt
 /* tab_update.c */
 extern int mnt_update_emit_event(struct libmnt_update *upd);
 extern int mnt_update_set_filename(struct libmnt_update *upd, const char *filename);
-extern int mnt_update_already_done(struct libmnt_update *upd,
-                                  struct libmnt_lock *lc);
+extern int mnt_update_already_done(struct libmnt_update *upd);
 
 #if __linux__
 /* btrfs.c */

diff --git a/libmount/src/tab_update.c b/libmount/src/tab_update.c
index 6a2c77b39c592b266d81c778c380193a7aa12c92..753e22c083c3a9d0d0de535e8e14fb6c1f7e6d91 100644 (file)
--- a/libmount/src/tab_update.c
+++ b/libmount/src/tab_update.c
@@ -40,6 +40,7 @@ struct libmnt_update {
                        missing_options : 1;
 
        struct libmnt_table *mountinfo;
+       struct libmnt_lock  *lock;
 };
 
 static int set_fs_root(struct libmnt_update *upd, struct libmnt_fs *fs, unsigned long mountflags);
@@ -75,6 +76,7 @@ void mnt_free_update(struct libmnt_update *upd)
 
        DBG(UPDATE, ul_debugobj(upd, "free"));
 
+       mnt_unref_lock(upd->lock);
        mnt_unref_fs(upd->fs);
        mnt_unref_table(upd->mountinfo);
        free(upd->target);
@@ -668,43 +670,42 @@ static int add_file_entry(struct libmnt_table *tb, struct libmnt_update *upd)
        return update_table(upd, tb);
 }
 
-static int update_add_entry(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_add_entry(struct libmnt_update *upd)
 {
        struct libmnt_table *tb;
        int rc = 0;
 
        assert(upd);
        assert(upd->fs);
+       assert(upd->lock);
 
        DBG(UPDATE, ul_debugobj(upd, "%s: add entry", upd->filename));
 
-       if (lc)
-               rc = mnt_lock_file(lc);
+       rc = mnt_lock_file(upd->lock);
        if (rc)
                return -MNT_ERR_LOCK;
 
        tb = __mnt_new_table_from_file(upd->filename, MNT_FMT_UTAB, 1);
        if (tb)
                rc = add_file_entry(tb, upd);
-       if (lc)
-               mnt_unlock_file(lc);
 
+       mnt_unlock_file(upd->lock);
        mnt_unref_table(tb);
        return rc;
 }
 
-static int update_remove_entry(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_remove_entry(struct libmnt_update *upd)
 {
        struct libmnt_table *tb;
        int rc = 0;
 
        assert(upd);
        assert(upd->target);
+       assert(upd->lock);
 
        DBG(UPDATE, ul_debugobj(upd, "%s: remove entry", upd->filename));
 
-       if (lc)
-               rc = mnt_lock_file(lc);
+       rc = mnt_lock_file(upd->lock);
        if (rc)
                return -MNT_ERR_LOCK;
 
@@ -716,23 +717,22 @@ static int update_remove_entry(struct libmnt_update *upd, struct libmnt_lock *lc
                        rc = update_table(upd, tb);
                }
        }
-       if (lc)
-               mnt_unlock_file(lc);
 
+       mnt_unlock_file(upd->lock);
        mnt_unref_table(tb);
        return rc;
 }
 
-static int update_modify_target(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_modify_target(struct libmnt_update *upd)
 {
        struct libmnt_table *tb = NULL;
        int rc = 0;
 
        assert(upd);
+       assert(upd->lock);
        DBG(UPDATE, ul_debugobj(upd, "%s: modify target", upd->filename));
 
-       if (lc)
-               rc = mnt_lock_file(lc);
+       rc = mnt_lock_file(upd->lock);
        if (rc)
                return -MNT_ERR_LOCK;
 
@@ -781,15 +781,13 @@ static int update_modify_target(struct libmnt_update *upd, struct libmnt_lock *l
        }
 
 done:
-       if (lc)
-               mnt_unlock_file(lc);
-
+       mnt_unlock_file(upd->lock);
        mnt_unref_table(tb);
        return rc;
 }
 
 /* replaces option in the table entry by new options from @udp */
-static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_modify_options(struct libmnt_update *upd)
 {
        struct libmnt_table *tb = NULL;
        int rc = 0;
@@ -797,13 +795,13 @@ static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *
 
        assert(upd);
        assert(upd->fs);
+       assert(upd->lock);
 
        DBG(UPDATE, ul_debugobj(upd, "%s: modify options", upd->filename));
 
        fs = upd->fs;
 
-       if (lc)
-               rc = mnt_lock_file(lc);
+       rc = mnt_lock_file(upd->lock);
        if (rc)
                return -MNT_ERR_LOCK;
 
@@ -822,15 +820,13 @@ static int update_modify_options(struct libmnt_update *upd, struct libmnt_lock *
                        rc = add_file_entry(tb, upd);   /* not found, add new */
        }
 
-       if (lc)
-               mnt_unlock_file(lc);
-
+       mnt_unlock_file(upd->lock);
        mnt_unref_table(tb);
        return rc;
 }
 
 /* add user options missing in the table entry by new options from @udp */
-static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
+static int update_add_options(struct libmnt_update *upd)
 {
        struct libmnt_table *tb = NULL;
        int rc = 0;
@@ -838,6 +834,7 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
 
        assert(upd);
        assert(upd->fs);
+       assert(upd->lock);
 
        if (!upd->fs->user_optstr)
                return 0;
@@ -846,8 +843,7 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
 
        fs = upd->fs;
 
-       if (lc)
-               rc = mnt_lock_file(lc);
+       rc = mnt_lock_file(upd->lock);
        if (rc)
                return -MNT_ERR_LOCK;
 
@@ -873,14 +869,30 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
                        rc = add_file_entry(tb, upd);   /* not found, add new */
        }
 
-       if (lc)
-               mnt_unlock_file(lc);
-
+       mnt_unlock_file(upd->lock);
        mnt_unref_table(tb);
        return rc;
 
 }
 
+static int update_init_lock(struct libmnt_update *upd, struct libmnt_lock *lc)
+{
+       assert(upd);
+
+       if (lc) {
+               mnt_unref_lock(upd->lock);
+               mnt_ref_lock(lc);
+               upd->lock = lc;
+       } else if (!upd->lock) {
+               upd->lock = mnt_new_lock(upd->filename, 0);
+               if (!upd->lock)
+                       return -ENOMEM;
+               mnt_lock_block_signals(lc, TRUE);
+       }
+
+       return 0;
+}
+
 /**
  * mnt_update_table:
  * @upd: update
@@ -897,7 +909,6 @@ static int update_add_options(struct libmnt_update *upd, struct libmnt_lock *lc)
  */
 int mnt_update_table(struct libmnt_update *upd, struct libmnt_lock *lc)
 {
-       struct libmnt_lock *lc0 = lc;
        int rc = -EINVAL;
 
        if (!upd || !upd->filename)
@@ -909,35 +920,32 @@ int mnt_update_table(struct libmnt_update *upd, struct libmnt_lock *lc)
        if (upd->fs) {
                DBG(UPDATE, mnt_fs_print_debug(upd->fs, stderr));
        }
-       if (!lc) {
-               lc = mnt_new_lock(upd->filename, 0);
-               if (lc)
-                       mnt_lock_block_signals(lc, TRUE);
-       }
+
+       rc = update_init_lock(upd, lc);
+       if (rc)
+               goto done;
 
        if (!upd->fs && upd->target)
-               rc = update_remove_entry(upd, lc);      /* umount */
+               rc = update_remove_entry(upd);          /* umount */
        else if (upd->mountflags & MS_MOVE)
-               rc = update_modify_target(upd, lc);     /* move */
+               rc = update_modify_target(upd);         /* move */
        else if (upd->mountflags & MS_REMOUNT)
-               rc = update_modify_options(upd, lc);    /* remount */
+               rc = update_modify_options(upd);        /* remount */
        else if (upd->fs && upd->missing_options)
-               rc = update_add_options(upd, lc);       /* mount by externel helper */
+               rc = update_add_options(upd);           /* mount by externel helper */
        else if (upd->fs)
-               rc = update_add_entry(upd, lc);         /* mount */
+               rc = update_add_entry(upd);             /* mount */
 
        upd->ready = 1;
+done:
        DBG(UPDATE, ul_debugobj(upd, "%s: update tab: done [rc=%d]",
                                upd->filename, rc));
-       if (lc != lc0)
-                mnt_free_lock(lc);
        return rc;
 }
 
-int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
+int mnt_update_already_done(struct libmnt_update *upd)
 {
        struct libmnt_table *tb = NULL;
-       struct libmnt_lock *lc0 = lc;
        int rc = 0;
 
        if (!upd || !upd->filename || (!upd->fs && !upd->target))
@@ -945,22 +953,7 @@ int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
 
        DBG(UPDATE, ul_debugobj(upd, "%s: checking for previous update", upd->filename));
 
-       if (!lc) {
-               lc = mnt_new_lock(upd->filename, 0);
-               if (lc)
-                       mnt_lock_block_signals(lc, TRUE);
-       }
-       if (lc) {
-               rc = mnt_lock_file(lc);
-               if (rc) {
-                       rc = -MNT_ERR_LOCK;
-                       goto done;
-               }
-       }
-
        tb = __mnt_new_table_from_file(upd->filename, MNT_FMT_UTAB, 1);
-       if (lc)
-               mnt_unlock_file(lc);
        if (!tb)
                goto done;
 
@@ -997,8 +990,6 @@ int mnt_update_already_done(struct libmnt_update *upd, struct libmnt_lock *lc)
 
        mnt_unref_table(tb);
 done:
-       if (lc && lc != lc0)
-               mnt_free_lock(lc);
        DBG(UPDATE, ul_debugobj(upd, "%s: previous update check done [rc=%d]",
                                upd->filename, rc));
        return rc;