Libguestfs - Jan 2023 - [PATCH nbdkit] New plugin: blkio [incomplete]

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.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

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

Thanks,
Stefano