Re: [Qemu-devel] [PATCH v2 0/3] HMP/snapshot changes - do not use ID anymore

[Dr. David Alan Gilbert]

* Daniel Henrique Barboza (address@hidden) 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 / address@hidden / Manchester, UK