Re: [Qemu-devel] [PATCH for-2.10 3/9] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only

[Jeff Cody <jcody@redhat.com>]

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.