KVM call agenda for Tuesday 28th

KVM call agenda for Tuesday 28th

[Juan Quintela]

Hi

Please send in any agenda items you are interested in covering.

Cheers,

Juan.

[Qemu-devel] KVM call agenda for Tuesday 28th

[Juan Quintela]

Hi

Please send in any agenda items you are interested in covering.

Cheers,

Juan.

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On 02/27/2012 05:22 AM, Juan Quintela wrote:
> 
> Hi
> 
> Please send in any agenda items you are interested in covering.

Given all the threads on snapshot/mirror/migrate/reopen in the blockdev
layer, that sounds like a worthwhile topic to discuss on a phone call.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 27/02/2012 18:21, Eric Blake ha scritto:
>> > Please send in any agenda items you are interested in covering.
> Given all the threads on snapshot/mirror/migrate/reopen in the blockdev
> layer, that sounds like a worthwhile topic to discuss on a phone call.

I put a description of the existing proposals here:

http://wiki.qemu.org/Features/SnapshotsMultipleDevices/CommandSetProposals

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 27/02/2012 18:21, Eric Blake ha scritto:
>> > Please send in any agenda items you are interested in covering.
> Given all the threads on snapshot/mirror/migrate/reopen in the blockdev
> layer, that sounds like a worthwhile topic to discuss on a phone call.

I put a description of the existing proposals here:

http://wiki.qemu.org/Features/SnapshotsMultipleDevices/CommandSetProposals

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Anthony Liguori]

On 02/27/2012 03:58 PM, Paolo Bonzini wrote:
> Il 27/02/2012 18:21, Eric Blake ha scritto:
>>>> Please send in any agenda items you are interested in covering.
>> Given all the threads on snapshot/mirror/migrate/reopen in the blockdev
>> layer, that sounds like a worthwhile topic to discuss on a phone call.
>
> I put a description of the existing proposals here:
>
> http://wiki.qemu.org/Features/SnapshotsMultipleDevices/CommandSetProposals

Thanks!  One thing I'm having trouble following on your proposal: What commands 
are valid within blockdev-start-transaction/blockdev-commit-transaction?

If I do:

blockdev-start-transaction
stop
drive-reopen
drive-mirror
blockdev-end-transaction

What state should I expect that my guest is in (paused or running)?

Regards,

Anthony Liguori

>
> Paolo
>

Re: KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 27/02/2012 23:06, Anthony Liguori ha scritto:
> 
> Thanks!  One thing I'm having trouble following on your proposal: What
> commands are valid within
> blockdev-start-transaction/blockdev-commit-transaction?
> 
> If I do:
> 
> blockdev-start-transaction
> stop
> drive-reopen
> drive-mirror
> blockdev-end-transaction
> 
> What state should I expect that my guest is in (paused or running)?

Paused.  Only the two new commands and blockdev-snapshot-sync are part
of the transaction (edited the wiki now).

What I like most in Jeff's new command is that it's not even a question.
 On the other hand we have to be sure that we can extend it, and perhaps
change its name already in 1.1...

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 27/02/2012 23:06, Anthony Liguori ha scritto:
> 
> Thanks!  One thing I'm having trouble following on your proposal: What
> commands are valid within
> blockdev-start-transaction/blockdev-commit-transaction?
> 
> If I do:
> 
> blockdev-start-transaction
> stop
> drive-reopen
> drive-mirror
> blockdev-end-transaction
> 
> What state should I expect that my guest is in (paused or running)?

Paused.  Only the two new commands and blockdev-snapshot-sync are part
of the transaction (edited the wiki now).

What I like most in Jeff's new command is that it's not even a question.
 On the other hand we have to be sure that we can extend it, and perhaps
change its name already in 1.1...

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Stefan Hajnoczi]

On Mon, Feb 27, 2012 at 10:06 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/27/2012 03:58 PM, Paolo Bonzini wrote:
>>
>> Il 27/02/2012 18:21, Eric Blake ha scritto:
>>>>>
>>>>> Please send in any agenda items you are interested in covering.
>>>
>>> Given all the threads on snapshot/mirror/migrate/reopen in the blockdev
>>> layer, that sounds like a worthwhile topic to discuss on a phone call.
>>
>>
>> I put a description of the existing proposals here:
>>
>> http://wiki.qemu.org/Features/SnapshotsMultipleDevices/CommandSetProposals
>
>
> Thanks!  One thing I'm having trouble following on your proposal: What
> commands are valid within
> blockdev-start-transaction/blockdev-commit-transaction?
>
> If I do:
>
> blockdev-start-transaction
> stop
> drive-reopen
> drive-mirror
> blockdev-end-transaction
>
> What state should I expect that my guest is in (paused or running)?

I'm not a fan of transactions or freeze/thaw (if used to atomically
perform other commands).

We should not export low-level block device operations so that
external software can micromanage via QMP.  I don't think this is a
good idea because it takes the block device offline and possibly
blocks the VM.  We're reaching a level comparable to an HTTP interface
for acquiring pthread mutex, doing some operations, and then another
HTTP request to unlock it.  This is micromanagement it will create
more problems because we will have to support lots of little API
functions.

I think we're only exposing low level operations because:
1. We haven't designed a block model that works.
2. Therefore, upper layers of the management stack have felt forced to
implement these operations on our behalf.  They want a micromanagement
interface in order to do that.

What we should really do is design the block device model for QEMU:

* What responsibilities does QEMU have for handling image files?  We
seem to go back and forth between file descriptor passing for security
and reopening images while QEMU is running.

* What user-visible operations does it need to support (snapshotting
groups of images, eject/insert media, hotplug disk, etc)?

We can look at existing hypervisors and virtualization APIs as inspiration.

Let's provide high-level commands via QMP and let's do it with -blockdev.

Or if we decide that QEMU shouldn't be in the business of doing these
operations then we need to radically simplify to a model that just
passes file descriptors and freezes/thaws I/O but doesn't do any of
the high-level operations at all.  Right now we have a half-way house
and adding more snapshot/transaction APIs isn't the answer.

Stefan

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
> I'm not a fan of transactions or freeze/thaw (if used to atomically
> perform other commands).
> 
> We should not export low-level block device operations so that
> external software can micromanage via QMP.  I don't think this is a
> good idea because it takes the block device offline and possibly
> blocks the VM.  We're reaching a level comparable to an HTTP interface
> for acquiring pthread mutex, doing some operations, and then another
> HTTP request to unlock it.  This is micromanagement it will create
> more problems because we will have to support lots of little API
> functions.

So you're for extending Jeff's patches to group mirroring etc.?

That's also my favorite one, assuming we can do it in time for 1.1.

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Paolo Bonzini]

Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
> I'm not a fan of transactions or freeze/thaw (if used to atomically
> perform other commands).
> 
> We should not export low-level block device operations so that
> external software can micromanage via QMP.  I don't think this is a
> good idea because it takes the block device offline and possibly
> blocks the VM.  We're reaching a level comparable to an HTTP interface
> for acquiring pthread mutex, doing some operations, and then another
> HTTP request to unlock it.  This is micromanagement it will create
> more problems because we will have to support lots of little API
> functions.

So you're for extending Jeff's patches to group mirroring etc.?

That's also my favorite one, assuming we can do it in time for 1.1.

Paolo

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Stefan Hajnoczi]

On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>> perform other commands).
>>
>> We should not export low-level block device operations so that
>> external software can micromanage via QMP.  I don't think this is a
>> good idea because it takes the block device offline and possibly
>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>> for acquiring pthread mutex, doing some operations, and then another
>> HTTP request to unlock it.  This is micromanagement it will create
>> more problems because we will have to support lots of little API
>> functions.
>
> So you're for extending Jeff's patches to group mirroring etc.?
>
> That's also my favorite one, assuming we can do it in time for 1.1.

Yes, that's the approach I like the most.  It's relatively clean and
leaves us space to develop -blockdev.

Stefan

Re: [Qemu-devel] KVM call agenda for Tuesday 28th

[Stefan Hajnoczi]

On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>> perform other commands).
>>
>> We should not export low-level block device operations so that
>> external software can micromanage via QMP.  I don't think this is a
>> good idea because it takes the block device offline and possibly
>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>> for acquiring pthread mutex, doing some operations, and then another
>> HTTP request to unlock it.  This is micromanagement it will create
>> more problems because we will have to support lots of little API
>> functions.
>
> So you're for extending Jeff's patches to group mirroring etc.?
>
> That's also my favorite one, assuming we can do it in time for 1.1.

Yes, that's the approach I like the most.  It's relatively clean and
leaves us space to develop -blockdev.

Stefan

blockdev operations [was: [Qemu-devel] KVM call agenda for Tuesday 28th]

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3417 bytes --]

On 02/28/2012 07:58 AM, Stefan Hajnoczi wrote:
> On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>>> perform other commands).
>>>
>>> We should not export low-level block device operations so that
>>> external software can micromanage via QMP.  I don't think this is a
>>> good idea because it takes the block device offline and possibly
>>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>>> for acquiring pthread mutex, doing some operations, and then another
>>> HTTP request to unlock it.  This is micromanagement it will create
>>> more problems because we will have to support lots of little API
>>> functions.
>>
>> So you're for extending Jeff's patches to group mirroring etc.?
>>
>> That's also my favorite one, assuming we can do it in time for 1.1.
> 
> Yes, that's the approach I like the most.  It's relatively clean and
> leaves us space to develop -blockdev.

Here's the idea I was forming based on today's call:

Jeff's idea of a group operation can be extended to allow multiple
operations while reusing the framework.  For oVirt, we need the ability
to open a mirror (by passing the mirror file alongside the name of the
new external snapshot), as well as reopening a blockdev (to pivot to the
other side of an already-open mirror).

Is there a way to express a designated union in QMP?  I'm thinking
something along the lines of having the overall group command take a
list of operations, where each operation can either be 'create a
snapshot', 'create a snapshot and mirror', or 'reopen a mirror'.

I'm thinking it might look something like:

{ 'enum': 'BlockdevOp',
  'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
{ 'type': 'BlockdevAction',
  'data': {'device': 'str', 'op': 'BlockdevOp',
           'file': 'str', '*format': 'str', '*reuse': 'bool',
           '*mirror': 'str', '*mirror-format': 'str' } }
{ 'command': 'blkdev-group-action-sync',
  'data': { 'actionlist': [ 'BlockdevAction' ] } }


The overall command is atomic - either all operations will succeed, or
the command returns an error pointing to the name of the device that
failed leaving all devices in their pre-command state.  Then, for each
requested operation:

If op is 'snapshot', then 'file' names the new snapshot file; 'reuse' is
optional (defaults to false) to say whether qemu creates the file from
scratch, or opens an existing file with the backing file already
populated correctly.  'format' gives the format of 'file', defaulting to
qcow2.  'mirror' and 'mirror-format' must not be given.

If op is 'snapshot-mirror', then 'mirror' is mandatory; and both 'file'
and 'mirror' are opened as a new mirrored snapshot.  Again, 'reuse'
affects whether qemu creates the new files from scratch or trusts oVirt
to pre-create both files with backing file information; and 'format' and
'mirror-format' allow control over the image format being opened.

If op is 'reopen', then 'file' is the name of the file to be opened to
replace the current file tied to the blockdev, with type given by
'format'.  'reuse', 'mirror', and 'mirror-format' must not be given.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

[Qemu-devel] blockdev operations [was: KVM call agenda for Tuesday 28th]

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3417 bytes --]

On 02/28/2012 07:58 AM, Stefan Hajnoczi wrote:
> On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>>> perform other commands).
>>>
>>> We should not export low-level block device operations so that
>>> external software can micromanage via QMP.  I don't think this is a
>>> good idea because it takes the block device offline and possibly
>>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>>> for acquiring pthread mutex, doing some operations, and then another
>>> HTTP request to unlock it.  This is micromanagement it will create
>>> more problems because we will have to support lots of little API
>>> functions.
>>
>> So you're for extending Jeff's patches to group mirroring etc.?
>>
>> That's also my favorite one, assuming we can do it in time for 1.1.
> 
> Yes, that's the approach I like the most.  It's relatively clean and
> leaves us space to develop -blockdev.

Here's the idea I was forming based on today's call:

Jeff's idea of a group operation can be extended to allow multiple
operations while reusing the framework.  For oVirt, we need the ability
to open a mirror (by passing the mirror file alongside the name of the
new external snapshot), as well as reopening a blockdev (to pivot to the
other side of an already-open mirror).

Is there a way to express a designated union in QMP?  I'm thinking
something along the lines of having the overall group command take a
list of operations, where each operation can either be 'create a
snapshot', 'create a snapshot and mirror', or 'reopen a mirror'.

I'm thinking it might look something like:

{ 'enum': 'BlockdevOp',
  'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
{ 'type': 'BlockdevAction',
  'data': {'device': 'str', 'op': 'BlockdevOp',
           'file': 'str', '*format': 'str', '*reuse': 'bool',
           '*mirror': 'str', '*mirror-format': 'str' } }
{ 'command': 'blkdev-group-action-sync',
  'data': { 'actionlist': [ 'BlockdevAction' ] } }


The overall command is atomic - either all operations will succeed, or
the command returns an error pointing to the name of the device that
failed leaving all devices in their pre-command state.  Then, for each
requested operation:

If op is 'snapshot', then 'file' names the new snapshot file; 'reuse' is
optional (defaults to false) to say whether qemu creates the file from
scratch, or opens an existing file with the backing file already
populated correctly.  'format' gives the format of 'file', defaulting to
qcow2.  'mirror' and 'mirror-format' must not be given.

If op is 'snapshot-mirror', then 'mirror' is mandatory; and both 'file'
and 'mirror' are opened as a new mirrored snapshot.  Again, 'reuse'
affects whether qemu creates the new files from scratch or trusts oVirt
to pre-create both files with backing file information; and 'format' and
'mirror-format' allow control over the image format being opened.

If op is 'reopen', then 'file' is the name of the file to be opened to
replace the current file tied to the blockdev, with type given by
'format'.  'reuse', 'mirror', and 'mirror-format' must not be given.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: blockdev operations [was: [Qemu-devel] KVM call agenda for Tuesday 28th]

[Paolo Bonzini]

Il 28/02/2012 17:07, Eric Blake ha scritto:
> { 'enum': 'BlockdevOp',
>   'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
> { 'type': 'BlockdevAction',
>   'data': {'device': 'str', 'op': 'BlockdevOp',
>            'file': 'str', '*format': 'str', '*reuse': 'bool',
>            '*mirror': 'str', '*mirror-format': 'str' } }
> { 'command': 'blkdev-group-action-sync',
>   'data': { 'actionlist': [ 'BlockdevAction' ] } }
> 
> 
> The overall command is atomic - either all operations will succeed, or
> the command returns an error pointing to the name of the device that
> failed leaving all devices in their pre-command state.  Then, for each
> requested operation:
> 
> If op is 'snapshot', then 'file' names the new snapshot file; 'reuse' is
> optional (defaults to false) to say whether qemu creates the file from
> scratch, or opens an existing file with the backing file already
> populated correctly.  'format' gives the format of 'file', defaulting to
> qcow2.  'mirror' and 'mirror-format' must not be given.
> 
> If op is 'snapshot-mirror', then 'mirror' is mandatory; and both 'file'
> and 'mirror' are opened as a new mirrored snapshot.  Again, 'reuse'
> affects whether qemu creates the new files from scratch or trusts oVirt
> to pre-create both files with backing file information; and 'format' and
> 'mirror-format' allow control over the image format being opened.

Could snapshot-mirror be done as two separate commands for snapshot (or
reopen) and mirror?  This removes the need for mirror and mirror-format.

> If op is 'reopen', then 'file' is the name of the file to be opened to
> replace the current file tied to the blockdev, with type given by
> 'format'.  'reuse', 'mirror', and 'mirror-format' must not be given.

Otherwise looks good.

Paolo

Re: [Qemu-devel] blockdev operations [was: KVM call agenda for Tuesday 28th]

[Paolo Bonzini]

Il 28/02/2012 17:07, Eric Blake ha scritto:
> { 'enum': 'BlockdevOp',
>   'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
> { 'type': 'BlockdevAction',
>   'data': {'device': 'str', 'op': 'BlockdevOp',
>            'file': 'str', '*format': 'str', '*reuse': 'bool',
>            '*mirror': 'str', '*mirror-format': 'str' } }
> { 'command': 'blkdev-group-action-sync',
>   'data': { 'actionlist': [ 'BlockdevAction' ] } }
> 
> 
> The overall command is atomic - either all operations will succeed, or
> the command returns an error pointing to the name of the device that
> failed leaving all devices in their pre-command state.  Then, for each
> requested operation:
> 
> If op is 'snapshot', then 'file' names the new snapshot file; 'reuse' is
> optional (defaults to false) to say whether qemu creates the file from
> scratch, or opens an existing file with the backing file already
> populated correctly.  'format' gives the format of 'file', defaulting to
> qcow2.  'mirror' and 'mirror-format' must not be given.
> 
> If op is 'snapshot-mirror', then 'mirror' is mandatory; and both 'file'
> and 'mirror' are opened as a new mirrored snapshot.  Again, 'reuse'
> affects whether qemu creates the new files from scratch or trusts oVirt
> to pre-create both files with backing file information; and 'format' and
> 'mirror-format' allow control over the image format being opened.

Could snapshot-mirror be done as two separate commands for snapshot (or
reopen) and mirror?  This removes the need for mirror and mirror-format.

> If op is 'reopen', then 'file' is the name of the file to be opened to
> replace the current file tied to the blockdev, with type given by
> 'format'.  'reuse', 'mirror', and 'mirror-format' must not be given.

Otherwise looks good.

Paolo

Re: [Qemu-devel] blockdev operations [was: KVM call agenda for Tuesday 28th]

[Kevin Wolf]

Am 28.02.2012 17:07, schrieb Eric Blake:
> On 02/28/2012 07:58 AM, Stefan Hajnoczi wrote:
>> On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>>>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>>>> perform other commands).
>>>>
>>>> We should not export low-level block device operations so that
>>>> external software can micromanage via QMP.  I don't think this is a
>>>> good idea because it takes the block device offline and possibly
>>>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>>>> for acquiring pthread mutex, doing some operations, and then another
>>>> HTTP request to unlock it.  This is micromanagement it will create
>>>> more problems because we will have to support lots of little API
>>>> functions.
>>>
>>> So you're for extending Jeff's patches to group mirroring etc.?
>>>
>>> That's also my favorite one, assuming we can do it in time for 1.1.
>>
>> Yes, that's the approach I like the most.  It's relatively clean and
>> leaves us space to develop -blockdev.
> 
> Here's the idea I was forming based on today's call:
> 
> Jeff's idea of a group operation can be extended to allow multiple
> operations while reusing the framework.  For oVirt, we need the ability
> to open a mirror (by passing the mirror file alongside the name of the
> new external snapshot), as well as reopening a blockdev (to pivot to the
> other side of an already-open mirror).
> 
> Is there a way to express a designated union in QMP?  I'm thinking
> something along the lines of having the overall group command take a
> list of operations, where each operation can either be 'create a
> snapshot', 'create a snapshot and mirror', or 'reopen a mirror'.
> 
> I'm thinking it might look something like:
> 
> { 'enum': 'BlockdevOp',
>   'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
> { 'type': 'BlockdevAction',
>   'data': {'device': 'str', 'op': 'BlockdevOp',
>            'file': 'str', '*format': 'str', '*reuse': 'bool',
>            '*mirror': 'str', '*mirror-format': 'str' } }
> { 'command': 'blkdev-group-action-sync',
>   'data': { 'actionlist': [ 'BlockdevAction' ] } }

I think the general approach is good.

Your implementation in QAPI is kind of ugly because you mix the
arguments of all three commands in BlockdevAction (how about
extensibility? And the "optional" flags aren't the full truth either,
we'd have to add checks in the handlers.). We really want to have some
real union support in QAPI that creates three different C structs.

So something like (I'll reintroduce the bad word transaction, because
it's really what we're doing here, just everything in one command):

{ 'type': 'BlockdevSnapshot',
  'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
{ 'union': 'BlockdevAction',
  'types': {
       'snapshot': 'BlockdevSnapshot',
       'mirror': 'BlockdevMirror', ...
   } }
{ 'command': 'blockdev-transaction',
  'data': { 'actionlist': [ 'BlockdevAction' ] } }

On the wire in QMP this would look like:

{ 'execute': 'blockdev-transaction',
  'arguments': {
    'actionlist': [
      { 'type': 'snapshot',
        'data': { 'device': 'ide-hd0', ... } },
      { 'type': 'mirror',
        'data': { ... }
    ]
  }
}

Now if you look closely, BlockdevSnapshot is exactly what Jeff called
SnapshotDev in his series, which in turn is the definition of the
blockdev-snapshot-sync command. We can reuse all of this even in the API
of a new more generic command.

So my conclusion for now is that we can merge the group snapshots right
now instead of waiting for the mirror design to be completed, and we can
reuse everything in a more complex transaction command later.

Kevin

Re: [Qemu-devel] blockdev operations [was: KVM call agenda for Tuesday 28th]

[Kevin Wolf]

Am 28.02.2012 17:07, schrieb Eric Blake:
> On 02/28/2012 07:58 AM, Stefan Hajnoczi wrote:
>> On Tue, Feb 28, 2012 at 2:47 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 28/02/2012 15:39, Stefan Hajnoczi ha scritto:
>>>> I'm not a fan of transactions or freeze/thaw (if used to atomically
>>>> perform other commands).
>>>>
>>>> We should not export low-level block device operations so that
>>>> external software can micromanage via QMP.  I don't think this is a
>>>> good idea because it takes the block device offline and possibly
>>>> blocks the VM.  We're reaching a level comparable to an HTTP interface
>>>> for acquiring pthread mutex, doing some operations, and then another
>>>> HTTP request to unlock it.  This is micromanagement it will create
>>>> more problems because we will have to support lots of little API
>>>> functions.
>>>
>>> So you're for extending Jeff's patches to group mirroring etc.?
>>>
>>> That's also my favorite one, assuming we can do it in time for 1.1.
>>
>> Yes, that's the approach I like the most.  It's relatively clean and
>> leaves us space to develop -blockdev.
> 
> Here's the idea I was forming based on today's call:
> 
> Jeff's idea of a group operation can be extended to allow multiple
> operations while reusing the framework.  For oVirt, we need the ability
> to open a mirror (by passing the mirror file alongside the name of the
> new external snapshot), as well as reopening a blockdev (to pivot to the
> other side of an already-open mirror).
> 
> Is there a way to express a designated union in QMP?  I'm thinking
> something along the lines of having the overall group command take a
> list of operations, where each operation can either be 'create a
> snapshot', 'create a snapshot and mirror', or 'reopen a mirror'.
> 
> I'm thinking it might look something like:
> 
> { 'enum': 'BlockdevOp',
>   'data': [ 'snapshot', 'snapshot-mirror', 'reopen' ] }
> { 'type': 'BlockdevAction',
>   'data': {'device': 'str', 'op': 'BlockdevOp',
>            'file': 'str', '*format': 'str', '*reuse': 'bool',
>            '*mirror': 'str', '*mirror-format': 'str' } }
> { 'command': 'blkdev-group-action-sync',
>   'data': { 'actionlist': [ 'BlockdevAction' ] } }

I think the general approach is good.

Your implementation in QAPI is kind of ugly because you mix the
arguments of all three commands in BlockdevAction (how about
extensibility? And the "optional" flags aren't the full truth either,
we'd have to add checks in the handlers.). We really want to have some
real union support in QAPI that creates three different C structs.

So something like (I'll reintroduce the bad word transaction, because
it's really what we're doing here, just everything in one command):

{ 'type': 'BlockdevSnapshot',
  'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
{ 'union': 'BlockdevAction',
  'types': {
       'snapshot': 'BlockdevSnapshot',
       'mirror': 'BlockdevMirror', ...
   } }
{ 'command': 'blockdev-transaction',
  'data': { 'actionlist': [ 'BlockdevAction' ] } }

On the wire in QMP this would look like:

{ 'execute': 'blockdev-transaction',
  'arguments': {
    'actionlist': [
      { 'type': 'snapshot',
        'data': { 'device': 'ide-hd0', ... } },
      { 'type': 'mirror',
        'data': { ... }
    ]
  }
}

Now if you look closely, BlockdevSnapshot is exactly what Jeff called
SnapshotDev in his series, which in turn is the definition of the
blockdev-snapshot-sync command. We can reuse all of this even in the API
of a new more generic command.

So my conclusion for now is that we can merge the group snapshots right
now instead of waiting for the mirror design to be completed, and we can
reuse everything in a more complex transaction command later.

Kevin