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

[Stefano Garzarella]

On Mon, Jan 16, 2023 at 3:46 PM Stefano Garzarella <sgarzare at redhat.com> 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

Thanks,
Stefano