LVM: New flag, LV_REBUILD

Any objections to a new flag?

brassow

Add new flag, LV_REBUILD.

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

This patch proposes and uses a new flag, LV_REBUILD.

Index: LVM2/lib/metadata/metadata-exported.h
===================================================================

lib/metadata/metadata-exported.h +2 -0

@@ -61,7 +61,9 @@
 //#define VIRTUAL		UINT64_C(0x00010000)	/* LV - internal use only */
 #define MIRROR_LOG		UINT64_C(0x00020000)	/* LV */
 #define MIRROR_IMAGE		UINT64_C(0x00040000)	/* LV */
+
 #define LV_NOTSYNCED		UINT64_C(0x00080000)	/* LV */
+#define LV_REBUILD		UINT64_C(0x00100000)	/* LV */
 //#define PRECOMMITTED		UINT64_C(0x00200000)	/* VG - internal use only */
 #define CONVERTING		UINT64_C(0x00400000)	/* LV */
 

Index: LVM2/lib/metadata/raid_manip.c
===================================================================

lib/metadata/raid_manip.c +4 -4

@@ -440,7 +440,7 @@ static int _alloc_image_component(struct
 		return 0;
 	}
 
-	status = LVM_READ | LVM_WRITE | LV_NOTSYNCED | type;
+	status = LVM_READ | LVM_WRITE | LV_REBUILD | type;
 	tmp_lv = lv_create_empty(img_name, NULL, status, ALLOC_INHERIT, lv->vg);
 	if (!tmp_lv) {
 		log_error("Failed to allocate new raid component, %s", img_name);

@@ -588,7 +588,7 @@ static int _raid_add_images(struct logic
 	 */
 	if (seg_is_linear(seg)) {
 		/* A complete resync will be done, no need to mark each sub-lv */
-		status_mask = ~(LV_NOTSYNCED);
+		status_mask = ~(LV_REBUILD);
 
 		if (!(lvl = dm_pool_alloc(lv->vg->vgmem, sizeof(*lvl)))) {
 			log_error("Memory allocation failed");

@@ -1335,8 +1335,8 @@ static int _convert_mirror_to_raid1(stru
 		log_debug("Adding %s to %s", lvl->lv->name, lv->name);
 
 		/* Images are known to be in-sync */
-		lvl->lv->status &= ~LV_NOTSYNCED;
-		first_seg(lvl->lv)->status &= ~LV_NOTSYNCED;
+		lvl->lv->status &= ~LV_REBUILD;
+		first_seg(lvl->lv)->status &= ~LV_REBUILD;
 		lv_set_hidden(lvl->lv);
 
 		if (!set_lv_segment_area_lv(seg, s, lvl->lv, 0,

Index: LVM2/lib/raid/raid.c
===================================================================

lib/raid/raid.c +1 -2

@@ -183,7 +183,7 @@ static int _raid_add_target_line(struct 
 	}
 
 	for (s = 0; s < seg->area_count; s++)
-		if (seg_lv(seg, s)->status & LV_NOTSYNCED)
+		if (seg_lv(seg, s)->status & LV_REBUILD)
 			rebuilds |= 1 << s;
 
 	if (!dm_tree_node_add_raid_target(node, len, _raid_name(seg),
 
 
--

lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

Dne 10.11.2011 23:26, Jonathan Brassow napsal(a):

Quoted 12 lines

Any objections to a new flag?

brassow

Add new flag, LV_REBUILD.

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

Quoted 1 line

This patch proposes and uses a new flag, LV_REBUILD.

Hmm and how is this going to work in some crash scenarios - are you always able to deduce the raid was not yet build ?

Also it seems we encode each flag into lib/format_text/flags.c

Changes from previous:
- New rebuild flag is now written to on-disk LVM metadata
- An additional metadata write/commit is necessary to clear the flag after a proper resume
- flags.c file updated

brassow

Add new flag, LV_REBUILD.

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

This patch proposes and uses a new flag, LV_REBUILD.

Dne 14.11.2011 19:06, Jonathan Brassow napsal(a):

Quoted 16 lines

Changes from previous:
- New rebuild flag is now written to on-disk LVM metadata
- An additional metadata write/commit is necessary to clear the flag after a proper resume
- flags.c file updated

brassow

Add new flag, LV_REBUILD.

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

Quoted 1 line

This patch proposes and uses a new flag, LV_REBUILD.

Hmm so now - when it's written to disk - it looks like LV_REBUILD and LV_NOTSYNCED is mostly the same meaning - except one is used in raid and original in mirror ?

So I think it's easier to keep just one flag ?
(Well the bit field space is somewhat limited)

Quoted 22 lines · Zdenek Kabelac

Dne 14.11.2011 19:06, Jonathan Brassow napsal(a):

Quoted 10 lines

Changes from previous:
- New rebuild flag is now written to on-disk LVM metadata
- An additional metadata write/commit is necessary to clear the flag after a proper resume
- flags.c file updated

brassow

Add new flag, LV_REBUILD.

Quoted 7 lines

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

This patch proposes and uses a new flag, LV_REBUILD.

Hmm so now - when it's written to disk - it looks like LV_REBUILD and LV_NOTSYNCED is mostly the same meaning - except one is used in raid and original in mirror ?

Quoted 2 lines

So I think it's easier to keep just one flag ?
(Well the bit field space is somewhat limited)

Perhaps. I've considered this and certainly it'd be easy for me to leave things the way they are. However, LV_NOTSYNCED means that the LV was created (or extended) with '--nosync', and signifies that it has not and will not undergo a complete synchronization. LV_REBUILD, on the other hand, means precisely the opposite - the LV is to undergo a complete synchronization now. LV_NOTSYNCED signifies an attribute of the LV, while LV_REBUILD signifies an action that needs to be taken.

The only reason the difference is not more obvious is because you can look at 'LV_NOTSYNCED' sideways and sort-of make it work. "These sub-LVs are not in-sync right now, and must be made to be in-sync." - or some similar thinking could get you there.

I'm good either way, but I'd prefer the new flag...

Quoted 31 lines · Zdenek Kabelac

Quoted 18 lines

Dne 14.11.2011 19:06, Jonathan Brassow napsal(a):

Quoted 16 lines

Changes from previous:
- New rebuild flag is now written to on-disk LVM metadata
- An additional metadata write/commit is necessary to clear the flag after a proper resume
- flags.c file updated

brassow

Add new flag, LV_REBUILD.

Until now, I had been using the LV_NOTSYNCED as a flag to indicate that RAID sub-LVs needed to be rebuilt. (The 'rebuild' parameter is then specified in the DM CTR table.) However, I don't want to use a flag that gets written to the LVM metadata... and the LV_NOTSYNCED flag's original meaning does not suite the purpose adequately.

Quoted 7 lines

Quoted 1 line

This patch proposes and uses a new flag, LV_REBUILD.

Hmm so now - when it's written to disk - it looks like LV_REBUILD and LV_NOTSYNCED is mostly the same meaning - except one is used in raid and original in mirror ?

So I think it's easier to keep just one flag ?
(Well the bit field space is somewhat limited)

Perhaps. I've considered this and certainly it'd be easy for me to leave things the way they are. However, LV_NOTSYNCED means that the LV was created (or extended) with '--nosync', and signifies that it has not and will not undergo a complete synchronization. LV_REBUILD, on the other hand, means precisely the opposite - the LV is to undergo a complete synchronization now. LV_NOTSYNCED signifies an attribute of the LV, while LV_REBUILD signifies an action that needs to be taken.

Quoted 3 lines

The only reason the difference is not more obvious is because you can look at 'LV_NOTSYNCED' sideways and sort-of make it work. "These sub-LVs are not in-sync right now, and must be made to be in-sync." - or some similar thinking could get you there.

I'm good either way, but I'd prefer the new flag...

Now that I think a little more on it, I'm becoming even more in favor of the separate flag.

Consider the case we discussed earlier, where the machine dies after the vg_commit but before the resume. This leaves the 'rebuild' DM table argument yet to be supplied to the kernel in order to clear the device's bitmap. When the machine comes up and it is required to clean-up the flags after activation, it would complicate the process to force the code to know when the LV_NOTSYNCED flag means that the LV was not synced and to leave the flag, or whether it means the LV needs to be synced/rebuilt and the flag must be cleared.