From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QgKkf-0005Qo-Cn for qemu-devel@nongnu.org; Mon, 11 Jul 2011 13:58:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QgKkd-0004eE-MQ for qemu-devel@nongnu.org; Mon, 11 Jul 2011 13:58:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15683) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QgKkd-0004e6-59 for qemu-devel@nongnu.org; Mon, 11 Jul 2011 13:58:35 -0400 Date: Mon, 11 Jul 2011 14:58:29 -0300 From: Luiz Capitulino Message-ID: <20110711145829.49bdf857@doriath> In-Reply-To: <4E1B37F5.9010003@redhat.com> References: <1310126413-13451-1-git-send-email-Jes.Sorensen@redhat.com> <1310126413-13451-2-git-send-email-Jes.Sorensen@redhat.com> <20110711133539.5442d454@doriath> <4E1B37F5.9010003@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: qemu-devel@nongnu.org On Mon, 11 Jul 2011 19:50:45 +0200 Jes Sorensen wrote: > On 07/11/11 18:35, Luiz Capitulino wrote: > > On Fri, 8 Jul 2011 14:00:13 +0200 > > Jes.Sorensen@redhat.com wrote: > > > >> From: Jes Sorensen > >> > >> 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 > >> --- > >> 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.