Am 11.10.2019 um 08:08 hat Markus Armbruster geschrieben:
> Kevin Wolf <kw...@redhat.com> writes:
>
> > Am 02.10.2019 um 13:57 hat Markus Armbruster geschrieben:
> >> Eric Blake <ebl...@redhat.com> writes:
> >>
> >> > On 10/1/19 2:34 PM, Markus Armbruster wrote:
> >> >> Peter Krempa <pkre...@redhat.com> 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.
>
> v3's commit message:
>
> qapi: Allow introspecting fix for savevm's cooperation with blockdev
>
> 'savevm' was buggy as it considered all monitor-owned block device nodes
> for snapshot. With introduction of -blockdev the common usage made all
> nodes including protocol and backing file nodes monitor-owned and thus
> considered for snapshot.
>
> This is a problem since the 'file' protocol nodes can't have internal
> snapshots and it does not make sense to take snapshot of nodes
> representing backing files.
>
> This was fixed by commit 05f4aced658a02b02 clients need to be able to
> detect whether this fix is present.
Something is missing in this sentence. I think you lost the "but" from
the original message.
> Since savevm does not have an QMP alternative, add the feature for the
> 'human-monitor-command' backdoor which is used to call this command in
> modern use.
>
> Signed-off-by: Peter Krempa <pkre...@redhat.com>
>
> Kevin, is this explanation sufficiently correct & complete?
Looks good to me otherwise.
Kevin
