* Kevin Wolf (kw...@redhat.com) wrote: > Am 10.01.2019 um 12:41 hat Dr. David Alan Gilbert geschrieben: > > * Kevin Wolf (kw...@redhat.com) wrote: > > > Am 09.01.2019 um 20:02 hat Eric Blake geschrieben: > > > > On 1/9/19 12:51 PM, Kevin Wolf wrote: > > > > > > > > >> Indeed, and libvirt IS using 'savevm' via HMP via QMP's > > > > >> human-monitor-command, since there is no QMP counterpart for internal > > > > >> snapshot. Even though lately we consistently tell people that > > > > >> internal > > > > >> snapshots are underdeveloped and you should use external snapshots, > > > > >> it > > > > >> does not get away from the fact that libvirt has been using 'savevm' > > > > >> to > > > > >> drive internal snapshots for years now, and that we MUST consider > > > > >> back-compat and/or add an introspectible QMP interface before making > > > > >> changes that would break libvirt. > > > > > > > > > > Okay, so what does libvirt do when you request a snapshot with a > > > > > numerical name? Without having looked at the code, the best case I > > > > > would > > > > > expect that it forbids them, and more realistically I suspect that we > > > > > may actually fix a bug for libvirt by changing the semantics. > > > > > > > > > > Or does libvirt really use snapshot IDs rather than names? > > > > > > > > At the moment, libvirt does not place any restriction on internal > > > > snapshot names, but passes the user's name through without thought of > > > > whether it is an id or a name. > > > > > > > > So yes, arguably tightening the semantics fixes a libvirt bug for > > > > libvirt having allowed internal snapshots to get into an inconsistent > > > > state. > > > > > > So there are two scenarios to consider with respect to breaking > > > backwards compatibility: > > > > > > 1. There may be code out there that relies on numeric arguments being > > > interpreted as IDs. This code will break if we make this change and > > > numeric snapshot names exist. That such code exists is speculation > > > (even though plausible) and we don't know how widespread it is. > > > > > > 2. There may be code out there that blindly assumes that whatever it > > > passes is interpreted as a name. Nobody considered that with a > > > numeric snapshot name, it maybe misinterpreted as an ID. We know that > > > this is true for libvirt, the single most used management tool for > > > QEMU. More code like this may (and probably does) exist. > > > > > > Essentially the same two categories exist for human users: those who > > > somehow found out that QEMU accepts IDs as monitor command arguments and > > > are using those (mitigated by not displaying IDs any more), and those > > > who are trapped because they wanted to access a numeric name, but > > > surprisingly got it interpreted as an ID. Both are speculation to some > > > degree, but my guess is that the latter group is larger. > > > > > > Given all this, this is a no-brainer for me. We simplify the interface, > > > we simplify the code, we make things less confusing for manual users and > > > we fix the management tool that everybody uses. How could we not make > > > this change? > > > > > > > But again, it falls in the category of "properly fixing this > > > > requires a lot of auditing, time, mental power, and unit tests", which > > > > makes it a rather daunting task to state for certain. > > > > > > Fix the world before we can fix anything? > > > > Can't we break this loop for the savevm command fairly easily; it's > > currently documnted as: > > > > savevm [tag|id] > > > > If we made that: > > > > savevm [-t] [-i] [tag|id] > > > > then: > > a) with neither -t or -i it would behave in the same roulette way > > as it does in the moment, and it might be a tag or id > > > > b) with -t we'd explicitly treat the parameter as a tag and it > > would error if it wasn't found > > > > c) With -i we'd explicitly treat the parameter as an id and > > it would error if it wasn't found > > > > Since we still allow (a) it doesn't break any existing code. > > If you can explain why we need both tag and id? > > And by keeping the current behaviour, we might not break hypothetically > existing correct code, but we leave currently actually existing broken > code like libvirt broken.
My only reason for leaving both tag & id was for the hypothetical existing current code; my assumption adding the above would be that we would then fix libvirt never to use (a), probably always (b). Dave > Kevin -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
