Fix use of orphan lock in commands

vgreduce, vgremove and vgcfgrestore were acquiring
the orphan lock in the midst of command processing
instead of at the start of the command.  (The orphan
lock moved to being acquired at the start of the
command back when pvcreate/vgcreate/vgextend were
reworked based on pvcreate_each_device.)

vgsplit also needed a small update to avoid reacquiring
a VG lock that it already held (for the new VG name).

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 641bf499e161e26884894f292e71ec9cddb65f1a..36a87b58f44050d7a57c830873ce37e46760a712 100644 (file)
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -693,6 +693,10 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
                             const char *vgid, uint32_t read_flags, uint32_t lockd_state);
 struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
                         const char *vgid, uint32_t read_flags, uint32_t lockd_state);
+struct volume_group *vg_read_orphans(struct cmd_context *cmd,
+                                             uint32_t warn_flags,
+                                             const char *orphan_vgname,
+                                             int *consistent);
 
 /* 
  * Test validity of a VG handle.

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 30b22c049a49b4b92c11dddab07ca678f64c3b75..9dc5627382d27acaea8d7cc92a9d180ad0c118c4 100644 (file)
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -600,14 +600,8 @@ int vg_remove(struct volume_group *vg)
 {
        int ret;
 
-       if (!lock_vol(vg->cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-               log_error("Can't get lock for orphan PVs");
-               return 0;
-       }
-
        ret = vg_remove_direct(vg);
 
-       unlock_vg(vg->cmd, vg, VG_ORPHANS);
        return ret;
 }
 
@@ -3280,7 +3274,7 @@ static int _vg_read_orphan_pv(struct lvmcache_info *info, void *baton)
 }
 
 /* Make orphan PVs look like a VG. */
-static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
+struct volume_group *vg_read_orphans(struct cmd_context *cmd,
                                             uint32_t warn_flags,
                                             const char *orphan_vgname,
                                             int *consistent)
@@ -3664,7 +3658,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
                                  "with pre-commit.");
                        return NULL;
                }
-               return _vg_read_orphans(cmd, warn_flags, vgname, consistent);
+               return vg_read_orphans(cmd, warn_flags, vgname, consistent);
        }
 
        uuid[0] = '\0';

diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index 400af8d4e570f1980776328f3aca33058d0f9752..d837a55cb6f42ca08e4000490e3bab67a9cb7198 100644 (file)
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -671,6 +671,7 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
 {
        struct pv_list *pvl;
        struct volume_group *orphan_vg = NULL;
+       int consistent;
        int r = 0;
        const char *name = pv_dev_name(pv);
 
@@ -679,6 +680,8 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
                return r;
        }
 
+       log_debug("vgreduce_single VG %s PV %s", vg->name, pv_dev_name(pv));
+
        if (pv_pe_alloc_count(pv)) {
                log_error("Physical volume \"%s\" still in use", name);
                return r;
@@ -690,11 +693,6 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
                return r;
        }
 
-       if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-               log_error("Can't get lock for orphan PVs");
-               return r;
-       }
-
        pvl = find_pv_in_vg(vg, name);
 
        if (!archive(vg))
@@ -716,8 +714,8 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
        vg->free_count -= pv_pe_count(pv) - pv_pe_alloc_count(pv);
        vg->extent_count -= pv_pe_count(pv);
 
-       orphan_vg = vg_read_for_update(cmd, vg->fid->fmt->orphan_vg_name,
-                                      NULL, 0, 0);
+       /* FIXME: we don't need to vg_read the orphan vg here */
+       orphan_vg = vg_read_orphans(cmd, 0, vg->fid->fmt->orphan_vg_name, &consistent);
 
        if (vg_read_error(orphan_vg))
                goto bad;
@@ -755,6 +753,6 @@ bad:
        /* If we are committing here or we had an error then we will free fid */
        if (pvl && (commit || r != 1))
                free_pv_fid(pvl->pv);
-       unlock_and_release_vg(cmd, orphan_vg, VG_ORPHANS);
+       release_vg(orphan_vg);
        return r;
 }

diff --git a/tools/vgcfgrestore.c b/tools/vgcfgrestore.c
index ec2c2400c767b093bbb67db02182715002c42e38..0b0368769c8714fba46eaa992bb5eed34b30d966 100644 (file)
--- a/tools/vgcfgrestore.c
+++ b/tools/vgcfgrestore.c
@@ -63,14 +63,14 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
                lvmetad_rescan = 1;
        }
 
-       if (!lock_vol(cmd, vg_name, LCK_VG_WRITE, NULL)) {
-               log_error("Unable to lock volume group %s.", vg_name);
+       if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
+               log_error("Unable to lock orphans.");
                return ECMD_FAILED;
        }
 
-       if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-               log_error("Unable to lock orphans.");
-               unlock_vg(cmd, NULL, vg_name);
+       if (!lock_vol(cmd, vg_name, LCK_VG_WRITE, NULL)) {
+               log_error("Unable to lock volume group %s.", vg_name);
+               unlock_vg(cmd, NULL, VG_ORPHANS);
                return ECMD_FAILED;
        }
 
@@ -83,8 +83,8 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
                                       arg_str_value(cmd, file_ARG, ""),
                                       arg_count(cmd, force_long_ARG)) :
              backup_restore(cmd, vg_name, arg_count(cmd, force_long_ARG)))) {
-               unlock_vg(cmd, NULL, VG_ORPHANS);
                unlock_vg(cmd, NULL, vg_name);
+               unlock_vg(cmd, NULL, VG_ORPHANS);
                log_error("Restore failed.");
                ret = ECMD_FAILED;
                goto rescan;

diff --git a/tools/vgreduce.c b/tools/vgreduce.c
index e8479a8dc81d27cd2975429c3925391509fb446d..ce3d155c768eb6ad2f7e774b075475c106a957bd 100644 (file)
--- a/tools/vgreduce.c
+++ b/tools/vgreduce.c
@@ -231,9 +231,20 @@ int vgreduce(struct cmd_context *cmd, int argc, char **argv)
        handle->custom_handle = &vp;
 
        if (!repairing) {
+               if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
+                       log_error("Can't get lock for orphan PVs");
+                       ret = ECMD_FAILED;
+                       goto out;
+               }
+
                /* FIXME: Pass private struct through to all these functions */
                /* and update in batch afterwards? */
-               ret = process_each_pv(cmd, argc, argv, vg_name, 0, READ_FOR_UPDATE, handle, _vgreduce_single);
+
+               ret = process_each_pv(cmd, argc, argv, vg_name, 0,
+                                     READ_FOR_UPDATE | PROCESS_SKIP_ORPHAN_LOCK,
+                                     handle, _vgreduce_single);
+
+               unlock_vg(cmd, NULL, VG_ORPHANS);
                goto out;
        }
 

diff --git a/tools/vgremove.c b/tools/vgremove.c
index 8bf38415135b131c3b3f12d0ecd829785016f1fe..5010e7d6bc4bdadb74564c7385c09d2d416be0fe 100644 (file)
--- a/tools/vgremove.c
+++ b/tools/vgremove.c
@@ -108,10 +108,17 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv)
         */
        cmd->lockd_gl_disable = 1;
 
+       if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
+               log_error("Can't get lock for orphan PVs");
+               return ECMD_FAILED;
+       }
+
        cmd->handles_missing_pvs = 1;
        ret = process_each_vg(cmd, argc, argv, NULL, NULL,
                              READ_FOR_UPDATE, 0,
                              NULL, &_vgremove_single);
 
+       unlock_vg(cmd, NULL, VG_ORPHANS);
+
        return ret;
 }

diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index 1e46e7ed33d7cf661f5186c39634b2ed4f9629a6..7ec00f8a25bfe47f5e4a9bb57020e464baa3d05b 100644 (file)
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -748,8 +748,10 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
 
        /*
         * Finally, remove the EXPORTED flag from the new VG and write it out.
+        * We need to unlock vg_to because vg_read_for_update wants to lock it.
         */
        if (!test_mode()) {
+               unlock_vg(cmd, NULL, vg_name_to);
                release_vg(vg_to);
                vg_to = vg_read_for_update(cmd, vg_name_to, NULL,
                                           READ_ALLOW_EXPORTED, 0);