On 09.01.19 17:57, Daniel Henrique Barboza wrote: > > > On 1/9/19 12:10 PM, Max Reitz wrote: >> On 06.09.18 13:11, Daniel Henrique Barboza 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. >> (Yes, very late reply, I'm sorry...) >> >> I do think it affects users of HMP, because right now you can delete >> snapshots with their ID, and after this series you cannot. > > That's true. My idea here was simple: the user can't reliably exclude > via snapshot ID today > because we're hiding the ID field in info snapshots: > > > (qemu) savevm 0 > (qemu) info snapshots > List of snapshots present on all disks: > ID TAG VM SIZE DATE VM CLOCK > -- 0 741M 2018-07-31 13:39:56 00:41:25.313 > > > Thus, what will end up happening is that the user will be forced to use the > TAG of the snapshot since this is the only available information.
But you can get it through e.g. qemu-img info. Snapshot list: ID TAG VM SIZE DATE VM CLOCK 1 0 1.6M 2019-01-09 18:01:21 00:00:02.657 So it's not impossible to get. >> I think we had a short discussion about just disallowing numeric >> snapshot names. How bad would that be? > > > This was my first idea when evaluating what to do in this case. I gave > it up because > I found it to be too extreme. People would start complaining "I was able > to do > savevm 0 and now I can't". True. But it wouldn't be impossible to do, we'd need to deprecate numeric names, print a warning for two releases, and then we can make it an error. Hm... If we had a proper deprecation warning in this series, I suppose it wouldn't be dangerous anymore. Can we just print a warning whenever the user specified an ID? (i.e. if some snapshot's ID matches the string given by the user and the snapshot's name does not) Max
signature.asc
Description: OpenPGP digital signature