| Message ID | 1429864368-15249-1-git-send-email-kchamart@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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 :)
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.
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.
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.
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
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(-)