All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/13] Block layer patches
@ 2021-10-06 10:59 Kevin Wolf
  2021-10-06 10:59 ` [PULL 01/13] include/block.h: remove outdated comment Kevin Wolf
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

The following changes since commit e3acc2c1961cbe22ca474cd5da4163b7bbf7cea3:

  tests/docker/dockerfiles: Bump fedora-i386-cross to fedora 34 (2021-10-05 16:40:39 -0700)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 3765315d4c84f9c0799744f43a314169baaccc05:

  iotests: Update for pylint 2.11.1 (2021-10-06 10:25:55 +0200)

----------------------------------------------------------------
Block layer patches

- Fix I/O errors because of incorrectly detected max_iov
- Fix not white-listed copy-before-write
- qemu-storage-daemon: Only display FUSE help when FUSE is built-in
- iotests: update environment and linting configuration

----------------------------------------------------------------
Emanuele Giuseppe Esposito (1):
      include/block.h: remove outdated comment

John Snow (5):
      iotests: add 'qemu' package location to PYTHONPATH in testenv
      iotests/linters: check mypy files all at once
      iotests/mirror-top-perms: Adjust imports
      iotests/migrate-bitmaps-test: delint
      iotests: Update for pylint 2.11.1

Paolo Bonzini (1):
      block: introduce max_hw_iov for use in scsi-generic

Philippe Mathieu-Daudé (1):
      qemu-storage-daemon: Only display FUSE help when FUSE is built-in

Vladimir Sementsov-Ogievskiy (5):
      block: implement bdrv_new_open_driver_opts()
      block: bdrv_insert_node(): fix and improve error handling
      block: bdrv_insert_node(): doc and style
      block: bdrv_insert_node(): don't use bdrv_open()
      iotests/image-fleecing: declare requirement of copy-before-write

 include/block/block.h                         |  8 ++-
 include/block/block_int.h                     |  7 +++
 include/sysemu/block-backend.h                |  1 +
 block.c                                       | 79 ++++++++++++++++++++++-----
 block/block-backend.c                         |  6 ++
 block/file-posix.c                            |  2 +-
 block/io.c                                    |  1 +
 hw/scsi/scsi-generic.c                        |  2 +-
 storage-daemon/qemu-storage-daemon.c          |  2 +
 tests/qemu-iotests/iotests.py                 |  2 -
 tests/qemu-iotests/testenv.py                 | 15 +++--
 tests/qemu-iotests/testrunner.py              |  7 ++-
 tests/qemu-iotests/235                        |  2 -
 tests/qemu-iotests/297                        | 52 +++++++-----------
 tests/qemu-iotests/300                        |  5 +-
 tests/qemu-iotests/pylintrc                   |  6 +-
 tests/qemu-iotests/tests/image-fleecing       |  1 +
 tests/qemu-iotests/tests/migrate-bitmaps-test | 50 +++++++++--------
 tests/qemu-iotests/tests/mirror-top-perms     | 12 ++--
 19 files changed, 164 insertions(+), 96 deletions(-)



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

* [PULL 01/13] include/block.h: remove outdated comment
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 02/13] qemu-storage-daemon: Only display FUSE help when FUSE is built-in Kevin Wolf
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

There are a couple of errors in bdrv_drained_begin header comment:
- block_job_pause does not exist anymore, it has been replaced
  with job_pause in b15de82867
- job_pause is automatically invoked as a .drained_begin callback
  (child_job_drained_begin) by the child_job BdrvChildClass struct
  in blockjob.c. So no additional pause should be required.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20210903113800.59970-1-eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 740038a892..ab987e8a99 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -751,9 +751,7 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive,
  * bdrv_drained_begin:
  *
  * Begin a quiesced section for exclusive access to the BDS, by disabling
- * external request sources including NBD server and device model. Note that
- * this doesn't block timers or coroutines from submitting more requests, which
- * means block_job_pause is still necessary.
+ * external request sources including NBD server, block jobs, and device model.
  *
  * This function can be recursive.
  */
-- 
2.31.1



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

* [PULL 02/13] qemu-storage-daemon: Only display FUSE help when FUSE is built-in
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
  2021-10-06 10:59 ` [PULL 01/13] include/block.h: remove outdated comment Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 03/13] block: implement bdrv_new_open_driver_opts() Kevin Wolf
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Philippe Mathieu-Daudé <philmd@redhat.com>

When configuring QEMU with --disable-fuse, the qemu-storage-daemon
still reports FUSE command line options in its help:

  $ qemu-storage-daemon -h
  Usage: qemu-storage-daemon [options]
  QEMU storage daemon

    --export [type=]fuse,id=<id>,node-name=<node-name>,mountpoint=<file>
             [,growable=on|off][,writable=on|off]
                           export the specified block node over FUSE

Remove this help message when FUSE is disabled, to avoid:

  $ qemu-storage-daemon --export fuse
  qemu-storage-daemon: --export fuse: Invalid parameter 'fuse'

Reported-by: Qing Wang <qinwang@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210816180442.2000642-1-philmd@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 storage-daemon/qemu-storage-daemon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index fc8b150629..10a1a33761 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -98,10 +98,12 @@ static void help(void)
 "                         export the specified block node over NBD\n"
 "                         (requires --nbd-server)\n"
 "\n"
+#ifdef CONFIG_FUSE
 "  --export [type=]fuse,id=<id>,node-name=<node-name>,mountpoint=<file>\n"
 "           [,growable=on|off][,writable=on|off]\n"
 "                         export the specified block node over FUSE\n"
 "\n"
+#endif /* CONFIG_FUSE */
 "  --monitor [chardev=]name[,mode=control][,pretty[=on|off]]\n"
 "                         configure a QMP monitor\n"
 "\n"
-- 
2.31.1



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

* [PULL 03/13] block: implement bdrv_new_open_driver_opts()
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
  2021-10-06 10:59 ` [PULL 01/13] include/block.h: remove outdated comment Kevin Wolf
  2021-10-06 10:59 ` [PULL 02/13] qemu-storage-daemon: Only display FUSE help when FUSE is built-in Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 04/13] block: bdrv_insert_node(): fix and improve error handling Kevin Wolf
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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

Add version of bdrv_new_open_driver() that supports QDict options.
We'll use it in further commit.

Simply add one more argument to bdrv_new_open_driver() is worse, as
there are too many invocations of bdrv_new_open_driver() to update
then.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210920115538.264372-2-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h |  4 ++++
 block.c               | 25 +++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index ab987e8a99..e5dd22b034 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -383,6 +383,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
                            const char *bdref_key, Error **errp);
 BlockDriverState *bdrv_open(const char *filename, const char *reference,
                             QDict *options, int flags, Error **errp);
+BlockDriverState *bdrv_new_open_driver_opts(BlockDriver *drv,
+                                            const char *node_name,
+                                            QDict *options, int flags,
+                                            Error **errp);
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
                                        int flags, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
diff --git a/block.c b/block.c
index 5ce08a79fd..917fb7faca 100644
--- a/block.c
+++ b/block.c
@@ -1604,16 +1604,26 @@ open_failed:
     return ret;
 }
 
-BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
-                                       int flags, Error **errp)
+/*
+ * Create and open a block node.
+ *
+ * @options is a QDict of options to pass to the block drivers, or NULL for an
+ * empty set of options. The reference to the QDict belongs to the block layer
+ * after the call (even on failure), so if the caller intends to reuse the
+ * dictionary, it needs to use qobject_ref() before calling bdrv_open.
+ */
+BlockDriverState *bdrv_new_open_driver_opts(BlockDriver *drv,
+                                            const char *node_name,
+                                            QDict *options, int flags,
+                                            Error **errp)
 {
     BlockDriverState *bs;
     int ret;
 
     bs = bdrv_new();
     bs->open_flags = flags;
-    bs->explicit_options = qdict_new();
-    bs->options = qdict_new();
+    bs->options = options ?: qdict_new();
+    bs->explicit_options = qdict_clone_shallow(bs->options);
     bs->opaque = NULL;
 
     update_options_from_flags(bs->options, flags);
@@ -1631,6 +1641,13 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
     return bs;
 }
 
+/* Create and open a block node. */
+BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
+                                       int flags, Error **errp)
+{
+    return bdrv_new_open_driver_opts(drv, node_name, NULL, flags, errp);
+}
+
 QemuOptsList bdrv_runtime_opts = {
     .name = "bdrv_common",
     .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
-- 
2.31.1



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

* [PULL 04/13] block: bdrv_insert_node(): fix and improve error handling
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 03/13] block: implement bdrv_new_open_driver_opts() Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 05/13] block: bdrv_insert_node(): doc and style Kevin Wolf
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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

 - use ERRP_GUARD(): function calls error_prepend(), so it must use
   ERRP_GUARD(), otherwise error_prepend() would not be called when
   passed errp is error_fatal

 - drop error propagation, handle return code instead

 - for symmetry, do error_prepend() for the second failure

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210920115538.264372-3-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 917fb7faca..5d49188073 100644
--- a/block.c
+++ b/block.c
@@ -5122,8 +5122,9 @@ static void bdrv_delete(BlockDriverState *bs)
 BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
                                    int flags, Error **errp)
 {
+    ERRP_GUARD();
+    int ret;
     BlockDriverState *new_node_bs;
-    Error *local_err = NULL;
 
     new_node_bs = bdrv_open(NULL, NULL, node_options, flags, errp);
     if (new_node_bs == NULL) {
@@ -5132,12 +5133,12 @@ BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
     }
 
     bdrv_drained_begin(bs);
-    bdrv_replace_node(bs, new_node_bs, &local_err);
+    ret = bdrv_replace_node(bs, new_node_bs, errp);
     bdrv_drained_end(bs);
 
-    if (local_err) {
+    if (ret < 0) {
+        error_prepend(errp, "Could not replace node: ");
         bdrv_unref(new_node_bs);
-        error_propagate(errp, local_err);
         return NULL;
     }
 
-- 
2.31.1



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

* [PULL 05/13] block: bdrv_insert_node(): doc and style
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 04/13] block: bdrv_insert_node(): fix and improve error handling Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 06/13] block: bdrv_insert_node(): don't use bdrv_open() Kevin Wolf
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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

 - options & flags is common pair for open-like functions, let's use it
 - add a comment that specifies use of @options

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210920115538.264372-4-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 5d49188073..3a90407b83 100644
--- a/block.c
+++ b/block.c
@@ -5119,14 +5119,23 @@ static void bdrv_delete(BlockDriverState *bs)
     g_free(bs);
 }
 
-BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
+
+/*
+ * Replace @bs by newly created block node.
+ *
+ * @options is a QDict of options to pass to the block drivers, or NULL for an
+ * empty set of options. The reference to the QDict belongs to the block layer
+ * after the call (even on failure), so if the caller intends to reuse the
+ * dictionary, it needs to use qobject_ref() before calling bdrv_open.
+ */
+BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *options,
                                    int flags, Error **errp)
 {
     ERRP_GUARD();
     int ret;
     BlockDriverState *new_node_bs;
 
-    new_node_bs = bdrv_open(NULL, NULL, node_options, flags, errp);
+    new_node_bs = bdrv_open(NULL, NULL, options, flags, errp);
     if (new_node_bs == NULL) {
         error_prepend(errp, "Could not create node: ");
         return NULL;
-- 
2.31.1



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

* [PULL 06/13] block: bdrv_insert_node(): don't use bdrv_open()
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 05/13] block: bdrv_insert_node(): doc and style Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 07/13] iotests/image-fleecing: declare requirement of copy-before-write Kevin Wolf
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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

Use bdrv_new_open_driver_opts() instead of complicated bdrv_open().

Among other extra things bdrv_open() also check for white-listed
formats, which we don't want for internal node creation: currently
backup doesn't work when copy-before-write filter is not white-listed.
As well block-stream doesn't work when copy-on-read is not
white-listed.

Fixes: 751cec7a261adaf1145dc7adf6de7c9c084e5a0b
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2004812
Reported-by: Yanan Fu
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210920115538.264372-5-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 3a90407b83..45f653a88b 100644
--- a/block.c
+++ b/block.c
@@ -5133,12 +5133,30 @@ BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *options,
 {
     ERRP_GUARD();
     int ret;
-    BlockDriverState *new_node_bs;
+    BlockDriverState *new_node_bs = NULL;
+    const char *drvname, *node_name;
+    BlockDriver *drv;
+
+    drvname = qdict_get_try_str(options, "driver");
+    if (!drvname) {
+        error_setg(errp, "driver is not specified");
+        goto fail;
+    }
+
+    drv = bdrv_find_format(drvname);
+    if (!drv) {
+        error_setg(errp, "Unknown driver: '%s'", drvname);
+        goto fail;
+    }
 
-    new_node_bs = bdrv_open(NULL, NULL, options, flags, errp);
-    if (new_node_bs == NULL) {
+    node_name = qdict_get_try_str(options, "node-name");
+
+    new_node_bs = bdrv_new_open_driver_opts(drv, node_name, options, flags,
+                                            errp);
+    options = NULL; /* bdrv_new_open_driver() eats options */
+    if (!new_node_bs) {
         error_prepend(errp, "Could not create node: ");
-        return NULL;
+        goto fail;
     }
 
     bdrv_drained_begin(bs);
@@ -5147,11 +5165,15 @@ BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *options,
 
     if (ret < 0) {
         error_prepend(errp, "Could not replace node: ");
-        bdrv_unref(new_node_bs);
-        return NULL;
+        goto fail;
     }
 
     return new_node_bs;
+
+fail:
+    qobject_unref(options);
+    bdrv_unref(new_node_bs);
+    return NULL;
 }
 
 /*
-- 
2.31.1



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

* [PULL 07/13] iotests/image-fleecing: declare requirement of copy-before-write
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 06/13] block: bdrv_insert_node(): don't use bdrv_open() Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 08/13] block: introduce max_hw_iov for use in scsi-generic Kevin Wolf
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

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

Now test fails if copy-before-write is not white-listed.
Let's skip test instead.

Fixes: c0605985696a19ef034fa25d04f53f3b3b383896
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210920115538.264372-6-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/tests/image-fleecing | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index f6318492c6..35164e9036 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -28,6 +28,7 @@ from iotests import log, qemu_img, qemu_io, qemu_io_silent
 iotests.script_initialize(
     supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk', 'vhdx', 'raw'],
     supported_platforms=['linux'],
+    required_fmts=['copy-before-write'],
 )
 
 patterns = [('0x5d', '0',         '64k'),
-- 
2.31.1



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

* [PULL 08/13] block: introduce max_hw_iov for use in scsi-generic
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (6 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 07/13] iotests/image-fleecing: declare requirement of copy-before-write Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 09/13] iotests: add 'qemu' package location to PYTHONPATH in testenv Kevin Wolf
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: Paolo Bonzini <pbonzini@redhat.com>

Linux limits the size of iovecs to 1024 (UIO_MAXIOV in the kernel
sources, IOV_MAX in POSIX).  Because of this, on some host adapters
requests with many iovecs are rejected with -EINVAL by the
io_submit() or readv()/writev() system calls.

In fact, the same limit applies to SG_IO as well.  To fix both the
EINVAL and the possible performance issues from using fewer iovecs
than allowed by Linux (some HBAs have max_segments as low as 128),
introduce a separate entry in BlockLimits to hold the max_segments
value from sysfs.  This new limit is used only for SG_IO and clamped
to bs->bl.max_iov anyway, just like max_hw_transfer is clamped to
bs->bl.max_transfer.

Reported-by: Halil Pasic <pasic@linux.ibm.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org
Cc: qemu-stable@nongnu.org
Fixes: 18473467d5 ("file-posix: try BLKSECTGET on block devices too, do not round to power of 2", 2021-06-25)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210923130436.1187591-1-pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block_int.h      | 7 +++++++
 include/sysemu/block-backend.h | 1 +
 block/block-backend.c          | 6 ++++++
 block/file-posix.c             | 2 +-
 block/io.c                     | 1 +
 hw/scsi/scsi-generic.c         | 2 +-
 6 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index ffe86068d4..f4c75e8ba9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -718,6 +718,13 @@ typedef struct BlockLimits {
      */
     uint64_t max_hw_transfer;
 
+    /* Maximal number of scatter/gather elements allowed by the hardware.
+     * Applies whenever transfers to the device bypass the kernel I/O
+     * scheduler, for example with SG_IO.  If larger than max_iov
+     * or if zero, blk_get_max_hw_iov will fall back to max_iov.
+     */
+    int max_hw_iov;
+
     /* memory alignment, in bytes so that no bounce buffer is needed */
     size_t min_mem_alignment;
 
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 29d4fdbf63..82bae55161 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -211,6 +211,7 @@ uint32_t blk_get_request_alignment(BlockBackend *blk);
 uint32_t blk_get_max_transfer(BlockBackend *blk);
 uint64_t blk_get_max_hw_transfer(BlockBackend *blk);
 int blk_get_max_iov(BlockBackend *blk);
+int blk_get_max_hw_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
 void *blk_try_blockalign(BlockBackend *blk, size_t size);
 void *blk_blockalign(BlockBackend *blk, size_t size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 6140d133e2..ba2b5ebb10 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1986,6 +1986,12 @@ uint32_t blk_get_max_transfer(BlockBackend *blk)
     return ROUND_DOWN(max, blk_get_request_alignment(blk));
 }
 
+int blk_get_max_hw_iov(BlockBackend *blk)
+{
+    return MIN_NON_ZERO(blk->root->bs->bl.max_hw_iov,
+                        blk->root->bs->bl.max_iov);
+}
+
 int blk_get_max_iov(BlockBackend *blk)
 {
     return blk->root->bs->bl.max_iov;
diff --git a/block/file-posix.c b/block/file-posix.c
index c62e42743d..53be0bdc1b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1273,7 +1273,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 
         ret = hdev_get_max_segments(s->fd, &st);
         if (ret > 0) {
-            bs->bl.max_iov = ret;
+            bs->bl.max_hw_iov = ret;
         }
     }
 }
diff --git a/block/io.c b/block/io.c
index 18d345a87a..bb0a254def 100644
--- a/block/io.c
+++ b/block/io.c
@@ -136,6 +136,7 @@ static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
     dst->min_mem_alignment = MAX(dst->min_mem_alignment,
                                  src->min_mem_alignment);
     dst->max_iov = MIN_NON_ZERO(dst->max_iov, src->max_iov);
+    dst->max_hw_iov = MIN_NON_ZERO(dst->max_hw_iov, src->max_hw_iov);
 }
 
 typedef struct BdrvRefreshLimitsState {
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 665baf900e..0306ccc7b1 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -180,7 +180,7 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s, int len)
         page = r->req.cmd.buf[2];
         if (page == 0xb0) {
             uint64_t max_transfer = blk_get_max_hw_transfer(s->conf.blk);
-            uint32_t max_iov = blk_get_max_iov(s->conf.blk);
+            uint32_t max_iov = blk_get_max_hw_iov(s->conf.blk);
 
             assert(max_transfer);
             max_transfer = MIN_NON_ZERO(max_transfer, max_iov * qemu_real_host_page_size)
-- 
2.31.1



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

* [PULL 09/13] iotests: add 'qemu' package location to PYTHONPATH in testenv
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (7 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 08/13] block: introduce max_hw_iov for use in scsi-generic Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 10/13] iotests/linters: check mypy files all at once Kevin Wolf
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: John Snow <jsnow@redhat.com>

We can drop the sys.path hacking in various places by doing
this. Additionally, by doing it in one place right up top, we can print
interesting warnings in case the environment does not look correct. (See
next commit.)

If we ever decide to change how the environment is crafted, all of the
"help me find my python packages" goop is all in one place, right in one
function.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210923180715.4168522-2-jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/iotests.py             |  2 --
 tests/qemu-iotests/testenv.py             | 15 +++++++++------
 tests/qemu-iotests/235                    |  2 --
 tests/qemu-iotests/297                    |  6 ------
 tests/qemu-iotests/300                    |  5 ++---
 tests/qemu-iotests/tests/mirror-top-perms |  7 +++----
 6 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce06cf5630..b06ad76e0c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -36,8 +36,6 @@
 
 from contextlib import contextmanager
 
-# pylint: disable=import-error, wrong-import-position
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import qtest
 from qemu.qmp import QMPMessage
 
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 70da0d60c8..99a57a69f3 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -108,12 +108,15 @@ def init_directories(self) -> None:
              SAMPLE_IMG_DIR
              OUTPUT_DIR
         """
-        self.pythonpath = os.getenv('PYTHONPATH')
-        if self.pythonpath:
-            self.pythonpath = self.source_iotests + os.pathsep + \
-                self.pythonpath
-        else:
-            self.pythonpath = self.source_iotests
+
+        # Path where qemu goodies live in this source tree.
+        qemu_srctree_path = Path(__file__, '../../../python').resolve()
+
+        self.pythonpath = os.pathsep.join(filter(None, (
+            self.source_iotests,
+            str(qemu_srctree_path),
+            os.getenv('PYTHONPATH'),
+        )))
 
         self.test_dir = os.getenv('TEST_DIR',
                                   os.path.join(os.getcwd(), 'scratch'))
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index 8aed45f9a7..4de920c380 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -24,8 +24,6 @@ import os
 import iotests
 from iotests import qemu_img_create, qemu_io, file_path, log
 
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-
 from qemu.machine import QEMUMachine
 
 iotests.script_initialize(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b04cba5366..467b712280 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -68,12 +68,6 @@ def run_linters():
     # Todo notes are fine, but fixme's or xxx's should probably just be
     # fixed (in tests, at least)
     env = os.environ.copy()
-    qemu_module_path = os.path.join(os.path.dirname(__file__),
-                                    '..', '..', 'python')
-    try:
-        env['PYTHONPATH'] += os.pathsep + qemu_module_path
-    except KeyError:
-        env['PYTHONPATH'] = qemu_module_path
     subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
                    env=env, check=False)
 
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index fe94de84ed..10f9f2a8da 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,11 +24,10 @@ import random
 import re
 from typing import Dict, List, Optional
 
+from qemu.machine import machine
+
 import iotests
 
-# Import qemu after iotests.py has amended sys.path
-# pylint: disable=wrong-import-order
-from qemu.machine import machine
 
 BlockBitmapMapping = List[Dict[str, object]]
 
diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index 2fc8dd66e0..73138a0ef9 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -20,13 +20,12 @@
 #
 
 import os
-import iotests
-from iotests import qemu_img
 
-# Import qemu after iotests.py has amended sys.path
-# pylint: disable=wrong-import-order
 import qemu
 
+import iotests
+from iotests import qemu_img
+
 
 image_size = 1 * 1024 * 1024
 source = os.path.join(iotests.test_dir, 'source.img')
-- 
2.31.1



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

* [PULL 10/13] iotests/linters: check mypy files all at once
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (8 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 09/13] iotests: add 'qemu' package location to PYTHONPATH in testenv Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 11/13] iotests/mirror-top-perms: Adjust imports Kevin Wolf
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: John Snow <jsnow@redhat.com>

We can circumvent the '__main__' redefinition problem by passing
--scripts-are-modules. Take mypy out of the loop per-filename and check
everything in one go: it's quite a bit faster.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210923180715.4168522-4-jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/297 | 46 +++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 467b712280..91ec34d952 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -74,32 +74,28 @@ def run_linters():
     print('=== mypy ===')
     sys.stdout.flush()
 
-    # We have to call mypy separately for each file.  Otherwise, it
-    # will interpret all given files as belonging together (i.e., they
-    # may not both define the same classes, etc.; most notably, they
-    # must not both define the __main__ module).
     env['MYPYPATH'] = env['PYTHONPATH']
-    for filename in files:
-        p = subprocess.run(('mypy',
-                            '--warn-unused-configs',
-                            '--disallow-subclassing-any',
-                            '--disallow-any-generics',
-                            '--disallow-incomplete-defs',
-                            '--disallow-untyped-decorators',
-                            '--no-implicit-optional',
-                            '--warn-redundant-casts',
-                            '--warn-unused-ignores',
-                            '--no-implicit-reexport',
-                            '--namespace-packages',
-                            filename),
-                           env=env,
-                           check=False,
-                           stdout=subprocess.PIPE,
-                           stderr=subprocess.STDOUT,
-                           universal_newlines=True)
-
-        if p.returncode != 0:
-            print(p.stdout)
+    p = subprocess.run(('mypy',
+                        '--warn-unused-configs',
+                        '--disallow-subclassing-any',
+                        '--disallow-any-generics',
+                        '--disallow-incomplete-defs',
+                        '--disallow-untyped-decorators',
+                        '--no-implicit-optional',
+                        '--warn-redundant-casts',
+                        '--warn-unused-ignores',
+                        '--no-implicit-reexport',
+                        '--namespace-packages',
+                        '--scripts-are-modules',
+                        *files),
+                       env=env,
+                       check=False,
+                       stdout=subprocess.PIPE,
+                       stderr=subprocess.STDOUT,
+                       universal_newlines=True)
+
+    if p.returncode != 0:
+        print(p.stdout)
 
 
 for linter in ('pylint-3', 'mypy'):
-- 
2.31.1



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

* [PULL 11/13] iotests/mirror-top-perms: Adjust imports
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (9 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 10/13] iotests/linters: check mypy files all at once Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 12/13] iotests/migrate-bitmaps-test: delint Kevin Wolf
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: John Snow <jsnow@redhat.com>

We need to import subpackages from the qemu namespace package; importing
the namespace package alone doesn't bring the subpackages with it --
unless someone else (like iotests.py) imports them too.

Adjust the imports.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210923180715.4168522-5-jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/tests/mirror-top-perms | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index 73138a0ef9..3d475aa3a5 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -21,7 +21,8 @@
 
 import os
 
-import qemu
+from qemu import qmp
+from qemu.machine import machine
 
 import iotests
 from iotests import qemu_img
@@ -46,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
     def tearDown(self):
         try:
             self.vm.shutdown()
-        except qemu.machine.machine.AbnormalShutdown:
+        except machine.AbnormalShutdown:
             pass
 
         if self.vm_b is not None:
@@ -101,7 +102,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
             self.vm_b.launch()
             print('ERROR: VM B launched successfully, this should not have '
                   'happened')
-        except qemu.qmp.QMPConnectError:
+        except qmp.QMPConnectError:
             assert 'Is another process using the image' in self.vm_b.get_log()
 
         result = self.vm.qmp('block-job-cancel',
-- 
2.31.1



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

* [PULL 12/13] iotests/migrate-bitmaps-test: delint
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (10 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 11/13] iotests/mirror-top-perms: Adjust imports Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 10:59 ` [PULL 13/13] iotests: Update for pylint 2.11.1 Kevin Wolf
  2021-10-06 15:49 ` [PULL 00/13] Block layer patches Richard Henderson
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: John Snow <jsnow@redhat.com>

Mostly uninteresting stuff. Move the test injections under a function
named main() so that the variables used during that process aren't in
the global scope.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210923180715.4168522-6-jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/tests/migrate-bitmaps-test | 50 +++++++++++--------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test
index dc431c35b3..c23df3d75c 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -19,10 +19,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-import os
 import itertools
 import operator
+import os
 import re
+
 import iotests
 from iotests import qemu_img, qemu_img_create, Timeout
 
@@ -224,25 +225,6 @@ def inject_test_case(klass, suffix, method, *args, **kwargs):
     setattr(klass, 'test_' + method + suffix, lambda self: mc(self))
 
 
-for cmb in list(itertools.product((True, False), repeat=5)):
-    name = ('_' if cmb[0] else '_not_') + 'persistent_'
-    name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
-    name += '_online' if cmb[2] else '_offline'
-    name += '_shared' if cmb[3] else '_nonshared'
-    if cmb[4]:
-        name += '__pre_shutdown'
-
-    inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
-                     *list(cmb))
-
-for cmb in list(itertools.product((True, False), repeat=2)):
-    name = ('_' if cmb[0] else '_not_') + 'persistent_'
-    name += ('_' if cmb[1] else '_not_') + 'migbitmap'
-
-    inject_test_case(TestDirtyBitmapMigration, name,
-                     'do_test_migration_resume_source', *list(cmb))
-
-
 class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
     def setUp(self):
         qemu_img_create('-f', iotests.imgfmt, base_a, size)
@@ -304,6 +286,30 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
 
+def main() -> None:
+    for cmb in list(itertools.product((True, False), repeat=5)):
+        name = ('_' if cmb[0] else '_not_') + 'persistent_'
+        name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
+        name += '_online' if cmb[2] else '_offline'
+        name += '_shared' if cmb[3] else '_nonshared'
+        if cmb[4]:
+            name += '__pre_shutdown'
+
+        inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
+                         *list(cmb))
+
+    for cmb in list(itertools.product((True, False), repeat=2)):
+        name = ('_' if cmb[0] else '_not_') + 'persistent_'
+        name += ('_' if cmb[1] else '_not_') + 'migbitmap'
+
+        inject_test_case(TestDirtyBitmapMigration, name,
+                         'do_test_migration_resume_source', *list(cmb))
+
+    iotests.main(
+        supported_fmts=['qcow2'],
+        supported_protocols=['file']
+    )
+
+
 if __name__ == '__main__':
-    iotests.main(supported_fmts=['qcow2'],
-                 supported_protocols=['file'])
+    main()
-- 
2.31.1



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

* [PULL 13/13] iotests: Update for pylint 2.11.1
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (11 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 12/13] iotests/migrate-bitmaps-test: delint Kevin Wolf
@ 2021-10-06 10:59 ` Kevin Wolf
  2021-10-06 15:49 ` [PULL 00/13] Block layer patches Richard Henderson
  13 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2021-10-06 10:59 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel

From: John Snow <jsnow@redhat.com>

1. Ignore the new f-strings warning, we're not interested in doing a
   full conversion at this time.

2. Just mute the unbalanced-tuple-unpacking warning, it's not a real
   error in this case and muting the dozens of callsites is just not
   worth it.

3. Add encodings to read_text().

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210923180715.4168522-7-jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/testrunner.py | 7 ++++---
 tests/qemu-iotests/pylintrc      | 6 +++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 4a6ec421ed..a56b6da396 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -266,12 +266,13 @@ def do_run_test(self, test: str) -> TestResult:
                               diff=file_diff(str(f_reference), str(f_bad)))
 
         if f_notrun.exists():
-            return TestResult(status='not run',
-                              description=f_notrun.read_text().strip())
+            return TestResult(
+                status='not run',
+                description=f_notrun.read_text(encoding='utf-8').strip())
 
         casenotrun = ''
         if f_casenotrun.exists():
-            casenotrun = f_casenotrun.read_text()
+            casenotrun = f_casenotrun.read_text(encoding='utf-8')
 
         diff = file_diff(str(f_reference), str(f_bad))
         if diff:
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index f2c0b522ac..8cb4e1d6a6 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -19,13 +19,17 @@ disable=invalid-name,
         too-many-public-methods,
         # pylint warns about Optional[] etc. as unsubscriptable in 3.9
         unsubscriptable-object,
+        # pylint's static analysis causes false positivies for file_path();
+        # If we really care to make it statically knowable, we'll use mypy.
+        unbalanced-tuple-unpacking,
         # Sometimes we need to disable a newly introduced pylint warning.
         # Doing so should not produce a warning in older versions of pylint.
         bad-option-value,
         # These are temporary, and should be removed:
         missing-docstring,
         too-many-return-statements,
-        too-many-statements
+        too-many-statements,
+        consider-using-f-string,
 
 [FORMAT]
 
-- 
2.31.1



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

* Re: [PULL 00/13] Block layer patches
  2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
                   ` (12 preceding siblings ...)
  2021-10-06 10:59 ` [PULL 13/13] iotests: Update for pylint 2.11.1 Kevin Wolf
@ 2021-10-06 15:49 ` Richard Henderson
  13 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2021-10-06 15:49 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: peter.maydell, qemu-devel

On 10/6/21 3:59 AM, Kevin Wolf wrote:
> The following changes since commit e3acc2c1961cbe22ca474cd5da4163b7bbf7cea3:
> 
>    tests/docker/dockerfiles: Bump fedora-i386-cross to fedora 34 (2021-10-05 16:40:39 -0700)
> 
> are available in the Git repository at:
> 
>    git://repo.or.cz/qemu/kevin.git tags/for-upstream
> 
> for you to fetch changes up to 3765315d4c84f9c0799744f43a314169baaccc05:
> 
>    iotests: Update for pylint 2.11.1 (2021-10-06 10:25:55 +0200)
> 
> ----------------------------------------------------------------
> Block layer patches
> 
> - Fix I/O errors because of incorrectly detected max_iov
> - Fix not white-listed copy-before-write
> - qemu-storage-daemon: Only display FUSE help when FUSE is built-in
> - iotests: update environment and linting configuration
> 
> ----------------------------------------------------------------
> Emanuele Giuseppe Esposito (1):
>        include/block.h: remove outdated comment
> 
> John Snow (5):
>        iotests: add 'qemu' package location to PYTHONPATH in testenv
>        iotests/linters: check mypy files all at once
>        iotests/mirror-top-perms: Adjust imports
>        iotests/migrate-bitmaps-test: delint
>        iotests: Update for pylint 2.11.1
> 
> Paolo Bonzini (1):
>        block: introduce max_hw_iov for use in scsi-generic
> 
> Philippe Mathieu-Daudé (1):
>        qemu-storage-daemon: Only display FUSE help when FUSE is built-in
> 
> Vladimir Sementsov-Ogievskiy (5):
>        block: implement bdrv_new_open_driver_opts()
>        block: bdrv_insert_node(): fix and improve error handling
>        block: bdrv_insert_node(): doc and style
>        block: bdrv_insert_node(): don't use bdrv_open()
>        iotests/image-fleecing: declare requirement of copy-before-write
> 
>   include/block/block.h                         |  8 ++-
>   include/block/block_int.h                     |  7 +++
>   include/sysemu/block-backend.h                |  1 +
>   block.c                                       | 79 ++++++++++++++++++++++-----
>   block/block-backend.c                         |  6 ++
>   block/file-posix.c                            |  2 +-
>   block/io.c                                    |  1 +
>   hw/scsi/scsi-generic.c                        |  2 +-
>   storage-daemon/qemu-storage-daemon.c          |  2 +
>   tests/qemu-iotests/iotests.py                 |  2 -
>   tests/qemu-iotests/testenv.py                 | 15 +++--
>   tests/qemu-iotests/testrunner.py              |  7 ++-
>   tests/qemu-iotests/235                        |  2 -
>   tests/qemu-iotests/297                        | 52 +++++++-----------
>   tests/qemu-iotests/300                        |  5 +-
>   tests/qemu-iotests/pylintrc                   |  6 +-
>   tests/qemu-iotests/tests/image-fleecing       |  1 +
>   tests/qemu-iotests/tests/migrate-bitmaps-test | 50 +++++++++--------
>   tests/qemu-iotests/tests/mirror-top-perms     | 12 ++--
>   19 files changed, 164 insertions(+), 96 deletions(-)

Applied, thanks.

r~



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

end of thread, other threads:[~2021-10-06 16:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 10:59 [PULL 00/13] Block layer patches Kevin Wolf
2021-10-06 10:59 ` [PULL 01/13] include/block.h: remove outdated comment Kevin Wolf
2021-10-06 10:59 ` [PULL 02/13] qemu-storage-daemon: Only display FUSE help when FUSE is built-in Kevin Wolf
2021-10-06 10:59 ` [PULL 03/13] block: implement bdrv_new_open_driver_opts() Kevin Wolf
2021-10-06 10:59 ` [PULL 04/13] block: bdrv_insert_node(): fix and improve error handling Kevin Wolf
2021-10-06 10:59 ` [PULL 05/13] block: bdrv_insert_node(): doc and style Kevin Wolf
2021-10-06 10:59 ` [PULL 06/13] block: bdrv_insert_node(): don't use bdrv_open() Kevin Wolf
2021-10-06 10:59 ` [PULL 07/13] iotests/image-fleecing: declare requirement of copy-before-write Kevin Wolf
2021-10-06 10:59 ` [PULL 08/13] block: introduce max_hw_iov for use in scsi-generic Kevin Wolf
2021-10-06 10:59 ` [PULL 09/13] iotests: add 'qemu' package location to PYTHONPATH in testenv Kevin Wolf
2021-10-06 10:59 ` [PULL 10/13] iotests/linters: check mypy files all at once Kevin Wolf
2021-10-06 10:59 ` [PULL 11/13] iotests/mirror-top-perms: Adjust imports Kevin Wolf
2021-10-06 10:59 ` [PULL 12/13] iotests/migrate-bitmaps-test: delint Kevin Wolf
2021-10-06 10:59 ` [PULL 13/13] iotests: Update for pylint 2.11.1 Kevin Wolf
2021-10-06 15:49 ` [PULL 00/13] Block layer patches Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.