| Message ID | 1380154568-5339-4-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 09/25/2013 06:16 PM, Wenchao Xia wrote: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > qemu-nbd.c | 11 ++++++++++- > qemu-nbd.texi | 11 ++++++++++- > 2 files changed, 20 insertions(+), 2 deletions(-) This should be squashed into 2/7. When adding new options, the documentation should be added at the same time. > +" the temporary one\n" > +" -l, --load-snapshot=SNAPSHOT_ID_OR_NAME\n" > +" load an internal snapshot inside FILE and export it\n" > +" as an read-only device\n" > +" -L, --load-snapshot1=SNAPSHOT_PARAM\n" > +" load an internal snapshot inside FILE and export it\n" > +" as an read-only device, SNAPSHOT_PARAM format is\n" > +" 'snapshot.id=[ID],snapshot.name=[NAME]'\n" Why can't ONE option be good enough? In other words, make the command line parser smart enough so that: --load-snapshot=name tries SNAPSHOT_ID_OR_NAME, while --load-snapshot=snapshot.id=xyz,snapshot.name=name tries the SNAPSHOT_PARAM form. In other words, if the optarg begins with 'snapshot.', assume the SNAPSHOT_PARAM form, otherwise use the SNAPSHOT_ID_OR_NAME form. Then you only burn one short option letter, and avoid the problem with ambiguous abbreviation that I complained about in 2/7.
δΊ 2013/10/1 22:49, Eric Blake ει: > On 09/25/2013 06:16 PM, Wenchao Xia wrote: >> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com> >> --- >> qemu-nbd.c | 11 ++++++++++- >> qemu-nbd.texi | 11 ++++++++++- >> 2 files changed, 20 insertions(+), 2 deletions(-) > This should be squashed into 2/7. When adding new options, the > documentation should be added at the same time. > OK. >> +" the temporary one\n" >> +" -l, --load-snapshot=SNAPSHOT_ID_OR_NAME\n" >> +" load an internal snapshot inside FILE and export it\n" >> +" as an read-only device\n" >> +" -L, --load-snapshot1=SNAPSHOT_PARAM\n" >> +" load an internal snapshot inside FILE and export it\n" >> +" as an read-only device, SNAPSHOT_PARAM format is\n" >> +" 'snapshot.id=[ID],snapshot.name=[NAME]'\n" > Why can't ONE option be good enough? In other words, make the command > line parser smart enough so that: > > --load-snapshot=name > > tries SNAPSHOT_ID_OR_NAME, while > > --load-snapshot=snapshot.id=xyz,snapshot.name=name > > tries the SNAPSHOT_PARAM form. In other words, if the optarg begins > with 'snapshot.', assume the SNAPSHOT_PARAM form, otherwise use the > SNAPSHOT_ID_OR_NAME form. Then you only burn one short option letter, > and avoid the problem with ambiguous abbreviation that I complained > about in 2/7. > I split the option as two item since want to keep capatiability for "-s snapshot.id=xyz" in qemu-img convert, it is possible some one already named a snapshot as "snapshot.id=xyz". But from the comments of Paolo, I think add a new option in qemu-img convert and deprecate -s, can solve the problem, so I will use your format in next version, thanks for tipping that.
diff --git a/qemu-nbd.c b/qemu-nbd.c index 6588a1f..49dfc14 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -80,7 +80,16 @@ static void usage(const char *name) "\n" "Block device options:\n" " -r, --read-only export read-only\n" -" -s, --snapshot use snapshot file\n" +" -s, --snapshot use FILE as an external snapshot, create a temporary\n" +" file with backing_file=FILE, redirect the write to\n" +" the temporary one\n" +" -l, --load-snapshot=SNAPSHOT_ID_OR_NAME\n" +" load an internal snapshot inside FILE and export it\n" +" as an read-only device\n" +" -L, --load-snapshot1=SNAPSHOT_PARAM\n" +" load an internal snapshot inside FILE and export it\n" +" as an read-only device, SNAPSHOT_PARAM format is\n" +" 'snapshot.id=[ID],snapshot.name=[NAME]'\n" " -n, --nocache disable host cache\n" " --cache=MODE set cache mode (none, writeback, ...)\n" #ifdef CONFIG_LINUX_AIO diff --git a/qemu-nbd.texi b/qemu-nbd.texi index 6055ec6..5940905 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -27,7 +27,16 @@ Export QEMU disk image using NBD protocol. @item -P, --partition=@var{num} only expose partition @var{num} @item -s, --snapshot - use snapshot file + use @var{filename} as an external snapshot, create a temporary + file with backing_file=@var{filename}, redirect the write to + the temporary one +@item -l, --load-snapshot=@var{snapshot_id_or_name} + load an internal snapshot inside @var{filename} and export it + as an read-only device +@item -L, --load-snapshot1=@var{snapshot_param} + load an internal snapshot inside @var{filename} and export it + as an read-only device, @var{snapshot_param} format is + 'snapshot.id=[ID],snapshot.name=[NAME]' @item -n, --nocache @itemx --cache=@var{cache} set cache mode to be used with the file. See the documentation of
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- qemu-nbd.c | 11 ++++++++++- qemu-nbd.texi | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-)