| Message ID | 20230724154611.178858-3-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/blkio: fix opening virtio-blk drivers | expand |
On Mon, Jul 24, 2023 at 05:46:11PM +0200, Stefano Garzarella wrote: > The way the virtio-blk driver is implemented in libblkio, > it's much easier to use blkio_set_int() instead of blkio_get_int() > and have it fail right away to see if `fd` is supported by the > transport. See https://gitlab.com/libblkio/libblkio/-/merge_requests/208 The commit description is vague about what's going on here. My understanding is: Setting the `fd` property fails with virtio-blk-* libblkio drivers that do not support fd passing since https://gitlab.com/libblkio/libblkio/-/merge_requests/208. Getting the `fd` property, on the other hand, always succeeds for virtio-blk-* libblkio drivers even when they don't support fd passing. This patch switches to setting the `fd` property because it is a better mechanism for probing fd passing support than getting the `fd` property. Please update the commit description. Thanks! > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > block/blkio.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/block/blkio.c b/block/blkio.c > index ca1149042a..719b19324b 100644 > --- a/block/blkio.c > +++ b/block/blkio.c > @@ -665,7 +665,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, > const char *blkio_driver = bs->drv->protocol_name; > BDRVBlkioState *s = bs->opaque; > bool fd_supported = false; > - int fd, ret; > + int ret; > > if (!path) { > error_setg(errp, "missing 'path' option"); > @@ -678,7 +678,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, > } > > if (strcmp(blkio_driver, "virtio-blk-vhost-vdpa") == 0 && > - blkio_get_int(s->blkio, "fd", &fd) == 0) { > + blkio_set_int(s->blkio, "fd", -1) == 0) { > fd_supported = true; > } > > @@ -688,7 +688,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, > * layer through the "/dev/fdset/N" special path. > */ > if (fd_supported) { > - int open_flags; > + int open_flags, fd; > > if (flags & BDRV_O_RDWR) { > open_flags = O_RDWR; > -- > 2.41.0 >
On Tue, Jul 25, 2023 at 04:05:38PM -0400, Stefan Hajnoczi wrote: >On Mon, Jul 24, 2023 at 05:46:11PM +0200, Stefano Garzarella wrote: >> The way the virtio-blk driver is implemented in libblkio, >> it's much easier to use blkio_set_int() instead of blkio_get_int() >> and have it fail right away to see if `fd` is supported by the >> transport. See https://gitlab.com/libblkio/libblkio/-/merge_requests/208 > >The commit description is vague about what's going on here. My >understanding is: > > Setting the `fd` property fails with virtio-blk-* libblkio drivers > that do not support fd passing since > https://gitlab.com/libblkio/libblkio/-/merge_requests/208. > > Getting the `fd` property, on the other hand, always succeeds for > virtio-blk-* libblkio drivers even when they don't support fd passing. > > This patch switches to setting the `fd` property because it is a > better mechanism for probing fd passing support than getting the `fd` > property. > >Please update the commit description. Thanks! Ack, I'll update it. Thanks, Stefano > >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> block/blkio.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/block/blkio.c b/block/blkio.c >> index ca1149042a..719b19324b 100644 >> --- a/block/blkio.c >> +++ b/block/blkio.c >> @@ -665,7 +665,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, >> const char *blkio_driver = bs->drv->protocol_name; >> BDRVBlkioState *s = bs->opaque; >> bool fd_supported = false; >> - int fd, ret; >> + int ret; >> >> if (!path) { >> error_setg(errp, "missing 'path' option"); >> @@ -678,7 +678,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, >> } >> >> if (strcmp(blkio_driver, "virtio-blk-vhost-vdpa") == 0 && >> - blkio_get_int(s->blkio, "fd", &fd) == 0) { >> + blkio_set_int(s->blkio, "fd", -1) == 0) { >> fd_supported = true; >> } >> >> @@ -688,7 +688,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, >> * layer through the "/dev/fdset/N" special path. >> */ >> if (fd_supported) { >> - int open_flags; >> + int open_flags, fd; >> >> if (flags & BDRV_O_RDWR) { >> open_flags = O_RDWR; >> -- >> 2.41.0 >>
diff --git a/block/blkio.c b/block/blkio.c index ca1149042a..719b19324b 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -665,7 +665,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, const char *blkio_driver = bs->drv->protocol_name; BDRVBlkioState *s = bs->opaque; bool fd_supported = false; - int fd, ret; + int ret; if (!path) { error_setg(errp, "missing 'path' option"); @@ -678,7 +678,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, } if (strcmp(blkio_driver, "virtio-blk-vhost-vdpa") == 0 && - blkio_get_int(s->blkio, "fd", &fd) == 0) { + blkio_set_int(s->blkio, "fd", -1) == 0) { fd_supported = true; } @@ -688,7 +688,7 @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, * layer through the "/dev/fdset/N" special path. */ if (fd_supported) { - int open_flags; + int open_flags, fd; if (flags & BDRV_O_RDWR) { open_flags = O_RDWR;
The way the virtio-blk driver is implemented in libblkio, it's much easier to use blkio_set_int() instead of blkio_get_int() and have it fail right away to see if `fd` is supported by the transport. See https://gitlab.com/libblkio/libblkio/-/merge_requests/208 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- block/blkio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)