[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [lvm-devel] [PATCH 1/11] lvconvert takes '--wait' instead of '--background'



Alasdair G Kergon wrote:
> On Fri, Jan 11, 2008 at 06:43:56PM -0500, Jun'ichi Nomura wrote:
>> Remove '--background' option and add '--wait' option to lvconvert.
>  
>> The option was inherited from pvmove.
>> However, since lvconvert used to exit immediately and it still does
>> for different types of conversion (e.g. linear to mirror conversion),
>> it's more consistent and intuitive to add '--wait' option for foreground
>> polling and do it in background by default.
>  
> I'm not yet persuaded about the need for this change.
> 
> The aim is that the behaviour of the tools be consistent across
> the whole command set.
> 
> Waiting for the operation to complete feels more intuitive to me.

OK. I didn't want to change the existing behavior of lvconvert.
But I agree the synchronous behavior is better choice for default case.

Attached is a patch to fix the inconsistency issue in the other way.
If it looks acceptable, please replace patches "1/11" and "14/14"
with this. Other patches won't be affected.

By default: lvconvert waits for initial sync

  Add a mirror to linear
    # lvs -o+stripes vg/lvol0; lvconvert -m+1 vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert #Str
      lvol0 vg   -wi-a- 1.00G                                          1
      vg/lvol0: Converted: 18.8%
      vg/lvol0: Converted: 39.6%
      vg/lvol0: Converted: 61.7%
      vg/lvol0: Converted: 83.6%
      vg/lvol0: Converted: 100.0%
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            2

  Add a mirror to mirror
    # lvs -o+stripes vg/lvol0; lvconvert -m+1 vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            2
      vg/lvol0: Converted: 53.7%
      vg/lvol0: Converted: 76.8%
      vg/lvol0: Converted: 99.8%
      vg/lvol0: Converted: 100.0%
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            3

  Remove mirror
    # lvs -o+stripes vg/lvol0; lvconvert -m-1 vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            3
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            2
    # lvs -o+stripes vg/lvol0; lvconvert -m-1 vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog 100.00            2
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert #Str
      lvol0 vg   -wi-a- 1.00G

With background option (-b):

  Add a mirror to linear
    # lvs -o+stripes vg/lvol0; lvconvert -m+1 -b vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert #Str
      lvol0 vg   -wi-a- 1.00G                                          1
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog   0.59            2

  Add a mirror to mirror
    # lvs -o+stripes vg/lvol0; lvconvert -m+1 -b vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog  11.52            2
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert           #Str
      lvol0 vg   cwi-a- 1.00G                          0.39 lvol0_mimagetmp_2    2

  Remove mirror
    # lvs -o+stripes vg/lvol0; lvconvert -m-1 -b vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert           #Str
      lvol0 vg   cwi-a- 1.00G                         17.77 lvol0_mimagetmp_2    2
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog  31.45            2
    # lvs -o+stripes vg/lvol0; lvconvert -m-1 -b vg/lvol0; lvs -o+stripes vg/lvol0
      LV    VG   Attr   LSize Origin Snap%  Move Log        Copy%  Convert #Str
      lvol0 vg   mwi-a- 1.00G                    lvol0_mlog  34.57            2
      Logical volume lvol0 converted.
      LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert #Str
      lvol0 vg   -wi-a- 1.00G                                          1

-- 
Jun'ichi Nomura, NEC Corporation of America
lvconvert to wait for mirror sync completion,
regardless of the necessity of collapsing.

For readability of the code, split the internal param 'wait_daemon'
to 'wait_completion' and 'need_polling'.
'wait_completion' represents an intent of the user whether he/she
waits to wait for the conversion to complete.
'need_polling' represents conversion specific requirement whether
the conversion needs help from polldaemon to check the completion.

Index: LVM2.work/tools/lvconvert.c
===================================================================
--- LVM2.work.orig/tools/lvconvert.c
+++ LVM2.work/tools/lvconvert.c
@@ -24,7 +24,8 @@ struct lvconvert_params {
 	const char *lv_name;
 	const char *lv_name_full;
 	const char *vg_name;
-	int wait_daemon;
+	int wait_completion;
+	int need_polling;
 
 	uint32_t chunk_size;
 	uint32_t region_size;
@@ -116,6 +117,9 @@ static int _read_params(struct lvconvert
 		return 0;
 	}
 
+	if (!arg_count(cmd, background_ARG))
+		lp->wait_completion = 1;
+
 	if (arg_count(cmd, snapshot_ARG))
 		lp->snapshot = 1;
 
@@ -386,7 +390,7 @@ static int lvconvert_mirrors(struct cmd_
 	/* If called with no argument, try collapsing the resync layers */
 	if (!arg_count(cmd, mirrors_ARG) && !arg_count(cmd, mirrorlog_ARG) &&
 	    !arg_count(cmd, corelog_ARG)) {
-		lp->wait_daemon = 1;
+		lp->need_polling = 1;
 		return 1;
 	}
 
@@ -488,6 +492,8 @@ static int lvconvert_mirrors(struct cmd_
 				    corelog ? 0U : 1U, lp->pvh, lp->alloc,
 				    MIRROR_BY_LV))
 			return_0;
+		if (lp->wait_completion)
+			lp->need_polling = 1;
 		goto commit_changes;
 	}
 
@@ -563,7 +569,7 @@ static int lvconvert_mirrors(struct cmd_
 				    MIRROR_BY_LV))
 			return_0;
 		lv->status |= CONVERTING;
-		lp->wait_daemon = 1;
+		lp->need_polling = 1;
 	} else {
 		/* Reduce number of mirrors */
 		if (!lv_remove_mirrors(cmd, lv, existing_mirrors - lp->mirrors,
@@ -598,7 +604,7 @@ commit_changes:
 		return 0;
 	}
 
-	if (!lp->wait_daemon)
+	if (!lp->need_polling)
 		log_print("Logical volume %s converted.", lv->name);
 
 	return 1;
@@ -754,13 +760,13 @@ int lvconvert(struct cmd_context * cmd, 
 error:
 	unlock_vg(cmd, lp.vg_name);
 
-	if (ret == ECMD_PROCESSED && lp.wait_daemon) {
+	if (ret == ECMD_PROCESSED && lp.need_polling) {
 		if (!lv_info(cmd, lvl->lv, &info, 1, 0) || !info.exists) {
 			log_print("Conversion starts after activation");
 			return ret;
 		}
 		ret = lvconvert_poll(cmd, lp.lv_name_full,
-				     arg_count(cmd, background_ARG) ? 1U : 0);
+				     lp.wait_completion ? 0 : 1U);
 	}
 
 	return ret;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]