Re: [PATCH] block/blkio: do not use open flags in qemu_open()

[Stefano Garzarella]

On Tue, Jul 25, 2023 at 12:15:40PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 25, 2023 at 01:11:55PM +0200, Stefano Garzarella wrote:
> > qemu_open() in blkio_virtio_blk_common_open() is used to open the
> > character device (e.g. /dev/vhost-vdpa-0 or /dev/vfio/vfio) or in
> > the future eventually the unix socket.
> >
> > In all these cases we cannot open the path in read-only mode,
> > when the `read-only` option of blockdev is on, because the exchange
> > of IOCTL commands for example will fail.
> >
> > In order to open the device read-only, we have to use the `read-only`
> > property of the libblkio driver as we already do in blkio_file_open().
> >
> > Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for 
> > virtio-blk")
> > Reported-by: Qing Wang <qinwang@redhat.com>
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  block/blkio.c | 10 +---------
> >  1 file changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/block/blkio.c b/block/blkio.c
> > index 1798648134..fe9bf8ea5f 100644
> > --- a/block/blkio.c
> > +++ b/block/blkio.c
> > @@ -686,15 +686,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState 
> > *bs,
> >       * layer through the "/dev/fdset/N" special path.
> >       */
> >      if (fd_supported) {
> > -        int open_flags;
> > -
> > -        if (flags & BDRV_O_RDWR) {
> > -            open_flags = O_RDWR;
> > -        } else {
> > -            open_flags = O_RDONLY;
> > -        }
> > -
> > -        fd = qemu_open(path, open_flags, errp);
>
> I'd suggest taking the paragraph from the commit message explaining why
> it is correct to hardcode O_RDWR and putting it into a comment here.

Ack, I'd also add:

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2225439

in the commit message.

I'll fix in v2.

Thanks,
Stefano

> > +        fd = qemu_open(path, O_RDWR, errp);
> >          if (fd < 0) {
> >              return -EINVAL;
> >          }
> > --
> > 2.41.0
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|