| Message ID | 1443787955-19823-1-git-send-email-kchamart@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/02/2015 06:12 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 > block-dirty-bitmap-add > block-dirty-bitmap-clear > > Also update the possible values for the "type" action array. > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> Significant enough changes that I would have dropped R-b before posting to ensure it gets looked at. But fortunately, you did a good job, so you can still keep my R-b.
On Fri, Oct 02, 2015 at 06:40:28AM -0600, Eric Blake wrote: > On 10/02/2015 06:12 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 > > block-dirty-bitmap-add > > block-dirty-bitmap-clear > > > > Also update the possible values for the "type" action array. > > > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > > Reviewed-by: Eric Blake <eblake@redhat.com> > > Significant enough changes that I would have dropped R-b before posting > to ensure it gets looked at. Yes, I was wary of this and thought I included a blurb "Removing Eric's R-b as this change involves more edits." My bad, seems to have missed it while message composition. And, your reminder doesn't hurt. > But fortunately, you did a good job, so you can still keep my R-b. Thanks.
On 02.10.2015 14:12, 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 > > Also update the possible values for the "type" action array. > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > In v8 ("v8" because Fam included this as part of a series that is > in its "v7" edition): > - Update the possible values for supported 'type' of operation > --- > qmp-commands.hx | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1270,11 +1270,22 @@ 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 > @@ -1301,8 +1312,12 @@ it later with qemu-img or other command. > Arguments: > > actions array: > - - "type": the operation to perform. The only supported > - value is "blockdev-snapshot-sync". (json-string) > + - "type": the operation to perform (json-string). Possible > + values: "drive-backup", "blockdev-backup", > + "blockdev-snapshot-sync", > + "blockdev-snapshot-internal-sync", > + "abort", "block-dirty-bitmap-add", > + "block-dirty-bitmap-clear" > - "data": a dictionary. The contents depend on the value > of "type". When "type" is "blockdev-snapshot-sync": > - "device": device name to snapshot (json-string) > Do you want to extend this list, too? Right now it only contains parameter information for blockdev-snapshot-sync and blockdev-snapshot-internal-sync. Max
[Sorry, I noticed your question only just now, as I was briefly away from this list.] On Fri, Oct 02, 2015 at 07:59:48PM +0200, Max Reitz wrote: > On 02.10.2015 14:12, 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 > > > > Also update the possible values for the "type" action array. > > > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > > Reviewed-by: Eric Blake <eblake@redhat.com> > > --- > > In v8 ("v8" because Fam included this as part of a series that is > > in its "v7" edition): > > - Update the possible values for supported 'type' of operation > > --- > > qmp-commands.hx | 29 ++++++++++++++++++++++------- > > 1 file changed, 22 insertions(+), 7 deletions(-) > > > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -1270,11 +1270,22 @@ 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 > > @@ -1301,8 +1312,12 @@ it later with qemu-img or other command. > > Arguments: > > > > actions array: > > - - "type": the operation to perform. The only supported > > - value is "blockdev-snapshot-sync". (json-string) > > + - "type": the operation to perform (json-string). Possible > > + values: "drive-backup", "blockdev-backup", > > + "blockdev-snapshot-sync", > > + "blockdev-snapshot-internal-sync", > > + "abort", "block-dirty-bitmap-add", > > + "block-dirty-bitmap-clear" > > - "data": a dictionary. The contents depend on the value > > of "type". When "type" is "blockdev-snapshot-sync": > > - "device": device name to snapshot (json-string) > > > > Do you want to extend this list, too? Right now it only contains > parameter information for blockdev-snapshot-sync and > blockdev-snapshot-internal-sync. You mean, all the parameters for rest of the transactionable commands that one can possibly add (taking block-core.json)? E.g. for 'drive-backup' command, all the possible data ('device', target', 'format', 'sync', etc) one can provide (which are enumerated in the DriveBackup struct): $ less qapi/block-core.json [...] { 'struct': 'DriveBackup', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', '*bitmap': 'str', '*on-source-error': 'BlockdevOnError', '*on-target-error': 'BlockdevOnError' } } [...] And, data provided by 'BlockdevBackup' struct for 'blockdev-backup', so on. If so, I can do try that if that's the preferred way, but maybe it can be done as a separate commit, since it's been that way for a while?
Am 02.10.2015 um 14:12 hat Kashyap Chamarthy geschrieben: > 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 > > Also update the possible values for the "type" action array. > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks, applied to the block branch. Kevin
On 10/12/2015 05:04 AM, Kashyap Chamarthy wrote: > [Sorry, I noticed your question only just now, as I was briefly away > from this list.] > > On Fri, Oct 02, 2015 at 07:59:48PM +0200, Max Reitz wrote: >> On 02.10.2015 14:12, 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: >>> >>> actions array: >>> - - "type": the operation to perform. The only supported >>> - value is "blockdev-snapshot-sync". (json-string) >>> + - "type": the operation to perform (json-string). Possible >>> + values: "drive-backup", "blockdev-backup", >>> + "blockdev-snapshot-sync", >>> + "blockdev-snapshot-internal-sync", >>> + "abort", "block-dirty-bitmap-add", >>> + "block-dirty-bitmap-clear" >>> - "data": a dictionary. The contents depend on the value >>> of "type". When "type" is "blockdev-snapshot-sync": >>> - "device": device name to snapshot (json-string) >>> >> >> Do you want to extend this list, too? Right now it only contains >> parameter information for blockdev-snapshot-sync and >> blockdev-snapshot-internal-sync. > > You mean, all the parameters for rest of the transactionable commands > that one can possibly add (taking block-core.json)? Or conversely, we could drop the action-specific arguments, and just say something like "see the documentation for specific additional fields appropriate for that action" > If so, I can do try that if that's the preferred way, but maybe it can > be done as a separate commit, since it's been that way for a while? Yes, at this point, a separate commit for any further cleanups is fine.
On 10/12/2015 09:54 AM, Eric Blake wrote: > On 10/12/2015 05:04 AM, Kashyap Chamarthy wrote: >> [Sorry, I noticed your question only just now, as I was briefly away >> from this list.] >> >> On Fri, Oct 02, 2015 at 07:59:48PM +0200, Max Reitz wrote: >>> On 02.10.2015 14:12, 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: >>>> > >>>> actions array: >>>> - - "type": the operation to perform. The only supported >>>> - value is "blockdev-snapshot-sync". (json-string) >>>> + - "type": the operation to perform (json-string). Possible >>>> + values: "drive-backup", "blockdev-backup", >>>> + "blockdev-snapshot-sync", >>>> + "blockdev-snapshot-internal-sync", >>>> + "abort", "block-dirty-bitmap-add", >>>> + "block-dirty-bitmap-clear" >>>> - "data": a dictionary. The contents depend on the value >>>> of "type". When "type" is "blockdev-snapshot-sync": >>>> - "device": device name to snapshot (json-string) >>>> >>> >>> Do you want to extend this list, too? Right now it only contains >>> parameter information for blockdev-snapshot-sync and >>> blockdev-snapshot-internal-sync. >> >> You mean, all the parameters for rest of the transactionable commands >> that one can possibly add (taking block-core.json)? > > Or conversely, we could drop the action-specific arguments, and just say > something like "see the documentation for specific additional fields > appropriate for that action" > This is the approach that's most flexible. As it stands upstream right now, all of the various actions simply have an identical command layout to their respective QMP versions, so it'd be easiest to just point to the various singlet commands. We'll prevent desync and bad documentation that way. If we ever add extra arguments per-transaction (like this patchset proposes,) we can always just document the difference. > >> If so, I can do try that if that's the preferred way, but maybe it can >> be done as a separate commit, since it's been that way for a while? > > Yes, at this point, a separate commit for any further cleanups is fine. >
Hi On Fri, Oct 2, 2015 at 2:12 PM, Kashyap Chamarthy <kchamart@redhat.com> 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 > > Also update the possible values for the "type" action array. Isn't the actions limited by the TransactionAction in qapi-schema.json? { 'union': 'TransactionAction', 'data': { 'blockdev-snapshot-sync': 'BlockdevSnapshot', 'drive-backup': 'DriveBackup', + 'blockdev-backup': 'BlockdevBackup', 'abort': 'Abort', 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal' } } It looks like block-dirty-bitmap-add/clear isn't in the list. Thanks > > Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > In v8 ("v8" because Fam included this as part of a series that is > in its "v7" edition): > - Update the possible values for supported 'type' of operation > --- > qmp-commands.hx | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1270,11 +1270,22 @@ 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 > @@ -1301,8 +1312,12 @@ it later with qemu-img or other command. > Arguments: > > actions array: > - - "type": the operation to perform. The only supported > - value is "blockdev-snapshot-sync". (json-string) > + - "type": the operation to perform (json-string). Possible > + values: "drive-backup", "blockdev-backup", > + "blockdev-snapshot-sync", > + "blockdev-snapshot-internal-sync", > + "abort", "block-dirty-bitmap-add", > + "block-dirty-bitmap-clear" > - "data": a dictionary. The contents depend on the value > of "type". When "type" is "blockdev-snapshot-sync": > - "device": device name to snapshot (json-string) > -- > 2.4.3 > >
Hi On Mon, Nov 23, 2015 at 7:58 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Isn't the actions limited by the TransactionAction in qapi-schema.json? > > { 'union': 'TransactionAction', > 'data': { > 'blockdev-snapshot-sync': 'BlockdevSnapshot', > 'drive-backup': 'DriveBackup', > + 'blockdev-backup': 'BlockdevBackup', > 'abort': 'Abort', > 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal' > } } > > It looks like block-dirty-bitmap-add/clear isn't in the list. my bad, I was looking at older version,
On 11/23/2015 01:58 PM, Marc-André Lureau wrote: > Hi > > On Fri, Oct 2, 2015 at 2:12 PM, Kashyap Chamarthy <kchamart@redhat.com> 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 >> >> Also update the possible values for the "type" action array. > > Isn't the actions limited by the TransactionAction in qapi-schema.json? > > { 'union': 'TransactionAction', > 'data': { > 'blockdev-snapshot-sync': 'BlockdevSnapshot', > 'drive-backup': 'DriveBackup', > + 'blockdev-backup': 'BlockdevBackup', > 'abort': 'Abort', > 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal' > } } > > It looks like block-dirty-bitmap-add/clear isn't in the list. > > Thanks > Maybe at the time. Your codebase is outdated. ## # @TransactionAction # # A discriminated record of operations that can be performed with # @transaction. # # Since 1.1 # # drive-backup since 1.6 # abort since 1.6 # blockdev-snapshot-internal-sync since 1.7 # blockdev-backup since 2.3 # blockdev-snapshot since 2.5 # block-dirty-bitmap-add since 2.5 # block-dirty-bitmap-clear since 2.5 ## { 'union': 'TransactionAction', 'data': { 'blockdev-snapshot': 'BlockdevSnapshot', 'blockdev-snapshot-sync': 'BlockdevSnapshotSync', 'drive-backup': 'DriveBackup', 'blockdev-backup': 'BlockdevBackup', 'abort': 'Abort', 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal', 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd', 'block-dirty-bitmap-clear': 'BlockDirtyBitmap' } } >> >> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> In v8 ("v8" because Fam included this as part of a series that is >> in its "v7" edition): >> - Update the possible values for supported 'type' of operation >> --- >> qmp-commands.hx | 29 ++++++++++++++++++++++------- >> 1 file changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -1270,11 +1270,22 @@ 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 >> @@ -1301,8 +1312,12 @@ it later with qemu-img or other command. >> Arguments: >> >> actions array: >> - - "type": the operation to perform. The only supported >> - value is "blockdev-snapshot-sync". (json-string) >> + - "type": the operation to perform (json-string). Possible >> + values: "drive-backup", "blockdev-backup", >> + "blockdev-snapshot-sync", >> + "blockdev-snapshot-internal-sync", >> + "abort", "block-dirty-bitmap-add", >> + "block-dirty-bitmap-clear" >> - "data": a dictionary. The contents depend on the value >> of "type". When "type" is "blockdev-snapshot-sync": >> - "device": device name to snapshot (json-string) >> -- >> 2.4.3 >> >> > > >
diff --git a/qmp-commands.hx b/qmp-commands.hx index d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1270,11 +1270,22 @@ 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 @@ -1301,8 +1312,12 @@ it later with qemu-img or other command. Arguments: actions array: - - "type": the operation to perform. The only supported - value is "blockdev-snapshot-sync". (json-string) + - "type": the operation to perform (json-string). Possible + values: "drive-backup", "blockdev-backup", + "blockdev-snapshot-sync", + "blockdev-snapshot-internal-sync", + "abort", "block-dirty-bitmap-add", + "block-dirty-bitmap-clear" - "data": a dictionary. The contents depend on the value of "type". When "type" is "blockdev-snapshot-sync": - "device": device name to snapshot (json-string)
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 Also update the possible values for the "type" action array. Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- In v8 ("v8" because Fam included this as part of a series that is in its "v7" edition): - Update the possible values for supported 'type' of operation --- qmp-commands.hx | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)