Attachment 900918 Details for Bug 1095834 – Concept patch to introduce a way to force lockfs with suspend

[patch] Concept patch to introduce a way to force lockfs with suspend

bug-1095834-flush-on-splitmirror.patch (text/plain), 8.27 KB, created by Jonathan Earl Brassow on 2014-05-30 17:06:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Earl Brassow

Created: 2014-05-30 17:06:36 UTC

Size: 8.27 KB

patch

obsolete

>mirror/raid1: Flush the file system when splitting off images
>
>Since it is possible (at least for mirrors) to be clustered, the
>option to flush a file system must take the form of a cluster
>communicated flag - a new LCK_*[_MODE] flag.  The 'lockfs' action
>was formerly inferred when snapshots were involved.  I have not
>come up with a clever way to infer the splitting of a mirror.  So,
>explicit direction is given through the new suspend_lv_flags macro.
>I've chosen 'suspend_lv_flags' rather than 'suspend_lv_lockfs'
>because there may be situations that require new flags or combinations
>of them.  For example, ORIGIN_ONLY currently implies lockfs, but
>it wouldn't need to - you could pass
>'suspend_lv_flags(cmd, lv, LCK_ORIGIN_ONLY | LCK_LOCKFS)'
>
>N.B. I haven't tested this patch and don't know if I have exceeded the
>number of bits available with 'LCK_LOCKFS_MODE 0x100'.
>Index: lvm2/daemons/clvmd/lvm-functions.c
>===================================================================
>--- lvm2.orig/daemons/clvmd/lvm-functions.c
>+++ lvm2/daemons/clvmd/lvm-functions.c
>@@ -440,6 +440,7 @@ static int do_suspend_lv(char *resource,
> {
> 	int oldmode;
> 	unsigned origin_only = (lock_flags & LCK_ORIGIN_ONLY_MODE) ? 1 : 0;
>+	unsigned lockfs = (lock_flags & LCK_LOCKFS_MODE) ? 1 : 0;
> 	unsigned exclusive;
> 
> 	/* Is it open ? */
>@@ -452,7 +453,7 @@ static int do_suspend_lv(char *resource,
> 	exclusive = (oldmode == LCK_EXCL) ? 1 : 0;
> 
> 	/* Always call lv_suspend to read commited and precommited data */
>-	if (!lv_suspend_if_active(cmd, resource, origin_only, exclusive, NULL, NULL))
>+	if (!lv_suspend_if_active(cmd, resource, origin_only, lockfs, exclusive, NULL, NULL))
> 		return EIO;
> 
> 	return 0;
>Index: lvm2/lib/activate/activate.c
>===================================================================
>--- lvm2.orig/lib/activate/activate.c
>+++ lvm2/lib/activate/activate.c
>@@ -1942,6 +1942,9 @@ static int _lv_suspend(struct cmd_contex
> 	if (laopts->origin_only && lv_is_thin_volume(ondisk_lv) && lv_is_thin_volume(incore_lv))
> 		lockfs = 1;
> 
>+	if (laopts->requires_lockfs)
>+		lockfs = 1;
>+
> 	/*
> 	 * Suspending an LV directly above a PVMOVE LV also
>  	 * suspends other LVs using that same PVMOVE LV.
>@@ -1986,10 +1989,11 @@ out:
>  *
>  * Returns success if the device is not active
>  */
>-int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only, unsigned exclusive, struct logical_volume *ondisk_lv, struct logical_volume *incore_lv)
>+int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only, unsigned requires_lockfs, unsigned exclusive, struct logical_volume *ondisk_lv, struct logical_volume *incore_lv)
> {
> 	struct lv_activate_opts laopts = {
> 		.origin_only = origin_only,
>+		.requires_lockfs = requires_lockfs,
> 		.exclusive = exclusive
> 	};
> 
>Index: lvm2/lib/activate/activate.h
>===================================================================
>--- lvm2.orig/lib/activate/activate.h
>+++ lvm2/lib/activate/activate.h
>@@ -38,6 +38,7 @@ struct lv_activate_opts {
> 	int skip_in_use;
> 	unsigned revert;
> 	unsigned read_only;
>+	unsigned requires_lockfs;	/* Flush the file system on suspend */
> 	unsigned noscan;	/* Mark this LV to avoid its scanning. This also
> 				   directs udev to use proper udev flag to avoid
> 				   any scanning in udev. This udev flag is automatically
>Index: lvm2/lib/locking/file_locking.c
>===================================================================
>--- lvm2.orig/lib/locking/file_locking.c
>+++ lvm2/lib/locking/file_locking.c
>@@ -46,6 +46,7 @@ static int _file_lock_resource(struct cm
> {
> 	char lockfile[PATH_MAX];
> 	unsigned origin_only = (flags & LCK_ORIGIN_ONLY) ? 1 : 0;
>+	unsigned lock_fs = (flags & LCK_LOCKFS) ? 1 : 0;
> 	unsigned revert = (flags & LCK_REVERT) ? 1 : 0;
> 
> 	switch (flags & LCK_SCOPE_MASK) {
>@@ -102,7 +103,7 @@ static int _file_lock_resource(struct cm
> 			break;
> 		case LCK_WRITE:
> 			log_very_verbose("Locking LV %s (W)%s", resource, origin_only ? " without snapshots" : "");
>-			if (!lv_suspend_if_active(cmd, resource, origin_only, 0, lv_ondisk(lv), lv))
>+			if (!lv_suspend_if_active(cmd, resource, origin_only, lock_fs, 0, lv_ondisk(lv), lv))
> 				return 0;
> 			break;
> 		case LCK_EXCL:
>Index: lvm2/lib/locking/locking.h
>===================================================================
>--- lvm2.orig/lib/locking/locking.h
>+++ lvm2/lib/locking/locking.h
>@@ -103,6 +103,7 @@ int check_lvm1_vg_inactive(struct cmd_co
> #define LCK_CACHE	0x00000100U	/* Operation on cache only using P_ lock */
> #define LCK_ORIGIN_ONLY	0x00000200U	/* Operation should bypass any snapshots */
> #define LCK_REVERT	0x00000400U	/* Revert any incomplete change */
>+#define LCK_LOCKFS	0x00000800U	/* Perform lockfs on FS and flush */
> 
> /*
>  * Additional lock bits for cluster communication via args[1]
>@@ -118,6 +119,7 @@ int check_lvm1_vg_inactive(struct cmd_co
> #define LCK_ORIGIN_ONLY_MODE		0x20	/* Same as above */
> #define LCK_DMEVENTD_MONITOR_IGNORE     0x40	/* Whether to ignore dmeventd */
> #define LCK_REVERT_MODE			0x80	/* Remove inactive tables */
>+#define LCK_LOCKFS_MODE			0x100	/* Flush the file system */
> 
> /*
>  * Special cases of VG locks.
>@@ -178,6 +180,7 @@ int check_lvm1_vg_inactive(struct cmd_co
> #define revert_lv(cmd, lv)	lock_lv_vol(cmd, lv, LCK_LV_RESUME | LCK_REVERT)
> #define suspend_lv(cmd, lv)	lock_lv_vol(cmd, lv, LCK_LV_SUSPEND | LCK_HOLD)
> #define suspend_lv_origin(cmd, lv)	lock_lv_vol(cmd, lv, LCK_LV_SUSPEND | LCK_HOLD | LCK_ORIGIN_ONLY)
>+#define suspend_lv_flags(cmd, lv, extra_flags) lock_lv_vol(cmd, lv, LCK_LV_SUSPEND | LCK_HOLD | extra_flags)
> #define deactivate_lv(cmd, lv)	lock_lv_vol(cmd, lv, LCK_LV_DEACTIVATE)
> 
> #define activate_lv(cmd, lv)	lock_lv_vol(cmd, lv, LCK_LV_ACTIVATE | LCK_HOLD)
>Index: lvm2/lib/locking/no_locking.c
>===================================================================
>--- lvm2.orig/lib/locking/no_locking.c
>+++ lvm2/lib/locking/no_locking.c
>@@ -36,6 +36,10 @@ static void _no_reset_locking(void)
> static int _no_lock_resource(struct cmd_context *cmd, const char *resource,
> 			     uint32_t flags, struct logical_volume *lv)
> {
>+	unsigned origin_only = (flags & LCK_ORIGIN_ONLY) ? 1 : 0;
>+	unsigned lock_fs = (flags & LCK_LOCKFS) ? 1 : 0;
>+	unsigned revert = (flags & LCK_REVERT) ? 1 : 0;
>+
> 	switch (flags & LCK_SCOPE_MASK) {
> 	case LCK_VG:
> 		if (!strcmp(resource, VG_SYNC_NAMES))
>@@ -46,12 +50,12 @@ static int _no_lock_resource(struct cmd_
> 		case LCK_NULL:
> 			return lv_deactivate(cmd, resource, lv_ondisk(lv));
> 		case LCK_UNLOCK:
>-			return lv_resume_if_active(cmd, resource, (flags & LCK_ORIGIN_ONLY) ? 1: 0, 0, (flags & LCK_REVERT) ? 1 : 0, lv_ondisk(lv));
>+			return lv_resume_if_active(cmd, resource, origin_only, 0, revert, lv_ondisk(lv));
> 		case LCK_READ:
> 			return lv_activate_with_filter(cmd, resource, 0, lv->status & LV_NOSCAN ? 1 : 0,
> 						       lv->status & LV_TEMPORARY ? 1 : 0, lv_ondisk(lv));
> 		case LCK_WRITE:
>-			return lv_suspend_if_active(cmd, resource, (flags & LCK_ORIGIN_ONLY) ? 1 : 0, 0, lv_ondisk(lv), lv);
>+			return lv_suspend_if_active(cmd, resource, origin_only, lock_fs, 0, lv_ondisk(lv), lv);
> 		case LCK_EXCL:
> 			return lv_activate_with_filter(cmd, resource, 1, lv->status & LV_NOSCAN ? 1 : 0,
> 						       lv->status & LV_TEMPORARY ? 1 : 0, lv_ondisk(lv));
>Index: lvm2/lib/metadata/mirror.c
>===================================================================
>--- lvm2.orig/lib/metadata/mirror.c
>+++ lvm2/lib/metadata/mirror.c
>@@ -756,8 +756,9 @@ static int _split_mirror_images(struct l
> 	/*
> 	 * Suspend the mirror - this includes all the sub-LVs and
> 	 *                      soon-to-be-split sub-LVs
>+	 * Flush the file system to ensure new LV has clean FS.
> 	 */
>-	if (!suspend_lv(cmd, mirrored_seg->lv)) {
>+	if (!suspend_lv_flags(cmd, mirrored_seg->lv, LCK_LOCKFS)) {
> 		log_error("Failed to lock %s", mirrored_seg->lv->name);
> 		vg_revert(mirrored_seg->lv->vg);
> 		return 0;
>Index: lvm2/lib/metadata/raid_manip.c
>===================================================================
>--- lvm2.orig/lib/metadata/raid_manip.c
>+++ lvm2/lib/metadata/raid_manip.c
>@@ -1127,7 +1127,11 @@ int lv_raid_split(struct logical_volume
> 		return 0;
> 	}
> 
>-	if (!suspend_lv(cmd, lv)) {
>+	/*
>+	 * Flush the file system when suspending so that the
>+	 * new LV has a clean file system.
>+	 */
>+	if (!suspend_lv_flags(cmd, lv, LCK_LOCKFS)) {
> 		log_error("Failed to suspend %s/%s before committing changes",
> 			  lv->vg->name, lv->name);
> 		return 0;

Actions: View | Diff

Attachments on bug 1095834: 900918