* Daniel Henrique Barboza (danielhb...@gmail.com) wrote: > changes in v2: > - removed the "RFC" marker; > - added a new patch (patch 2) that removes > bdrv_snapshot_delete_by_id_or_name from the code; > - made changes in patch 1 as suggested by Murilo; > - previous patch set link: > https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg04658.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. > > I guess there's room for discussion about considering this change an API > change or not. It doesn't affect users of HMP and it doesn't affect Libvirt, > but simplifying the meaning of the parameters of savevm/loadvm/delvm.
Can you clarify a couple of things: a) What is it about libvirt's use that means it's OK ? Does it never use numeric tags? b) After this series are you always guaranteed to be able to fix any existing oddly named snapshots? Dave > > 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 | 24 ++++++++++++------------ > include/block/snapshot.h | 3 --- > qemu-img.c | 15 +++++++++++---- > 5 files changed, 26 insertions(+), 46 deletions(-) > > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
