[Qemu-devel] [PULL 0/6] Block layer patches

[Qemu-devel] [PULL 0/6] Block layer patches

[Kevin Wolf]

The following changes since commit df34fe314b5da628bc9a2664fb1b887bc0a6cc6d:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190708' into staging (2019-07-08 14:23:32 +0100)

are available in the Git repository at:

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

for you to fetch changes up to f7077c9860a438087c2d9a8cc27cb8438c98a748:

  qcow2: Allow -o compat=v3 during qemu-img amend (2019-07-08 16:00:31 +0200)

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

- virtio-scsi: Fix request resubmission after I/O error with iothreads
- qcow2: Fix missing v2/v3 subformat aliases for amend
- qcow(1): More specific error message for wrong format version
- MAINTAINERS: update RBD block maintainer

----------------------------------------------------------------
Eric Blake (1):
      qcow2: Allow -o compat=v3 during qemu-img amend

Jason Dillaman (1):
      MAINTAINERS: update RBD block maintainer

John Snow (1):
      block/qcow: Improve error when opening qcow2 files as qcow

Stefan Hajnoczi (3):
      vl: add qemu_add_vm_change_state_handler_prio()
      qdev: add qdev_add_vm_change_state_handler()
      virtio-scsi: restart DMA after iothread

 include/hw/qdev-core.h            |  5 ++++
 include/sysemu/sysemu.h           |  2 ++
 block/qcow.c                      |  7 ++++-
 block/qcow2.c                     |  6 ++--
 hw/core/vm-change-state-handler.c | 61 +++++++++++++++++++++++++++++++++++++++
 hw/scsi/scsi-bus.c                |  4 +--
 hw/virtio/virtio.c                |  4 +--
 vl.c                              | 59 +++++++++++++++++++++++++++++--------
 MAINTAINERS                       |  2 +-
 hw/core/Makefile.objs             |  1 +
 10 files changed, 130 insertions(+), 21 deletions(-)
 create mode 100644 hw/core/vm-change-state-handler.c

[Qemu-devel] [PULL 1/6] vl: add qemu_add_vm_change_state_handler_prio()

[Kevin Wolf]

From: Stefan Hajnoczi <stefanha@redhat.com>

Add an API for registering vm change state handlers with a well-defined
ordering.  This is necessary when handlers depend on each other.

Small coding style fixes are included to make checkpatch.pl happy.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/sysemu/sysemu.h |  2 ++
 vl.c                    | 59 ++++++++++++++++++++++++++++++++---------
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 61579ae71e..984c439ac9 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -29,6 +29,8 @@ typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
 
 VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
                                                      void *opaque);
+VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
+        VMChangeStateHandler *cb, void *opaque, int priority);
 void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
 void vm_state_notify(int running, RunState state);
 
diff --git a/vl.c b/vl.c
index 280e709e2c..5089fce6c5 100644
--- a/vl.c
+++ b/vl.c
@@ -1365,28 +1365,57 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
 struct vm_change_state_entry {
     VMChangeStateHandler *cb;
     void *opaque;
-    QLIST_ENTRY (vm_change_state_entry) entries;
+    QTAILQ_ENTRY(vm_change_state_entry) entries;
+    int priority;
 };
 
-static QLIST_HEAD(, vm_change_state_entry) vm_change_state_head;
+static QTAILQ_HEAD(, vm_change_state_entry) vm_change_state_head;
 
-VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
-                                                     void *opaque)
+/**
+ * qemu_add_vm_change_state_handler_prio:
+ * @cb: the callback to invoke
+ * @opaque: user data passed to the callback
+ * @priority: low priorities execute first when the vm runs and the reverse is
+ *            true when the vm stops
+ *
+ * Register a callback function that is invoked when the vm starts or stops
+ * running.
+ *
+ * Returns: an entry to be freed using qemu_del_vm_change_state_handler()
+ */
+VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
+        VMChangeStateHandler *cb, void *opaque, int priority)
 {
     VMChangeStateEntry *e;
+    VMChangeStateEntry *other;
 
-    e = g_malloc0(sizeof (*e));
-
+    e = g_malloc0(sizeof(*e));
     e->cb = cb;
     e->opaque = opaque;
-    QLIST_INSERT_HEAD(&vm_change_state_head, e, entries);
+    e->priority = priority;
+
+    /* Keep list sorted in ascending priority order */
+    QTAILQ_FOREACH(other, &vm_change_state_head, entries) {
+        if (priority < other->priority) {
+            QTAILQ_INSERT_BEFORE(other, e, entries);
+            return e;
+        }
+    }
+
+    QTAILQ_INSERT_TAIL(&vm_change_state_head, e, entries);
     return e;
 }
 
+VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
+                                                     void *opaque)
+{
+    return qemu_add_vm_change_state_handler_prio(cb, opaque, 0);
+}
+
 void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
 {
-    QLIST_REMOVE (e, entries);
-    g_free (e);
+    QTAILQ_REMOVE(&vm_change_state_head, e, entries);
+    g_free(e);
 }
 
 void vm_state_notify(int running, RunState state)
@@ -1395,8 +1424,14 @@ void vm_state_notify(int running, RunState state)
 
     trace_vm_state_notify(running, state, RunState_str(state));
 
-    QLIST_FOREACH_SAFE(e, &vm_change_state_head, entries, next) {
-        e->cb(e->opaque, running, state);
+    if (running) {
+        QTAILQ_FOREACH_SAFE(e, &vm_change_state_head, entries, next) {
+            e->cb(e->opaque, running, state);
+        }
+    } else {
+        QTAILQ_FOREACH_REVERSE_SAFE(e, &vm_change_state_head, entries, next) {
+            e->cb(e->opaque, running, state);
+        }
     }
 }
 
@@ -2911,7 +2946,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    QLIST_INIT (&vm_change_state_head);
+    QTAILQ_INIT(&vm_change_state_head);
     os_setup_early_signal_handling();
 
     cpu_option = NULL;
-- 
2.20.1

[Qemu-devel] [PULL 2/6] qdev: add qdev_add_vm_change_state_handler()

[Kevin Wolf]

From: Stefan Hajnoczi <stefanha@redhat.com>

Children sometimes depend on their parent's vm change state handler
having completed.  Add a vm change state handler API for devices that
guarantees tree depth ordering.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/hw/qdev-core.h            |  5 +++
 hw/core/vm-change-state-handler.c | 61 +++++++++++++++++++++++++++++++
 hw/core/Makefile.objs             |  1 +
 3 files changed, 67 insertions(+)
 create mode 100644 hw/core/vm-change-state-handler.c

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index fa55dc10ae..e157fc4acd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -6,6 +6,7 @@
 #include "qom/object.h"
 #include "hw/irq.h"
 #include "hw/hotplug.h"
+#include "sysemu/sysemu.h"
 
 enum {
     DEV_NVECTORS_UNSPECIFIED = -1,
@@ -450,4 +451,8 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
 void device_listener_register(DeviceListener *listener);
 void device_listener_unregister(DeviceListener *listener);
 
+VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
+                                                     VMChangeStateHandler *cb,
+                                                     void *opaque);
+
 #endif
diff --git a/hw/core/vm-change-state-handler.c b/hw/core/vm-change-state-handler.c
new file mode 100644
index 0000000000..f814813bdd
--- /dev/null
+++ b/hw/core/vm-change-state-handler.c
@@ -0,0 +1,61 @@
+/*
+ *  qdev vm change state handlers
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License,
+ *  or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+static int qdev_get_dev_tree_depth(DeviceState *dev)
+{
+    int depth;
+
+    for (depth = 0; dev; depth++) {
+        BusState *bus = dev->parent_bus;
+
+        if (!bus) {
+            break;
+        }
+
+        dev = bus->parent;
+    }
+
+    return depth;
+}
+
+/**
+ * qdev_add_vm_change_state_handler:
+ * @dev: the device that owns this handler
+ * @cb: the callback function to be invoked
+ * @opaque: user data passed to the callback function
+ *
+ * This function works like qemu_add_vm_change_state_handler() except callbacks
+ * are invoked in qdev tree depth order.  Ordering is desirable when callbacks
+ * of children depend on their parent's callback having completed first.
+ *
+ * For example, when qdev_add_vm_change_state_handler() is used, a host
+ * controller's callback is invoked before the children on its bus when the VM
+ * starts running.  The order is reversed when the VM stops running.
+ *
+ * Returns: an entry to be freed with qemu_del_vm_change_state_handler()
+ */
+VMChangeStateEntry *qdev_add_vm_change_state_handler(DeviceState *dev,
+                                                     VMChangeStateHandler *cb,
+                                                     void *opaque)
+{
+    int depth = qdev_get_dev_tree_depth(dev);
+
+    return qemu_add_vm_change_state_handler_prio(cb, opaque, depth);
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 585b734358..f8481d959f 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -7,6 +7,7 @@ common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
 common-obj-y += irq.o
 common-obj-y += hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += nmi.o
+common-obj-$(CONFIG_SOFTMMU) += vm-change-state-handler.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_XILINX_AXI) += stream.o
-- 
2.20.1

[Qemu-devel] [PULL 3/6] virtio-scsi: restart DMA after iothread

[Kevin Wolf]

From: Stefan Hajnoczi <stefanha@redhat.com>

When the 'cont' command resumes guest execution the vm change state
handlers are invoked.  Unfortunately there is no explicit ordering
between classic qemu_add_vm_change_state_handler() callbacks.  When two
layers of code both use vm change state handlers, we don't control which
handler runs first.

virtio-scsi with iothreads hits a deadlock when a failed SCSI command is
restarted and completes before the iothread is re-initialized.

This patch uses the new qdev_add_vm_change_state_handler() API to
guarantee that virtio-scsi's virtio change state handler executes before
the SCSI bus children.  This way DMA is restarted after the iothread has
re-initialized.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi/scsi-bus.c | 4 ++--
 hw/virtio/virtio.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 196136a307..fdc3a0e4e0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -207,8 +207,8 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
         error_propagate(errp, local_err);
         return;
     }
-    dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
-                                                     dev);
+    dev->vmsentry = qdev_add_vm_change_state_handler(DEVICE(dev),
+            scsi_dma_restart_cb, dev);
 }
 
 static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 18f9f4c372..a94ea18a9c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2362,8 +2362,8 @@ void virtio_init(VirtIODevice *vdev, const char *name,
     } else {
         vdev->config = NULL;
     }
-    vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change,
-                                                     vdev);
+    vdev->vmstate = qdev_add_vm_change_state_handler(DEVICE(vdev),
+            virtio_vmstate_change, vdev);
     vdev->device_endian = virtio_default_endian();
     vdev->use_guest_notifier_mask = true;
 }
-- 
2.20.1

[Qemu-devel] [PULL 4/6] block/qcow: Improve error when opening qcow2 files as qcow

[Kevin Wolf]

From: John Snow <jsnow@redhat.com>

Reported-by: radmehrsaeed7@gmail.com
Fixes: https://bugs.launchpad.net/bugs/1832914
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/qcow.c b/block/qcow.c
index 6dee5bb792..5bdf72ba33 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -156,7 +156,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
     if (header.version != QCOW_VERSION) {
-        error_setg(errp, "Unsupported qcow version %" PRIu32, header.version);
+        error_setg(errp, "qcow (v%d) does not support qcow version %" PRIu32,
+                   QCOW_VERSION, header.version);
+        if (header.version == 2 || header.version == 3) {
+            error_append_hint(errp, "Try the 'qcow2' driver instead.\n");
+        }
+
         ret = -ENOTSUP;
         goto fail;
     }
-- 
2.20.1

[Qemu-devel] [PULL 5/6] MAINTAINERS: update RBD block maintainer

[Kevin Wolf]

From: Jason Dillaman <dillaman@redhat.com>

Remove Josh as per his request since he is no longer the upstream RBD
tech lead. Add myself as the maintainer since I am the current RBD tech
lead.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2cce50287a..cc9636b43a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2385,7 +2385,7 @@ S: Supported
 F: block/vmdk.c
 
 RBD
-M: Josh Durgin <jdurgin@redhat.com>
+M: Jason Dillaman <dillaman@redhat.com>
 L: qemu-block@nongnu.org
 S: Supported
 F: block/rbd.c
-- 
2.20.1

[Qemu-devel] [PULL 6/6] qcow2: Allow -o compat=v3 during qemu-img amend

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

Commit b76b4f60 allowed '-o compat=v3' as an alias for the
less-appealing '-o compat=1.1' for 'qemu-img create' since we want to
use the QMP form as much as possible, but forgot to do likewise for
qemu-img amend.  Also, it doesn't help that '-o help' doesn't list our
new preferred spellings.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2a59eb27fe..039bdc2f7e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4823,9 +4823,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
             compat = qemu_opt_get(opts, BLOCK_OPT_COMPAT_LEVEL);
             if (!compat) {
                 /* preserve default */
-            } else if (!strcmp(compat, "0.10")) {
+            } else if (!strcmp(compat, "0.10") || !strcmp(compat, "v2")) {
                 new_version = 2;
-            } else if (!strcmp(compat, "1.1")) {
+            } else if (!strcmp(compat, "1.1") || !strcmp(compat, "v3")) {
                 new_version = 3;
             } else {
                 error_setg(errp, "Unknown compatibility level %s", compat);
@@ -5098,7 +5098,7 @@ static QemuOptsList qcow2_create_opts = {
         {
             .name = BLOCK_OPT_COMPAT_LEVEL,
             .type = QEMU_OPT_STRING,
-            .help = "Compatibility level (0.10 or 1.1)"
+            .help = "Compatibility level (v2 [0.10] or v3 [1.1])"
         },
         {
             .name = BLOCK_OPT_BACKING_FILE,
-- 
2.20.1

Re: [Qemu-devel] [PULL 0/6] Block layer patches

[Peter Maydell]

On Mon, 8 Jul 2019 at 15:18, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit df34fe314b5da628bc9a2664fb1b887bc0a6cc6d:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190708' into staging (2019-07-08 14:23:32 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to f7077c9860a438087c2d9a8cc27cb8438c98a748:
>
>   qcow2: Allow -o compat=v3 during qemu-img amend (2019-07-08 16:00:31 +0200)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - virtio-scsi: Fix request resubmission after I/O error with iothreads
> - qcow2: Fix missing v2/v3 subformat aliases for amend
> - qcow(1): More specific error message for wrong format version
> - MAINTAINERS: update RBD block maintainer
>


Applied, thanks.

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

-- PMM