From: Eric Blake <eblake@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, mreitz@redhat.com, pkrempa@redhat.com,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 3/8] block: Require auto-read-only for existing fallbacks
Date: Tue, 16 Oct 2018 13:51:27 -0500 [thread overview]
Message-ID: <d4c232b8-dfd4-46ca-e582-3c74d990c9ba@redhat.com> (raw)
In-Reply-To: <20181016141243.GH5620@dhcp-200-186.str.redhat.com>
On 10/16/18 9:12 AM, Kevin Wolf wrote:
> Am 12.10.2018 um 19:02 hat Eric Blake geschrieben:
>> On 10/12/18 6:55 AM, Kevin Wolf wrote:
>>> Some block drivers have traditionally changed their node to read-only
>>> mode without asking the user. This behaviour has been marked deprecated
>>> since 2.11, expecting users to provide an explicit read-only=on option.
>>>
>>> Now that we have auto-read-only=on, enable these drivers to make use of
>>> the option.
>>>
>>> This is the only use of bdrv_set_read_only(), so we can make it a bit
>>> more specific and turn it into a bdrv_apply_auto_read_only() that is
>>> more convenient for drivers to use.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>
>> Worth documenting the -EINVAL (copy-on-read prevents setting read-only)
>> failure as well? (The -EPERM failure of bdrv_can_set_read_only() is not
>> reachable, since this new function never clears readonly).
>
> In fact, -EINVAL and the error string from bdrv_can_set_read_only() may
> be confusing because the user didn't explicitly request a read-only
> image. Maybe it would be better to just turn this case into -EACCES with
> the same error message.
>
> What do you think?
So, how would it trigger in practice? The user requests a copy-on-read
action with the BDS as destination (thus the BDS must be writable, and
can't be set to readonly); they omitted read-only (because they know
they want copy-on-read); they supplied auto-read-only=true (because they
are lazy and want to always use that flag if it is available); but the
particular BDS they selected is not writable (whether read-only file
system, read-only NBD server, etc). In short, we can't grant them
read-write to begin with, and can't gracefully fall back to read-only
because it would violate their request for copy-on-read, so as long as
we give them a sane error message about their request being impossible,
we're good. Yes, -EACCES sounds reasonable, if you want to code that in.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
next prev parent reply other threads:[~2018-10-16 18:51 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-12 11:55 [Qemu-devel] [PATCH v2 0/8] block: Add auto-read-only option Kevin Wolf
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 1/8] block: Update flags in bdrv_set_read_only() Kevin Wolf
2018-10-12 16:19 ` Eric Blake
2018-10-17 9:02 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 2/8] block: Add auto-read-only option Kevin Wolf
2018-10-12 16:47 ` Eric Blake
2018-10-15 9:37 ` Kevin Wolf
2018-10-16 18:46 ` Eric Blake
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 3/8] block: Require auto-read-only for existing fallbacks Kevin Wolf
2018-10-12 17:02 ` Eric Blake
2018-10-16 14:12 ` Kevin Wolf
2018-10-16 18:51 ` Eric Blake [this message]
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 4/8] nbd: Support auto-read-only option Kevin Wolf
2018-10-12 14:09 ` Eric Blake
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 5/8] file-posix: " Kevin Wolf
2018-10-12 17:24 ` Eric Blake
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 6/8] curl: " Kevin Wolf
2018-10-12 17:30 ` Eric Blake
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 7/8] gluster: " Kevin Wolf
2018-10-12 17:31 ` Eric Blake
2018-10-14 11:04 ` [Qemu-devel] [Qemu-block] " Niels de Vos
2018-10-12 11:55 ` [Qemu-devel] [PATCH v2 8/8] iscsi: " Kevin Wolf
2018-10-12 17:32 ` Eric Blake
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=d4c232b8-dfd4-46ca-e582-3c74d990c9ba@redhat.com \
--to=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pkrempa@redhat.com \
--cc=qemu-block@nongnu.org \
--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).