From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Fam Zheng <famz@redhat.com>,
berrange@redhat.com, qemu-block@nongnu.org, rjones@redhat.com,
Markus Armbruster <armbru@redhat.com>,
qemu-devel@nongnu.org, stefanha@redhat.com, den@openvz.org,
pbonzini@redhat.com, jcody@redhat.com
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking
Date: Tue, 6 Sep 2016 18:45:55 +0200 [thread overview]
Message-ID: <20160906164555.GI4667@noname.redhat.com> (raw)
In-Reply-To: <b59c2716-54d4-d1f2-2ffd-35fca09238ce@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4341 bytes --]
Am 08.07.2016 um 15:05 hat Max Reitz geschrieben:
> On 08.07.2016 11:50, Kevin Wolf wrote:
> > Am 08.07.2016 um 04:56 hat Fam Zheng geschrieben:
> >> On Tue, 07/05 15:37, Kevin Wolf wrote:
> >>> Am 17.06.2016 um 11:23 hat Kevin Wolf geschrieben:
> >>>> Am 03.06.2016 um 10:48 hat Fam Zheng geschrieben:
> >>>>> Respect the locking mode from CLI or QMP, and set the open flags
> >>>>> accordingly.
> >>>>>
> >>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
> >>>
> >>>> This is the wrong level to implement the feature. You would only be able
> >>>> to configure the locking on the top level image this way (because what
> >>>> we're doing here is BB, not BDS configuration). If you want to allow
> >>>> configuration per node, you need to put the options into
> >>>> bdrv_runtime_opts and interpret them in bdrv_open_common().
> >>>>
> >>>> Otherwise we would have to specify in the blockdev-add documentation
> >>>> that this works only on the top level, but I don't see a reason for
> >>>> such a restriction.
> >>>
> >>> And actually, after some more thinking about block device configuration,
> >>> I'm not sure any more whether letting the user configure this on the
> >>> node level, at least as the primary interface.
> >>>
> >>> A node usually knows by itself what locking mode it needs to request
> >>> from its children, depending on the locking mode that the parent node
> >>> requested for it. It could be passing down the locking mode (raw
> >>> format), it could require a stricter locking mode (non-raw formats never
> >>> work with r/w sharing) or it could even be less strict (backing files
> >>> are normally ro/ and can therefore be shared, even if the guest can't
> >>> share its image).
> >>>
> >>> The real origin of the locking requirement is the top level. So maybe
> >>> the right interface for guest devices is adding a qdev option that tells
> >>> whether the guest can share the image. For NBD servers, we'd add a QMP
> >>
> >> I think most block devices are not designed to share the data, so in general
> >> it's hard to imagine this as a device property.
> >
> > Well, it's really a guest OS (or even guest application) property, but
> > obviously that doesn't exist. And the device is the qemu component that
> > is the closest to the guest. We generally have options about behaviour
> > that the guest expects at the device level.
>
> Can the guest device really make a qualified decision here? If you're
> using raw as the image format, sharing may be fine, if you're using
> qcow2, it most likely is not.
Just noticed while looking at v7 that I never replied here...
Yes, you're right that the guest device can't make the decision for the
exact locking mode of the images. But the device is the point where the
user has to decide whether or not sharing is okay. For everything below
it, qemu knows by itself whether it can share the image.
This is essentially what I already described above:
> >>> A node usually knows by itself what locking mode it needs to request
> >>> from its children, depending on the locking mode that the parent node
> >>> requested for it. It could be passing down the locking mode (raw
> >>> format), it could require a stricter locking mode (non-raw formats never
> >>> work with r/w sharing) or it could even be less strict (backing files
> >>> are normally ro/ and can therefore be shared, even if the guest can't
> >>> share its image).
> I think whether to lock a BDS chain is a host-side property and has not
> a lot to do with the guest, thus it should be a BDS property. I can
> imagine that a guest may say that sharing should be disallowed under all
> circumstances, but a guest is never able to decide to allow sharing.
Well, yes and no. The decision which lock mode to use is at the node
level, no doubt. But it's not something that a user can configure.
Non-raw images simply can't be shared and the user can't do anything
about it. Why should they have an option to specify a lock mode when
there is only one correct setting?
The only possible exception where an option on the node level could make
sense (which I already mentioned earlier in this thread) is if the user
wants to be stricter than what qemu needs.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2016-09-06 16:46 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-03 8:48 [Qemu-devel] [PATCH v6 00/22] block: Lock images when opening Fam Zheng
2016-06-03 8:48 ` [Qemu-devel] [PATCH v6 01/22] block: Add flag bits for image locking Fam Zheng
2016-06-17 9:05 ` Kevin Wolf
2016-06-03 8:48 ` [Qemu-devel] [PATCH v6 02/22] qapi: Add lock-mode in blockdev-add options Fam Zheng
2016-06-17 9:17 ` Kevin Wolf
2016-06-18 11:16 ` Fam Zheng
2016-06-20 13:24 ` Kevin Wolf
2016-06-03 8:48 ` [Qemu-devel] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking Fam Zheng
2016-06-17 9:23 ` Kevin Wolf
2016-07-05 13:37 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2016-07-08 2:56 ` Fam Zheng
2016-07-08 9:50 ` Kevin Wolf
2016-07-08 13:05 ` Max Reitz
2016-09-06 16:45 ` Kevin Wolf [this message]
2016-09-06 16:51 ` Daniel P. Berrange
2016-09-06 17:15 ` Kevin Wolf
2016-06-03 8:48 ` [Qemu-devel] [PATCH v6 04/22] block: Introduce image file locking Fam Zheng
2016-06-08 1:51 ` Jason Dillaman
2016-06-08 2:45 ` Fam Zheng
2016-06-17 11:34 ` Kevin Wolf
2016-06-22 7:23 ` Fam Zheng
2016-06-22 8:30 ` Kevin Wolf
2016-06-22 9:04 ` Fam Zheng
2016-06-03 8:48 ` [Qemu-devel] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd Fam Zheng
2016-06-17 12:12 ` Kevin Wolf
2016-06-22 7:34 ` Fam Zheng
2016-06-22 8:34 ` Kevin Wolf
2016-06-22 9:10 ` Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 06/22] osdep: Introduce qemu_dup Fam Zheng
2016-06-17 12:32 ` Kevin Wolf
2016-06-17 13:08 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2016-06-22 7:37 ` Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 07/22] raw-posix: Use qemu_dup Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 08/22] raw-posix: Add image locking support Fam Zheng
2016-06-03 23:53 ` Fam Zheng
2016-06-17 13:07 ` Kevin Wolf
2016-06-22 8:27 ` Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 09/22] qemu-io: Add "-L" option for BDRV_O_NO_LOCK Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 10/22] qemu-img: Add "-L" option to sub commands Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 11/22] qemu-img: Update documentation of "-L" option Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 12/22] qemu-nbd: Add "--no-lock/-L" option Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 13/22] block: Don't lock drive-backup target image in none mode Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 14/22] mirror: Disable image locking on target backing chain Fam Zheng
2016-06-06 15:03 ` Max Reitz
2016-06-08 3:22 ` Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 15/22] qemu-iotests: 046: Move version detection out from verify_io Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 16/22] qemu-iotests: Wait for QEMU processes before checking image in 091 Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 17/22] qemu-iotests: 030: Disable image locking when checking test image Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 18/22] iotests: 087: Disable image locking in cases where file is shared Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 19/22] iotests: Disable image locking in 085 Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 20/22] tests: Use null-co:// instead of /dev/null Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 21/22] block: Turn on image locking by default Fam Zheng
2016-06-03 8:49 ` [Qemu-devel] [PATCH v6 22/22] qemu-iotests: Add test case 153 for image locking Fam Zheng
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=20160906164555.GI4667@noname.redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=den@openvz.org \
--cc=famz@redhat.com \
--cc=jcody@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rjones@redhat.com \
--cc=stefanha@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.