qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/10] Block patches for 5.0-rc1
@ 2020-03-26 14:29 Max Reitz
  2020-03-26 14:29 ` [PULL 01/10] block/mirror: fix use after free of local_err Max Reitz
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

The following changes since commit 01e38186ecb1fc6275720c5425332eed280ea93d:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200325b' into staging (2020-03-26 09:28:11 +0000)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2020-03-26

for you to fetch changes up to a507c51790fa955c1fccd4deca3c50476a862b83:

  iotests/138: Test leaks/corruptions fixed report (2020-03-26 14:52:43 +0100)

----------------------------------------------------------------
Block patches for 5.0-rc1:
- Fix qemu-img convert with a host device or iscsi target
- Use-after-free fix in mirror
- Some minor qcow2 fixes
- Minor sheepdog fix
- Minor qemu-img check report fix

----------------------------------------------------------------
Eric Blake (4):
  qcow2: Comment typo fixes
  qcow2: List autoclear bit names in header
  qcow2: Avoid feature name extension on small cluster size
  sheepdog: Consistently set bdrv_has_zero_init_truncate

Max Reitz (3):
  qemu-img: Fix check's leak/corruption fix report
  iotests: Add poke_file_[bl]e functions
  iotests/138: Test leaks/corruptions fixed report

Maxim Levitsky (2):
  block: pass BlockDriver reference to the .bdrv_co_create
  block: trickle down the fallback image creation function use to the
    block drivers

Vladimir Sementsov-Ogievskiy (1):
  block/mirror: fix use after free of local_err

 docs/interop/qcow2.txt       |  3 ++-
 include/block/block.h        |  1 +
 include/block/block_int.h    | 14 +++++++++++-
 block.c                      | 38 +++++++++++++++++++--------------
 block/crypto.c               |  3 ++-
 block/file-posix.c           | 11 ++++++++--
 block/file-win32.c           |  4 +++-
 block/gluster.c              |  3 ++-
 block/iscsi.c                | 16 ++++----------
 block/mirror.c               |  1 +
 block/nbd.c                  |  6 ++++++
 block/nfs.c                  |  4 +++-
 block/nvme.c                 |  3 +++
 block/parallels.c            |  3 ++-
 block/qcow.c                 |  3 ++-
 block/qcow2.c                | 33 +++++++++++++++++++++++------
 block/qed.c                  |  3 ++-
 block/raw-format.c           |  4 +++-
 block/rbd.c                  |  3 ++-
 block/sheepdog.c             |  6 +++++-
 block/ssh.c                  |  4 +++-
 block/vdi.c                  |  4 +++-
 block/vhdx.c                 |  3 ++-
 block/vmdk.c                 |  4 +++-
 block/vpc.c                  |  6 ++++--
 qemu-img.c                   |  9 ++++++--
 tests/qemu-iotests/031.out   |  8 +++----
 tests/qemu-iotests/036       |  6 ++++--
 tests/qemu-iotests/036.out   |  4 ++--
 tests/qemu-iotests/061       |  6 ++++--
 tests/qemu-iotests/061.out   | 14 ++++++------
 tests/qemu-iotests/138       | 41 ++++++++++++++++++++++++++++++++++--
 tests/qemu-iotests/138.out   | 14 ++++++++++++
 tests/qemu-iotests/common.rc | 24 +++++++++++++++++++++
 34 files changed, 233 insertions(+), 76 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PULL 01/10] block/mirror: fix use after free of local_err
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 02/10] block: pass BlockDriver reference to the .bdrv_co_create Max Reitz
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

local_err is used again in mirror_exit_common() after
bdrv_set_backing_hd(), so we must zero it. Otherwise try to set
non-NULL local_err will crash.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200324153630.11882-3-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/mirror.c b/block/mirror.c
index 447051dbc6..6203e5946e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -678,6 +678,7 @@ static int mirror_exit_common(Job *job)
             bdrv_set_backing_hd(target_bs, backing, &local_err);
             if (local_err) {
                 error_report_err(local_err);
+                local_err = NULL;
                 ret = -EPERM;
             }
         }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 02/10] block: pass BlockDriver reference to the .bdrv_co_create
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
  2020-03-26 14:29 ` [PULL 01/10] block/mirror: fix use after free of local_err Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 03/10] block: trickle down the fallback image creation function use to the block drivers Max Reitz
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Maxim Levitsky <mlevitsk@redhat.com>

This will allow the reuse of a single generic .bdrv_co_create
implementation for several drivers.
No functional changes.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200326011218.29230-2-mlevitsk@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h | 3 ++-
 block.c                   | 3 ++-
 block/crypto.c            | 3 ++-
 block/file-posix.c        | 4 +++-
 block/file-win32.c        | 4 +++-
 block/gluster.c           | 3 ++-
 block/nfs.c               | 4 +++-
 block/parallels.c         | 3 ++-
 block/qcow.c              | 3 ++-
 block/qcow2.c             | 4 +++-
 block/qed.c               | 3 ++-
 block/raw-format.c        | 4 +++-
 block/rbd.c               | 3 ++-
 block/sheepdog.c          | 4 +++-
 block/ssh.c               | 4 +++-
 block/vdi.c               | 4 +++-
 block/vhdx.c              | 3 ++-
 block/vmdk.c              | 4 +++-
 block/vpc.c               | 6 ++++--
 19 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index ae9c4da4d0..57c8ea24b2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -135,7 +135,8 @@ struct BlockDriver {
     void (*bdrv_close)(BlockDriverState *bs);
     int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
                                        Error **errp);
-    int coroutine_fn (*bdrv_co_create_opts)(const char *filename,
+    int coroutine_fn (*bdrv_co_create_opts)(BlockDriver *drv,
+                                            const char *filename,
                                             QemuOpts *opts,
                                             Error **errp);
     int (*bdrv_make_empty)(BlockDriverState *bs);
diff --git a/block.c b/block.c
index cccae5add9..ff23e20443 100644
--- a/block.c
+++ b/block.c
@@ -483,7 +483,8 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
     CreateCo *cco = opaque;
     assert(cco->drv);
 
-    ret = cco->drv->bdrv_co_create_opts(cco->filename, cco->opts, &local_err);
+    ret = cco->drv->bdrv_co_create_opts(cco->drv,
+                                        cco->filename, cco->opts, &local_err);
     error_propagate(&cco->err, local_err);
     cco->ret = ret;
 }
diff --git a/block/crypto.c b/block/crypto.c
index 4425ebeb47..d577f89659 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -601,7 +601,8 @@ fail:
     return ret;
 }
 
-static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
+static int coroutine_fn block_crypto_co_create_opts_luks(BlockDriver *drv,
+                                                         const char *filename,
                                                          QemuOpts *opts,
                                                          Error **errp)
 {
diff --git a/block/file-posix.c b/block/file-posix.c
index 9bc3838b2a..65bc980bc4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2405,7 +2405,9 @@ out:
     return result;
 }
 
-static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
                                            Error **errp)
 {
     BlockdevCreateOptions options;
diff --git a/block/file-win32.c b/block/file-win32.c
index 77e8ff7b68..15859839a1 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -588,7 +588,9 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
     return 0;
 }
 
-static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
                                            Error **errp)
 {
     BlockdevCreateOptions options;
diff --git a/block/gluster.c b/block/gluster.c
index 4fa4a77a47..0aa1f2cda4 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1130,7 +1130,8 @@ out:
     return ret;
 }
 
-static int coroutine_fn qemu_gluster_co_create_opts(const char *filename,
+static int coroutine_fn qemu_gluster_co_create_opts(BlockDriver *drv,
+                                                    const char *filename,
                                                     QemuOpts *opts,
                                                     Error **errp)
 {
diff --git a/block/nfs.c b/block/nfs.c
index 9a6311e270..cc2413d5ab 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -662,7 +662,9 @@ out:
     return ret;
 }
 
-static int coroutine_fn nfs_file_co_create_opts(const char *url, QemuOpts *opts,
+static int coroutine_fn nfs_file_co_create_opts(BlockDriver *drv,
+                                                const char *url,
+                                                QemuOpts *opts,
                                                 Error **errp)
 {
     BlockdevCreateOptions *create_options;
diff --git a/block/parallels.c b/block/parallels.c
index 7a01997659..6d4ed77f16 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -609,7 +609,8 @@ exit:
     goto out;
 }
 
-static int coroutine_fn parallels_co_create_opts(const char *filename,
+static int coroutine_fn parallels_co_create_opts(BlockDriver *drv,
+                                                 const char *filename,
                                                  QemuOpts *opts,
                                                  Error **errp)
 {
diff --git a/block/qcow.c b/block/qcow.c
index fce8989868..8973e4e565 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -934,7 +934,8 @@ exit:
     return ret;
 }
 
-static int coroutine_fn qcow_co_create_opts(const char *filename,
+static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,
+                                            const char *filename,
                                             QemuOpts *opts, Error **errp)
 {
     BlockdevCreateOptions *create_options = NULL;
diff --git a/block/qcow2.c b/block/qcow2.c
index d1da3d91db..5f65fce924 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3558,7 +3558,9 @@ out:
     return ret;
 }
 
-static int coroutine_fn qcow2_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
+                                             const char *filename,
+                                             QemuOpts *opts,
                                              Error **errp)
 {
     BlockdevCreateOptions *create_options = NULL;
diff --git a/block/qed.c b/block/qed.c
index d8c4e5fb1e..1af9b3cb1d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -720,7 +720,8 @@ out:
     return ret;
 }
 
-static int coroutine_fn bdrv_qed_co_create_opts(const char *filename,
+static int coroutine_fn bdrv_qed_co_create_opts(BlockDriver *drv,
+                                                const char *filename,
                                                 QemuOpts *opts,
                                                 Error **errp)
 {
diff --git a/block/raw-format.c b/block/raw-format.c
index 3a76ec7dd2..93b25e1b6b 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -419,7 +419,9 @@ static int raw_has_zero_init_truncate(BlockDriverState *bs)
     return bdrv_has_zero_init_truncate(bs->file->bs);
 }
 
-static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
                                            Error **errp)
 {
     return bdrv_create_file(filename, opts, errp);
diff --git a/block/rbd.c b/block/rbd.c
index 84115d34b4..e637639a07 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -437,7 +437,8 @@ static int qemu_rbd_co_create(BlockdevCreateOptions *options, Error **errp)
     return qemu_rbd_do_create(options, NULL, NULL, errp);
 }
 
-static int coroutine_fn qemu_rbd_co_create_opts(const char *filename,
+static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv,
+                                                const char *filename,
                                                 QemuOpts *opts,
                                                 Error **errp)
 {
diff --git a/block/sheepdog.c b/block/sheepdog.c
index cfa84338a2..a8a7e32a41 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2157,7 +2157,9 @@ out:
     return ret;
 }
 
-static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn sd_co_create_opts(BlockDriver *drv,
+                                          const char *filename,
+                                          QemuOpts *opts,
                                           Error **errp)
 {
     BlockdevCreateOptions *create_options = NULL;
diff --git a/block/ssh.c b/block/ssh.c
index b4375cf7d2..84e92821c0 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -963,7 +963,9 @@ fail:
     return ret;
 }
 
-static int coroutine_fn ssh_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn ssh_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
                                            Error **errp)
 {
     BlockdevCreateOptions *create_options;
diff --git a/block/vdi.c b/block/vdi.c
index 0142da7233..e1a11f2aa0 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -896,7 +896,9 @@ static int coroutine_fn vdi_co_create(BlockdevCreateOptions *create_options,
     return vdi_co_do_create(create_options, DEFAULT_CLUSTER_SIZE, errp);
 }
 
-static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn vdi_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
                                            Error **errp)
 {
     QDict *qdict = NULL;
diff --git a/block/vhdx.c b/block/vhdx.c
index f02d2611be..33e57cd656 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -2046,7 +2046,8 @@ delete_and_exit:
     return ret;
 }
 
-static int coroutine_fn vhdx_co_create_opts(const char *filename,
+static int coroutine_fn vhdx_co_create_opts(BlockDriver *drv,
+                                            const char *filename,
                                             QemuOpts *opts,
                                             Error **errp)
 {
diff --git a/block/vmdk.c b/block/vmdk.c
index 8466051bc9..218d9c9800 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2588,7 +2588,9 @@ exit:
     return blk;
 }
 
-static int coroutine_fn vmdk_co_create_opts(const char *filename, QemuOpts *opts,
+static int coroutine_fn vmdk_co_create_opts(BlockDriver *drv,
+                                            const char *filename,
+                                            QemuOpts *opts,
                                             Error **errp)
 {
     Error *local_err = NULL;
diff --git a/block/vpc.c b/block/vpc.c
index a65550298e..6df75e22dc 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -1089,8 +1089,10 @@ out:
     return ret;
 }
 
-static int coroutine_fn vpc_co_create_opts(const char *filename,
-                                           QemuOpts *opts, Error **errp)
+static int coroutine_fn vpc_co_create_opts(BlockDriver *drv,
+                                           const char *filename,
+                                           QemuOpts *opts,
+                                           Error **errp)
 {
     BlockdevCreateOptions *create_options = NULL;
     QDict *qdict;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 03/10] block: trickle down the fallback image creation function use to the block drivers
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
  2020-03-26 14:29 ` [PULL 01/10] block/mirror: fix use after free of local_err Max Reitz
  2020-03-26 14:29 ` [PULL 02/10] block: pass BlockDriver reference to the .bdrv_co_create Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 04/10] qcow2: Comment typo fixes Max Reitz
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Maxim Levitsky <mlevitsk@redhat.com>

Instead of checking the .bdrv_co_create_opts to see if we need the
fallback, just implement the .bdrv_co_create_opts in the drivers that
need it.

This way we don't break various places that need to know if the
underlying protocol/format really supports image creation, and this way
we still allow some drivers to not support image creation.

Fixes: fd17146cd93d1704cd96d7c2757b325fc7aac6fd
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1816007

Note that technically this driver reverts the image creation fallback
for the vxhs driver since I don't have a means to test it, and IMHO it
is better to leave it not supported as it was prior to generic image
creation patches.

Also drop iscsi_create_opts which was left accidentally.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200326011218.29230-3-mlevitsk@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
[mreitz: Fixed alignment, and moved bdrv_co_create_opts_simple() and
         bdrv_create_opts_simple from block.h into block_int.h]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block.h     |  1 +
 include/block/block_int.h | 11 +++++++++++
 block.c                   | 35 ++++++++++++++++++++---------------
 block/file-posix.c        |  7 ++++++-
 block/iscsi.c             | 16 ++++------------
 block/nbd.c               |  6 ++++++
 block/nvme.c              |  3 +++
 7 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index e569a4d747..b05995fe9c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -283,6 +283,7 @@ BlockDriver *bdrv_find_format(const char *format_name);
 int bdrv_create(BlockDriver *drv, const char* filename,
                 QemuOpts *opts, Error **errp);
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
+
 BlockDriverState *bdrv_new(void);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
                  Error **errp);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 57c8ea24b2..4c3587ea19 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1331,4 +1331,15 @@ int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
 void bdrv_set_monitor_owned(BlockDriverState *bs);
 BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp);
 
+/**
+ * Simple implementation of bdrv_co_create_opts for protocol drivers
+ * which only support creation via opening a file
+ * (usually existing raw storage device)
+ */
+int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
+                                            const char *filename,
+                                            QemuOpts *opts,
+                                            Error **errp);
+extern QemuOptsList bdrv_create_opts_simple;
+
 #endif /* BLOCK_INT_H */
diff --git a/block.c b/block.c
index ff23e20443..af3faf664e 100644
--- a/block.c
+++ b/block.c
@@ -598,8 +598,15 @@ static int create_file_fallback_zero_first_sector(BlockBackend *blk,
     return 0;
 }
 
-static int bdrv_create_file_fallback(const char *filename, BlockDriver *drv,
-                                     QemuOpts *opts, Error **errp)
+/**
+ * Simple implementation of bdrv_co_create_opts for protocol drivers
+ * which only support creation via opening a file
+ * (usually existing raw storage device)
+ */
+int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
+                                            const char *filename,
+                                            QemuOpts *opts,
+                                            Error **errp)
 {
     BlockBackend *blk;
     QDict *options;
@@ -663,11 +670,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
         return -ENOENT;
     }
 
-    if (drv->bdrv_co_create_opts) {
-        return bdrv_create(drv, filename, opts, errp);
-    } else {
-        return bdrv_create_file_fallback(filename, drv, opts, errp);
-    }
+    return bdrv_create(drv, filename, opts, errp);
 }
 
 int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp)
@@ -1592,9 +1595,9 @@ QemuOptsList bdrv_runtime_opts = {
     },
 };
 
-static QemuOptsList fallback_create_opts = {
-    .name = "fallback-create-opts",
-    .head = QTAILQ_HEAD_INITIALIZER(fallback_create_opts.head),
+QemuOptsList bdrv_create_opts_simple = {
+    .name = "simple-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(bdrv_create_opts_simple.head),
     .desc = {
         {
             .name = BLOCK_OPT_SIZE,
@@ -5963,13 +5966,15 @@ void bdrv_img_create(const char *filename, const char *fmt,
         return;
     }
 
+    if (!proto_drv->create_opts) {
+        error_setg(errp, "Protocol driver '%s' does not support image creation",
+                   proto_drv->format_name);
+        return;
+    }
+
     /* Create parameter list */
     create_opts = qemu_opts_append(create_opts, drv->create_opts);
-    if (proto_drv->create_opts) {
-        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
-    } else {
-        create_opts = qemu_opts_append(create_opts, &fallback_create_opts);
-    }
+    create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
 
     opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
 
diff --git a/block/file-posix.c b/block/file-posix.c
index 65bc980bc4..7e19bbff5f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3513,6 +3513,8 @@ static BlockDriver bdrv_host_device = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
+    .bdrv_co_create_opts = bdrv_co_create_opts_simple,
+    .create_opts         = &bdrv_create_opts_simple,
     .mutable_opts        = mutable_opts,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
     .bdrv_co_pwrite_zeroes = hdev_co_pwrite_zeroes,
@@ -3639,10 +3641,11 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
+    .bdrv_co_create_opts = bdrv_co_create_opts_simple,
+    .create_opts         = &bdrv_create_opts_simple,
     .mutable_opts        = mutable_opts,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
 
-
     .bdrv_co_preadv         = raw_co_preadv,
     .bdrv_co_pwritev        = raw_co_pwritev,
     .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
@@ -3771,6 +3774,8 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
+    .bdrv_co_create_opts = bdrv_co_create_opts_simple,
+    .create_opts         = &bdrv_create_opts_simple,
     .mutable_opts       = mutable_opts,
 
     .bdrv_co_preadv         = raw_co_preadv,
diff --git a/block/iscsi.c b/block/iscsi.c
index 682abd8e09..14680a436a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2399,18 +2399,6 @@ out_unlock:
     return r;
 }
 
-static QemuOptsList iscsi_create_opts = {
-    .name = "iscsi-create-opts",
-    .head = QTAILQ_HEAD_INITIALIZER(iscsi_create_opts.head),
-    .desc = {
-        {
-            .name = BLOCK_OPT_SIZE,
-            .type = QEMU_OPT_SIZE,
-            .help = "Virtual disk size"
-        },
-        { /* end of list */ }
-    }
-};
 
 static const char *const iscsi_strong_runtime_opts[] = {
     "transport",
@@ -2434,6 +2422,8 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_parse_filename    = iscsi_parse_filename,
     .bdrv_file_open         = iscsi_open,
     .bdrv_close             = iscsi_close,
+    .bdrv_co_create_opts    = bdrv_co_create_opts_simple,
+    .create_opts            = &bdrv_create_opts_simple,
     .bdrv_reopen_prepare    = iscsi_reopen_prepare,
     .bdrv_reopen_commit     = iscsi_reopen_commit,
     .bdrv_co_invalidate_cache = iscsi_co_invalidate_cache,
@@ -2471,6 +2461,8 @@ static BlockDriver bdrv_iser = {
     .bdrv_parse_filename    = iscsi_parse_filename,
     .bdrv_file_open         = iscsi_open,
     .bdrv_close             = iscsi_close,
+    .bdrv_co_create_opts    = bdrv_co_create_opts_simple,
+    .create_opts            = &bdrv_create_opts_simple,
     .bdrv_reopen_prepare    = iscsi_reopen_prepare,
     .bdrv_reopen_commit     = iscsi_reopen_commit,
     .bdrv_co_invalidate_cache  = iscsi_co_invalidate_cache,
diff --git a/block/nbd.c b/block/nbd.c
index 976be76647..2160859f64 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -2038,6 +2038,8 @@ static BlockDriver bdrv_nbd = {
     .protocol_name              = "nbd",
     .instance_size              = sizeof(BDRVNBDState),
     .bdrv_parse_filename        = nbd_parse_filename,
+    .bdrv_co_create_opts        = bdrv_co_create_opts_simple,
+    .create_opts                = &bdrv_create_opts_simple,
     .bdrv_file_open             = nbd_open,
     .bdrv_reopen_prepare        = nbd_client_reopen_prepare,
     .bdrv_co_preadv             = nbd_client_co_preadv,
@@ -2063,6 +2065,8 @@ static BlockDriver bdrv_nbd_tcp = {
     .protocol_name              = "nbd+tcp",
     .instance_size              = sizeof(BDRVNBDState),
     .bdrv_parse_filename        = nbd_parse_filename,
+    .bdrv_co_create_opts        = bdrv_co_create_opts_simple,
+    .create_opts                = &bdrv_create_opts_simple,
     .bdrv_file_open             = nbd_open,
     .bdrv_reopen_prepare        = nbd_client_reopen_prepare,
     .bdrv_co_preadv             = nbd_client_co_preadv,
@@ -2088,6 +2092,8 @@ static BlockDriver bdrv_nbd_unix = {
     .protocol_name              = "nbd+unix",
     .instance_size              = sizeof(BDRVNBDState),
     .bdrv_parse_filename        = nbd_parse_filename,
+    .bdrv_co_create_opts        = bdrv_co_create_opts_simple,
+    .create_opts                = &bdrv_create_opts_simple,
     .bdrv_file_open             = nbd_open,
     .bdrv_reopen_prepare        = nbd_client_reopen_prepare,
     .bdrv_co_preadv             = nbd_client_co_preadv,
diff --git a/block/nvme.c b/block/nvme.c
index d41c4bda6e..7b7c0cc5d6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1333,6 +1333,9 @@ static BlockDriver bdrv_nvme = {
     .protocol_name            = "nvme",
     .instance_size            = sizeof(BDRVNVMeState),
 
+    .bdrv_co_create_opts      = bdrv_co_create_opts_simple,
+    .create_opts              = &bdrv_create_opts_simple,
+
     .bdrv_parse_filename      = nvme_parse_filename,
     .bdrv_file_open           = nvme_file_open,
     .bdrv_close               = nvme_close,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 04/10] qcow2: Comment typo fixes
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (2 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 03/10] block: trickle down the fallback image creation function use to the block drivers Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 05/10] qcow2: List autoclear bit names in header Max Reitz
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Eric Blake <eblake@redhat.com>

Various trivial typos noticed while working on this file.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200324174233.1622067-2-eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 5f65fce924..b565cf912e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -177,7 +177,7 @@ static ssize_t qcow2_crypto_hdr_write_func(QCryptoBlock *block, size_t offset,
 }
 
 
-/* 
+/*
  * read qcow2 extension and fill bs
  * start reading from start_offset
  * finish reading upon magic of value 0 or when end_offset reached
@@ -3255,7 +3255,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
      * inconsistency later.
      *
      * We do need a refcount table because growing the refcount table means
-     * allocating two new refcount blocks - the seconds of which would be at
+     * allocating two new refcount blocks - the second of which would be at
      * 2 GB for 64k clusters, and we don't want to have a 2 GB initial file
      * size for any qcow2 image.
      */
@@ -3500,7 +3500,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
         goto out;
     }
 
-    /* Want a backing file? There you go.*/
+    /* Want a backing file? There you go. */
     if (qcow2_opts->has_backing_file) {
         const char *backing_format = NULL;
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 05/10] qcow2: List autoclear bit names in header
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (3 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 04/10] qcow2: Comment typo fixes Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 06/10] qcow2: Avoid feature name extension on small cluster size Max Reitz
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Eric Blake <eblake@redhat.com>

The feature table is supposed to advertise the name of all feature
bits that we support; however, we forgot to update the table for
autoclear bits.  While at it, move the table to read-only memory in
code, and tweak the qcow2 spec to name the second autoclear bit.
Update iotests that are affected by the longer header length.

Fixes: 88ddffae
Fixes: 93c24936
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200324174233.1622067-3-eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 docs/interop/qcow2.txt     |  3 ++-
 block/qcow2.c              | 12 +++++++++++-
 tests/qemu-iotests/031.out |  8 ++++----
 tests/qemu-iotests/036.out |  4 ++--
 tests/qemu-iotests/061.out | 14 +++++++-------
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 5597e24474..640e0eca40 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -143,7 +143,8 @@ the next fields through header_length.
                                 bit is unset, the bitmaps extension data must be
                                 considered inconsistent.
 
-                    Bit 1:      If this bit is set, the external data file can
+                    Bit 1:      Raw external data bit
+                                If this bit is set, the external data file can
                                 be read as a consistent standalone raw image
                                 without looking at the qcow2 metadata.
 
diff --git a/block/qcow2.c b/block/qcow2.c
index b565cf912e..b74cbeb047 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2825,7 +2825,7 @@ int qcow2_update_header(BlockDriverState *bs)
 
     /* Feature table */
     if (s->qcow_version >= 3) {
-        Qcow2Feature features[] = {
+        static const Qcow2Feature features[] = {
             {
                 .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
                 .bit  = QCOW2_INCOMPAT_DIRTY_BITNR,
@@ -2846,6 +2846,16 @@ int qcow2_update_header(BlockDriverState *bs)
                 .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
                 .name = "lazy refcounts",
             },
+            {
+                .type = QCOW2_FEAT_TYPE_AUTOCLEAR,
+                .bit  = QCOW2_AUTOCLEAR_BITMAPS_BITNR,
+                .name = "bitmaps",
+            },
+            {
+                .type = QCOW2_FEAT_TYPE_AUTOCLEAR,
+                .bit  = QCOW2_AUTOCLEAR_DATA_FILE_RAW_BITNR,
+                .name = "raw external data",
+            },
         };
 
         ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out
index d535e407bc..46f97c5a4e 100644
--- a/tests/qemu-iotests/031.out
+++ b/tests/qemu-iotests/031.out
@@ -117,7 +117,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 Header extension:
@@ -150,7 +150,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 Header extension:
@@ -164,7 +164,7 @@ No errors were found on the image.
 
 magic                     0x514649fb
 version                   3
-backing_file_offset       0x178
+backing_file_offset       0x1d8
 backing_file_size         0x17
 cluster_bits              16
 size                      67108864
@@ -188,7 +188,7 @@ data                      'host_device'
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 Header extension:
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index 0b52b934e1..23b699ce06 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -26,7 +26,7 @@ compatible_features       []
 autoclear_features        [63]
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 
@@ -38,7 +38,7 @@ compatible_features       []
 autoclear_features        []
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 *** done
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 8b3091a412..413cc4e0f4 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -26,7 +26,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 magic                     0x514649fb
@@ -84,7 +84,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 magic                     0x514649fb
@@ -140,7 +140,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 ERROR cluster 5 refcount=0 reference=1
@@ -195,7 +195,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 magic                     0x514649fb
@@ -264,7 +264,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 read 65536/65536 bytes at offset 44040192
@@ -298,7 +298,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 ERROR cluster 5 refcount=0 reference=1
@@ -327,7 +327,7 @@ header_length             104
 
 Header extension:
 magic                     0x6803f857
-length                    192
+length                    288
 data                      <binary>
 
 read 131072/131072 bytes at offset 0
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 06/10] qcow2: Avoid feature name extension on small cluster size
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (4 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 05/10] qcow2: List autoclear bit names in header Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 07/10] sheepdog: Consistently set bdrv_has_zero_init_truncate Max Reitz
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Eric Blake <eblake@redhat.com>

As the feature name table can be quite large (over 9k if all 64 bits
of all three feature fields have names; a mere 8 features leaves only
8 bytes for a backing file name in a 512-byte cluster), it is unwise
to emit this optional header in images with small cluster sizes.

Update iotest 036 to skip running on small cluster sizes; meanwhile,
note that iotest 061 never passed on alternative cluster sizes
(however, I limited this patch to tests with output affected by adding
feature names, rather than auditing for other tests that are not
robust to alternative cluster sizes).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200324174233.1622067-4-eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c          | 11 +++++++++--
 tests/qemu-iotests/036 |  6 ++++--
 tests/qemu-iotests/061 |  6 ++++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b74cbeb047..2bb536b014 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2823,8 +2823,15 @@ int qcow2_update_header(BlockDriverState *bs)
         buflen -= ret;
     }
 
-    /* Feature table */
-    if (s->qcow_version >= 3) {
+    /*
+     * Feature table.  A mere 8 feature names occupies 392 bytes, and
+     * when coupled with the v3 minimum header of 104 bytes plus the
+     * 8-byte end-of-extension marker, that would leave only 8 bytes
+     * for a backing file name in an image with 512-byte clusters.
+     * Thus, we choose to omit this header for cluster sizes 4k and
+     * smaller.
+     */
+    if (s->qcow_version >= 3 && s->cluster_size > 4096) {
         static const Qcow2Feature features[] = {
             {
                 .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index 512598421c..cf522de7a1 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -44,8 +44,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 # Only qcow2v3 and later supports feature bits;
-# qcow2.py does not support external data files
-_unsupported_imgopts 'compat=0.10' data_file
+# qcow2.py does not support external data files;
+# this test requires a cluster size large enough for the feature table
+_unsupported_imgopts 'compat=0.10' data_file \
+		     'cluster_size=\(512\|1024\|2048\|4096\)'
 
 echo
 echo === Image with unknown incompatible feature bit ===
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 36b040491f..ce285d3084 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -44,8 +44,10 @@ _supported_os Linux
 # Conversion between different compat versions can only really work
 # with refcount_bits=16;
 # we have explicit tests for data_file here, but the whole test does
-# not work with it
-_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
+# not work with it;
+# we have explicit tests for various cluster sizes, the remaining tests
+# require the default 64k cluster
+_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file cluster_size
 
 echo
 echo "=== Testing version downgrade with zero expansion ==="
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 07/10] sheepdog: Consistently set bdrv_has_zero_init_truncate
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (5 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 06/10] qcow2: Avoid feature name extension on small cluster size Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 08/10] qemu-img: Fix check's leak/corruption fix report Max Reitz
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

From: Eric Blake <eblake@redhat.com>

block_int.h claims that .bdrv_has_zero_init must return 0 if
.bdrv_has_zero_init_truncate does likewise; but this is violated if
only the former callback is provided if .bdrv_co_truncate also exists.
When adding the latter callback, it was mistakenly added to only one
of the three possible sheepdog instantiations.

Fixes: 1dcaf527
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200324174233.1622067-5-eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/sheepdog.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a8a7e32a41..59f7ebb171 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -3271,6 +3271,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
+    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
@@ -3309,6 +3310,7 @@ static BlockDriver bdrv_sheepdog_unix = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
+    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 08/10] qemu-img: Fix check's leak/corruption fix report
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (6 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 07/10] sheepdog: Consistently set bdrv_has_zero_init_truncate Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 09/10] iotests: Add poke_file_[bl]e functions Max Reitz
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

There are two problems with qemu-img check's report on how many leaks
and/or corruptions have been fixed:

(1) ImageCheck.has_leaks_fixed and ImageCheck.has_corruptions_fixed are
only true when ImageCheck.leaks or ImageCheck.corruptions (respectively)
are non-zero.  qcow2's check implementation will set the latter to zero
after it has fixed leaks and corruptions, though, so leaks-fixed and
corruptions-fixed are actually never reported after successful repairs.
We should always report them when they are non-zero, just like all the
other fields of ImageCheck.

(2) After something has been fixed and we run the check a second time,
leaks_fixed and corruptions_fixed are taken from the first run; but
has_leaks_fixed and has_corruptions_fixed are not.  The second run
actually cannot fix anything, so with (1) fixed, has_leaks_fixed and
has_corruptions_fixed will always be false here.  (With (1) unfixed,
they will at least be false on successful runs, because then the number
of leaks and corruptions found in the second run should be 0.)
We should save has_leaks_fixed and has_corruptions_fixed just like we
save leaks_fixed and corruptions_fixed.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200324172757.1173824-2-mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index afddf33f08..b167376bd7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -647,9 +647,9 @@ static int collect_image_check(BlockDriverState *bs,
     check->leaks                    = result.leaks;
     check->has_leaks                = result.leaks != 0;
     check->corruptions_fixed        = result.corruptions_fixed;
-    check->has_corruptions_fixed    = result.corruptions != 0;
+    check->has_corruptions_fixed    = result.corruptions_fixed != 0;
     check->leaks_fixed              = result.leaks_fixed;
-    check->has_leaks_fixed          = result.leaks != 0;
+    check->has_leaks_fixed          = result.leaks_fixed != 0;
     check->image_end_offset         = result.image_end_offset;
     check->has_image_end_offset     = result.image_end_offset != 0;
     check->total_clusters           = result.bfi.total_clusters;
@@ -803,9 +803,12 @@ static int img_check(int argc, char **argv)
 
     if (check->corruptions_fixed || check->leaks_fixed) {
         int corruptions_fixed, leaks_fixed;
+        bool has_leaks_fixed, has_corruptions_fixed;
 
         leaks_fixed         = check->leaks_fixed;
+        has_leaks_fixed     = check->has_leaks_fixed;
         corruptions_fixed   = check->corruptions_fixed;
+        has_corruptions_fixed = check->has_corruptions_fixed;
 
         if (output_format == OFORMAT_HUMAN) {
             qprintf(quiet,
@@ -822,7 +825,9 @@ static int img_check(int argc, char **argv)
         ret = collect_image_check(bs, check, filename, fmt, 0);
 
         check->leaks_fixed          = leaks_fixed;
+        check->has_leaks_fixed      = has_leaks_fixed;
         check->corruptions_fixed    = corruptions_fixed;
+        check->has_corruptions_fixed = has_corruptions_fixed;
     }
 
     if (!ret) {
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 09/10] iotests: Add poke_file_[bl]e functions
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (7 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 08/10] qemu-img: Fix check's leak/corruption fix report Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 14:29 ` [PULL 10/10] iotests/138: Test leaks/corruptions fixed report Max Reitz
  2020-03-26 16:54 ` [PULL 00/10] Block patches for 5.0-rc1 Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

Similarly to peek_file_[bl]e, we may want to write binary integers into
a file.  Currently, this often means messing around with poke_file and
raw binary strings.  I hope these functions make it a bit more
comfortable.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Code-suggested-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200324172757.1173824-3-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.rc | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 4c246c0450..bf3b9fdea0 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -53,6 +53,30 @@ poke_file()
     printf "$3" | dd "of=$1" bs=1 "seek=$2" conv=notrunc &>/dev/null
 }
 
+# poke_file_le $img_filename $offset $byte_width $value
+# Example: poke_file_le "$TEST_IMG" 512 2 65534
+poke_file_le()
+{
+    local img=$1 ofs=$2 len=$3 val=$4 str=''
+
+    while ((len--)); do
+        str+=$(printf '\\x%02x' $((val & 0xff)))
+        val=$((val >> 8))
+    done
+
+    poke_file "$img" "$ofs" "$str"
+}
+
+# poke_file_be $img_filename $offset $byte_width $value
+# Example: poke_file_be "$TEST_IMG" 512 2 65279
+poke_file_be()
+{
+    local img=$1 ofs=$2 len=$3 val=$4
+    local str=$(printf "%0$((len * 2))x\n" $val | sed 's/\(..\)/\\x\1/g')
+
+    poke_file "$img" "$ofs" "$str"
+}
+
 # peek_file_le 'test.img' 512 2 => 65534
 peek_file_le()
 {
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 10/10] iotests/138: Test leaks/corruptions fixed report
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (8 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 09/10] iotests: Add poke_file_[bl]e functions Max Reitz
@ 2020-03-26 14:29 ` Max Reitz
  2020-03-26 16:54 ` [PULL 00/10] Block patches for 5.0-rc1 Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Max Reitz @ 2020-03-26 14:29 UTC (permalink / raw)
  To: qemu-block; +Cc: Kevin Wolf, Peter Maydell, qemu-devel, Max Reitz

Test that qemu-img check reports the number of leaks and corruptions
fixed in its JSON report (after a successful run).

While touching the _unsupported_imgopts line, adjust the note on why
data_file does not work with this test: The current comment sounds a bit
like it is a mistake for qemu-img check not to check external data
files' refcounts.  But there are no such refcounts, so it is no mistake.
Just say that qemu-img check does not do much for external data files,
and this is why this test does not work with them.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200324172757.1173824-4-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/138     | 41 ++++++++++++++++++++++++++++++++++++--
 tests/qemu-iotests/138.out | 14 +++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
index 54b01046ad..1d5b0bed6d 100755
--- a/tests/qemu-iotests/138
+++ b/tests/qemu-iotests/138
@@ -41,8 +41,10 @@ _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
 # With an external data file, data clusters are not refcounted
-# (and so qemu-img check does not check their refcount)
-_unsupported_imgopts data_file
+# (so qemu-img check would not do much);
+# we want to modify the refcounts, so we need them to have a specific
+# format (namely u16)
+_unsupported_imgopts data_file 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
 
 echo
 echo '=== Check on an image with a multiple of 2^32 clusters ==='
@@ -65,6 +67,41 @@ poke_file "$TEST_IMG" $((2048 + 8)) "\x00\x80\x00\x00\x00\x00\x00\x00"
 # allocate memory", we have an error showing that l2 entry is invalid.
 _check_test_img
 
+echo
+echo '=== Check leaks-fixed/corruptions-fixed report'
+echo
+
+# After leaks and corruptions were fixed, those numbers should be
+# reported by qemu-img check
+_make_test_img 64k
+
+# Allocate data cluster
+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+reftable_ofs=$(peek_file_be "$TEST_IMG" 48 8)
+refblock_ofs=$(peek_file_be "$TEST_IMG" $reftable_ofs 8)
+
+# Introduce a leak: Make the image header's refcount 2
+poke_file_be "$TEST_IMG" "$refblock_ofs" 2 2
+
+l1_ofs=$(peek_file_be "$TEST_IMG" 40 8)
+
+# Introduce a corruption: Drop the COPIED flag from the (first) L1 entry
+l1_entry=$(peek_file_be "$TEST_IMG" $l1_ofs 8)
+l1_entry=$((l1_entry & ~(1 << 63)))
+poke_file_be "$TEST_IMG" $l1_ofs 8 $l1_entry
+
+echo
+# Should print the number of corruptions and leaks fixed
+# (Filter out all JSON fields (recognizable by their four-space
+# indentation), but keep the "-fixed" fields (by removing two spaces
+# from their indentation))
+# (Also filter out the L1 entry, because why not)
+_check_test_img -r all --output=json \
+    | sed -e 's/^  \(.*\)-fixed"/\1-fixed"/' \
+          -e '/^    /d' \
+          -e "s/\\([^0-9a-f]\\)$(printf %x $l1_entry)\\([^0-9a-f]\\)/\1L1_ENTRY_VALUE\2/"
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/138.out b/tests/qemu-iotests/138.out
index aca7d47a80..79681e7cc9 100644
--- a/tests/qemu-iotests/138.out
+++ b/tests/qemu-iotests/138.out
@@ -9,4 +9,18 @@ ERROR: counting reference for region exceeding the end of the file by one cluste
 
 1 errors were found on the image.
 Data may be corrupted, or further writes to the image may corrupt it.
+
+=== Check leaks-fixed/corruptions-fixed report
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Leaked cluster 0 refcount=2 reference=1
+Repairing cluster 0 refcount=2 reference=1
+Repairing OFLAG_COPIED L2 cluster: l1_index=0 l1_entry=L1_ENTRY_VALUE refcount=1
+{
+  "corruptions-fixed": 1,
+  "leaks-fixed": 1,
+}
 *** done
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PULL 00/10] Block patches for 5.0-rc1
  2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
                   ` (9 preceding siblings ...)
  2020-03-26 14:29 ` [PULL 10/10] iotests/138: Test leaks/corruptions fixed report Max Reitz
@ 2020-03-26 16:54 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2020-03-26 16:54 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, QEMU Developers, Qemu-block

On Thu, 26 Mar 2020 at 14:29, Max Reitz <mreitz@redhat.com> wrote:
>
> The following changes since commit 01e38186ecb1fc6275720c5425332eed280ea93d:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200325b' into staging (2020-03-26 09:28:11 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/XanClic/qemu.git tags/pull-block-2020-03-26
>
> for you to fetch changes up to a507c51790fa955c1fccd4deca3c50476a862b83:
>
>   iotests/138: Test leaks/corruptions fixed report (2020-03-26 14:52:43 +0100)
>
> ----------------------------------------------------------------
> Block patches for 5.0-rc1:
> - Fix qemu-img convert with a host device or iscsi target
> - Use-after-free fix in mirror
> - Some minor qcow2 fixes
> - Minor sheepdog fix
> - Minor qemu-img check report fix
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-26 16:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 14:29 [PULL 00/10] Block patches for 5.0-rc1 Max Reitz
2020-03-26 14:29 ` [PULL 01/10] block/mirror: fix use after free of local_err Max Reitz
2020-03-26 14:29 ` [PULL 02/10] block: pass BlockDriver reference to the .bdrv_co_create Max Reitz
2020-03-26 14:29 ` [PULL 03/10] block: trickle down the fallback image creation function use to the block drivers Max Reitz
2020-03-26 14:29 ` [PULL 04/10] qcow2: Comment typo fixes Max Reitz
2020-03-26 14:29 ` [PULL 05/10] qcow2: List autoclear bit names in header Max Reitz
2020-03-26 14:29 ` [PULL 06/10] qcow2: Avoid feature name extension on small cluster size Max Reitz
2020-03-26 14:29 ` [PULL 07/10] sheepdog: Consistently set bdrv_has_zero_init_truncate Max Reitz
2020-03-26 14:29 ` [PULL 08/10] qemu-img: Fix check's leak/corruption fix report Max Reitz
2020-03-26 14:29 ` [PULL 09/10] iotests: Add poke_file_[bl]e functions Max Reitz
2020-03-26 14:29 ` [PULL 10/10] iotests/138: Test leaks/corruptions fixed report Max Reitz
2020-03-26 16:54 ` [PULL 00/10] Block patches for 5.0-rc1 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).