[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.10 3/9] block: honor BDRV_O_ALLOW_RDWR whe
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH for-2.10 3/9] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only |
Date: |
Wed, 5 Apr 2017 20:55:49 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Apr 05, 2017 at 08:20:28PM -0400, Jeff Cody wrote:
> On Wed, Apr 05, 2017 at 02:26:53PM -0500, Eric Blake wrote:
> > On 04/05/2017 02:20 PM, John Snow wrote:
> >
> > > Conceptually straightforward.
> > >
> > > looks like this might change behavior for... RBD and vvfat, right?
> > > RBD is the subject of this series so we'll just assume that was broken
> > > and stupid.
> > >
>
> Yes on RBD, and that change is intentional.
>
> > > What's vvfat's story? It always set the read-only property to false
> > > regardless of what you asked for?
> >
> > vvfat is even stupider than that - it has its own independent property
> > 'rw' that determines whether to allow write operations, separate from
> > the inherited BDS readonly property.
> >
>
> Yes, it is very odd. But if we have copy_on_read enabled, or explicitly set
> the block device to read-only via QAPI or -drive, I think that those should
> take precedence.
>
I meant to add an example - currently, with this drive commandline:
"-drive format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on"
The drive is not read-only, but writable. That seems broken.
After this patch, this ends up throwing an error now (which I think is a
logical thing to do):
qemu-system-x86_64: -drive
format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on: Node '#block238' is read
only
How this affects vvfat (and rbd) should be documented in the commit message,
however, so I should ammend that if we keep this behavior.
One other possible option is to treat the vvfat 'rw' option as meaning
"enable writes, iff the block device is writable". With that
interpretation, we could do something different in the above scenario:
silently fail to set bs->read_only to 'true' in the vvfat driver, and keep
it r/o.
[Qemu-devel] [PATCH for-2.10 4/9] block: code movement, Jeff Cody, 2017/04/05
[Qemu-devel] [PATCH for-2.10 2/9] block: do not set BDS read_only if copy_on_read enabled, Jeff Cody, 2017/04/05
[Qemu-devel] [PATCH for-2.10 5/9] block: introduce bdrv_try_set_read_only(), Jeff Cody, 2017/04/05