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

On Mon, Jan 16, 2023 at 3:46 PM Stefano Garzarella <sgarzare(a)redhat.com&gt; wrote: > 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). MR posted: https://gitlab.com/libblkio/libblkio/-/merge_requests/161