Re: [PATCH v2 2/2] qapi: Allow introspecting fix for savevm's cooperation with blockdev

[Kevin Wolf]

Am 02.10.2019 um 13:57 hat Markus Armbruster geschrieben:
> Eric Blake <address@hidden> writes:
> 
> > On 10/1/19 2:34 PM, Markus Armbruster wrote:
> >> Peter Krempa <address@hidden> writes:
> >>
> >>> savevm was buggy as it considered all monitor owned block device nodes
> >>
> >> Recommend "monitor-owned block device nodes" or "block device nodes
> >> owned by a monitor"
> >>
> >>> for snapshot. With introduction of -blockdev the common usage made all
> >>> nodes including protocol nodes monitor owned and thus considered for
> >>> snapshot.
> >>
> >> What exactly is / was the problem?
> >
> >
> > Old way: using QMP add_device, you create a drive backend with two BDS
> > (format and protocol) assigned to it; the drive backend has your given
> > name, and both BDS have a generated name (beginning with '#').  The
> > two BDS are not monitor-owned, rather, the drive is.
> >
> > New way: using QMP blockdev_add, you create the two BDS manually with
> > names of your choice, then plug that blockdev into an unnamed
> > blockbackend (the drive no longer needs a name, because you can get at
> > everything through the BDS name).  You _could_ do this in one step
> > (the QAPI allows self-recursion where you can define both the format
> > and protocol in one step), but it is easier to do in two steps (define
> > the protocol BDS first, then define the format BDS using a "string"
> > name of the protocol BDS instead of a { "driver":..., args... } object
> > of the protocol layer.  But by making two calls,  now both BDS are
> > monitor-owned.
> >
> > At snapshot-time, the code currently looks for all monitor-owned nodes
> > when deciding what to snapshot.  In the old way, this finds the named
> > drive, picks up its associated top-most node, and snapshots the format
> > layer.  In the new way, the drive is unnamed so it is skipped, while
> > there are two named BDS, but we don't want a snapshot of the protocol
> > layer.
> 
> So the problem is certain (common & sane) -blockdev use makes savevm
> create additional, unwanted snapshots.

Actually, the most common protocol driver is file-posix, which doesn't
support snapshots, so usually the result was that savevm just fails
because it can't snapshot something that it (incorrectly) thinks it
should snapshot.

Kevin

> Your explanation should be worked into the commit message along with ...
> 
> >>>            This was fixed but clients need to be able to detect whether
> >>> this fix is present.
> >>
> >> Fixed where?  Commit hash, if possible.
> >
> > Pull request:
> > https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04773.html
> > (assuming it doesn't need a respin before landing, 8ec72832)
> 
> ... a pointer to this fix.
> 
> Thanks!
> 
> [...]