block/qcow2-snapshot.c | 5 ----- block/snapshot.c | 25 +++---------------------- hmp-commands.hx | 32 ++++++++++++++++++++------------ include/block/snapshot.h | 3 --- qemu-img.c | 15 +++++++++++---- 5 files changed, 34 insertions(+), 46 deletions(-)
changes in v3: - rebased to v3.1.0-rc0 tag - hmp-commands.hx documentation now mentions the change of semantics starting version 3.2. - previous version link: http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00629.html It is not uncommon to see bugs being opened by testers that attempt to create VM snapshots using HMP. It turns out that "0" and "1" are quite common snapshot names and they trigger a lot of bugs. I gave an example in the commit message of patch 1, but to sum up here: QEMU treats the input of savevm/loadvm/delvm sometimes as 'ID', sometimes as 'name'. It is documented as such, but this can lead to strange situations. Given that it is strange for an API to consider a parameter to be 2 fields at the same time, and inadvently treating them as one or the other, and that removing the ID field is too drastic, my idea here is to keep the ID field for internal control, but do not let the user set it. This series simplifies the meaning of savevm/loadvm/delvm to be up to par to what the QEMU code (and Libvirt) is already doing: snapshot operations using "tag" semantics only, leaving the "id" to be automatically calculated by the block drivers and used internally only. This change of semantics does not affect existing snapshots. What changes is that any HMP operations with them will use the updated semantics. Daniel Henrique Barboza (3): block/snapshot.c: eliminate use of ID input in snapshot operations block/snapshot: remove bdrv_snapshot_delete_by_id_or_name qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call block/qcow2-snapshot.c | 5 ----- block/snapshot.c | 25 +++---------------------- hmp-commands.hx | 32 ++++++++++++++++++++------------ include/block/snapshot.h | 3 --- qemu-img.c | 15 +++++++++++---- 5 files changed, 34 insertions(+), 46 deletions(-) -- 2.17.2
Ping On 11/7/18 11:09 AM, Daniel Henrique Barboza wrote: > changes in v3: > - rebased to v3.1.0-rc0 tag > - hmp-commands.hx documentation now mentions the change of semantics > starting version 3.2. > - previous version link: > http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00629.html > > > It is not uncommon to see bugs being opened by testers that attempt to > create VM snapshots using HMP. It turns out that "0" and "1" are quite > common snapshot names and they trigger a lot of bugs. I gave an example > in the commit message of patch 1, but to sum up here: QEMU treats the > input of savevm/loadvm/delvm sometimes as 'ID', sometimes as 'name'. It > is documented as such, but this can lead to strange situations. > > Given that it is strange for an API to consider a parameter to be 2 fields > at the same time, and inadvently treating them as one or the other, and > that removing the ID field is too drastic, my idea here is to keep the > ID field for internal control, but do not let the user set it. > > This series simplifies the meaning of savevm/loadvm/delvm to be up to > par to what the QEMU code (and Libvirt) is already doing: snapshot > operations using "tag" semantics only, leaving the "id" to be > automatically calculated by the block drivers and used internally > only. > > This change of semantics does not affect existing snapshots. What > changes is that any HMP operations with them will use the > updated semantics. > > > Daniel Henrique Barboza (3): > block/snapshot.c: eliminate use of ID input in snapshot operations > block/snapshot: remove bdrv_snapshot_delete_by_id_or_name > qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call > > block/qcow2-snapshot.c | 5 ----- > block/snapshot.c | 25 +++---------------------- > hmp-commands.hx | 32 ++++++++++++++++++++------------ > include/block/snapshot.h | 3 --- > qemu-img.c | 15 +++++++++++---- > 5 files changed, 34 insertions(+), 46 deletions(-) >
Ping I believe all the discussion that happened in v2 applies here as well. Do we have a plan for this series? Should I add something else (warning message or doc note) to indicate a deprecation of the old meaning of the arguments? Thanks, DHB On 11/7/18 11:09 AM, Daniel Henrique Barboza wrote: > changes in v3: > - rebased to v3.1.0-rc0 tag > - hmp-commands.hx documentation now mentions the change of semantics > starting version 3.2. > - previous version link: > http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00629.html > > > It is not uncommon to see bugs being opened by testers that attempt to > create VM snapshots using HMP. It turns out that "0" and "1" are quite > common snapshot names and they trigger a lot of bugs. I gave an example > in the commit message of patch 1, but to sum up here: QEMU treats the > input of savevm/loadvm/delvm sometimes as 'ID', sometimes as 'name'. It > is documented as such, but this can lead to strange situations. > > Given that it is strange for an API to consider a parameter to be 2 fields > at the same time, and inadvently treating them as one or the other, and > that removing the ID field is too drastic, my idea here is to keep the > ID field for internal control, but do not let the user set it. > > This series simplifies the meaning of savevm/loadvm/delvm to be up to > par to what the QEMU code (and Libvirt) is already doing: snapshot > operations using "tag" semantics only, leaving the "id" to be > automatically calculated by the block drivers and used internally > only. > > This change of semantics does not affect existing snapshots. What > changes is that any HMP operations with them will use the > updated semantics. > > > Daniel Henrique Barboza (3): > block/snapshot.c: eliminate use of ID input in snapshot operations > block/snapshot: remove bdrv_snapshot_delete_by_id_or_name > qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call > > block/qcow2-snapshot.c | 5 ----- > block/snapshot.c | 25 +++---------------------- > hmp-commands.hx | 32 ++++++++++++++++++++------------ > include/block/snapshot.h | 3 --- > qemu-img.c | 15 +++++++++++---- > 5 files changed, 34 insertions(+), 46 deletions(-) >
Am 07.11.2018 um 14:09 hat Daniel Henrique Barboza geschrieben: > changes in v3: > - rebased to v3.1.0-rc0 tag > - hmp-commands.hx documentation now mentions the change of semantics > starting version 3.2. > - previous version link: > http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00629.html > > > It is not uncommon to see bugs being opened by testers that attempt to > create VM snapshots using HMP. It turns out that "0" and "1" are quite > common snapshot names and they trigger a lot of bugs. I gave an example > in the commit message of patch 1, but to sum up here: QEMU treats the > input of savevm/loadvm/delvm sometimes as 'ID', sometimes as 'name'. It > is documented as such, but this can lead to strange situations. > > Given that it is strange for an API to consider a parameter to be 2 fields > at the same time, and inadvently treating them as one or the other, and > that removing the ID field is too drastic, my idea here is to keep the > ID field for internal control, but do not let the user set it. > > This series simplifies the meaning of savevm/loadvm/delvm to be up to > par to what the QEMU code (and Libvirt) is already doing: snapshot > operations using "tag" semantics only, leaving the "id" to be > automatically calculated by the block drivers and used internally > only. > > This change of semantics does not affect existing snapshots. What > changes is that any HMP operations with them will use the > updated semantics. Thanks, applied to the block branch. Kevin
© 2016 - 2024 Red Hat, Inc.