[Qemu-devel] [PULL 00/17] Block and testing patches

[Qemu-devel] [PULL 00/17] Block and testing patches

[Fam Zheng]

The following changes since commit 38441756b70eec5807b5f60dad11a93a91199866:

  Update version for v3.0.0 release (2018-08-14 16:38:43 +0100)

are available in the Git repository at:

  git://github.com/famz/qemu.git tags/block-and-testing-pull-request

for you to fetch changes up to 37a81812f7b8367422a039eb09d915df543983ee:

  aio-posix: Improve comment around marking node deleted (2018-08-15 10:12:35 +0800)

----------------------------------------------------------------
Block and testing patches for 3.1

- aio fixes by me
- nvme fixes by Paolo and me
- test improvements by Peter, Phil and me

----------------------------------------------------------------

Fam Zheng (10):
  nvme: Fix nvme_init error handling
  aio-posix: Don't count ctx->notifier as progress when polling
  aio: Do aio_notify_accept only during blocking aio_poll
  docker: Install more packages in centos7
  tests: Add an option for snapshot (default: off)
  tests: Allow overriding archive path with SRC_ARCHIVE
  tests: Add centos VM testing
  tests: vm: Add vm-clean-all
  tests/vm: Add vm-build-all/vm-clean-all in help text
  aio-posix: Improve comment around marking node deleted

Paolo Bonzini (1):
  nvme: simplify plug/unplug

Peter Maydell (4):
  tests/vm: Pass the jobs parallelism setting to 'make check'
  tests/vm: Propagate V=1 down into the make inside the VM
  tests/vm: Bump guest RAM up from 2G to 4G
  tests/vm: Use make's --output-sync option

Philippe Mathieu-Daudé (2):
  tests/vm: Only use -cpu 'host' if KVM is available
  tests/vm: Add flex and bison to the vm image

 block/nvme.c                            | 57 +++++++----------
 docs/devel/testing.rst                  |  1 +
 tests/docker/Makefile.include           |  7 ++-
 tests/docker/dockerfiles/centos7.docker |  3 +
 tests/vm/Makefile.include               | 12 +++-
 tests/vm/basevm.py                      | 17 +++--
 tests/vm/centos                         | 84 +++++++++++++++++++++++++
 tests/vm/freebsd                        |  4 +-
 tests/vm/netbsd                         |  4 +-
 tests/vm/openbsd                        |  4 +-
 tests/vm/ubuntu.i386                    |  6 +-
 util/aio-posix.c                        |  9 +--
 util/aio-win32.c                        |  3 +-
 13 files changed, 154 insertions(+), 57 deletions(-)
 create mode 100755 tests/vm/centos

-- 
2.17.1

[Qemu-devel] [PULL 01/17] tests/vm: Only use -cpu 'host' if KVM is available

[Fam Zheng]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

If KVM is not available, then use the 'max' cpu.

This fixes:

  ERROR:root:Log:
  ERROR:root:qemu-system-x86_64: CPU model 'host' requires KVM
  Failed to prepare guest environment
  error: [Errno 104] Connection reset by peer
  source/qemu/tests/vm/Makefile.include:25: recipe for target 'tests/vm/ubuntu.i386.img' failed
  make: *** [tests/vm/ubuntu.i386.img] Error 2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180628153535.1411-4-f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/basevm.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3643117816..d80cc8ab85 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -65,7 +65,6 @@ class BaseVM(object):
             self._stdout = self._devnull
         self._args = [ \
             "-nodefaults", "-m", "2G",
-            "-cpu", "host",
             "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22",
             "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20",
@@ -73,9 +72,11 @@ class BaseVM(object):
         if vcpus:
             self._args += ["-smp", str(vcpus)]
         if os.access("/dev/kvm", os.R_OK | os.W_OK):
+            self._args += ["-cpu", "host"]
             self._args += ["-enable-kvm"]
         else:
             logging.info("KVM not available, not using -enable-kvm")
+            self._args += ["-cpu", "max"]
         self._data_args = []
 
     def _download_with_cache(self, url, sha256sum=None):
-- 
2.17.1

[Qemu-devel] [PULL 02/17] tests/vm: Add flex and bison to the vm image

[Fam Zheng]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Similar to 79f24568e5e70, this fixes the following warnings:

           CHK version_gen.h
           LEX convert-dtsv0-lexer.lex.c
  make[1]: flex: Command not found
           BISON dtc-parser.tab.c
  make[1]: bison: Command not found
           LEX dtc-lexer.lex.c
  make[1]: flex: Command not found

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180628153535.1411-5-f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/ubuntu.i386 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index fc319e0e6e..fc27b6935e 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -77,7 +77,7 @@ class UbuntuX86VM(basevm.BaseVM):
         # The previous update sometimes doesn't survive a reboot, so do it again
         self.ssh_root_check("apt-get update")
         self.ssh_root_check("apt-get build-dep -y qemu")
-        self.ssh_root_check("apt-get install -y libfdt-dev")
+        self.ssh_root_check("apt-get install -y libfdt-dev flex bison")
         self.ssh_root("poweroff")
         self.wait()
         if os.path.exists(img):
-- 
2.17.1

[Qemu-devel] [PULL 03/17] nvme: Fix nvme_init error handling

[Fam Zheng]

It is wrong to leave this field as 1, as nvme_close() called in the
error handling code in nvme_file_open() will use it and try to free
s->queues again.

Another problem is the cleaning ups are duplicated between the fail*
labels of nvme_init() and nvme_file_open(), which calls nvme_close().

A third problem is nvme_close() misses g_free() and
event_notifier_cleanup().

Fix all of them.

Cc: qemu-stable@nongnu.org
Signed-off-by: Fam Zheng <famz@redhat.com>

Message-Id: <20180712025420.4932-1-famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/nvme.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 6f71122bf5..37805e8890 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -569,13 +569,13 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     s->vfio = qemu_vfio_open_pci(device, errp);
     if (!s->vfio) {
         ret = -EINVAL;
-        goto fail;
+        goto out;
     }
 
     s->regs = qemu_vfio_pci_map_bar(s->vfio, 0, 0, NVME_BAR_SIZE, errp);
     if (!s->regs) {
         ret = -EINVAL;
-        goto fail;
+        goto out;
     }
 
     /* Perform initialize sequence as described in NVMe spec "7.6.1
@@ -585,7 +585,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     if (!(cap & (1ULL << 37))) {
         error_setg(errp, "Device doesn't support NVMe command set");
         ret = -EINVAL;
-        goto fail;
+        goto out;
     }
 
     s->page_size = MAX(4096, 1 << (12 + ((cap >> 48) & 0xF)));
@@ -603,7 +603,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
                              PRId64 " ms)",
                        timeout_ms);
             ret = -ETIMEDOUT;
-            goto fail;
+            goto out;
         }
     }
 
@@ -613,7 +613,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     s->queues[0] = nvme_create_queue_pair(bs, 0, NVME_QUEUE_SIZE, errp);
     if (!s->queues[0]) {
         ret = -EINVAL;
-        goto fail;
+        goto out;
     }
     QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000);
     s->regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE);
@@ -633,14 +633,14 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
                              PRId64 " ms)",
                        timeout_ms);
             ret = -ETIMEDOUT;
-            goto fail_queue;
+            goto out;
         }
     }
 
     ret = qemu_vfio_pci_init_irq(s->vfio, &s->irq_notifier,
                                  VFIO_PCI_MSIX_IRQ_INDEX, errp);
     if (ret) {
-        goto fail_queue;
+        goto out;
     }
     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
                            false, nvme_handle_event, nvme_poll_cb);
@@ -649,30 +649,15 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
     if (local_err) {
         error_propagate(errp, local_err);
         ret = -EIO;
-        goto fail_handler;
+        goto out;
     }
 
     /* Set up command queues. */
     if (!nvme_add_io_queue(bs, errp)) {
         ret = -EIO;
-        goto fail_handler;
     }
-    return 0;
-
-fail_handler:
-    aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
-                           false, NULL, NULL);
-fail_queue:
-    nvme_free_queue_pair(bs, s->queues[0]);
-fail:
-    g_free(s->queues);
-    if (s->regs) {
-        qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE);
-    }
-    if (s->vfio) {
-        qemu_vfio_close(s->vfio);
-    }
-    event_notifier_cleanup(&s->irq_notifier);
+out:
+    /* Cleaning up is done in nvme_file_open() upon error. */
     return ret;
 }
 
@@ -739,8 +724,10 @@ static void nvme_close(BlockDriverState *bs)
     for (i = 0; i < s->nr_queues; ++i) {
         nvme_free_queue_pair(bs, s->queues[i]);
     }
+    g_free(s->queues);
     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
                            false, NULL, NULL);
+    event_notifier_cleanup(&s->irq_notifier);
     qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE);
     qemu_vfio_close(s->vfio);
 }
-- 
2.17.1

[Qemu-devel] [PULL 04/17] nvme: simplify plug/unplug

[Fam Zheng]

From: Paolo Bonzini <pbonzini@redhat.com>

bdrv_io_plug/bdrv_io_unplug take care of keeping a nesting count,
so change s->plugged to just a bool.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20180813144320.12382-2-pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/nvme.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 37805e8890..781d77d6d2 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -104,7 +104,7 @@ typedef struct {
     uint64_t nsze; /* Namespace size reported by identify command */
     int nsid;      /* The namespace id to read/write data. */
     uint64_t max_transfer;
-    int plugged;
+    bool plugged;
 
     CoMutex dma_map_lock;
     CoQueue dma_flush_queue;
@@ -1101,7 +1101,8 @@ static void nvme_attach_aio_context(BlockDriverState *bs,
 static void nvme_aio_plug(BlockDriverState *bs)
 {
     BDRVNVMeState *s = bs->opaque;
-    s->plugged++;
+    assert(!s->plugged);
+    s->plugged = true;
 }
 
 static void nvme_aio_unplug(BlockDriverState *bs)
@@ -1109,14 +1110,13 @@ static void nvme_aio_unplug(BlockDriverState *bs)
     int i;
     BDRVNVMeState *s = bs->opaque;
     assert(s->plugged);
-    if (!--s->plugged) {
-        for (i = 1; i < s->nr_queues; i++) {
-            NVMeQueuePair *q = s->queues[i];
-            qemu_mutex_lock(&q->lock);
-            nvme_kick(s, q);
-            nvme_process_completion(s, q);
-            qemu_mutex_unlock(&q->lock);
-        }
+    s->plugged = false;
+    for (i = 1; i < s->nr_queues; i++) {
+        NVMeQueuePair *q = s->queues[i];
+        qemu_mutex_lock(&q->lock);
+        nvme_kick(s, q);
+        nvme_process_completion(s, q);
+        qemu_mutex_unlock(&q->lock);
     }
 }
 
-- 
2.17.1

[Qemu-devel] [PULL 05/17] aio-posix: Don't count ctx->notifier as progress when polling

[Fam Zheng]

The same logic exists in fd polling. This change is especially important
to avoid busy loop once we limit aio_notify_accept() to blocking
aio_poll().

Cc: qemu-stable@nongnu.org
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180809132259.18402-2-famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 util/aio-posix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 118bf5784b..b5c7f463aa 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -494,7 +494,8 @@ static bool run_poll_handlers_once(AioContext *ctx)
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
         if (!node->deleted && node->io_poll &&
             aio_node_check(ctx, node->is_external) &&
-            node->io_poll(node->opaque)) {
+            node->io_poll(node->opaque) &&
+            node->opaque != &ctx->notifier) {
             progress = true;
         }
 
-- 
2.17.1

[Qemu-devel] [PULL 06/17] aio: Do aio_notify_accept only during blocking aio_poll

[Fam Zheng]

An aio_notify() pairs with an aio_notify_accept(). The former should
happen in the main thread or a vCPU thread, and the latter should be
done in the IOThread.

There is one rare case that the main thread or vCPU thread may "steal"
the aio_notify() event just raised by itself, in bdrv_set_aio_context()
[1]. The sequence is like this:

    main thread                     IO Thread
    ===============================================================
    bdrv_drained_begin()
      aio_disable_external(ctx)
                                    aio_poll(ctx, true)
                                      ctx->notify_me += 2
    ...
    bdrv_drained_end()
      ...
        aio_notify()
    ...
    bdrv_set_aio_context()
      aio_poll(ctx, false)
[1]     aio_notify_accept(ctx)
                                      ppoll() /* Hang! */

[1] is problematic. It will clear the ctx->notifier event so that
the blocked ppoll() will not return.

(For the curious, this bug was noticed when booting a number of VMs
simultaneously in RHV.  One or two of the VMs will hit this race
condition, making the VIRTIO device unresponsive to I/O commands. When
it hangs, Seabios is busy waiting for a read request to complete (read
MBR), right after initializing the virtio-blk-pci device, using 100%
guest CPU. See also https://bugzilla.redhat.com/show_bug.cgi?id=1562750
for the original bug analysis.)

aio_notify() only injects an event when ctx->notify_me is set,
correspondingly aio_notify_accept() is only useful when ctx->notify_me
_was_ set. Move the call to it into the "blocking" branch. This will
effectively skip [1] and fix the hang.

Furthermore, blocking aio_poll is only allowed on home thread
(in_aio_context_home_thread), because otherwise two blocking
aio_poll()'s can steal each other's ctx->notifier event and cause
hanging just like described above.

Cc: qemu-stable@nongnu.org
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180809132259.18402-3-famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 util/aio-posix.c | 4 ++--
 util/aio-win32.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index b5c7f463aa..b5c609b68b 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -591,6 +591,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
      * so disable the optimization now.
      */
     if (blocking) {
+        assert(in_aio_context_home_thread(ctx));
         atomic_add(&ctx->notify_me, 2);
     }
 
@@ -633,6 +634,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 
     if (blocking) {
         atomic_sub(&ctx->notify_me, 2);
+        aio_notify_accept(ctx);
     }
 
     /* Adjust polling time */
@@ -676,8 +678,6 @@ bool aio_poll(AioContext *ctx, bool blocking)
         }
     }
 
-    aio_notify_accept(ctx);
-
     /* if we have any readable fds, dispatch event */
     if (ret > 0) {
         for (i = 0; i < npfd; i++) {
diff --git a/util/aio-win32.c b/util/aio-win32.c
index e676a8d9b2..c58957cc4b 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -373,11 +373,12 @@ bool aio_poll(AioContext *ctx, bool blocking)
         ret = WaitForMultipleObjects(count, events, FALSE, timeout);
         if (blocking) {
             assert(first);
+            assert(in_aio_context_home_thread(ctx));
             atomic_sub(&ctx->notify_me, 2);
+            aio_notify_accept(ctx);
         }
 
         if (first) {
-            aio_notify_accept(ctx);
             progress |= aio_bh_poll(ctx);
             first = false;
         }
-- 
2.17.1

[Qemu-devel] [PULL 07/17] docker: Install more packages in centos7

[Fam Zheng]

This makes test-block work.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180711065813.14894-1-famz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/docker/dockerfiles/centos7.docker | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
index 575de29a0a..83462b7205 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -3,6 +3,7 @@ RUN yum install -y epel-release centos-release-xen
 RUN yum -y update
 ENV PACKAGES \
     bison \
+    bzip2 \
     bzip2-devel \
     ccache \
     csnappy-devel \
@@ -12,10 +13,12 @@ ENV PACKAGES \
     gettext \
     git \
     glib2-devel \
+    libaio-devel \
     libepoxy-devel \
     libfdt-devel \
     librdmacm-devel \
     lzo-devel \
+    nettle-devel \
     make \
     mesa-libEGL-devel \
     mesa-libgbm-devel \
-- 
2.17.1

[Qemu-devel] [PULL 08/17] tests: Add an option for snapshot (default: off)

[Fam Zheng]

Not using snapshot has the benefit of automatically persisting useful
test harnesses, such as docker images and ccache database. Although it
will lose some cleanness, it is imaginably useful for patchew.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180712012829.20231-2-famz@redhat.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/basevm.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index d80cc8ab85..1f1579dc9a 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -217,6 +217,8 @@ def parse_args(vm_name):
                       help="build QEMU from source in guest")
     parser.add_option("--interactive", "-I", action="store_true",
                       help="Interactively run command")
+    parser.add_option("--snapshot", "-s", action="store_true",
+                      help="run tests with a snapshot")
     parser.disable_interspersed_args()
     return parser.parse_args()
 
@@ -242,7 +244,10 @@ def main(vmcls):
                    jobs=args.jobs)]
         else:
             cmd = argv
-        vm.boot(args.image + ",snapshot=on")
+        img = args.image
+        if args.snapshot:
+            img += ",snapshot=on"
+        vm.boot(img)
         vm.wait_ssh()
     except Exception as e:
         if isinstance(e, SystemExit) and e.code == 0:
-- 
2.17.1

[Qemu-devel] [PULL 09/17] tests: Allow overriding archive path with SRC_ARCHIVE

[Fam Zheng]

In VM based tests, the source archive is created in host, we don't have
to run archive-source.sh again, as it complicates the Makefile and
scripts.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180712012829.20231-4-famz@redhat.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/docker/Makefile.include | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 1aaa795743..d3101afecd 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -27,8 +27,11 @@ DOCKER_SRC_COPY := $(BUILD_DIR)/docker-src.$(CUR_TIME)
 
 $(DOCKER_SRC_COPY):
 	@mkdir $@
-	$(call quiet-command, cd $(SRC_PATH) && scripts/archive-source.sh $@/qemu.tar, \
-		"GEN", "$@/qemu.tar")
+	$(if $(SRC_ARCHIVE), \
+		$(call quiet-command, cp "$(SRC_ARCHIVE)" $@/qemu.tar, \
+			"CP", "$@/qemu.tar"), \
+		$(call quiet-command, cd $(SRC_PATH) && scripts/archive-source.sh $@/qemu.tar, \
+			"GEN", "$@/qemu.tar"))
 	$(call quiet-command, cp $(SRC_PATH)/tests/docker/run $@/run, \
 		"COPY","RUNNER")
 
-- 
2.17.1

[Qemu-devel] [PULL 10/17] tests: Add centos VM testing

[Fam Zheng]

This one does docker testing in the VM. It is intended to replace the
native docker testing on patchew testers.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180712012829.20231-5-famz@redhat.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/Makefile.include |  3 +-
 tests/vm/centos           | 84 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100755 tests/vm/centos

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 5daa2a3b73..af19b7a4e6 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -2,7 +2,7 @@
 
 .PHONY: vm-build-all
 
-IMAGES := ubuntu.i386 freebsd netbsd openbsd
+IMAGES := ubuntu.i386 freebsd netbsd openbsd centos
 IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
 
 .PRECIOUS: $(IMAGE_FILES)
@@ -14,6 +14,7 @@ vm-test:
 	@echo "  vm-build-freebsd                - Build QEMU in FreeBSD VM"
 	@echo "  vm-build-netbsd                 - Build QEMU in NetBSD VM"
 	@echo "  vm-build-openbsd                - Build QEMU in OpenBSD VM"
+	@echo "  vm-build-centos                 - Build QEMU in CentOS VM, with Docker"
 
 vm-build-all: $(addprefix vm-build-, $(IMAGES))
 
diff --git a/tests/vm/centos b/tests/vm/centos
new file mode 100755
index 0000000000..afd560c564
--- /dev/null
+++ b/tests/vm/centos
@@ -0,0 +1,84 @@
+#!/usr/bin/env python
+#
+# CentOS image
+#
+# Copyright 2018 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This code is licensed under the GPL version 2 or later.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+import time
+
+class CentosVM(basevm.BaseVM):
+    name = "centos"
+    BUILD_SCRIPT = """
+        set -e;
+        cd $(mktemp -d);
+        export SRC_ARCHIVE=/dev/vdb;
+        sudo chmod a+r $SRC_ARCHIVE;
+        tar -xf $SRC_ARCHIVE;
+        make docker-test-block@centos7 V={verbose} J={jobs};
+        make docker-test-quick@centos7 V={verbose} J={jobs};
+        make docker-test-mingw@fedora V={verbose} J={jobs};
+    """
+
+    def _gen_cloud_init_iso(self):
+        cidir = self._tmpdir
+        mdata = open(os.path.join(cidir, "meta-data"), "w")
+        mdata.writelines(["instance-id: centos-vm-0\n",
+                          "local-hostname: centos-guest\n"])
+        mdata.close()
+        udata = open(os.path.join(cidir, "user-data"), "w")
+        udata.writelines(["#cloud-config\n",
+                          "chpasswd:\n",
+                          "  list: |\n",
+                          "    root:%s\n" % self.ROOT_PASS,
+                          "    %s:%s\n" % (self.GUEST_USER, self.GUEST_PASS),
+                          "  expire: False\n",
+                          "users:\n",
+                          "  - name: %s\n" % self.GUEST_USER,
+                          "    sudo: ALL=(ALL) NOPASSWD:ALL\n",
+                          "    ssh-authorized-keys:\n",
+                          "    - %s\n" % basevm.SSH_PUB_KEY,
+                          "  - name: root\n",
+                          "    ssh-authorized-keys:\n",
+                          "    - %s\n" % basevm.SSH_PUB_KEY,
+                          "locale: en_US.UTF-8\n"])
+        udata.close()
+        subprocess.check_call(["genisoimage", "-output", "cloud-init.iso",
+                               "-volid", "cidata", "-joliet", "-rock",
+                               "user-data", "meta-data"],
+                               cwd=cidir,
+                               stdin=self._devnull, stdout=self._stdout,
+                               stderr=self._stdout)
+        return os.path.join(cidir, "cloud-init.iso")
+
+    def build_image(self, img):
+        cimg = self._download_with_cache("https://cloud.centos.org/centos/7/images/CentOS-7-x86_64-GenericCloud-1802.qcow2.xz")
+        img_tmp = img + ".tmp"
+        subprocess.check_call(["cp", "-f", cimg, img_tmp + ".xz"])
+        subprocess.check_call(["xz", "-df", img_tmp + ".xz"])
+        subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
+        self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()])
+        self.wait_ssh()
+        self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
+        self.ssh_root_check("yum update -y")
+        self.ssh_root_check("yum install -y docker make git")
+        self.ssh_root_check("systemctl enable docker")
+        self.ssh_root("poweroff")
+        self.wait()
+        if os.path.exists(img):
+            os.remove(img)
+        os.rename(img_tmp, img)
+        return 0
+
+if __name__ == "__main__":
+    sys.exit(basevm.main(CentosVM))
-- 
2.17.1

[Qemu-devel] [PULL 11/17] tests: vm: Add vm-clean-all

[Fam Zheng]

The images are big. Add a rule to clean up easily.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180716020008.31468-1-famz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/Makefile.include | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index af19b7a4e6..a5abb569c5 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -1,6 +1,6 @@
 # Makefile for VM tests
 
-.PHONY: vm-build-all
+.PHONY: vm-build-all vm-clean-all
 
 IMAGES := ubuntu.i386 freebsd netbsd openbsd centos
 IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
@@ -18,6 +18,9 @@ vm-test:
 
 vm-build-all: $(addprefix vm-build-, $(IMAGES))
 
+vm-clean-all:
+	rm -f $(IMAGE_FILES)
+
 tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
 		$(SRC_PATH)/tests/vm/basevm.py \
 		$(SRC_PATH)/tests/vm/Makefile.include
-- 
2.17.1

[Qemu-devel] [PULL 12/17] tests/vm: Pass the jobs parallelism setting to 'make check'

[Fam Zheng]

From: Peter Maydell <peter.maydell@linaro.org>

Our test suite works for parallel execution too, and this can
noticeably speed up a test run; pass the 'jobs' setting to
it as well as to the build proper.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180803085230.30574-3-peter.maydell@linaro.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/freebsd     | 2 +-
 tests/vm/netbsd      | 2 +-
 tests/vm/openbsd     | 2 +-
 tests/vm/ubuntu.i386 | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 039dad8f69..2187a17327 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -24,7 +24,7 @@ class FreeBSDVM(basevm.BaseVM):
         tar -xf /dev/vtbd1;
         ./configure {configure_opts};
         gmake -j{jobs};
-        gmake check;
+        gmake -j{jobs} check;
     """
 
     def build_image(self, img):
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 3972d8b45c..2cc4798f0c 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -24,7 +24,7 @@ class NetBSDVM(basevm.BaseVM):
         tar -xf /dev/rld1a;
         ./configure --python=python2.7 {configure_opts};
         gmake -j{jobs};
-        gmake check;
+        gmake -j{jobs} check;
     """
 
     def build_image(self, img):
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 6ae16d97fd..df6b79fe7d 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -25,7 +25,7 @@ class OpenBSDVM(basevm.BaseVM):
         ./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 --python=python2.7 {configure_opts};
         gmake -j{jobs};
         # XXX: "gmake check" seems to always hang or fail
-        #gmake check;
+        #gmake -j{jobs} check;
     """
 
     def build_image(self, img):
diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index fc27b6935e..2498fc7570 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -26,7 +26,7 @@ class UbuntuX86VM(basevm.BaseVM):
         tar -xf /dev/vdb;
         ./configure {configure_opts};
         make -j{jobs};
-        make check;
+        make -j{jobs} check;
     """
 
     def _gen_cloud_init_iso(self):
-- 
2.17.1

[Qemu-devel] [PULL 13/17] tests/vm: Propagate V=1 down into the make inside the VM

[Fam Zheng]

From: Peter Maydell <peter.maydell@linaro.org>

Invoking 'make vm-build-freebsd' and friends with V=1 should
propagate that verbosity setting down into the build run
inside the VM. Make sure we do that. This brings it into
line with how the container tests handle V=1.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180803085230.30574-4-peter.maydell@linaro.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 docs/devel/testing.rst    | 1 +
 tests/vm/Makefile.include | 1 +
 tests/vm/basevm.py        | 5 ++++-
 tests/vm/freebsd          | 4 ++--
 tests/vm/netbsd           | 4 ++--
 tests/vm/openbsd          | 4 ++--
 tests/vm/ubuntu.i386      | 2 +-
 7 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 8e1fa3a66e..727c4019b5 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -434,6 +434,7 @@ Debugging
 
 Add ``DEBUG=1`` and/or ``V=1`` to the make command to allow interactive
 debugging and verbose output. If this is not enough, see the next section.
+``V=1`` will be propagated down into the make jobs in the guest.
 
 Manual invocation
 -----------------
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index a5abb569c5..76f416286e 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -40,6 +40,7 @@ vm-build-%: tests/vm/%.img
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(DEBUG), --interactive) \
 		$(if $(J),--jobs $(J)) \
+		$(if $(V),--verbose) \
 		--image "$<" \
 		--build-qemu $(SRC_PATH), \
 		"  VM-BUILD $*")
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 1f1579dc9a..d73cba9edb 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -211,6 +211,8 @@ def parse_args(vm_name):
                       help="force build image even if image exists")
     parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() / 2,
                       help="number of virtual CPUs")
+    parser.add_option("--verbose", "-V", action="store_true",
+                      help="Pass V=1 to builds within the guest")
     parser.add_option("--build-image", "-b", action="store_true",
                       help="build image")
     parser.add_option("--build-qemu",
@@ -241,7 +243,8 @@ def main(vmcls):
             vm.add_source_dir(args.build_qemu)
             cmd = [vm.BUILD_SCRIPT.format(
                    configure_opts = " ".join(argv),
-                   jobs=args.jobs)]
+                   jobs=args.jobs,
+                   verbose = "V=1" if args.verbose else "")]
         else:
             cmd = argv
         img = args.image
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 2187a17327..795f739c7b 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -23,8 +23,8 @@ class FreeBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/vtbd1;
         ./configure {configure_opts};
-        gmake -j{jobs};
-        gmake -j{jobs} check;
+        gmake -j{jobs} {verbose};
+        gmake -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 2cc4798f0c..c211672bcb 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -23,8 +23,8 @@ class NetBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/rld1a;
         ./configure --python=python2.7 {configure_opts};
-        gmake -j{jobs};
-        gmake -j{jobs} check;
+        gmake -j{jobs} {verbose};
+        gmake -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index df6b79fe7d..1e0c2500ad 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -23,9 +23,9 @@ class OpenBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/rsd1c;
         ./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 --python=python2.7 {configure_opts};
-        gmake -j{jobs};
+        gmake -j{jobs} {verbose};
         # XXX: "gmake check" seems to always hang or fail
-        #gmake -j{jobs} check;
+        #gmake -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index 2498fc7570..160b9be034 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -26,7 +26,7 @@ class UbuntuX86VM(basevm.BaseVM):
         tar -xf /dev/vdb;
         ./configure {configure_opts};
         make -j{jobs};
-        make -j{jobs} check;
+        make check -j{jobs} {verbose};
     """
 
     def _gen_cloud_init_iso(self):
-- 
2.17.1

[Qemu-devel] [PULL 14/17] tests/vm: Bump guest RAM up from 2G to 4G

[Fam Zheng]

From: Peter Maydell <peter.maydell@linaro.org>

Currently we run the guests in a VM which is given only 2G of RAM.
Since the guests are configured without any swap space, builds
can fail because the system runs out of memory and kills the
compiler, especially if the job count is set for a lot of
parallelism. Bump the setting up from 2G to 4G to give us some
more headroom.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180803085230.30574-5-peter.maydell@linaro.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/basevm.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index d73cba9edb..d7149dea7d 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -64,7 +64,7 @@ class BaseVM(object):
         else:
             self._stdout = self._devnull
         self._args = [ \
-            "-nodefaults", "-m", "2G",
+            "-nodefaults", "-m", "4G",
             "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22",
             "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20",
-- 
2.17.1

[Qemu-devel] [PULL 15/17] tests/vm: Use make's --output-sync option

[Fam Zheng]

From: Peter Maydell <peter.maydell@linaro.org>

Use make's --output-sync option when running tests inside VMs,
so that if we're building with parallelization the output doesn't
get scrambled.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180803085230.30574-6-peter.maydell@linaro.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/freebsd     | 4 ++--
 tests/vm/netbsd      | 4 ++--
 tests/vm/openbsd     | 4 ++--
 tests/vm/ubuntu.i386 | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 795f739c7b..0a6ec4614a 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -23,8 +23,8 @@ class FreeBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/vtbd1;
         ./configure {configure_opts};
-        gmake -j{jobs} {verbose};
-        gmake -j{jobs} check {verbose};
+        gmake --output-sync -j{jobs} {verbose};
+        gmake --output-sync -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index c211672bcb..45c9260dc0 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -23,8 +23,8 @@ class NetBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/rld1a;
         ./configure --python=python2.7 {configure_opts};
-        gmake -j{jobs} {verbose};
-        gmake -j{jobs} check {verbose};
+        gmake --output-sync -j{jobs} {verbose};
+        gmake --output-sync -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 1e0c2500ad..98edfbca4b 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -23,9 +23,9 @@ class OpenBSDVM(basevm.BaseVM):
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/rsd1c;
         ./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 --python=python2.7 {configure_opts};
-        gmake -j{jobs} {verbose};
+        gmake --output-sync -j{jobs} {verbose};
         # XXX: "gmake check" seems to always hang or fail
-        #gmake -j{jobs} check {verbose};
+        #gmake --output-sync -j{jobs} check {verbose};
     """
 
     def build_image(self, img):
diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index 160b9be034..3f6ed48b74 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -25,8 +25,8 @@ class UbuntuX86VM(basevm.BaseVM):
         sudo chmod a+r /dev/vdb;
         tar -xf /dev/vdb;
         ./configure {configure_opts};
-        make -j{jobs};
-        make check -j{jobs} {verbose};
+        make --output-sync -j{jobs};
+        make --output-sync check -j{jobs} {verbose};
     """
 
     def _gen_cloud_init_iso(self):
-- 
2.17.1

[Qemu-devel] [PULL 16/17] tests/vm: Add vm-build-all/vm-clean-all in help text

[Fam Zheng]

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180727083445.21436-1-famz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 76f416286e..a98fb3027f 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -15,6 +15,9 @@ vm-test:
 	@echo "  vm-build-netbsd                 - Build QEMU in NetBSD VM"
 	@echo "  vm-build-openbsd                - Build QEMU in OpenBSD VM"
 	@echo "  vm-build-centos                 - Build QEMU in CentOS VM, with Docker"
+	@echo ""
+	@echo "  vm-build-all                    - Build QEMU in all VMs"
+	@echo "  vm-clean-all                    - Clean up VM images"
 
 vm-build-all: $(addprefix vm-build-, $(IMAGES))
 
-- 
2.17.1

[Qemu-devel] [PULL 17/17] aio-posix: Improve comment around marking node deleted

[Fam Zheng]

The counter is for qemu_lockcnt_inc/dec sections (read side),
qemu_lockcnt_lock/unlock is for the write side.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180803063917.30292-1-famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 util/aio-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index b5c609b68b..131ba6b4a8 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -232,7 +232,7 @@ void aio_set_fd_handler(AioContext *ctx,
             g_source_remove_poll(&ctx->source, &node->pfd);
         }
 
-        /* If the lock is held, just mark the node as deleted */
+        /* If a read is in progress, just mark the node as deleted */
         if (qemu_lockcnt_count(&ctx->list_lock)) {
             node->deleted = 1;
             node->pfd.revents = 0;
-- 
2.17.1

Re: [Qemu-devel] [PULL 00/17] Block and testing patches

[Peter Maydell]

On 15 August 2018 at 04:12, Fam Zheng <famz@redhat.com> wrote:
> The following changes since commit 38441756b70eec5807b5f60dad11a93a91199866:
>
>   Update version for v3.0.0 release (2018-08-14 16:38:43 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/famz/qemu.git tags/block-and-testing-pull-request
>
> for you to fetch changes up to 37a81812f7b8367422a039eb09d915df543983ee:
>
>   aio-posix: Improve comment around marking node deleted (2018-08-15 10:12:35 +0800)
>
> ----------------------------------------------------------------
> Block and testing patches for 3.1
>
> - aio fixes by me
> - nvme fixes by Paolo and me
> - test improvements by Peter, Phil and me
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 00/17] Block and testing patches

[Peter Maydell]

On 15 August 2018 at 04:12, Fam Zheng <famz@redhat.com> wrote:
> The following changes since commit 38441756b70eec5807b5f60dad11a93a91199866:
>
>   Update version for v3.0.0 release (2018-08-14 16:38:43 +0100)


>   tests/vm: Only use -cpu 'host' if KVM is available

Incidentally, why did you go with this one rather than just
using -cpu max all the time? The point of -cpu max is to work
everywhere, so you don't need to have special casing to decide
whether to use it or something else...

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/17] Block and testing patches

[Fam Zheng]

On Wed, 08/15 16:35, Peter Maydell wrote:
> On 15 August 2018 at 04:12, Fam Zheng <famz@redhat.com> wrote:
> > The following changes since commit 38441756b70eec5807b5f60dad11a93a91199866:
> >
> >   Update version for v3.0.0 release (2018-08-14 16:38:43 +0100)
> 
> 
> >   tests/vm: Only use -cpu 'host' if KVM is available
> 
> Incidentally, why did you go with this one rather than just
> using -cpu max all the time? The point of -cpu max is to work
> everywhere, so you don't need to have special casing to decide
> whether to use it or something else...

My apology for dropping the patch but forgot to send a note.

The only reason is this was queued first, and yours couldn't be applied without
rebasing (which effectively drops the former). But I agree with your point, and
am open for a further clean up.

Fam