Patchwork [V3,3/7] qemu-nbd: add doc for internal snapshot export

login
register
mail settings
Submitter Wayne Xia
Date Sept. 26, 2013, 12:16 a.m.
Message ID <1380154568-5339-4-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/278049/
State New
Headers show

Comments

Wayne Xia - Sept. 26, 2013, 12:16 a.m.
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(-)
Eric Blake - Oct. 1, 2013, 2:49 p.m.
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.
Wayne Xia - Oct. 10, 2013, 6:04 a.m.
于 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.

Patch

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