Re: [PATCH 4/5] kpartx: default to running in sync mode

[Martin Wilck <mwilck@suse.com>]

Hi Ben,

On Mon, 2017-04-24 at 17:39 -0500, Benjamin Marzinski wrote:

> When users run kpartx, they would naturally assume that when it
> completes, the devices have been created. However, kpartx runs in
> async
> mode by default.  This seems like it is likely to trip up users. 

Is this just likely, or do you have evidence that it really did
irritate users? You introduced "sync mode", together with the "-s"
flag, yourself in 2010, and unless I'm mistaken, kpartx operated in
"async" mode before that, too, because there was no "sync" mode. 

I'm not too fond of the idea to change a default which has been
established for such a long time just because user mistakes are
"likely". For one thing, such changes are difficult to document in a
way that doesn't cause confusion. Also, if someone writes a script in
which she wants kpartx to run asynchronously, it will be difficult to
do so in a manner which is portable between distributions if some
distributions include this patch and some don't.

As an alternative, we might simply warn about the default asynchronous
behavior in a prominent place in the man page.

Cheers,
Martin

>  So,
> switch the default to sync mode, add a -n option to enable async
> mode,
> and set async mode when kpartx is called by the udev rules.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/kpartx.c     | 10 +++++++---
>  kpartx/kpartx.rules |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index 58e60ff..d1edd5e 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
> @@ -58,7 +58,7 @@ struct pt {
>  } pts[MAXTYPES];
>  
>  int ptct = 0;
> -int udev_sync = 0;
> +int udev_sync = 1;
>  
>  static void
>  addpts(char *t, ptreader f)
> @@ -86,7 +86,7 @@ initpts(void)
>  	addpts("ps3", read_ps3_pt);
>  }
>  
> -static char short_opts[] = "rladfgvp:t:su";
> +static char short_opts[] = "rladfgvp:t:snu";
>  
>  /* Used in gpt.c */
>  int force_gpt=0;
> @@ -105,7 +105,8 @@ usage(void) {
>  	printf("\t-g force GUID partition table (GPT)\n");
>  	printf("\t-f force devmap create\n");
>  	printf("\t-v verbose\n");
> -	printf("\t-s sync mode. Don't return until the partitions
> are created\n");
> +	printf("\t-n nosync mode. Return before the partitions are
> created\n");
> +	printf("\t-s sync mode. Don't return until the partitions
> are created. Default.\n");
>  	return 1;
>  }
>  
> @@ -291,6 +292,9 @@ main(int argc, char **argv){
>  		case 's':
>  			udev_sync = 1;
>  			break;
> +		case 'n':
> +			udev_sync = 0;
> +			break;
>  		case 'u':
>  			what = UPDATE;
>  			break;
> diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> index 48a4d6c..a958791 100644
> --- a/kpartx/kpartx.rules
> +++ b/kpartx/kpartx.rules
> @@ -40,6 +40,6 @@ ENV{DM_NR_VALID_PATHS}=="0", GOTO="kpartx_end"
>  ENV{ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1",
> IMPORT{db}="DM_SUBSYSTEM_UDEV_FLAG1"
>  ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="kpartx_end"
>  ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
> -	RUN+="/sbin/kpartx -u -p -part /dev/$name"
> +	RUN+="/sbin/kpartx -un -p -part /dev/$name"
>  
>  LABEL="kpartx_end"

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel