Am 09.03.2011 18:03, schrieb Anthony Liguori: > 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, [email protected] wrote: >>>> From: Jes Sorensen<[email protected]> >>>> >>>> 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<[email protected]> >>>> --- >>>> 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?
I think it's a drive name, like 'ide-hd0' or what they were called. I'm not completely sure if this is the same namespace as blockdev_add would use, but at first sight it would make some sense. Markus? Kevin
