* [PATCH 1/5] Fix clustered and no-lock locking with HOTFIX
2011-02-02 13:07 [PATCH 0/5] Fixes for relase Zdenek Kabelac
@ 2011-02-02 13:07 ` Zdenek Kabelac
2011-02-02 13:07 ` [PATCH 2/5] Move add_dev_node to DM_DEVICE_RESUME Zdenek Kabelac
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Zdenek Kabelac @ 2011-02-02 13:07 UTC (permalink / raw)
To: lvm-devel
Patch is fixing clustered locking - but currently cluster wide
message passing does not work with sync_dev_names().
To avoid any udev synchronizaiton problems with leaving unrelased
udev cookies, revert to the old slow behavior - and check for udev
cookie with after every activate and deactivate operation inside
do_lock_vol().
Once the new message will pass around the cluster - these extra fs_unlock()
shall be removed.
The problem I have now:
Cluster does not pass any message around cluster related to unlock_vg.
So the initial idea to use this point for fs_unlock() works only on
the local/single node.
For the real multinode cluster we need to pass extra message around
- but currently I've not managed to pass this new message around.
We cannot reuse any other message like DROP_CACHE or VG_BACKUP
as this are send when MDA is modifed - however activation does not
modify anything - yet it needs to flush udev cookie.
sync_local_dev_names() - serves for local cluster synchronization
sync_dev_names() - is supposed to be send to all nodes in cluster to
sychronizes its dev nodes. This should happen when vg is unlocked.
For this - macro 'unlock_vg()' is modifed to call sync_dev_names for
clustered locking before it actually releases VG lock - it's called
before lock_vol code to avoid recusion.
Mornfal expressed concerns about possibility to unlock VG by plain
call of lock_vol() with right parameters - this would miss the
sync call - however I'm currently not seeing better way, how
to achieve message passing around - unless we find the way to
to pass send this message within clvmd unlock_vg processing - the
code is currently not very clear in this area.
So once we figure out, how to pass such parameter - we may add
implicit sync_dev_names() into unlock clvmd code and no-locking
code.
For filesystem and external locking implicit fs_unlock() is called
with lock_vol as there is no recursive rick in this case.
For sync names - new LCK is defined:
LCK_VG_SYNC - which should differ from LCK_VG_BACKUP by LCK_HOLD bit.
But somehow I fail to see the reason why this message is not send
around the cluster and only local node is processing this message.
Difference between local & global sync is LCL_LOCAL and usage of
VG_SYNC_NAMES for local message.
As a potential solution could be - to overload DROP_CACHE
and with special paramater use this for as VG_SYNC_NAMES
So the cache would be dropped in other cases.
But my head already spins quite a lot around....
So any help would be appreciated.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
daemons/clvmd/clvmd-command.c | 5 ++++-
daemons/clvmd/clvmd.c | 3 +++
daemons/clvmd/lvm-functions.c | 9 +++++++--
daemons/clvmd/lvm-functions.h | 2 +-
lib/locking/cluster_locking.c | 6 +++---
lib/locking/locking.c | 7 ++++---
lib/locking/locking.h | 22 ++++++++++++++++++++--
7 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index 6d0dee4..c2bddc2 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -139,7 +139,10 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
break;
case CLVMD_CMD_SYNC_NAMES:
- lvm_do_fs_unlock();
+ lock_cmd = args[0];
+ lock_flags = args[1];
+ lockname = &args[2];
+ lvm_do_fs_unlock(lock_cmd, lock_flags, lockname, client ? 1 : 0);
break;
case CLVMD_CMD_SET_DEBUG:
diff --git a/daemons/clvmd/clvmd.c b/daemons/clvmd/clvmd.c
index 8e21732..f38894c 100644
--- a/daemons/clvmd/clvmd.c
+++ b/daemons/clvmd/clvmd.c
@@ -296,6 +296,9 @@ static const char *decode_cmd(unsigned char cmdl)
case CLVMD_CMD_RESTART:
command = "RESTART";
break;
+ case CLVMD_CMD_SYNC_NAMES:
+ command = "SYNC_NAMES";
+ break;
default:
command = "unknown";
break;
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index f06657d..8dc200e 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -507,6 +507,7 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
switch (command & LCK_MASK) {
case LCK_LV_EXCLUSIVE:
status = do_activate_lv(resource, lock_flags, LCK_EXCL);
+ fs_unlock();
break;
case LCK_LV_SUSPEND:
@@ -520,10 +521,12 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
case LCK_LV_ACTIVATE:
status = do_activate_lv(resource, lock_flags, LCK_READ);
+ fs_unlock();
break;
case LCK_LV_DEACTIVATE:
status = do_deactivate_lv(resource, lock_flags);
+ fs_unlock();
break;
default:
@@ -894,11 +897,13 @@ struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name)
return v;
}
-void lvm_do_fs_unlock(void)
+void lvm_do_fs_unlock(int lock_cmd, int lock_flags, char *lockname, int is_client)
{
pthread_mutex_lock(&lvm_lock);
- DEBUGLOG("Syncing device names\n");
+ DEBUGLOG("Syncing device names cmd=%d flags=%d name=%s client=%d\n",
+ lock_cmd, lock_flags, lockname, is_client);
fs_unlock();
+
pthread_mutex_unlock(&lvm_lock);
}
diff --git a/daemons/clvmd/lvm-functions.h b/daemons/clvmd/lvm-functions.h
index a132f4a..1b60c4f 100644
--- a/daemons/clvmd/lvm-functions.h
+++ b/daemons/clvmd/lvm-functions.h
@@ -36,6 +36,6 @@ extern char *get_last_lvm_error(void);
extern void do_lock_vg(unsigned char command, unsigned char lock_flags,
char *resource);
extern struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name);
-void lvm_do_fs_unlock(void);
+void lvm_do_fs_unlock(int lock_cmd, int lock_flags, char *lockname, int is_client);
#endif
diff --git a/lib/locking/cluster_locking.c b/lib/locking/cluster_locking.c
index 83b1cd2..cecd52d 100644
--- a/lib/locking/cluster_locking.c
+++ b/lib/locking/cluster_locking.c
@@ -401,10 +401,10 @@ int lock_resource(struct cmd_context *cmd, const char *resource, uint32_t flags)
switch (flags & LCK_SCOPE_MASK) {
case LCK_VG:
- if (!strcmp(resource, VG_SYNC_NAMES)) {
- log_very_verbose("Requesting sync names.");
+ if ((flags & ~LCK_LOCAL) == LCK_VG_SYNC) {
+ log_very_verbose("Requesting sync names for %s.", resource);
return _lock_for_cluster(cmd, CLVMD_CMD_SYNC_NAMES,
- flags & ~LCK_HOLD, resource);
+ flags, resource);
}
if (flags == LCK_VG_BACKUP) {
log_very_verbose("Requesting backup of VG metadata for %s",
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index c923610..a12db52 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -443,9 +443,10 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
if (!check_lvm1_vg_inactive(cmd, vol))
return_0;
- /* Before unlocking VG wait until devices are available. */
- if ((flags & LCK_TYPE_MASK) == LCK_UNLOCK)
- sync_local_dev_names(cmd);
+ /* For non clustered unlock - implicit fs_unlock() */
+ if ((lck_type == LCK_UNLOCK) && !(flags & LCK_CACHE) &&
+ is_real_vg(vol) && !locking_is_clustered())
+ fs_unlock();
break;
case LCK_LV:
/* All LV locks are non-blocking. */
diff --git a/lib/locking/locking.h b/lib/locking/locking.h
index 1b5e1fc..122d2fc 100644
--- a/lib/locking/locking.h
+++ b/lib/locking/locking.h
@@ -126,6 +126,7 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
#define LCK_VG_REVERT (LCK_VG | LCK_READ | LCK_CACHE | LCK_HOLD)
#define LCK_VG_BACKUP (LCK_VG | LCK_CACHE)
+#define LCK_VG_SYNC (LCK_VG | LCK_CACHE | LCK_HOLD)
#define LCK_LV_EXCLUSIVE (LCK_LV | LCK_EXCL)
#define LCK_LV_SUSPEND (LCK_LV | LCK_WRITE)
@@ -143,7 +144,20 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
lock_vol(cmd, (lv)->lvid.s, flags | LCK_LV_CLUSTERED(lv)) : \
0)
-#define unlock_vg(cmd, vol) lock_vol(cmd, vol, LCK_VG_UNLOCK)
+/*
+ * For clustered locking (cluster, no_lock) explicit call
+ * of cluster wide sync_dev_names() is made before unlocking vg.
+ * For non-clustered locking (filesystem, external)
+ * use of fs_unlock() is implicit in lock_vol().
+ */
+#define unlock_vg(cmd, vol) \
+ do { \
+ if (is_real_vg(vol) && \
+ locking_is_clustered()) \
+ sync_dev_names(cmd, vol); \
+ lock_vol(cmd, vol, LCK_VG_UNLOCK); \
+ } while (0)
+
#define unlock_and_free_vg(cmd, vg, vol) \
do { \
unlock_vg(cmd, vol); \
@@ -170,8 +184,12 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
lock_vol((vg)->cmd, (vg)->name, LCK_VG_REVERT)
#define remote_backup_metadata(vg) \
lock_vol((vg)->cmd, (vg)->name, LCK_VG_BACKUP)
+/* Synchronize local device names on cluster node */
#define sync_local_dev_names(cmd) \
- lock_vol(cmd, VG_SYNC_NAMES, LCK_NONE | LCK_CACHE | LCK_LOCAL)
+ lock_vol(cmd, VG_SYNC_NAMES, LCK_VG_SYNC | LCK_LOCAL)
+/* Synchronize global device names on all cluster nodes */
+#define sync_dev_names(cmd, vol) \
+ lock_vol(cmd, vol, LCK_VG_SYNC)
/* Process list of LVs */
int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs);
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] Move add_dev_node to DM_DEVICE_RESUME
2011-02-02 13:07 [PATCH 0/5] Fixes for relase Zdenek Kabelac
2011-02-02 13:07 ` [PATCH 1/5] Fix clustered and no-lock locking with HOTFIX Zdenek Kabelac
@ 2011-02-02 13:07 ` Zdenek Kabelac
2011-02-02 13:07 ` [PATCH 3/5] Fix operation node stacking Zdenek Kabelac
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Zdenek Kabelac @ 2011-02-02 13:07 UTC (permalink / raw)
To: lvm-devel
This patch came out from discussion with Petr.
We need valid 'check_udev' for stacked ops processing.
==
Current code:
dmsetup create xxx --notable -> create node in /dev/mapper/xxx
(check_udev has incorrect 0 value) and we are forcing node
creation in /dev directory - while only udevd should have
full control over the nodes available there.
(i.e. selinux policy might allo creation only in udev process)
==
New code creates /dev/mapper node with:
dmsetup resume xxx
As just at this moment udev is registering such device through
a CHANGE event.
==
This patch is causing incompatibilities with some old
hand written scripts which may relay on existance of device node
even when such node has only empty table.
But the functionality is needed for following patch which prunes
operation on the same node when they are all combined within one
udev cookie transaction.
We cannot insert there 'ADD' operation which pretends it should not
check for udev and mix them with udev operation - as we would again
modify content of /dev dir.
Also this patch allows to further cleanup libdevmapper code
from other quirks.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
libdm/ioctl/libdm-iface.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c
index fbe4b14..bb9dac5 100644
--- a/libdm/ioctl/libdm-iface.c
+++ b/libdm/ioctl/libdm-iface.c
@@ -2064,10 +2064,6 @@ repeat_ioctl:
switch (dmt->type) {
case DM_DEVICE_CREATE:
- if (dmt->dev_name && *dmt->dev_name && !udev_only)
- add_dev_node(dmt->dev_name, MAJOR(dmi->dev),
- MINOR(dmi->dev), dmt->uid, dmt->gid,
- dmt->mode, check_udev);
break;
case DM_DEVICE_REMOVE:
/* FIXME Kernel needs to fill in dmi->name */
@@ -2083,6 +2079,10 @@ repeat_ioctl:
break;
case DM_DEVICE_RESUME:
+ if (dmt->dev_name && *dmt->dev_name && !udev_only)
+ add_dev_node(dmt->dev_name, MAJOR(dmi->dev),
+ MINOR(dmi->dev), dmt->uid, dmt->gid,
+ dmt->mode, check_udev);
/* FIXME Kernel needs to fill in dmi->name */
set_dev_node_read_ahead(dmt->dev_name, dmt->read_ahead,
dmt->read_ahead_flags);
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] Fix operation node stacking
2011-02-02 13:07 [PATCH 0/5] Fixes for relase Zdenek Kabelac
2011-02-02 13:07 ` [PATCH 1/5] Fix clustered and no-lock locking with HOTFIX Zdenek Kabelac
2011-02-02 13:07 ` [PATCH 2/5] Move add_dev_node to DM_DEVICE_RESUME Zdenek Kabelac
@ 2011-02-02 13:07 ` Zdenek Kabelac
2011-02-02 13:08 ` [PATCH 4/5] lv_info handle udev_sync Zdenek Kabelac
2011-02-02 13:08 ` [PATCH 5/5] Remove open_count read from some lv_info calls Zdenek Kabelac
4 siblings, 0 replies; 6+ messages in thread
From: Zdenek Kabelac @ 2011-02-02 13:07 UTC (permalink / raw)
To: lvm-devel
With ability to stack many operations in one udev transaction -
in same cases we are adding and removing same device at the same time.
This leads to a problem of checking stacked operations:
i.e. remove /dev/node1 followed by create /dev/node1
If the node creation is handled with udev - there is a problem as
stacked operation gives warning about existing node1 and will try to
remove it - while next operation needs to recreate it.
Current code removes all previous stacked operation if the fs op is
FS_DEL - patch adds similar behavior for FS_ADD - it will try to
remove any 'delete' operation if udev is in use.
For FS_RENAME operation it seems to be more complex. But as we
are always stacking FS_READ_AHEAD after FS_ADD operation -
should be safe to remove all previous operation on the node
when udev is running.
Code does same checking for stacking libdm and liblvm operations.
As a very simple optimization counters were added for each stacked ops
type to avoid unneeded list scans if some operation does not exists in
the list.
WARNING:
Patch checks variable 'check_udev' flag so the previous patch is mandatory
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/activate/fs.c | 61 ++++++++++++++++++++++++++++++++++++++++--
libdm/libdm-common.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 121 insertions(+), 11 deletions(-)
diff --git a/lib/activate/fs.c b/lib/activate/fs.c
index ae94763..a3db5a7 100644
--- a/lib/activate/fs.c
+++ b/lib/activate/fs.c
@@ -257,7 +257,8 @@ static int _rm_link(const char *dev_dir, const char *vg_name,
typedef enum {
FS_ADD,
FS_DEL,
- FS_RENAME
+ FS_RENAME,
+ NUM_FS_OPS
} fs_op_t;
static int _do_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
@@ -283,12 +284,18 @@ static int _do_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
if (!_mk_link(dev_dir, vg_name, lv_name, dev, check_udev))
stack;
+ default:;
}
return 1;
}
static DM_LIST_INIT(_fs_ops);
+/*
+ * Count number of stacked fs_op_t operations to allow to skip dm_list search.
+ * FIXME: handling of FS_RENAME
+ */
+static int _c_fs_ops[NUM_FS_OPS];
struct fs_op_parms {
struct dm_list list;
@@ -309,15 +316,63 @@ static void _store_str(char **pos, char **ptr, const char *str)
*pos += strlen(*ptr) + 1;
}
+static int _other_fs_ops(fs_op_t type)
+{
+ int i;
+
+ for (i = 0; i < NUM_FS_OPS; i++)
+ if (type != i && _c_fs_ops[i])
+ return 1;
+ return 0;
+}
+
+static void _del_fs_op(struct fs_op_parms *fsp)
+{
+ _c_fs_ops[fsp->type]--;
+ dm_list_del(&fsp->list);
+ dm_free(fsp);
+}
+
static int _stack_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
const char *lv_name, const char *dev,
const char *old_lv_name, int check_udev)
{
+ struct dm_list *fsph, *fspht;
struct fs_op_parms *fsp;
size_t len = strlen(dev_dir) + strlen(vg_name) + strlen(lv_name) +
strlen(dev) + strlen(old_lv_name) + 5;
char *pos;
+ if ((type == FS_DEL) && _other_fs_ops(type))
+ dm_list_iterate_safe(fsph, fspht, &_fs_ops) {
+ fsp = dm_list_item(fsph, struct fs_op_parms);
+ if (!strcmp(lv_name, fsp->lv_name) &&
+ !strcmp(vg_name, fsp->vg_name)) {
+ _del_fs_op(fsp);
+ if (!_other_fs_ops(type))
+ break;
+ }
+ }
+ else if ((type == FS_ADD) && _c_fs_ops[FS_DEL] && check_udev &&
+ dm_udev_get_sync_support() && dm_udev_get_checking())
+ dm_list_iterate_safe(fsph, fspht, &_fs_ops) {
+ fsp = dm_list_item(fsph, struct fs_op_parms);
+ if ((fsp->type == FS_DEL) &&
+ !strcmp(lv_name, fsp->lv_name) &&
+ !strcmp(vg_name, fsp->vg_name)) {
+ _del_fs_op(fsp);
+ break;
+ }
+ }
+ else if ((type == FS_RENAME) && check_udev &&
+ dm_udev_get_sync_support() && dm_udev_get_checking())
+ dm_list_iterate_safe(fsph, fspht, &_fs_ops) {
+ fsp = dm_list_item(fsph, struct fs_op_parms);
+ if (!strcmp(old_lv_name, fsp->lv_name) &&
+ !strcmp(vg_name, fsp->vg_name))
+ _del_fs_op(fsp);
+ }
+
if (!(fsp = dm_malloc(sizeof(*fsp) + len))) {
log_error("No space to stack fs operation");
return 0;
@@ -333,6 +388,7 @@ static int _stack_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
_store_str(&pos, &fsp->dev, dev);
_store_str(&pos, &fsp->old_lv_name, old_lv_name);
+ _c_fs_ops[type]++;
dm_list_add(&_fs_ops, &fsp->list);
return 1;
@@ -347,8 +403,7 @@ static void _pop_fs_ops(void)
fsp = dm_list_item(fsph, struct fs_op_parms);
_do_fs_op(fsp->type, fsp->dev_dir, fsp->vg_name, fsp->lv_name,
fsp->dev, fsp->old_lv_name, fsp->check_udev);
- dm_list_del(&fsp->list);
- dm_free(fsp);
+ _del_fs_op(fsp);
}
}
diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index a181c5f..9593e75 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -724,7 +724,8 @@ typedef enum {
NODE_ADD,
NODE_DEL,
NODE_RENAME,
- NODE_READ_AHEAD
+ NODE_READ_AHEAD,
+ NUM_NODES
} node_op_t;
static int _do_node_op(node_op_t type, const char *dev_name, uint32_t major,
@@ -743,12 +744,14 @@ static int _do_node_op(node_op_t type, const char *dev_name, uint32_t major,
case NODE_READ_AHEAD:
return _set_dev_node_read_ahead(dev_name, read_ahead,
read_ahead_flags);
+ default:;
}
return 1;
}
static DM_LIST_INIT(_node_ops);
+static int _c_node_ops[NUM_NODES];
struct node_op_parms {
struct dm_list list;
@@ -773,6 +776,25 @@ static void _store_str(char **pos, char **ptr, const char *str)
*pos += strlen(*ptr) + 1;
}
+static int _other_node_ops(node_op_t type)
+{
+ int i;
+
+ for (i = 0; i < NUM_NODES; i++)
+ if (type != i && _c_node_ops[i])
+ return 1;
+
+ return 0;
+}
+
+static void _del_node_op(struct node_op_parms *nop)
+{
+ _c_node_ops[nop->type]--;
+ dm_list_del(&nop->list);
+ dm_free(nop);
+
+}
+
static int _stack_node_op(node_op_t type, const char *dev_name, uint32_t major,
uint32_t minor, uid_t uid, gid_t gid, mode_t mode,
const char *old_name, uint32_t read_ahead,
@@ -784,17 +806,50 @@ static int _stack_node_op(node_op_t type, const char *dev_name, uint32_t major,
char *pos;
/*
- * Ignore any outstanding operations on the node if deleting it
+ * Note: check_udev must have valid content
*/
- if (type == NODE_DEL) {
+ if ((type == NODE_DEL) && _other_node_ops(type))
+ /*
+ * Ignore any outstanding operations on the node if deleting it
+ */
dm_list_iterate_safe(noph, nopht, &_node_ops) {
nop = dm_list_item(noph, struct node_op_parms);
if (!strcmp(dev_name, nop->dev_name)) {
- dm_list_del(&nop->list);
- dm_free(nop);
+ _del_node_op(nop);
+ if (!_other_node_ops(type))
+ break; /* no other non DEL ops */
}
}
- }
+ else if ((type == NODE_ADD) &&
+ _c_node_ops[NODE_DEL] && check_udev &&
+ dm_udev_get_sync_support() && dm_udev_get_checking())
+ /*
+ * Ignore previous del operation when udev is running
+ * Any other stacked operation for dev_node is error
+ */
+ dm_list_iterate_safe(noph, nopht, &_node_ops) {
+ nop = dm_list_item(noph, struct node_op_parms);
+ if ((nop->type == NODE_DEL) &&
+ !strcmp(dev_name, nop->dev_name)) {
+ _del_node_op(nop);
+ break;
+ }
+ }
+ else if ((type == NODE_RENAME) && check_udev &&
+ dm_udev_get_sync_support() && dm_udev_get_checking())
+ /*
+ * Ignore any outstanding operations on the node if renaming it
+ *
+ * Currently RENAME operation goes through suspend - resume
+ * on resume devnode is added with read_ahead settings, so it
+ * it's safe to remove any stacked ADD, RENAME, READ_AHEAD operation
+ * There cannot be any DEL operation on the renamed node.
+ */
+ dm_list_iterate_safe(noph, nopht, &_node_ops) {
+ nop = dm_list_item(noph, struct node_op_parms);
+ if (!strcmp(old_name, nop->dev_name))
+ _del_node_op(nop);
+ }
if (!(nop = dm_malloc(sizeof(*nop) + len))) {
log_error("Insufficient memory to stack mknod operation");
@@ -815,6 +870,7 @@ static int _stack_node_op(node_op_t type, const char *dev_name, uint32_t major,
_store_str(&pos, &nop->dev_name, dev_name);
_store_str(&pos, &nop->old_name, old_name);
+ _c_node_ops[type]++;
dm_list_add(&_node_ops, &nop->list);
return 1;
@@ -831,8 +887,7 @@ static void _pop_node_ops(void)
nop->uid, nop->gid, nop->mode, nop->old_name,
nop->read_ahead, nop->read_ahead_flags,
nop->check_udev);
- dm_list_del(&nop->list);
- dm_free(nop);
+ _del_node_op(nop);
}
}
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] lv_info handle udev_sync
2011-02-02 13:07 [PATCH 0/5] Fixes for relase Zdenek Kabelac
` (2 preceding siblings ...)
2011-02-02 13:07 ` [PATCH 3/5] Fix operation node stacking Zdenek Kabelac
@ 2011-02-02 13:08 ` Zdenek Kabelac
2011-02-02 13:08 ` [PATCH 5/5] Remove open_count read from some lv_info calls Zdenek Kabelac
4 siblings, 0 replies; 6+ messages in thread
From: Zdenek Kabelac @ 2011-02-02 13:08 UTC (permalink / raw)
To: lvm-devel
In case open_count is requested via lv_info - check if there
are any udev operations in-progress - and wait for them
before checking for lv_info
Remove fs_unlock after _lv_resume - as it's now handled via
lv_info query.
Note: some those lv_info() which do not need open_count,
needs to pass 0 for 'with_open_count' parameter.
NoteII: locking_is_clustered() is needed to detect in which context
lv_info() is called as we cannot use sync_local_dev_names() when
locking is not defined.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/activate/activate.c | 13 ++++++++++++-
lib/activate/fs.c | 5 +++++
lib/activate/fs.h | 1 +
3 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 3670ea4..ff84f4f 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -460,6 +460,18 @@ int lv_info(struct cmd_context *cmd, const struct logical_volume *lv, unsigned o
if (!activation())
return 0;
+ /*
+ * If open_count info is requested and we have to be sure our own udev
+ * transactions are finished
+ * For non-clustered locking type we are only interested for non-delete operation
+ * in progress - as only those could lead to opened files
+ */
+ if (with_open_count) {
+ if (locking_is_clustered())
+ sync_local_dev_names(cmd); /* Wait to have udev in sync */
+ else if (fs_has_non_delete_ops())
+ fs_unlock(); /* For non clustered - wait if there are non-delete ops */
+ }
if (!dev_manager_info(lv->vg->cmd->mem, lv, origin_only ? "real" : NULL, with_open_count,
with_read_ahead, &dminfo, &info->read_ahead))
@@ -1112,7 +1124,6 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s,
goto_out;
memlock_dec(cmd);
- fs_unlock();
if (!monitor_dev_for_events(cmd, lv, origin_only, 1))
stack;
diff --git a/lib/activate/fs.c b/lib/activate/fs.c
index a3db5a7..31a71b1 100644
--- a/lib/activate/fs.c
+++ b/lib/activate/fs.c
@@ -477,3 +477,8 @@ void fs_set_cookie(uint32_t cookie)
{
_fs_cookie = cookie;
}
+
+int fs_has_non_delete_ops(void)
+{
+ return _other_fs_ops(FS_DEL);
+}
diff --git a/lib/activate/fs.h b/lib/activate/fs.h
index 17ecedf..43e2846 100644
--- a/lib/activate/fs.h
+++ b/lib/activate/fs.h
@@ -32,5 +32,6 @@ int fs_rename_lv(struct logical_volume *lv, const char *dev,
/* void fs_unlock(void); moved to activate.h */
uint32_t fs_get_cookie(void);
void fs_set_cookie(uint32_t cookie);
+int fs_has_non_delete_ops(void);
#endif
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] Remove open_count read from some lv_info calls
2011-02-02 13:07 [PATCH 0/5] Fixes for relase Zdenek Kabelac
` (3 preceding siblings ...)
2011-02-02 13:08 ` [PATCH 4/5] lv_info handle udev_sync Zdenek Kabelac
@ 2011-02-02 13:08 ` Zdenek Kabelac
4 siblings, 0 replies; 6+ messages in thread
From: Zdenek Kabelac @ 2011-02-02 13:08 UTC (permalink / raw)
To: lvm-devel
PLEASE CHECK ME!!!
It's not completely clear what could be the side effect
lv_infoi() in these places does not read 'info.open_count' value -
so it should not need to set this value - unless I'm missing some side-effect??
This change is needed to minimalize the amount of fs_unlock() calls.
As we need to make sure no udev operation is in progress when we
check for open_count.
Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
lib/activate/activate.c | 2 +-
liblvm/lvm_lv.c | 4 ++--
tools/lvconvert.c | 2 +-
tools/lvscan.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index ff84f4f..d8f31ba 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -1218,7 +1218,7 @@ int lv_deactivate(struct cmd_context *cmd, const char *lvid_s)
r = _lv_deactivate(lv);
memlock_dec(cmd);
- if (!lv_info(cmd, lv, 0, &info, 1, 0) || info.exists)
+ if (!lv_info(cmd, lv, 0, &info, 0, 0) || info.exists)
r = 0;
out:
if (lv) {
diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
index b77f78c..664c39d 100644
--- a/liblvm/lvm_lv.c
+++ b/liblvm/lvm_lv.c
@@ -62,7 +62,7 @@ struct lvm_property_value lvm_lvseg_get_property(const lvseg_t lvseg,
uint64_t lvm_lv_is_active(const lv_t lv)
{
struct lvinfo info;
- if (lv_info(lv->vg->cmd, lv, 0, &info, 1, 0) &&
+ if (lv_info(lv->vg->cmd, lv, 0, &info, 0, 0) &&
info.exists && info.live_table)
return 1;
return 0;
@@ -71,7 +71,7 @@ uint64_t lvm_lv_is_active(const lv_t lv)
uint64_t lvm_lv_is_suspended(const lv_t lv)
{
struct lvinfo info;
- if (lv_info(lv->vg->cmd, lv, 0, &info, 1, 0) &&
+ if (lv_info(lv->vg->cmd, lv, 0, &info, 0, 0) &&
info.exists && info.suspended)
return 1;
return 0;
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 04c9f41..b35bc81 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -1637,7 +1637,7 @@ static int poll_logical_volume(struct cmd_context *cmd, struct logical_volume *l
{
struct lvinfo info;
- if (!lv_info(cmd, lv, 0, &info, 1, 0) || !info.exists) {
+ if (!lv_info(cmd, lv, 0, &info, 0, 0) || !info.exists) {
log_print("Conversion starts after activation.");
return ECMD_PROCESSED;
}
diff --git a/tools/lvscan.c b/tools/lvscan.c
index 1f91d39..753b00d 100644
--- a/tools/lvscan.c
+++ b/tools/lvscan.c
@@ -28,7 +28,7 @@ static int lvscan_single(struct cmd_context *cmd, struct logical_volume *lv,
if (!arg_count(cmd, all_ARG) && !lv_is_visible(lv))
return ECMD_PROCESSED;
- inkernel = lv_info(cmd, lv, 0, &info, 1, 0) && info.exists;
+ inkernel = lv_info(cmd, lv, 0, &info, 0, 0) && info.exists;
if (lv_is_origin(lv)) {
dm_list_iterate_items_gen(snap_seg, &lv->snapshot_segs,
origin_list) {
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread