Series comparison

-[PULL 0/3] Block patches
+[PULL for-8.1 0/2] Block patches
-The following changes since commit 73e0f70e097b7c92a5ce16ee35b53afe119b20d7:
+The following changes since commit 9ba37026fcf6b7f3f096c0cca3e1e7307802486b:
-  Merge tag 'pull-lu-20211123' of https://gitlab.com/rth7680/qemu into staging (2021-11-23 11:33:14 +0100)
+  Update version for v8.1.0-rc2 release (2023-08-02 08:22:45 -0700)
 are available in the Git repository at:
-  https://gitlab.com/hreitz/qemu.git tags/pull-block-2021-11-23
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 4dd218fd0717ed3cddb69c01eeb9da630107d89d:
+for you to fetch changes up to 9b06d0d076271d76e5384d767ef94a676f0a9efd:
-  iotests/149: Skip on unsupported ciphers (2021-11-23 15:39:12 +0100)
+  block/blkio: add more comments on the fd passing handling (2023-08-03 11:28:43 -0400)
 ----------------------------------------------------------------
-Block patches for 6.2-rc2:
+Pull request
-- Fix memory leak in vvfat when vvfat_open() fails
-- iotest fixes for the gnutls crypto backend
+Fix for an fd leak in the blkio block driver.
 ----------------------------------------------------------------
-Daniella Lee (1):
-  block/vvfat.c fix leak when failure occurs
-Hanna Reitz (2):
+Stefano Garzarella (2):
-  iotests: Use aes-128-cbc
+  block/blkio: close the fd when blkio_connect() fails
-  iotests/149: Skip on unsupported ciphers
+  block/blkio: add more comments on the fd passing handling
- block/vvfat.c              | 16 ++++++++++++----
+ block/blkio.c | 28 +++++++++++++++++++++-------
- tests/qemu-iotests/149     | 23 ++++++++++++++++++-----
+file changed, 21 insertions(+), 7 deletions(-)
  tests/qemu-iotests/206     |  4 ++--
  tests/qemu-iotests/206.out |  6 +++---
  tests/qemu-iotests/210     |  4 ++--
  tests/qemu-iotests/210.out |  6 +++---
 files changed, 40 insertions(+), 19 deletions(-)
 --
-.33.1
+.41.0

-[PULL 1/3] block/vvfat.c fix leak when failure occurs
+Deleted patch
-From: Daniella Lee <daniellalee111@gmail.com>
-Function vvfat_open called function enable_write_target and init_directories,
-and these functions malloc new memory for BDRVVVFATState::qcow_filename,
-BDRVVVFATState::used_clusters, and BDRVVVFATState::cluster_buff.
-When the specified folder does not exist ,it may contains memory leak.
-After init_directories function is executed, the vvfat_open return -EIO,
-and bdrv_open_driver goto label open_failed,
-the program use g_free(bs->opaque) to release BDRVVVFATState struct
-without members mentioned.
-command line:
-qemu-system-x86_64 -hdb <vdisk qcow file>  -usb -device usb-storage,drive=fat16
--drive file=fat:rw:fat-type=16:"<path of a host folder does not exist>",
-id=fat16,format=raw,if=none
-enable_write_target called:
-(gdb) bt
-    at ../block/vvfat.c:3114
-    flags=155650, errp=0x7fffffffd780) at ../block/vvfat.c:1236
-    node_name=0x0, options=0x555556fa45d0, open_flags=155650,
-    errp=0x7fffffffd890) at ../block.c:1558
-    errp=0x7fffffffd890) at ../block.c:1852
-    reference=0x0, options=0x555556fa45d0, flags=40962, parent=0x555556f98cd0,
-    child_class=0x555556b1d6a0 <child_of_bds>, child_role=19,
-    errp=0x7fffffffda90) at ../block.c:3779
-    options=0x555556f9cfc0, bdref_key=0x555556239bb8 "file",
-    parent=0x555556f98cd0, child_class=0x555556b1d6a0 <child_of_bds>,
-    child_role=19, allow_none=true, errp=0x7fffffffda90) at ../block.c:3419
-    reference=0x0, options=0x555556f9cfc0, flags=8194, parent=0x0,
-    child_class=0x0, child_role=0, errp=0x555556c98c40 <error_fatal>)
-    at ../block.c:3726
-    options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
-    at ../block.c:3872
-    options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
-    at ../block/block-backend.c:436
-    bs_opts=0x555556f757b0, errp=0x555556c98c40 <error_fatal>)
-    at ../blockdev.c:608
-    errp=0x555556c98c40 <error_fatal>) at ../blockdev.c:992
-......
-Signed-off-by: Daniella Lee <daniellalee111@gmail.com>
-Message-Id: <20211119112553.352222-1-daniellalee111@gmail.com>
-[hreitz: Took commit message from v1]
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
----
- block/vvfat.c | 16 ++++++++++++----
-file changed, 12 insertions(+), 4 deletions(-)
-diff --git a/block/vvfat.c b/block/vvfat.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/vvfat.c
-+++ b/block/vvfat.c
-@@ -XXX,XX +XXX,XX @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
-     qemu_co_mutex_init(&s->lock);
--    ret = 0;
-+    qemu_opts_del(opts);
-+
-+    return 0;
-+
- fail:
-+    g_free(s->qcow_filename);
-+    s->qcow_filename = NULL;
-+    g_free(s->cluster_buffer);
-+    s->cluster_buffer = NULL;
-+    g_free(s->used_clusters);
-+    s->used_clusters = NULL;
-+
-     qemu_opts_del(opts);
-     return ret;
- }
-@@ -XXX,XX +XXX,XX @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
-     int size = sector2cluster(s, s->sector_count);
-     QDict *options;
--    s->used_clusters = calloc(size, 1);
-+    s->used_clusters = g_malloc0(size);
-     array_init(&(s->commits), sizeof(commit_t));
-@@ -XXX,XX +XXX,XX @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
-     return 0;
- err:
--    g_free(s->qcow_filename);
--    s->qcow_filename = NULL;
-     return ret;
- }
---
-.33.1

-[PULL 3/3] iotests/149: Skip on unsupported ciphers
+[PULL for-8.1 1/2] block/blkio: close the fd when blkio_connect() fails
-Whenever qemu-img or qemu-io report that some cipher is unsupported,
+From: Stefano Garzarella <sgarzare@redhat.com>
 skip the whole test, because that is probably because qemu has been
 configured with the gnutls crypto backend.
-We could taylor the algorithm list to what gnutls supports, but this is
+libblkio drivers take ownership of `fd` only after a successful
-a test that is run rather rarely anyway (because it requires
+blkio_connect(), so if it fails, we are still the owners.
 password-less sudo), and so it seems better and easier to skip it.  When
 this test is intentionally run to check LUKS compatibility, it seems
 better not to limit the algorithms but keep the list extensive.
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
+Fixes: cad2ccc395 ("block/blkio: use qemu_open() to support fd passing for virtio-blk")
-Message-Id: <20211117151707.52549-3-hreitz@redhat.com>
+Suggested-by: Hanna Czenczek <hreitz@redhat.com>
-Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
 Message-id: 20230803082825.25293-2-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
+ block/blkio.c | 11 ++++++++---
-file changed, 18 insertions(+), 5 deletions(-)
+file changed, 8 insertions(+), 3 deletions(-)
-diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
+diff --git a/block/blkio.c b/block/blkio.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/149
+--- a/block/blkio.c
-+++ b/tests/qemu-iotests/149
++++ b/block/blkio.c
-@@ -XXX,XX +XXX,XX @@ def create_image(config, size_mb):
+@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
-         fn.truncate(size_mb * 1024 * 1024)
+     const char *path = qdict_get_try_str(options, "path");
+     BDRVBlkioState *s = bs->opaque;
+     bool fd_supported = false;
-+def check_cipher_support(config, output):
+-    int fd, ret;
-+    """Check the output of qemu-img or qemu-io for mention of the respective
++    int fd = -1, ret;
-+    cipher algorithm being unsupported, and if so, skip this test.
-+    (Returns `output` for convenience.)"""
+     if (!path) {
          error_setg(errp, "missing 'path' option");
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
              if (ret < 0) {
                  fd_supported = false;
                  qemu_close(fd);
 +                fd = -1;
              }
          }
      }
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
      }
      ret = blkio_connect(s->blkio);
 +    if (ret < 0 && fd >= 0) {
 +        /* Failed to give the FD to libblkio, close it */
 +        qemu_close(fd);
 +        fd = -1;
 +    }
 +
-+    if 'Unsupported cipher algorithm' in output:
+     /*
-+        iotests.notrun('Unsupported cipher algorithm '
+      * If the libblkio driver doesn't support the `fd` property, blkio_connect()
-+                       f'{config.cipher}-{config.keylen}-{config.mode}; '
+      * will fail with -EINVAL. So let's try calling blkio_connect() again by
-+                       'consider configuring qemu with a different crypto '
+      * directly setting `path`.
-+                       'backend')
+      */
-+    return output
+     if (fd_supported && ret == -EINVAL) {
-+
+-        qemu_close(fd);
- def qemu_img_create(config, size_mb):
+-
-     """Create and format a disk image with LUKS using qemu-img"""
+         /*
+          * We need to clear the `fd` property we set previously by setting
-@@ -XXX,XX +XXX,XX @@ def qemu_img_create(config, size_mb):
+          * it to -1.
              "%dM" % size_mb]
      iotests.log("qemu-img " + " ".join(args), filters=[iotests.filter_test_dir])
 -    iotests.log(iotests.qemu_img_pipe(*args), filters=[iotests.filter_test_dir])
 +    iotests.log(check_cipher_support(config, iotests.qemu_img_pipe(*args)),
 +                filters=[iotests.filter_test_dir])
  def qemu_io_image_args(config, dev=False):
      """Get the args for access an image or device with qemu-io"""
@@ -XXX,XX +XXX,XX @@ def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
      args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
      args.extend(qemu_io_image_args(config, dev))
      iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
 -    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
 -                                                 iotests.filter_qemu_io])
 +    iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
 +                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
  def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
@@ -XXX,XX +XXX,XX @@ def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
      args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
      args.extend(qemu_io_image_args(config, dev))
      iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
 -    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
 -                                                 iotests.filter_qemu_io])
 +    iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
 +                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
  def test_once(config, qemu_img=False):
 --
-.33.1
+.41.0

-[PULL 2/3] iotests: Use aes-128-cbc
+[PULL for-8.1 2/2] block/blkio: add more comments on the fd passing handling
-Our gnutls crypto backend (which is the default as of 8bd0931f6)
+From: Stefano Garzarella <sgarzare@redhat.com>
 supports neither twofish-128 nor the CTR mode.  CBC and aes-128 are
 supported by all of our backends (as far as I can tell), so use
 aes-128-cbc in our iotests.
-(We could also use e.g. aes-256-cbc, but the different key sizes would
+As Hanna pointed out, it is not clear in the code why qemu_open()
-lead to different key slot offsets and so change the reference output
+can fail, and why blkio_set_int("fd") is not enough to discover
-more, which is why I went with aes-128.)
+the `fd` property support.
-Signed-off-by: Hanna Reitz <hreitz@redhat.com>
+Let's fix them by adding more details in the code comments.
-Message-Id: <20211117151707.52549-2-hreitz@redhat.com>
-Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
+Suggested-by: Hanna Czenczek <hreitz@redhat.com>
-Tested-by: Thomas Huth <thuth@redhat.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
 Message-id: 20230803082825.25293-3-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/206     | 4 ++--
+ block/blkio.c | 15 ++++++++++++---
- tests/qemu-iotests/206.out | 6 +++---
+file changed, 12 insertions(+), 3 deletions(-)
  tests/qemu-iotests/210     | 4 ++--
  tests/qemu-iotests/210.out | 6 +++---
 files changed, 10 insertions(+), 10 deletions(-)
-diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
+diff --git a/block/blkio.c b/block/blkio.c
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/206
 +++ b/tests/qemu-iotests/206
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.qcow2') as disk_path, \
                           'encrypt': {
                               'format': 'luks',
                               'key-secret': 'keysec0',
 -                             'cipher-alg': 'twofish-128',
 -                             'cipher-mode': 'ctr',
 +                             'cipher-alg': 'aes-128',
 +                             'cipher-mode': 'cbc',
                               'ivgen-alg': 'plain64',
                               'ivgen-hash-alg': 'md5',
                               'hash-alg': 'sha1',
 diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
 index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/206.out
+--- a/block/blkio.c
-+++ b/tests/qemu-iotests/206.out
++++ b/block/blkio.c
-@@ -XXX,XX +XXX,XX @@ Format specific information:
+@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
+          */
- === Successful image creation (encrypted) ===
+         fd = qemu_open(path, O_RDWR, NULL);
+         if (fd < 0) {
--{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
++            /*
-+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
++             * qemu_open() can fail if the user specifies a path that is not
- {"return": {}}
++             * a file or device, for example in the case of Unix Domain Socket
- {"execute": "job-dismiss", "arguments": {"id": "job0"}}
++             * for the virtio-blk-vhost-user driver. In such cases let's have
- {"return": {}}
++             * libblkio open the path directly.
-@@ -XXX,XX +XXX,XX @@ Format specific information:
++             */
-     encrypt:
+             fd_supported = false;
-         ivgen alg: plain64
+         } else {
-         hash alg: sha1
+             ret = blkio_set_int(s->blkio, "fd", fd);
--        cipher alg: twofish-128
+@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
-+        cipher alg: aes-128
+     }
-         uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
-         format: luks
+     /*
--        cipher mode: ctr
+-     * If the libblkio driver doesn't support the `fd` property, blkio_connect()
-+        cipher mode: cbc
+-     * will fail with -EINVAL. So let's try calling blkio_connect() again by
-         slots:
+-     * directly setting `path`.
-             [0]:
++     * Before https://gitlab.com/libblkio/libblkio/-/merge_requests/208
-                 active: true
++     * (libblkio <= v1.3.0), setting the `fd` property is not enough to check
-diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
++     * whether the driver supports the `fd` property or not. In that case,
-index XXXXXXX..XXXXXXX 100755
++     * blkio_connect() will fail with -EINVAL.
---- a/tests/qemu-iotests/210
++     * So let's try calling blkio_connect() again by directly setting `path`
-+++ b/tests/qemu-iotests/210
++     * to cover this scenario.
-@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.luks') as disk_path, \
+      */
-                          },
+     if (fd_supported && ret == -EINVAL) {
-                          'size': size,
+         /*
                           'key-secret': 'keysec0',
 -                         'cipher-alg': 'twofish-128',
 -                         'cipher-mode': 'ctr',
 +                         'cipher-alg': 'aes-128',
 +                         'cipher-mode': 'cbc',
                           'ivgen-alg': 'plain64',
                           'ivgen-hash-alg': 'md5',
                           'hash-alg': 'sha1',
 diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/210.out
 +++ b/tests/qemu-iotests/210.out
@@ -XXX,XX +XXX,XX @@ Format specific information:
  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
  {"return": {}}
 -{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
 +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
  {"return": {}}
  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
  {"return": {}}
@@ -XXX,XX +XXX,XX @@ encrypted: yes
  Format specific information:
      ivgen alg: plain64
      hash alg: sha1
 -    cipher alg: twofish-128
 +    cipher alg: aes-128
      uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
 -    cipher mode: ctr
 +    cipher mode: cbc
      slots:
          [0]:
              active: true
 --
-.33.1
+.41.0

The following changes since commit 73e0f70e097b7c92a5ce16ee35b53afe119b20d7:

Merge tag 'pull-lu-20211123' of https://gitlab.com/rth7680/qemu into staging (2021-11-23 11:33:14 +0100)

are available in the Git repository at:

https://gitlab.com/hreitz/qemu.git tags/pull-block-2021-11-23

for you to fetch changes up to 4dd218fd0717ed3cddb69c01eeb9da630107d89d:

iotests/149: Skip on unsupported ciphers (2021-11-23 15:39:12 +0100)

----------------------------------------------------------------
Block patches for 6.2-rc2:
- Fix memory leak in vvfat when vvfat_open() fails
- iotest fixes for the gnutls crypto backend

----------------------------------------------------------------
Daniella Lee (1):
  block/vvfat.c fix leak when failure occurs

Hanna Reitz (2):
  iotests: Use aes-128-cbc
  iotests/149: Skip on unsupported ciphers

-- 
2.33.1

From: Daniella Lee <daniellalee111@gmail.com>

Function vvfat_open called function enable_write_target and init_directories,
and these functions malloc new memory for BDRVVVFATState::qcow_filename,
BDRVVVFATState::used_clusters, and BDRVVVFATState::cluster_buff.

When the specified folder does not exist ,it may contains memory leak.
After init_directories function is executed, the vvfat_open return -EIO,
and bdrv_open_driver goto label open_failed,
the program use g_free(bs->opaque) to release BDRVVVFATState struct
without members mentioned.

command line:
qemu-system-x86_64 -hdb <vdisk qcow file>  -usb -device usb-storage,drive=fat16
-drive file=fat:rw:fat-type=16:"<path of a host folder does not exist>",
id=fat16,format=raw,if=none

enable_write_target called:
(gdb) bt
    at ../block/vvfat.c:3114
    flags=155650, errp=0x7fffffffd780) at ../block/vvfat.c:1236
    node_name=0x0, options=0x555556fa45d0, open_flags=155650,
    errp=0x7fffffffd890) at ../block.c:1558
    errp=0x7fffffffd890) at ../block.c:1852
    reference=0x0, options=0x555556fa45d0, flags=40962, parent=0x555556f98cd0,
    child_class=0x555556b1d6a0 <child_of_bds>, child_role=19,
    errp=0x7fffffffda90) at ../block.c:3779
    options=0x555556f9cfc0, bdref_key=0x555556239bb8 "file",
    parent=0x555556f98cd0, child_class=0x555556b1d6a0 <child_of_bds>,
    child_role=19, allow_none=true, errp=0x7fffffffda90) at ../block.c:3419
    reference=0x0, options=0x555556f9cfc0, flags=8194, parent=0x0,
    child_class=0x0, child_role=0, errp=0x555556c98c40 <error_fatal>)
    at ../block.c:3726
    options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
    at ../block.c:3872
    options=0x555556f757b0, flags=0, errp=0x555556c98c40 <error_fatal>)
    at ../block/block-backend.c:436
    bs_opts=0x555556f757b0, errp=0x555556c98c40 <error_fatal>)
    at ../blockdev.c:608
    errp=0x555556c98c40 <error_fatal>) at ../blockdev.c:992
......

Signed-off-by: Daniella Lee <daniellalee111@gmail.com>
Message-Id: <20211119112553.352222-1-daniellalee111@gmail.com>
[hreitz: Took commit message from v1]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 block/vvfat.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -XXX,XX +XXX,XX @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
 
     qemu_co_mutex_init(&s->lock);
 
-    ret = 0;
+    qemu_opts_del(opts);
+
+    return 0;
+
 fail:
+    g_free(s->qcow_filename);
+    s->qcow_filename = NULL;
+    g_free(s->cluster_buffer);
+    s->cluster_buffer = NULL;
+    g_free(s->used_clusters);
+    s->used_clusters = NULL;
+
     qemu_opts_del(opts);
     return ret;
 }
@@ -XXX,XX +XXX,XX @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
     int size = sector2cluster(s, s->sector_count);
     QDict *options;
 
-    s->used_clusters = calloc(size, 1);
+    s->used_clusters = g_malloc0(size);
 
     array_init(&(s->commits), sizeof(commit_t));
 
@@ -XXX,XX +XXX,XX @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
     return 0;
 
 err:
-    g_free(s->qcow_filename);
-    s->qcow_filename = NULL;
     return ret;
 }
 
-- 
2.33.1

Our gnutls crypto backend (which is the default as of 8bd0931f6)
supports neither twofish-128 nor the CTR mode.  CBC and aes-128 are
supported by all of our backends (as far as I can tell), so use
aes-128-cbc in our iotests.

(We could also use e.g. aes-256-cbc, but the different key sizes would
lead to different key slot offsets and so change the reference output
more, which is why I went with aes-128.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211117151707.52549-2-hreitz@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/206     | 4 ++--
 tests/qemu-iotests/206.out | 6 +++---
 tests/qemu-iotests/210     | 4 ++--
 tests/qemu-iotests/210.out | 6 +++---
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.qcow2') as disk_path, \
                          'encrypt': {
                              'format': 'luks',
                              'key-secret': 'keysec0',
-                             'cipher-alg': 'twofish-128',
-                             'cipher-mode': 'ctr',
+                             'cipher-alg': 'aes-128',
+                             'cipher-mode': 'cbc',
                              'ivgen-alg': 'plain64',
                              'ivgen-hash-alg': 'md5',
                              'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -XXX,XX +XXX,XX @@ Format specific information:
 
 === Successful image creation (encrypted) ===
 
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
@@ -XXX,XX +XXX,XX @@ Format specific information:
     encrypt:
         ivgen alg: plain64
         hash alg: sha1
-        cipher alg: twofish-128
+        cipher alg: aes-128
         uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
         format: luks
-        cipher mode: ctr
+        cipher mode: cbc
         slots:
             [0]:
                 active: true
diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.luks') as disk_path, \
                          },
                          'size': size,
                          'key-secret': 'keysec0',
-                         'cipher-alg': 'twofish-128',
-                         'cipher-mode': 'ctr',
+                         'cipher-alg': 'aes-128',
+                         'cipher-mode': 'cbc',
                          'ivgen-alg': 'plain64',
                          'ivgen-hash-alg': 'md5',
                          'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -XXX,XX +XXX,XX @@ Format specific information:
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
 
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
@@ -XXX,XX +XXX,XX @@ encrypted: yes
 Format specific information:
     ivgen alg: plain64
     hash alg: sha1
-    cipher alg: twofish-128
+    cipher alg: aes-128
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
-    cipher mode: ctr
+    cipher mode: cbc
     slots:
         [0]:
             active: true
-- 
2.33.1

Whenever qemu-img or qemu-io report that some cipher is unsupported,
skip the whole test, because that is probably because qemu has been
configured with the gnutls crypto backend.

We could taylor the algorithm list to what gnutls supports, but this is
a test that is run rather rarely anyway (because it requires
password-less sudo), and so it seems better and easier to skip it.  When
this test is intentionally run to check LUKS compatibility, it seems
better not to limit the algorithms but keep the list extensive.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211117151707.52549-3-hreitz@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -XXX,XX +XXX,XX @@ def create_image(config, size_mb):
         fn.truncate(size_mb * 1024 * 1024)
 
 
+def check_cipher_support(config, output):
+    """Check the output of qemu-img or qemu-io for mention of the respective
+    cipher algorithm being unsupported, and if so, skip this test.
+    (Returns `output` for convenience.)"""
+
+    if 'Unsupported cipher algorithm' in output:
+        iotests.notrun('Unsupported cipher algorithm '
+                       f'{config.cipher}-{config.keylen}-{config.mode}; '
+                       'consider configuring qemu with a different crypto '
+                       'backend')
+    return output
+
 def qemu_img_create(config, size_mb):
     """Create and format a disk image with LUKS using qemu-img"""
 
@@ -XXX,XX +XXX,XX @@ def qemu_img_create(config, size_mb):
             "%dM" % size_mb]
 
     iotests.log("qemu-img " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_img_pipe(*args), filters=[iotests.filter_test_dir])
+    iotests.log(check_cipher_support(config, iotests.qemu_img_pipe(*args)),
+                filters=[iotests.filter_test_dir])
 
 def qemu_io_image_args(config, dev=False):
     """Get the args for access an image or device with qemu-io"""
@@ -XXX,XX +XXX,XX @@ def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
     args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
     args.extend(qemu_io_image_args(config, dev))
     iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
-                                                 iotests.filter_qemu_io])
+    iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
 
 
 def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
@@ -XXX,XX +XXX,XX @@ def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
     args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
     args.extend(qemu_io_image_args(config, dev))
     iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
-                                                 iotests.filter_qemu_io])
+    iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
 
 
 def test_once(config, qemu_img=False):
-- 
2.33.1

From: Stefano Garzarella <sgarzare@redhat.com>

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 <hreitz@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-id: 20230803082825.25293-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/blkio.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
     const char *path = qdict_get_try_str(options, "path");
     BDRVBlkioState *s = bs->opaque;
     bool fd_supported = false;
-    int fd, ret;
+    int fd = -1, ret;
 
     if (!path) {
         error_setg(errp, "missing 'path' option");
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
             if (ret < 0) {
                 fd_supported = false;
                 qemu_close(fd);
+                fd = -1;
             }
         }
     }
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
     }
 
     ret = blkio_connect(s->blkio);
+    if (ret < 0 && fd >= 0) {
+        /* Failed to give the FD to libblkio, close it */
+        qemu_close(fd);
+        fd = -1;
+    }
+
     /*
      * If the libblkio driver doesn't support the `fd` property, blkio_connect()
      * will fail with -EINVAL. So let's try calling blkio_connect() again by
      * directly setting `path`.
      */
     if (fd_supported && ret == -EINVAL) {
-        qemu_close(fd);
-
         /*
          * We need to clear the `fd` property we set previously by setting
          * it to -1.
-- 
2.41.0

From: Stefano Garzarella <sgarzare@redhat.com>

As Hanna pointed out, it is not clear in the code why qemu_open()
can fail, and why blkio_set_int("fd") is not enough to discover
the `fd` property support.

Let's fix them by adding more details in the code comments.

Suggested-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20230803082825.25293-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/blkio.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index XXXXXXX..XXXXXXX 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
          */
         fd = qemu_open(path, O_RDWR, NULL);
         if (fd < 0) {
+            /*
+             * qemu_open() can fail if the user specifies a path that is not
+             * a file or device, for example in the case of Unix Domain Socket
+             * for the virtio-blk-vhost-user driver. In such cases let's have
+             * libblkio open the path directly.
+             */
             fd_supported = false;
         } else {
             ret = blkio_set_int(s->blkio, "fd", fd);
@@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options,
     }
 
     /*
-     * If the libblkio driver doesn't support the `fd` property, blkio_connect()
-     * will fail with -EINVAL. So let's try calling blkio_connect() again by
-     * directly setting `path`.
+     * Before https://gitlab.com/libblkio/libblkio/-/merge_requests/208
+     * (libblkio <= v1.3.0), setting the `fd` property is not enough to check
+     * whether the driver supports the `fd` property or not. In that case,
+     * blkio_connect() will fail with -EINVAL.
+     * So let's try calling blkio_connect() again by directly setting `path`
+     * to cover this scenario.
      */
     if (fd_supported && ret == -EINVAL) {
         /*
-- 
2.41.0