From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38955 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PxLbK-0000Qs-LX for qemu-devel@nongnu.org; Wed, 09 Mar 2011 10:47:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PxLbD-00005J-E9 for qemu-devel@nongnu.org; Wed, 09 Mar 2011 10:47:02 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:56317) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PxLbD-00005B-9o for qemu-devel@nongnu.org; Wed, 09 Mar 2011 10:46:55 -0500 Received: by iym7 with SMTP id 7so750135iym.4 for ; Wed, 09 Mar 2011 07:46:54 -0800 (PST) Message-ID: <4D77A0EA.4060507@codemonkey.ws> Date: Wed, 09 Mar 2011 09:46:50 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command References: <1299685061-24234-1-git-send-email-Jes.Sorensen@redhat.com> In-Reply-To: <1299685061-24234-1-git-send-email-Jes.Sorensen@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes.Sorensen@redhat.com Cc: Kevin Wolf , qemu-devel@nongnu.org, lcapitulino@redhat.com On 03/09/2011 09:37 AM, Jes.Sorensen@redhat.com wrote: > From: Jes Sorensen > > Add QMP bits for snapshot_blkdev_sync command. This is the same as > snapshot_blkdev in the human monitor, but added _sync to the name to > make it explicit that the command is synchronous and leave space for a > future async version. > > Signed-off-by: Jes Sorensen > --- > qmp-commands.hx | 19 +++++++++++++++++++ > 1 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 9d3cc31..e32187e 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -667,6 +667,25 @@ Example: > EQMP > > { > + .name = "snapshot_blkdev_sync", > + .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)", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = do_snapshot_blkdev, > + }, > + > +SQMP > +Synchronous snapshot of block device, using snapshot file as target > +if provided. Please document the error semantics. The documentation in .help is discarded for QMP. You should put the docs in the SQMP section. Also, QMP should use '-' instead of '_'. We should also try to follow the form 'noun'-'verb' so the name would be better as 'blkdev-snapshot-sync' I'm not sure blkdev is the right prefix. Kevin, what are your thoughts here? Does 'blkdev' make sense for any command operating on a block device (that is, a qdev device that happens to have a block drive, not the same thing as -blockdev that we've discussed in the past). Regards, Anthony Liguori > +EQMP > + > + { > .name = "balloon", > .args_type = "value:M", > .params = "target",