* Remove NONBLOCK flag from higher-level tools.
@ 2009-05-12 22:21 Dave Wysochanski
2009-05-12 22:21 ` [PATCH] Remove LOCK_NONBLOCKING from new vg_read() interface Dave Wysochanski
0 siblings, 1 reply; 4+ messages in thread
From: Dave Wysochanski @ 2009-05-12 22:21 UTC (permalink / raw)
To: lvm-devel
The following two patches are two approaches to solving the
same problem: remove the NONBLOCK flag from the higher-level
vg_read() function. This simplifies the API for the user.
Both patches take a similar approach - enforce a policy that
the second VG lock implicitly sets the NONBLOCK flag. One
patch places the code inside lock_vol() and should apply
cleanly on upstream LVM. The other patch places the code
inside the new vg_read() interface and should apply cleanly
on top of Petr's vg_read() patches submitted on May 3.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Remove LOCK_NONBLOCKING from new vg_read() interface.
2009-05-12 22:21 Remove NONBLOCK flag from higher-level tools Dave Wysochanski
@ 2009-05-12 22:21 ` Dave Wysochanski
2009-05-12 22:21 ` [PATCH] Remove NON_BLOCKING flag from the tools and set a policy to auto-set Dave Wysochanski
0 siblings, 1 reply; 4+ messages in thread
From: Dave Wysochanski @ 2009-05-12 22:21 UTC (permalink / raw)
To: lvm-devel
This patch simplifies the vg_read() interface by removing a flag,
LOCK_NONBLOCKING. We can remove the flag if we set the following
policy:
- The first vg lock taken is taken non-blocking (w/out LCK_NONBLOCK)
- Any subsequent locks taken after the first are non-blocking.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
lib/metadata/metadata-exported.h | 1 -
lib/metadata/metadata.c | 9 +++++++--
tools/vgextend.c | 2 +-
tools/vgmerge.c | 3 +--
tools/vgreduce.c | 2 +-
tools/vgremove.c | 2 +-
tools/vgrename.c | 2 +-
tools/vgsplit.c | 2 +-
8 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 1523c79..8116f16 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -108,7 +108,6 @@ struct pv_segment;
#define READ_CHECK_EXISTENCE 0x00080000U /* Also used in vg->read_status */
/* FIXME Deduce these next requirements internally instead of having caller specify. */
-#define LOCK_NONBLOCKING 0x00000100U /* Fail if not available immediately. */
#define LOCK_KEEP 0x00000200U /* Do not unlock upon read failure. */
/* A meta-flag, useful with toollib for_each_* functions. */
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index ff37fba..7b4c512 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -2737,7 +2737,12 @@ vg_t *vg_read(struct cmd_context *cmd, const char *vg_name,
if (flags & READ_REQUIRE_RESIZEABLE)
status |= RESIZEABLE_VG;
- if (flags & LOCK_NONBLOCKING)
+ /*
+ * Automatically set LCK_NONBLOCK if one or more VGs locked.
+ * This will enforce correctness and prevent deadlocks rather
+ * than relying on the caller to set the flag properly.
+ */
+ if (vgs_locked())
lock_flags |= LCK_NONBLOCK;
return _vg_lock_and_read(cmd, vg_name, vgid, lock_flags, status, flags);
diff --git a/tools/vgextend.c b/tools/vgextend.c
index d589005..db611e8 100644
--- a/tools/vgextend.c
+++ b/tools/vgextend.c
@@ -43,7 +43,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
log_verbose("Checking for volume group \"%s\"", vg_name);
vg = vg_read_for_update(cmd, vg_name, NULL,
- READ_REQUIRE_RESIZEABLE | LOCK_NONBLOCKING);
+ READ_REQUIRE_RESIZEABLE);
if (vg_read_error(vg)) {
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
diff --git a/tools/vgmerge.c b/tools/vgmerge.c
index adba52e..292cf82 100644
--- a/tools/vgmerge.c
+++ b/tools/vgmerge.c
@@ -33,8 +33,7 @@ static int _vgmerge_single(struct cmd_context *cmd, const char *vg_name_to,
return ECMD_FAILED;
log_verbose("Checking for volume group \"%s\"", vg_name_from);
- vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
- LOCK_NONBLOCKING);
+ vg_from = vg_read_for_update(cmd, vg_name_from, NULL, 0);
if (vg_read_error(vg_from)) {
unlock_release_vg(cmd, vg_to, vg_name_to);
return ECMD_FAILED;
diff --git a/tools/vgreduce.c b/tools/vgreduce.c
index 749b45e..8ffa05f 100644
--- a/tools/vgreduce.c
+++ b/tools/vgreduce.c
@@ -419,7 +419,7 @@ static int _vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
vg->extent_count -= pv_pe_count(pv);
orphan_vg = vg_read_for_update(cmd, vg->fid->fmt->orphan_vg_name,
- NULL, LOCK_NONBLOCKING);
+ NULL, 0);
if (vg_read_error(orphan_vg))
goto bad;
diff --git a/tools/vgremove.c b/tools/vgremove.c
index 1d0849d..3363a1e 100644
--- a/tools/vgremove.c
+++ b/tools/vgremove.c
@@ -41,7 +41,7 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv)
}
ret = process_each_vg(cmd, argc, argv,
- READ_FOR_UPDATE | LOCK_NONBLOCKING,
+ READ_FOR_UPDATE,
NULL, &vgremove_single);
unlock_vg(cmd, VG_ORPHANS);
diff --git a/tools/vgrename.c b/tools/vgrename.c
index 447b808..ea9c17e 100644
--- a/tools/vgrename.c
+++ b/tools/vgrename.c
@@ -86,7 +86,7 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path,
log_verbose("Checking for new volume group \"%s\"", vg_name_new);
vg_new = vg_read_for_update(cmd, vg_name_new, NULL, LOCK_KEEP |
- READ_CHECK_EXISTENCE | LOCK_NONBLOCKING);
+ READ_CHECK_EXISTENCE);
if (vg_read_error(vg_new))
goto error;
diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index b96babd..0334d7a 100644
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -329,7 +329,7 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
log_verbose("Checking for new volume group \"%s\"", vg_name_to);
vg_to = vg_read_for_update(cmd, vg_name_to, NULL,
READ_REQUIRE_RESIZEABLE |
- LOCK_NONBLOCKING | LOCK_KEEP |
+ LOCK_KEEP |
READ_CHECK_EXISTENCE);
if (vg_read_error(vg_to))
--
1.6.0.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] Remove NON_BLOCKING flag from the tools and set a policy to auto-set.
2009-05-12 22:21 ` [PATCH] Remove LOCK_NONBLOCKING from new vg_read() interface Dave Wysochanski
@ 2009-05-12 22:21 ` Dave Wysochanski
2009-05-13 1:54 ` Alasdair G Kergon
0 siblings, 1 reply; 4+ messages in thread
From: Dave Wysochanski @ 2009-05-12 22:21 UTC (permalink / raw)
To: lvm-devel
As a simplification to the tools and further liblvm, this patch pushes
the setting of NON_BLOCKING lock flag inside the lock_vol() call.
The policy we set is if any existing VGs are currently locked, we
set the NON_BLOCKING flag.
Should be no functional change.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
lib/locking/locking.c | 8 ++++++++
tools/vgcfgrestore.c | 4 ++--
tools/vgcreate.c | 4 ++--
tools/vgextend.c | 4 ++--
tools/vgmerge.c | 4 ++--
tools/vgreduce.c | 2 +-
tools/vgremove.c | 4 ++--
tools/vgrename.c | 2 +-
tools/vgsplit.c | 2 +-
9 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index ad93ddd..37136ee 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -376,6 +376,14 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
switch (flags & LCK_SCOPE_MASK) {
case LCK_VG:
+ /*
+ * Automatically set LCK_NONBLOCK if one or more VGs locked.
+ * This will enforce correctness and prevent deadlocks rather
+ * than relying on the caller to set the flag properly.
+ */
+ if (vgs_locked())
+ flags |= LCK_NONBLOCK;
+
/* Lock VG to change on-disk metadata. */
/* If LVM1 driver knows about the VG, it can't be accessed. */
if (!check_lvm1_vg_inactive(cmd, vol))
diff --git a/tools/vgcfgrestore.c b/tools/vgcfgrestore.c
index 3c66b62..6ec20d1 100644
--- a/tools/vgcfgrestore.c
+++ b/tools/vgcfgrestore.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -48,7 +48,7 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
}
- if (!lock_vol(cmd, vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) {
log_error("Unable to lock volume group %s", vg_name);
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
diff --git a/tools/vgcreate.c b/tools/vgcreate.c
index add394a..0b8b769 100644
--- a/tools/vgcreate.c
+++ b/tools/vgcreate.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -51,7 +51,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
}
- if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE)) {
log_error("Can't get lock for %s", vp_new.vg_name);
unlock_vg(cmd, VG_ORPHANS);
return ECMD_FAILED;
diff --git a/tools/vgextend.c b/tools/vgextend.c
index 5d99cce..280017f 100644
--- a/tools/vgextend.c
+++ b/tools/vgextend.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -42,7 +42,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
}
log_verbose("Checking for volume group \"%s\"", vg_name);
- if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE | LCK_NONBLOCK,
+ if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE,
CLUSTERED | EXPORTED_VG |
LVM_WRITE | RESIZEABLE_VG,
CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) {
diff --git a/tools/vgmerge.c b/tools/vgmerge.c
index c847c7e..9a276d6 100644
--- a/tools/vgmerge.c
+++ b/tools/vgmerge.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -35,7 +35,7 @@ static int _vgmerge_single(struct cmd_context *cmd, const char *vg_name_to,
log_verbose("Checking for volume group \"%s\"", vg_name_from);
if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, NULL,
- LCK_VG_WRITE | LCK_NONBLOCK,
+ LCK_VG_WRITE,
CLUSTERED | EXPORTED_VG | LVM_WRITE,
CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) {
unlock_release_vg(cmd, vg_to, vg_name_to);
diff --git a/tools/vgreduce.c b/tools/vgreduce.c
index 8d40793..cf55afe 100644
--- a/tools/vgreduce.c
+++ b/tools/vgreduce.c
@@ -397,7 +397,7 @@ static int _vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
return ECMD_FAILED;
}
- if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE)) {
log_error("Can't get lock for orphan PVs");
return ECMD_FAILED;
}
diff --git a/tools/vgremove.c b/tools/vgremove.c
index 8c4357e..38f9ee3 100644
--- a/tools/vgremove.c
+++ b/tools/vgremove.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2009 Red Hat, Inc. All rights reserved.
*
* This file is part of LVM2.
*
@@ -41,7 +41,7 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv)
}
ret = process_each_vg(cmd, argc, argv,
- LCK_VG_WRITE | LCK_NONBLOCK, 1,
+ LCK_VG_WRITE, 1,
NULL, &vgremove_single);
unlock_vg(cmd, VG_ORPHANS);
diff --git a/tools/vgrename.c b/tools/vgrename.c
index 697d27b..205ba3e 100644
--- a/tools/vgrename.c
+++ b/tools/vgrename.c
@@ -100,7 +100,7 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path,
log_verbose("Checking for new volume group \"%s\"", vg_name_new);
- if (!lock_vol(cmd, vg_name_new, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ if (!lock_vol(cmd, vg_name_new, LCK_VG_WRITE)) {
unlock_release_vg(cmd, vg, vg_name_old);
log_error("Can't get lock for %s", vg_name_new);
return 0;
diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index 87e74a4..2b30871 100644
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -328,7 +328,7 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
log_verbose("Checking for new volume group \"%s\"", vg_name_to);
- if (!lock_vol(cmd, vg_name_to, LCK_VG_WRITE | LCK_NONBLOCK)) {
+ if (!lock_vol(cmd, vg_name_to, LCK_VG_WRITE)) {
log_error("Can't get lock for %s", vg_name_to);
unlock_release_vg(cmd, vg_from, vg_name_from);
return ECMD_FAILED;
--
1.6.0.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] Remove NON_BLOCKING flag from the tools and set a policy to auto-set.
2009-05-12 22:21 ` [PATCH] Remove NON_BLOCKING flag from the tools and set a policy to auto-set Dave Wysochanski
@ 2009-05-13 1:54 ` Alasdair G Kergon
0 siblings, 0 replies; 4+ messages in thread
From: Alasdair G Kergon @ 2009-05-13 1:54 UTC (permalink / raw)
To: lvm-devel
On Tue, May 12, 2009 at 06:21:11PM -0400, Dave Wysochanski wrote:
> As a simplification to the tools and further liblvm, this patch pushes
> the setting of NON_BLOCKING lock flag inside the lock_vol() call.
> The policy we set is if any existing VGs are currently locked, we
> set the NON_BLOCKING flag.
Ack, assuming you checked all the places setting it today have
VG locks already held.
Alasdair
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-05-13 1:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-12 22:21 Remove NONBLOCK flag from higher-level tools Dave Wysochanski
2009-05-12 22:21 ` [PATCH] Remove LOCK_NONBLOCKING from new vg_read() interface Dave Wysochanski
2009-05-12 22:21 ` [PATCH] Remove NON_BLOCKING flag from the tools and set a policy to auto-set Dave Wysochanski
2009-05-13 1:54 ` Alasdair G Kergon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.