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

[Stefan Hajnoczi]

On Mon, Jan 16, 2023 at 04:45:28PM +0100, Stefano Garzarella wrote:
> 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

Great, thanks for doing this. I will review the MR.

Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20230116/7a085aea/attachment.sig>