* 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.
Dave
> Kevin
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
