From e6923120b9ebb56d5ba9828c218df1722dc7ea23 Mon Sep 17 00:00:00 2001 From: Dave Wysochanski Date: Fri, 24 Jul 2009 15:01:43 +0000 Subject: [PATCH] Revert previous patch that moved VG_ORPHAN lock inside vg_extend. We must hold the VG_ORPHAN lock until we commit to disk. Otherwise, we risk a race condition on vgcreate / vgextend. Reverts the following commit: commit 72a41480ba66d7dc2d05ef8583080b6b08208507 Author: Dave Wysochanski Date: Fri Jul 10 20:09:21 2009 +0000 Move orphan lock obtain/release inside vg_extend(). With this change we now have vgcreate/vgextend liblvm functions. Note that this changes the lock order of the following functions as the orphan lock is now obtained first. With our policy of non-blocking second locks, this should not be a problem. Signed-off-by: Dave Wysochanski --- WHATS_NEW | 2 ++ lib/metadata/metadata.c | 7 ------- tools/vgcreate.c | 8 ++++++++ tools/vgextend.c | 7 +++++++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 217c1adc0..9a36f32b5 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,7 @@ Version 2.02.50 - ================================ + Fix race condition with vgcreate and vgextend on same device (2.02.49). + Remove redundant validate_name call from vgreduce. Add lvm_{pv|vg|lv}_get_{name|uuid} liblvm functions. Add lvm_vg_list_pvs and lvm_vg_list_lvs liblvm functions. Remove unused handles lvseg, pvseg inside liblvm/lvm.h. diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 03e0810b6..62a0291c8 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -518,11 +518,6 @@ int vg_extend(struct volume_group *vg, int pv_count, char **pv_names) if (_vg_bad_status_bits(vg, RESIZEABLE_VG)) return 0; - if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { - log_error("Can't get lock for orphan PVs"); - return 0; - } - /* attach each pv */ for (i = 0; i < pv_count; i++) { if (!(pv = pv_by_path(vg->fid->fmt->cmd, pv_names[i]))) { @@ -536,13 +531,11 @@ int vg_extend(struct volume_group *vg, int pv_count, char **pv_names) /* FIXME Decide whether to initialise and add new mdahs to format instance */ - unlock_vg(cmd, VG_ORPHANS); return 1; bad: log_error("Unable to add physical volume '%s' to " "volume group '%s'.", pv_names[i], vg->name); - unlock_vg(cmd, VG_ORPHANS); return 0; } diff --git a/tools/vgcreate.c b/tools/vgcreate.c index c41c50f60..dff3a5d13 100644 --- a/tools/vgcreate.c +++ b/tools/vgcreate.c @@ -57,6 +57,11 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) !vg_set_alloc_policy(vg, vp_new.alloc)) goto_bad; + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { + log_error("Can't get lock for orphan PVs"); + goto bad_orphan; + } + /* attach the pv's */ if (!vg_extend(vg, argc - 1, argv + 1)) goto_bad; @@ -106,6 +111,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) goto bad; } + unlock_vg(cmd, VG_ORPHANS); unlock_vg(cmd, vp_new.vg_name); backup(vg); @@ -117,6 +123,8 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv) return ECMD_PROCESSED; bad: + unlock_vg(cmd, VG_ORPHANS); +bad_orphan: vg_release(vg); unlock_vg(cmd, vp_new.vg_name); return ECMD_FAILED; diff --git a/tools/vgextend.c b/tools/vgextend.c index 1dfbc116f..37f4dba96 100644 --- a/tools/vgextend.c +++ b/tools/vgextend.c @@ -42,6 +42,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) vg_release(vg); return ECMD_FAILED; } + /********** FIXME log_print("maximum logical volume size is %s", (dummy = lvm_show_size(LVM_LV_SIZE_MAX(vg) / 2, LONG))); @@ -52,6 +53,11 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) if (!archive(vg)) goto error; + if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) { + log_error("Can't get lock for orphan PVs"); + return ECMD_FAILED; + } + /* extend vg */ if (!vg_extend(vg, argc, argv)) goto error; @@ -69,6 +75,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv) r = ECMD_PROCESSED; error: + unlock_vg(cmd, VG_ORPHANS); unlock_and_release_vg(cmd, vg, vg_name); return r; } -- 2.43.5