[Libguestfs] [PATCH nbdkit] New plugin: blkio [incomplete]

[Stefano Garzarella]

On Mon, Jan 16, 2023 at 03:39:29PM +0100, Stefano Garzarella wrote:
>On Wed, Jan 11, 2023 at 04:49:14PM +0000, Richard W.M. Jones wrote:
>>On Wed, Jan 11, 2023 at 11:34:50AM -0500, Stefan Hajnoczi wrote:
>>>On Sat, Jan 07, 2023 at 07:44:37PM +0000, Richard W.M. Jones wrote:
>>>>
>>>> This is upstream in nbdkit now:
>>>> https://gitlab.com/nbdkit/nbdkit/-/tree/master/plugins/blkio
>>>>
>>>> Another question:
>>>>
>>>> (6) vhost-user + "read-only" property acts a bit strangely.  After
>>>> opening virtio-blk-vhost-user it throws an EROFS error if you try to
>>>> "blkio_start" it.  However if you set the "read-only" property to true
>>>> then it's OK to start it, and subsequent read operations work too.
>>>>
>>>> I would expect that any device can be started without needing to set
>>>> this property, and in general I don't understand why vhost-user is r/o
>>>> since everything I read about it seems to indicate it's a general
>>>> purpose writable protocol.
>>>
>>>I guess the vhost-user-blk server is read-only and that's why it's
>>>refusing to blkio_start() unless readonly was set to true.
>>>
>>>If you're using qemu-storage-daemon, set the writable=on parameter on
>>>the export.
>>
>>So it turned out to be the case.  Alberto pointed out that unless you
>>set writable=on explicitly, q-s-d makes the export read-only.  This
>>was the reverse of what I expected.  But it is covered in the q-s-d
>>man page.
>>
>>But about libblkio: The problem is basically you must know if the
>>export is read-only or not before you connect.  For a readonly
>>vhost-user export this is the observed behaviour:
>>
>> blkio_create                 ==> OK
>> blkio_get_bool ("read-only") ==> returns false
>> blkio_connect                ==> OK
>> blkio_get_bool ("read-only") ==> returns false
>> blkio_start                  ==> fails "Device is read-only"
>>
>>At least after connecting it seems we should be able to find out that
>>the export is read-only.
>
>Yes, indeed IIUC we could take this information, but currently 
>"read-only" property for virtio-blk is not filled by querying the 
>device.
>
>At this point I wonder if for virtio-blk devices we should provide the 
>"read-only" property not writable (sorry for the confusion...) so that 
>it reflects whether the device is read-only or not.
>
>Otherwise we could also leave it writable, but surely after the 
>connect we would have to set it with the value exported from the 
>device and allow the user to change it before the start (I think it's 
>a bit different from what we do in io-uring, or maybe it is comparable 
>when we use "fd" instead of "path" in the io-uring driver).

I just checked, and IIUC when we use the io-uring driver and use the 
"fd" property instead of "path" to specify the backend, the "read-only" 
property is ignored if set by the user and overwritten (if the fd was 
opened read-only).

So I think for virtio-blk we should do the same. (I'll prepare a MR and 
maybe we can discuss there).

Rich thanks for bringing it up!

Stefano