From: Kevin Wolf <kwolf@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Peter Krempa <pkrempa@redhat.com>,
qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 2/2] qapi: Allow introspecting fix for savevm's cooperation with blockdev
Date: Fri, 11 Oct 2019 11:00:36 +0200 [thread overview]
Message-ID: <20191011090036.GA5158@localhost.localdomain> (raw)
In-Reply-To: <87r23jlsql.fsf@dusky.pond.sub.org>
Am 11.10.2019 um 08:08 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Am 02.10.2019 um 13:57 hat Markus Armbruster geschrieben:
> >> Eric Blake <eblake@redhat.com> writes:
> >>
> >> > On 10/1/19 2:34 PM, Markus Armbruster wrote:
> >> >> Peter Krempa <pkrempa@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 <pkrempa@redhat.com>
>
> Kevin, is this explanation sufficiently correct & complete?
Looks good to me otherwise.
Kevin
next prev parent reply other threads:[~2019-10-11 9:02 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-20 14:26 [PATCH v2 0/2] qapi: Add detection for the 'savevm' fix for blockdev Peter Krempa
2019-09-20 14:26 ` [PATCH v2 1/2] qapi: Add feature flags to commands in qapi introspection Peter Krempa
2019-10-01 6:40 ` Markus Armbruster
2019-10-01 14:17 ` Peter Krempa
2019-10-01 20:01 ` Markus Armbruster
2019-10-02 6:15 ` Markus Armbruster
2019-09-20 14:26 ` [PATCH v2 2/2] qapi: Allow introspecting fix for savevm's cooperation with blockdev Peter Krempa
2019-10-01 19:34 ` Markus Armbruster
2019-10-01 21:07 ` Eric Blake
2019-10-02 11:57 ` Markus Armbruster
2019-10-10 15:07 ` Kevin Wolf
2019-10-11 6:08 ` Markus Armbruster
2019-10-11 9:00 ` Kevin Wolf [this message]
2019-10-11 11:10 ` Markus Armbruster
2019-09-30 13:04 ` [PATCH v2 0/2] qapi: Add detection for the 'savevm' fix for blockdev Peter Krempa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191011090036.GA5158@localhost.localdomain \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=pkrempa@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).