[Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Kashyap Chamarthy]

Although the canonical source of reference for QMP commands is
qapi-schema.json, for consistency's sake, update qmp-commands.hx to
state the list of supported transactionable operations, namely:

    drive-backup
    blockdev-backup
    blockdev-snapshot-internal-sync
    abort
    block-dirty-bitmap-add
    block-dirty-bitmap-clear

NB: The 'block-dirty-bitmap-add' and 'block-dirty-bitmap-clear' commands
will be available once the in-review "transactionless incremental
backup" patch series[1] is merged upstream.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02161.html

Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
>From v1 -> v2:

  - Update the "NB" part of the commit message by removing the duplicate
    command entry and add the missing one ('block-dirty-bitmap-clear')
  - Fix grammer per Eric Blake's review: s/refer/refer to the
---
 qmp-commands.hx | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0bffeb23778f877410f6e2038943da46c0..b7fe31ca37afe0f4ae8f4b6f7be5d379b361b1b8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1200,11 +1200,14 @@ SQMP
 transaction
 -----------
 
-Atomically operate on one or more block devices.  The only supported operations
-for now are drive-backup, internal and external snapshotting.  A list of
-dictionaries is accepted, that contains the actions to be performed.
-If there is any failure performing any of the operations, all operations
-for the group are abandoned.
+Atomically operate on one or more block devices.  Operations that are
+currently supported: drive-backup, blockdev-backup,
+blockdev-snapshot-sync, blockdev-snapshot-internal-sync, abort,
+block-dirty-bitmap-add, block-dirty-bitmap-clear (refer to the
+qemu/qapi-schema.json file for minimum required QEMU versions for these
+operations).  A list of dictionaries is accepted, that contains the
+actions to be performed.  If there is any failure performing any of the
+operations, all operations for the group are abandoned.
 
 For external snapshots, the dictionary contains the device, the file to use for
 the new snapshot, and the format.  The default format, if not specified, is
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[John Snow]

On 04/24/2015 04:32 AM, Kashyap Chamarthy wrote:
> Although the canonical source of reference for QMP commands is
> qapi-schema.json, for consistency's sake, update qmp-commands.hx to
> state the list of supported transactionable operations, namely:
>
>      drive-backup
>      blockdev-backup
>      blockdev-snapshot-internal-sync
>      abort

These:

>      block-dirty-bitmap-add
>      block-dirty-bitmap-clear
>

Aren't merged yet, so it might be a little confusing. We could tack this 
on to the end of the transaction series if you'd like, and hopefully 
that all goes in at once before 2.4.

> NB: The 'block-dirty-bitmap-add' and 'block-dirty-bitmap-clear' commands
> will be available once the in-review "transactionless incremental
> backup" patch series[1] is merged upstream.
>
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02161.html
>
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  From v1 -> v2:
>
>    - Update the "NB" part of the commit message by removing the duplicate
>      command entry and add the missing one ('block-dirty-bitmap-clear')
>    - Fix grammer per Eric Blake's review: s/refer/refer to the
> ---
>   qmp-commands.hx | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 3a42ad0bffeb23778f877410f6e2038943da46c0..b7fe31ca37afe0f4ae8f4b6f7be5d379b361b1b8 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1200,11 +1200,14 @@ SQMP
>   transaction
>   -----------
>
> -Atomically operate on one or more block devices.  The only supported operations
> -for now are drive-backup, internal and external snapshotting.  A list of
> -dictionaries is accepted, that contains the actions to be performed.
> -If there is any failure performing any of the operations, all operations
> -for the group are abandoned.
> +Atomically operate on one or more block devices.  Operations that are
> +currently supported: drive-backup, blockdev-backup,
> +blockdev-snapshot-sync, blockdev-snapshot-internal-sync, abort,
> +block-dirty-bitmap-add, block-dirty-bitmap-clear (refer to the
> +qemu/qapi-schema.json file for minimum required QEMU versions for these
> +operations).  A list of dictionaries is accepted, that contains the
> +actions to be performed.  If there is any failure performing any of the
> +operations, all operations for the group are abandoned.
>
>   For external snapshots, the dictionary contains the device, the file to use for
>   the new snapshot, and the format.  The default format, if not specified, is
>

-- 
—js

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Kashyap Chamarthy]

On Fri, Apr 24, 2015 at 11:52:00AM -0400, John Snow wrote:
> 
> On 04/24/2015 04:32 AM, Kashyap Chamarthy wrote:

[. . .]

> These:
> 
> >     block-dirty-bitmap-add
> >     block-dirty-bitmap-clear
> >
> 
> Aren't merged yet, so it might be a little confusing.

Yeah, I expected someone to call that out, that's the reason I added the
NB at the end of the commit message which explicitly states that these
will be usable once your series (linked below) is merged, hoping that'd
clarify.

> We could tack this on to the end of the transaction series if you'd
> like, and hopefully that all goes in at once before 2.4.

That works too. 

If it's preferable, I can respin this to remove the mention of above two
commands and resubmit a v3. 

-- 
/kashyap

> >NB: The 'block-dirty-bitmap-add' and 'block-dirty-bitmap-clear' commands
> >will be available once the in-review "transactionless incremental
> >backup" patch series[1] is merged upstream.
> >
> >[1] http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02161.html
> >
> >Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> >Reviewed-by: Eric Blake <eblake@redhat.com>
> >---
> > From v1 -> v2:
> >
> >   - Update the "NB" part of the commit message by removing the duplicate
> >     command entry and add the missing one ('block-dirty-bitmap-clear')
> >   - Fix grammer per Eric Blake's review: s/refer/refer to the
> >---
> >  qmp-commands.hx | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> >diff --git a/qmp-commands.hx b/qmp-commands.hx
> >index 3a42ad0bffeb23778f877410f6e2038943da46c0..b7fe31ca37afe0f4ae8f4b6f7be5d379b361b1b8 100644
> >--- a/qmp-commands.hx
> >+++ b/qmp-commands.hx
> >@@ -1200,11 +1200,14 @@ SQMP
> >  transaction
> >  -----------
> >
> >-Atomically operate on one or more block devices.  The only supported operations
> >-for now are drive-backup, internal and external snapshotting.  A list of
> >-dictionaries is accepted, that contains the actions to be performed.
> >-If there is any failure performing any of the operations, all operations
> >-for the group are abandoned.
> >+Atomically operate on one or more block devices.  Operations that are
> >+currently supported: drive-backup, blockdev-backup,
> >+blockdev-snapshot-sync, blockdev-snapshot-internal-sync, abort,
> >+block-dirty-bitmap-add, block-dirty-bitmap-clear (refer to the
> >+qemu/qapi-schema.json file for minimum required QEMU versions for these
> >+operations).  A list of dictionaries is accepted, that contains the
> >+actions to be performed.  If there is any failure performing any of the
> >+operations, all operations for the group are abandoned.
> >
> >  For external snapshots, the dictionary contains the device, the file to use for
> >  the new snapshot, and the format.  The default format, if not specified, is
> >

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[John Snow]

On 04/24/2015 12:20 PM, Kashyap Chamarthy wrote:
> On Fri, Apr 24, 2015 at 11:52:00AM -0400, John Snow wrote:
>>
>> On 04/24/2015 04:32 AM, Kashyap Chamarthy wrote:
>
> [. . .]
>
>> These:
>>
>>>      block-dirty-bitmap-add
>>>      block-dirty-bitmap-clear
>>>
>>
>> Aren't merged yet, so it might be a little confusing.
>
> Yeah, I expected someone to call that out, that's the reason I added the
> NB at the end of the commit message which explicitly states that these
> will be usable once your series (linked below) is merged, hoping that'd
> clarify.
>
>> We could tack this on to the end of the transaction series if you'd
>> like, and hopefully that all goes in at once before 2.4.
>
> That works too.
>
> If it's preferable, I can respin this to remove the mention of above two
> commands and resubmit a v3.
>

I will defer to Eric Blake, who already gave you an R-B :)

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Eric Blake]

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

On 04/24/2015 10:20 AM, Kashyap Chamarthy wrote:
> On Fri, Apr 24, 2015 at 11:52:00AM -0400, John Snow wrote:
>>
>> On 04/24/2015 04:32 AM, Kashyap Chamarthy wrote:
> 
> [. . .]
> 
>> These:
>>
>>>     block-dirty-bitmap-add
>>>     block-dirty-bitmap-clear
>>>
>>
>> Aren't merged yet, so it might be a little confusing.
> 
> Yeah, I expected someone to call that out, that's the reason I added the
> NB at the end of the commit message which explicitly states that these
> will be usable once your series (linked below) is merged, hoping that'd
> clarify.
> 
>> We could tack this on to the end of the transaction series if you'd
>> like, and hopefully that all goes in at once before 2.4.
> 
> That works too. 
> 
> If it's preferable, I can respin this to remove the mention of above two
> commands and resubmit a v3. 

I would have just added a note after the '---' line that the patch
depends on the transaction series going in first.  That is, basically
appending this on to the end of the transaction series is the best
approach for now, rather than trying to respin without the new commands.

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Kashyap Chamarthy]

On Fri, Apr 24, 2015 at 02:51:59PM -0600, Eric Blake wrote:
> On 04/24/2015 10:20 AM, Kashyap Chamarthy wrote:
> > On Fri, Apr 24, 2015 at 11:52:00AM -0400, John Snow wrote:
> >>
> >> On 04/24/2015 04:32 AM, Kashyap Chamarthy wrote:
> > 
> > [. . .]
> > 
> >> These:
> >>
> >>>     block-dirty-bitmap-add
> >>>     block-dirty-bitmap-clear
> >>>
> >>
> >> Aren't merged yet, so it might be a little confusing.
> > 
> > Yeah, I expected someone to call that out, that's the reason I added the
> > NB at the end of the commit message which explicitly states that these
> > will be usable once your series (linked below) is merged, hoping that'd
> > clarify.
> > 
> >> We could tack this on to the end of the transaction series if you'd
> >> like, and hopefully that all goes in at once before 2.4.
> > 
> > That works too. 
> > 
> > If it's preferable, I can respin this to remove the mention of above two
> > commands and resubmit a v3. 
> 
> I would have just added a note after the '---' line that the patch
> depends on the transaction series going in first.  That is, basically
> appending this on to the end of the transaction series is the best
> approach for now, rather than trying to respin without the new
> commands.

Agreed. 

John: as you suggested, too, please "tack this on to the end of" your
transaction patch series.

Thanks.

-- 
/kashyap

Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: Update the supported 'transaction' operations

[Stefan Hajnoczi]

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

On Fri, Apr 24, 2015 at 10:32:47AM +0200, Kashyap Chamarthy wrote:
> Although the canonical source of reference for QMP commands is
> qapi-schema.json, for consistency's sake, update qmp-commands.hx to
> state the list of supported transactionable operations, namely:
> 
>     drive-backup
>     blockdev-backup
>     blockdev-snapshot-internal-sync
>     abort
>     block-dirty-bitmap-add
>     block-dirty-bitmap-clear
> 
> NB: The 'block-dirty-bitmap-add' and 'block-dirty-bitmap-clear' commands
> will be available once the in-review "transactionless incremental
> backup" patch series[1] is merged upstream.
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02161.html

The transactionless incremental backup series has been merged into
block-next and does not contain
block-dirty-bitmap-add/block-dirty-bitmap-clear transactions.  They are
part of the transactions series that John Snow sent.

John: Please include this patch in future revisions of your transactions
series.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]