* [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
@ 2011-03-09 15:37 Jes.Sorensen
2011-03-09 15:46 ` Anthony Liguori
0 siblings, 1 reply; 6+ messages in thread
From: Jes.Sorensen @ 2011-03-09 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
From: Jes Sorensen <Jes.Sorensen@redhat.com>
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 <Jes.Sorensen@redhat.com>
---
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.
+EQMP
+
+ {
.name = "balloon",
.args_type = "value:M",
.params = "target",
--
1.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
2011-03-09 15:37 [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command Jes.Sorensen
@ 2011-03-09 15:46 ` Anthony Liguori
2011-03-09 16:01 ` Kevin Wolf
0 siblings, 1 reply; 6+ messages in thread
From: Anthony Liguori @ 2011-03-09 15:46 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: Kevin Wolf, qemu-devel, lcapitulino
On 03/09/2011 09:37 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> 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<Jes.Sorensen@redhat.com>
> ---
> 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",
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
2011-03-09 15:46 ` Anthony Liguori
@ 2011-03-09 16:01 ` Kevin Wolf
2011-03-09 17:03 ` Anthony Liguori
0 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2011-03-09 16:01 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Jes.Sorensen, qemu-devel, lcapitulino
Am 09.03.2011 16:46, schrieb Anthony Liguori:
> On 03/09/2011 09:37 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> 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<Jes.Sorensen@redhat.com>
>> ---
>> 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...
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
2011-03-09 16:01 ` Kevin Wolf
@ 2011-03-09 17:03 ` Anthony Liguori
2011-03-10 9:10 ` Jes Sorensen
2011-03-10 9:21 ` Kevin Wolf
0 siblings, 2 replies; 6+ messages in thread
From: Anthony Liguori @ 2011-03-09 17:03 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Jes.Sorensen, qemu-devel, lcapitulino
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<Jes.Sorensen@redhat.com>
>>>
>>> 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<Jes.Sorensen@redhat.com>
>>> ---
>>> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
2011-03-09 17:03 ` Anthony Liguori
@ 2011-03-10 9:10 ` Jes Sorensen
2011-03-10 9:21 ` Kevin Wolf
1 sibling, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2011-03-10 9:10 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel, lcapitulino
On 03/09/11 18:03, Anthony Liguori wrote:
> On 03/09/2011 10:01 AM, Kevin Wolf wrote:
>>> 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?
It is a device in the sense of what you get from 'info block' in the
human monitor.
> 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 used blkdev in the human monitor command to reduce the length of the
command name.
Anyway, if you can decide what you want to call the command, I'll rename
it accordingly.
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
2011-03-09 17:03 ` Anthony Liguori
2011-03-10 9:10 ` Jes Sorensen
@ 2011-03-10 9:21 ` Kevin Wolf
1 sibling, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2011-03-10 9:21 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Jes.Sorensen, qemu-devel, Markus Armbruster, lcapitulino
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, Jes.Sorensen@redhat.com wrote:
>>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>>
>>>> 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<Jes.Sorensen@redhat.com>
>>>> ---
>>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-10 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-09 15:37 [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command Jes.Sorensen
2011-03-09 15:46 ` Anthony Liguori
2011-03-09 16:01 ` Kevin Wolf
2011-03-09 17:03 ` Anthony Liguori
2011-03-10 9:10 ` Jes Sorensen
2011-03-10 9:21 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).