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

2023-07-26 Thread Stefan Hajnoczi 
On Wed, Jul 26, 2023 at 09:48:07AM +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")
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2225439
> Reported-by: Qing Wang 
> Signed-off-by: Stefano Garzarella 
> ---
> 
> Notes:
> v2:
> - added comment on top of qemu_open() [Daniel]
> 
> v1: 
> https://lore.kernel.org/qemu-devel/2023072555.85426-1-sgarz...@redhat.com/
> 
>  block/blkio.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

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

2023-07-26 Thread Daniel P . Berrangé 
On Wed, Jul 26, 2023 at 09:48:07AM +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")
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2225439
> Reported-by: Qing Wang 
> Signed-off-by: Stefano Garzarella 
> ---
> 
> Notes:
> v2:
> - added comment on top of qemu_open() [Daniel]
> 
> v1: 
> https://lore.kernel.org/qemu-devel/2023072555.85426-1-sgarz...@redhat.com/
> 
>  block/blkio.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé 

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 :|

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

2023-07-26 Thread Stefano Garzarella 
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")
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2225439
Reported-by: Qing Wang 
Signed-off-by: Stefano Garzarella 
---

Notes:
v2:
- added comment on top of qemu_open() [Daniel]

v1: 
https://lore.kernel.org/qemu-devel/2023072555.85426-1-sgarz...@redhat.com/

 block/blkio.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index 1798648134..cd6d2e55e7 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -686,15 +686,18 @@ 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);
+/*
+ * `path` can contain the path of a character device
+ * (e.g. /dev/vhost-vdpa-0 or /dev/vfio/vfio) or a unix socket.
+ *
+ * So, we should always open it with O_RDWR flag, also if BDRV_O_RDWR
+ * is not set in the open flags, because the exchange of IOCTL commands
+ * for example will fail.
+ *
+ * In order to open the device read-only, we are using the `read-only`
+ * property of the libblkio driver in blkio_file_open().
+ */
+fd = qemu_open(path, O_RDWR, errp);
 if (fd < 0) {
 return -EINVAL;
 }
-- 
2.41.0