[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command |
Date: |
Mon, 11 Jul 2011 14:58:29 -0300 |
On Mon, 11 Jul 2011 19:50:45 +0200
Jes Sorensen <address@hidden> wrote:
> On 07/11/11 18:35, Luiz Capitulino wrote:
> > On Fri, 8 Jul 2011 14:00:13 +0200
> > address@hidden wrote:
> >
> >> From: Jes Sorensen <address@hidden>
> >>
> >> Add QMP bits for snapshot_blkdev command. This is the same as
> >> snapshot_blkdev in the human monitor. The command is synchronous.
> >>
> >> In the future async commands and or a break down of the functionality
> >> into multiple commands might be added.
> >>
> >> Signed-off-by: Jes Sorensen <address@hidden>
> >> ---
> >> qmp-commands.hx | 38 ++++++++++++++++++++++++++++++++++++++
> >> 1 files changed, 38 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 92c5c3a..eb135c1 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -694,6 +694,44 @@ Example:
> >> EQMP
> >>
> >> {
> >> + .name = "blockdev-snapshot",
> >
> > blockdev-snapshot-sync.
>
> argh, will fix
>
> >> + .args_type = "device:B,snapshot_file:s?,format:s?",
> >> + .params = "device [new-image-file] [format]",
> >> + .help = "initiates a live snapshot\n\t\t\t"
> >> + "of device. If a new image file is specified,
> >> the\n\t\t\t"
> >> + "new image file will become the new root
> >> image.\n\t\t\t"
> >> + "If format is specified, the snapshot file
> >> will\n\t\t\t"
> >> + "be created in that format. Otherwise the\n\t\t\t"
> >> + "snapshot will be internal! (currently
> >> unsupported)",
> >
> > The 'otherwise' part is a bit confusing. You document something as if it
> > were
> > supported then you say it's not supported. I recommend to just not document
> > it
> > instead.
>
> Ok
>
> >> + .user_print = monitor_user_noop,
> >> + .mhandler.cmd_new = do_snapshot_blkdev,
> >> + },
> >> +
> >> +SQMP
> >> +blockdev-snapshot-sync
> >> +----------------------
> >> +
> >> +Synchronous snapshot of block device, using snapshot file as target,
> >> +if provided.
> >
> > In QMP only this text is used, the text in '.help' is discarded. Please,
> > move all command documentation here.
>
> If .help isn't used, then please remove it from the struct. It is really
> pointless to keep carrying both around.
Yes, sorry for that. It will be completely dropped when we move to the QAPI
(which uses a schema file).
>
> >> +
> >> +Arguments:
> >> +
> >> +- "device": device name to snapshot (json-string)
> >> +- "snapshot_file": name of new image file (json-string)
> >> +- "format": format of new image (json-string)
> >> +
> >> +Example:
> >> +
> >> +-> { "execute": "blockdev-snapshot", "arguments": { "device": "ide-hd0",
> >> + "snapshot_file":
> >
> > We are trying to standardize the use of hyphen in QMP.
>
> Sorry, but I haven't got a clue what you mean here.
The argument is called "snapshot_file" but it should be called snapshot-file.