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

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).