On Wed, Aug 02, 2023 at 01:15:40PM +0200, Hanna Czenczek wrote:
On 01.08.23 18:03, Stefano Garzarella wrote:
libblkio drivers take ownership of `fd` only after a successful
blkio_connect(), so if it fails, we are still the owners.
Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for
virtio-blk")
Suggested-by: Hanna Czenczek <hre...@redhat.com>
Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
---
block/blkio.c | 9 +++++++++
1 file changed, 9 insertions(+)
Works, so:
Reviewed-by: Hanna Czenczek <hre...@redhat.com>
But personally, instead of having `fd_supported` track whether we have
a valid FD or not, I’d find it more intuitive to track ownership
through the `fd` variable itself, i.e. initialize it to -1, and set it
-1 when ownership is transferred, and then close it once we don’t need
it anymore, but failed to transfer ownership to blkio. The elaborate
way would be something like
...
-int fd, ret;
+int fd = -1;
+int ret;
...
ret = blkio_connect(s->blkio);
+if (!ret) {
+ /* If we had an FD, libblkio now has ownership of it */
+ fd = -1;
+}
+if (fd >= 0) {
+ /* We still have FD ownership, but no longer need it, so close it */
+ qemu_close(fd);
+ fd = -1;
+}
/*
* [...]
*/
if (fd_supported && ret == -EINVAL) {
- qemu_close(fd);
-
...
Or the shorter less-verbose version would be:
...
-int fd, ret;
+int fd = -1;
+int ret;
...
ret = blkio_connect(s->blkio);
+if (fd >= 0 && ret < 0) {
+ /* Failed to give the FD to libblkio, close it */
+ qemu_close(fd);
+}
I like this, I'll do it in v2!
Thanks,
Stefano
