From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51857 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PxMnz-0005Ge-5m for qemu-devel@nongnu.org; Wed, 09 Mar 2011 12:04:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PxMnp-0002EL-Cf for qemu-devel@nongnu.org; Wed, 09 Mar 2011 12:04:03 -0500 Received: from mail-gw0-f45.google.com ([74.125.83.45]:63560) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PxMnp-0002E3-4V for qemu-devel@nongnu.org; Wed, 09 Mar 2011 12:04:01 -0500 Received: by gwb19 with SMTP id 19so90981gwb.4 for ; Wed, 09 Mar 2011 09:04:00 -0800 (PST) Message-ID: <4D77B2FD.3050304@codemonkey.ws> Date: Wed, 09 Mar 2011 11:03:57 -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> <4D77A0EA.4060507@codemonkey.ws> <4D77A44C.5090904@redhat.com> In-Reply-To: <4D77A44C.5090904@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: Kevin Wolf Cc: Jes.Sorensen@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com On 03/09/2011 10:01 AM, Kevin Wolf wrote: > Am 09.03.2011 16:46, schrieb Anthony Liguori: >> 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). > Doesn't this command work on a -blockdev style thing, i.e. > BlockDriverState or DriveInfo? I don't think we have any commands that > refer to qdev devices that happen to be block devices. You could > probably argue that some of them should... 'device' is a device name though, right? Or is it a name associated with a BlockDriverState that currently happens to be a qdev name? Would I be able to eventually pass a qdev path here? If the answer is that this is a bdrv_name, then should we at least use blockdev instead of blkdev? Regards, Anthony Liguori > Kevin >