All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
@ 2020-04-16 20:36 Maxim Levitsky
  2020-04-16 20:36 ` [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find Maxim Levitsky
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-04-16 20:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, Maxim Levitsky,
	Paolo Bonzini

Hi!

This is a patch series that is a result of my discussion with Paulo on
how to correctly fix the root cause of the BZ #1812399.

The root cause of this bug is the fact that IO thread is running mostly
unlocked versus main thread on which device hotplug is done.

qdev_device_add first creates the device object, then places it on the bus,
and only then realizes it.

However some drivers and currently only virtio-scsi enumerate its child bus
devices on each request that is received from the guest and that can happen on the IO
thread.

Thus we have a window when new device is on the bus but not realized and can be accessed
by the virtio-scsi driver in that state.

Fix that by doing two things:

1. Add partial RCU protection to the list of a bus's child devices.
This allows the scsi IO thread to safely enumerate the child devices
while it races with the hotplug placing the device on the bus.

2. Make the virtio-scsi driver check .realized property of the scsi device
and avoid touching the device if it isn't

I don't think that this is very pretty way to solve this, we discussed this
with Paulo and it kind of looks like the lesser evil. I am open to your thoughts about this.

Note that this patch series doesn't pass some unit tests and in particular qtest 'drive_del-test'
I did some light debug of this test and I see that the reason for this is that now child device deletion
can be delayed due to RCU. This is also something I would like to discuss in this RFC.

Note also that I might have some code style errors and bugs in this since I haven't
tested the code in depth yet, because I am not yet sure that this is the right way
to fix that bug

Also note that in the particular bug report the issue wasn't a race but rather due
to combination of things, the .realize code in the middle managed to trigger IO on the virtqueue
which caused the virtio-scsi driver to access the half realized device. However
since this can happen as well with real IO thread, this patch series was done,
which fixes this as well.

Best regards,
	Maxim Levitsky

Maxim Levitsky (4):
  scsi/scsi_bus: switch search direction in scsi_device_find
  device-core: use RCU for list of childs of a bus
  device-core: use atomic_set on .realized property
  virtio-scsi: don't touch scsi devices that are not yet realized

 hw/core/bus.c                  | 43 ++++++++++++++++++++----------
 hw/core/qdev.c                 | 48 ++++++++++++++++++++++------------
 hw/scsi/scsi-bus.c             | 27 ++++++++++++++++---
 hw/scsi/virtio-scsi.c          | 24 +++++++++++++++--
 include/hw/qdev-core.h         |  3 +++
 include/hw/virtio/virtio-bus.h |  7 +++--
 6 files changed, 114 insertions(+), 38 deletions(-)

-- 
2.17.2



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

* [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
@ 2020-04-16 20:36 ` Maxim Levitsky
  2020-04-16 20:36 ` [PATCH 2/4] device-core: use RCU for list of childs of a bus Maxim Levitsky
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-04-16 20:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, Maxim Levitsky,
	Paolo Bonzini

This change will allow us to convert the bus children list to RCU,
while not changing the logic of this function

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/scsi/scsi-bus.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 1c980cab38..7bbc37acec 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1584,7 +1584,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -1592,7 +1592,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
             if (dev->lun == lun) {
                 return dev;
             }
-            target_dev = dev;
+
+            /*
+             * If we don't find exact match (channel/bus/lun),
+             * we will return the first device which matches channel/bus
+             */
+
+            if (!target_dev) {
+                target_dev = dev;
+            }
         }
     }
     return target_dev;
-- 
2.17.2



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

* [PATCH 2/4] device-core: use RCU for list of childs of a bus
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
  2020-04-16 20:36 ` [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find Maxim Levitsky
@ 2020-04-16 20:36 ` Maxim Levitsky
  2020-05-04 10:41   ` Stefan Hajnoczi
  2020-04-16 20:36 ` [PATCH 3/4] device-core: use atomic_set on .realized property Maxim Levitsky
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2020-04-16 20:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, Maxim Levitsky,
	Paolo Bonzini

This fixes the race between device emulation code that tries to find
a child device to dispatch the request to (e.g a scsi disk),
and hotplug of a new device to that bus.

Note that this doesn't convert all the readers of the list
but only these that might go over that list without BQL held.

This is a very small first step to make this code thread safe.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/core/bus.c                  | 43 ++++++++++++++++++++-----------
 hw/core/qdev.c                 | 46 +++++++++++++++++++++++-----------
 hw/scsi/scsi-bus.c             | 17 ++++++++++---
 hw/scsi/virtio-scsi.c          |  6 ++++-
 include/hw/qdev-core.h         |  3 +++
 include/hw/virtio/virtio-bus.h |  7 ++++--
 6 files changed, 87 insertions(+), 35 deletions(-)

diff --git a/hw/core/bus.c b/hw/core/bus.c
index 3dc0a825f0..cb7756ded1 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -49,12 +49,14 @@ int qbus_walk_children(BusState *bus,
         }
     }
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        err = qdev_walk_children(kid->child,
-                                 pre_devfn, pre_busfn,
-                                 post_devfn, post_busfn, opaque);
-        if (err < 0) {
-            return err;
+    WITH_RCU_READ_LOCK_GUARD(){
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            err = qdev_walk_children(kid->child,
+                                     pre_devfn, pre_busfn,
+                                     post_devfn, post_busfn, opaque);
+            if (err < 0) {
+                return err;
+            }
         }
     }
 
@@ -90,9 +92,13 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
     BusState *bus = BUS(obj);
     BusChild *kid;
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+    rcu_read_lock();
+
+    QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
         cb(OBJECT(kid->child), opaque, type);
     }
+
+    rcu_read_unlock();
 }
 
 static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
@@ -138,10 +144,15 @@ static void bus_unparent(Object *obj)
     /* Only the main system bus has no parent, and that bus is never freed */
     assert(bus->parent);
 
-    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
+    rcu_read_lock();
+
+    while ((kid = QTAILQ_FIRST_RCU(&bus->children)) != NULL) {
         DeviceState *dev = kid->child;
         object_unparent(OBJECT(dev));
     }
+
+    rcu_read_unlock();
+
     QLIST_REMOVE(bus, sibling);
     bus->parent->num_child_bus--;
     bus->parent = NULL;
@@ -185,14 +196,18 @@ static void bus_set_realized(Object *obj, bool value, Error **errp)
 
         /* TODO: recursive realization */
     } else if (!value && bus->realized) {
-        QTAILQ_FOREACH(kid, &bus->children, sibling) {
-            DeviceState *dev = kid->child;
-            object_property_set_bool(OBJECT(dev), false, "realized",
-                                     &local_err);
-            if (local_err != NULL) {
-                break;
+
+        WITH_RCU_READ_LOCK_GUARD(){
+            QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+                DeviceState *dev = kid->child;
+                object_property_set_bool(OBJECT(dev), false, "realized",
+                                         &local_err);
+                if (local_err != NULL) {
+                    break;
+                }
             }
         }
+
         if (bc->unrealize && local_err == NULL) {
             bc->unrealize(bus, &local_err);
         }
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 85f062def7..f0c87e582e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -50,26 +50,37 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
     return dc->vmsd;
 }
 
+static void bus_free_bus_child(BusChild *kid)
+{
+    object_unref(OBJECT(kid->child));
+    g_free(kid);
+}
+
 static void bus_remove_child(BusState *bus, DeviceState *child)
 {
     BusChild *kid;
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+    rcu_read_lock();
+
+    QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
         if (kid->child == child) {
             char name[32];
 
             snprintf(name, sizeof(name), "child[%d]", kid->index);
-            QTAILQ_REMOVE(&bus->children, kid, sibling);
+            QTAILQ_REMOVE_RCU(&bus->children, kid, sibling);
 
             bus->num_children--;
 
             /* This gives back ownership of kid->child back to us.  */
             object_property_del(OBJECT(bus), name, NULL);
-            object_unref(OBJECT(kid->child));
-            g_free(kid);
-            return;
+
+            /* free the bus kid, when it is safe to do so*/
+            call_rcu(kid, bus_free_bus_child, rcu);
+            break;
         }
     }
+
+    rcu_read_unlock();
 }
 
 static void bus_add_child(BusState *bus, DeviceState *child)
@@ -82,7 +93,9 @@ static void bus_add_child(BusState *bus, DeviceState *child)
     kid->child = child;
     object_ref(OBJECT(kid->child));
 
-    QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
+    rcu_read_lock();
+    QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
+    rcu_read_unlock();
 
     /* This transfers ownership of kid->child to the property.  */
     snprintf(name, sizeof(name), "child[%d]", kid->index);
@@ -681,20 +694,23 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     DeviceState *ret;
     BusState *child;
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        DeviceState *dev = kid->child;
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            DeviceState *dev = kid->child;
 
-        if (dev->id && strcmp(dev->id, id) == 0) {
-            return dev;
-        }
+            if (dev->id && strcmp(dev->id, id) == 0) {
+                return dev;
+            }
 
-        QLIST_FOREACH(child, &dev->child_bus, sibling) {
-            ret = qdev_find_recursive(child, id);
-            if (ret) {
-                return ret;
+            QLIST_FOREACH(child, &dev->child_bus, sibling) {
+                ret = qdev_find_recursive(child, id);
+                if (ret) {
+                    return ret;
+                }
             }
         }
     }
+
     return NULL;
 }
 
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 7bbc37acec..2bf6d005f3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -412,7 +412,10 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     id = r->req.dev->id;
     found_lun0 = false;
     n = 0;
-    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+
+    rcu_read_lock();
+
+    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -433,7 +436,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     memset(r->buf, 0, len);
     stl_be_p(&r->buf[0], n);
     i = found_lun0 ? 8 : 16;
-    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -442,6 +445,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
             i += 8;
         }
     }
+
+    rcu_read_unlock();
+
     assert(i == n + 8);
     r->len = len;
     return true;
@@ -1584,12 +1590,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+    rcu_read_lock();
+
+    QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
         if (dev->channel == channel && dev->id == id) {
             if (dev->lun == lun) {
+                rcu_read_unlock();
                 return dev;
             }
 
@@ -1603,6 +1612,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
             }
         }
     }
+
+    rcu_read_unlock();
     return target_dev;
 }
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 472bbd233b..b0f4a35f81 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
     case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
         target = req->req.tmf.lun[1];
         s->resetting++;
-        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+
+        rcu_read_lock();
+        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
              d = SCSI_DEVICE(kid->child);
              if (d->channel == 0 && d->id == target) {
                 qdev_reset_all(&d->qdev);
              }
         }
+        rcu_read_unlock();
+
         s->resetting--;
         break;
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1405b8a990..196253978b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -3,6 +3,8 @@
 
 #include "qemu/queue.h"
 #include "qemu/bitmap.h"
+#include "qemu/rcu.h"
+#include "qemu/rcu_queue.h"
 #include "qom/object.h"
 #include "hw/hotplug.h"
 #include "hw/resettable.h"
@@ -218,6 +220,7 @@ struct BusClass {
 };
 
 typedef struct BusChild {
+    struct rcu_head rcu;
     DeviceState *child;
     int index;
     QTAILQ_ENTRY(BusChild) sibling;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 38c9399cd4..58733f28e2 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -128,8 +128,11 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
 static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
 {
     BusState *qbus = &bus->parent_obj;
-    BusChild *kid = QTAILQ_FIRST(&qbus->children);
-    DeviceState *qdev = kid ? kid->child : NULL;
+    BusChild *kid;
+    DeviceState *qdev;
+
+    kid = QTAILQ_FIRST(&qbus->children);
+    qdev = kid ? kid->child : NULL;
 
     /* This is used on the data path, the cast is guaranteed
      * to succeed by the qdev machinery.
-- 
2.17.2



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

* [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
  2020-04-16 20:36 ` [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find Maxim Levitsky
  2020-04-16 20:36 ` [PATCH 2/4] device-core: use RCU for list of childs of a bus Maxim Levitsky
@ 2020-04-16 20:36 ` Maxim Levitsky
  2020-05-04 10:45   ` Stefan Hajnoczi
  2020-04-16 20:36 ` [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized Maxim Levitsky
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2020-04-16 20:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, Maxim Levitsky,
	Paolo Bonzini

Some code might race with placement of new devices on a bus.
We currently first place a (unrealized) device on the bus
and then realize it.

As a workaround, users that scan the child device list, can
check the realized property to see if it is safe to access such a device.
Use an atomic write here too to aid with this.

A separate discussion is what to do with devices that are unrealized:
It looks like for this case we only call the hotplug handler's unplug
callback and its up to it to unrealize the device.
An atomic operation doesn't cause harm for this code path though.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/core/qdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f0c87e582e..bbb1ae3eb3 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     }
 
     assert(local_err == NULL);
-    dev->realized = value;
+    atomic_set(&dev->realized, value);
     return;
 
 child_realize_fail:
-- 
2.17.2



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

* [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
                   ` (2 preceding siblings ...)
  2020-04-16 20:36 ` [PATCH 3/4] device-core: use atomic_set on .realized property Maxim Levitsky
@ 2020-04-16 20:36 ` Maxim Levitsky
  2020-05-04 11:24   ` Paolo Bonzini
  2020-04-16 21:33 ` [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread no-reply
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2020-04-16 20:36 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, Maxim Levitsky,
	Paolo Bonzini

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 hw/scsi/virtio-scsi.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b0f4a35f81..e360b4e03e 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun)
 
 static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
 {
+    SCSIDevice *device = NULL;
+
     if (lun[0] != 1) {
         return NULL;
     }
     if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) {
         return NULL;
     }
-    return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
+
+    device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
+
+    /* This function might run on the IO thread and we might race against
+     * main thread hot-plugging the device.
+     *
+     * We assume that as soon as .realized is set to true we can let
+     * the user access the device.
+     */
+
+    if (!device || !atomic_read(&device->qdev.realized)) {
+        return NULL;
+    }
+
+    return device;
 }
 
 void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
-- 
2.17.2



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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
                   ` (3 preceding siblings ...)
  2020-04-16 20:36 ` [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized Maxim Levitsky
@ 2020-04-16 21:33 ` no-reply
  2020-04-16 21:35 ` no-reply
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2020-04-16 21:33 UTC (permalink / raw)
  To: mlevitsk; +Cc: fam, berrange, ehabkost, mst, qemu-devel, mlevitsk, pbonzini

Patchew URL: https://patchew.org/QEMU/20200416203624.32366-1-mlevitsk@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
Message-id: 20200416203624.32366-1-mlevitsk@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200416212349.731404-1-eblake@redhat.com -> patchew/20200416212349.731404-1-eblake@redhat.com
Switched to a new branch 'test'
5e47155 virtio-scsi: don't touch scsi devices that are not yet realized
7a4e20b device-core: use atomic_set on .realized property
c8c6d72 device-core: use RCU for list of childs of a bus
7b3ca63 scsi/scsi_bus: switch search direction in scsi_device_find

=== OUTPUT BEGIN ===
1/4 Checking commit 7b3ca636be2f (scsi/scsi_bus: switch search direction in scsi_device_find)
2/4 Checking commit c8c6d7230602 (device-core: use RCU for list of childs of a bus)
ERROR: space required before the open brace '{'
#34: FILE: hw/core/bus.c:52:
+    WITH_RCU_READ_LOCK_GUARD(){

ERROR: space required before the open brace '{'
#88: FILE: hw/core/bus.c:200:
+        WITH_RCU_READ_LOCK_GUARD(){

total: 2 errors, 0 warnings, 255 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/4 Checking commit 7a4e20bf9d25 (device-core: use atomic_set on .realized property)
4/4 Checking commit 5e47155794bf (virtio-scsi: don't touch scsi devices that are not yet realized)
WARNING: Block comments use a leading /* on a separate line
#33: FILE: hw/scsi/virtio-scsi.c:49:
+    /* This function might run on the IO thread and we might race against

total: 0 errors, 1 warnings, 30 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200416203624.32366-1-mlevitsk@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
                   ` (4 preceding siblings ...)
  2020-04-16 21:33 ` [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread no-reply
@ 2020-04-16 21:35 ` no-reply
  2020-04-16 21:47 ` no-reply
  2020-05-04 10:59 ` Stefan Hajnoczi
  7 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2020-04-16 21:35 UTC (permalink / raw)
  To: mlevitsk; +Cc: fam, berrange, ehabkost, mst, qemu-devel, mlevitsk, pbonzini

Patchew URL: https://patchew.org/QEMU/20200416203624.32366-1-mlevitsk@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 3 fdc-test /x86_64/fdc/read_without_media
PASS 1 check-qnull /public/qnull_ref
PASS 2 check-qnull /public/qnull_visit
==8099==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/check-qobject -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="check-qobject" 
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
---
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" 
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
==8157==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8157==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd2b90d000; bottom 0x7f7ffd19e000; size: 0x007d2e76f000 (537650458624)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
==8172==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
PASS 15 test-aio /aio/coroutine/queue-chaining
---
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
==8180==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
==8186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 2 ide-test /x86_64/ide/flush
==8193==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==8195==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
PASS 2 test-aio-multithread /aio/multi/schedule
==8212==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==8223==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" 
==8245==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" 
==8251==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==8247==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap" 
---
PASS 14 test-hbitmap /hbitmap/set/twice
PASS 15 test-hbitmap /hbitmap/set/overlap
PASS 16 test-hbitmap /hbitmap/reset/empty
==8326==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 17 test-hbitmap /hbitmap/reset/general
PASS 18 test-hbitmap /hbitmap/reset/all
PASS 19 test-hbitmap /hbitmap/truncate/nop
---
PASS 39 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
PASS 40 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_after_truncate
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain" 
==8333==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
---
PASS 41 test-bdrv-drain /bdrv-drain/bdrv_drop_intermediate/poll
PASS 42 test-bdrv-drain /bdrv-drain/replace_child/mid-drain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-graph-mod -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-graph-mod" 
==8372==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-graph-mod /bdrv-graph-mod/update-perm-tree
PASS 2 test-bdrv-graph-mod /bdrv-graph-mod/should-update-child
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob" 
==8376==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob /blockjob/ids
PASS 2 test-blockjob /blockjob/cancel/created
PASS 3 test-blockjob /blockjob/cancel/running
---
PASS 7 test-blockjob /blockjob/cancel/pending
PASS 8 test-blockjob /blockjob/cancel/concluded
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob-txn -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob-txn" 
==8380==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob-txn /single/success
PASS 2 test-blockjob-txn /single/failure
PASS 3 test-blockjob-txn /single/cancel
---
PASS 6 test-blockjob-txn /pair/cancel
PASS 7 test-blockjob-txn /pair/fail-cancel-race
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-backend -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-backend" 
==8386==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-backend /block-backend/drain_aio_error
PASS 2 test-block-backend /block-backend/drain_all_aio_error
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-iothread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-iothread" 
==8382==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8393==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-iothread /sync-op/pread
PASS 2 test-block-iothread /sync-op/pwrite
PASS 3 test-block-iothread /sync-op/load_vmstate
---
PASS 15 test-block-iothread /propagate/diamond
PASS 16 test-block-iothread /propagate/mirror
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-image-locking -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-image-locking" 
==8414==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-image-locking /image-locking/basic
PASS 2 test-image-locking /image-locking/set-perm-abort
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-x86-cpuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid" 
---
PASS 1 rcutorture /rcu/torture/1reader
PASS 2 rcutorture /rcu/torture/10readers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-list" 
==8472==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-rcu-list /rcu/qlist/single-threaded
PASS 2 test-rcu-list /rcu/qlist/short-few
PASS 3 test-rcu-list /rcu/qlist/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-simpleq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-simpleq" 
PASS 1 test-rcu-simpleq /rcu/qsimpleq/single-threaded
==8517==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-rcu-simpleq /rcu/qsimpleq/short-few
PASS 3 test-rcu-simpleq /rcu/qsimpleq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-tailq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-tailq" 
PASS 1 test-rcu-tailq /rcu/qtailq/single-threaded
PASS 2 test-rcu-tailq /rcu/qtailq/short-few
==8577==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-tailq /rcu/qtailq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-slist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-slist" 
PASS 1 test-rcu-slist /rcu/qslist/single-threaded
---
PASS 7 test-qdist /qdist/binning/expand
PASS 8 test-qdist /qdist/binning/shrink
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht" 
==8619==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8629==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/various_prdts
==8635==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8635==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd46bc8000; bottom 0x7fd821dfe000; size: 0x002524dca000 (159532228608)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-qht /qht/mode/default
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht-par -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht-par" 
PASS 7 ide-test /x86_64/ide/flush/nodev
PASS 1 test-qht-par /qht/parallel/2threads-0%updates-1s
==8655==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 ide-test /x86_64/ide/flush/empty_drive
==8666==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 ide-test /x86_64/ide/flush/retry_pci
PASS 2 test-qht-par /qht/parallel/2threads-20%updates-1s
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bitops -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitops" 
---
PASS 5 test-bitops /bitops/half_unshuffle32
PASS 6 test-bitops /bitops/half_unshuffle64
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bitcnt -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitcnt" 
==8672==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 ide-test /x86_64/ide/flush/retry_isa
PASS 1 test-bitcnt /bitcnt/ctpop8
PASS 2 test-bitcnt /bitcnt/ctpop16
---
PASS 18 test-qemu-opts /qemu-opts/to_qdict/filtered
PASS 19 test-qemu-opts /qemu-opts/to_qdict/duplicates
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-keyval -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-keyval" 
==8684==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-keyval /keyval/keyval_parse
PASS 2 test-keyval /keyval/keyval_parse/list
PASS 3 test-keyval /keyval/visit/bool
---
PASS 3 test-crypto-hmac /crypto/hmac/prealloc
PASS 4 test-crypto-hmac /crypto/hmac/digest
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-cipher -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-cipher" 
==8721==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-cipher /crypto/cipher/aes-ecb-128
PASS 2 test-crypto-cipher /crypto/cipher/aes-ecb-192
PASS 3 test-crypto-cipher /crypto/cipher/aes-ecb-256
---
PASS 16 test-crypto-secret /crypto/secret/crypt/badiv
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlscredsx509 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlscredsx509" 
PASS 12 ide-test /x86_64/ide/cdrom/pio_large
==8750==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 ide-test /x86_64/ide/cdrom/dma
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" 
PASS 1 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectserver
---
PASS 6 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca1
PASS 7 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca2
PASS 8 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca3
==8764==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ahci-test /x86_64/ahci/sanity
==8770==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver1
PASS 10 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver2
PASS 2 ahci-test /x86_64/ahci/pci_spec
==8776==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ahci-test /x86_64/ahci/pci_enable
==8782==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver3
PASS 4 ahci-test /x86_64/ahci/hba_spec
PASS 12 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver4
==8788==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ahci-test /x86_64/ahci/hba_enable
==8794==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver5
PASS 14 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver6
PASS 6 ahci-test /x86_64/ahci/identify
PASS 15 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver7
==8800==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 16 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver1
PASS 17 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver2
PASS 18 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver3
---
PASS 33 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive2
PASS 34 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive3
PASS 7 ahci-test /x86_64/ahci/max
==8806==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 ahci-test /x86_64/ahci/reset
PASS 35 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain1
PASS 36 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain2
---
PASS 38 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingserver
PASS 39 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingclient
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlssession -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlssession" 
==8812==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8812==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff6aafe000; bottom 0x7f709a3fe000; size: 0x008ed0700000 (613382356992)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
==8822==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8822==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd79476000; bottom 0x7f70fd1fe000; size: 0x008c7c278000 (603378384896)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
PASS 1 test-crypto-tlssession /qcrypto/tlssession/psk
==8828==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8828==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffec7b93000; bottom 0x7fcb549fe000; size: 0x003373195000 (220974370816)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
PASS 2 test-crypto-tlssession /qcrypto/tlssession/basicca
PASS 3 test-crypto-tlssession /qcrypto/tlssession/differentca
==8834==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8834==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd4f062000; bottom 0x7ff9b01fe000; size: 0x00039ee64000 (15550791680)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
PASS 4 test-crypto-tlssession /qcrypto/tlssession/altname1
==8840==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-crypto-tlssession /qcrypto/tlssession/altname2
==8840==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc262a2000; bottom 0x7fd7813fe000; size: 0x0024a4ea4000 (157385637888)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
PASS 6 test-crypto-tlssession /qcrypto/tlssession/altname3
PASS 7 test-crypto-tlssession /qcrypto/tlssession/altname4
==8846==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8846==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffec40bc000; bottom 0x7f11209fe000; size: 0x00eda36be000 (1020649005056)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
PASS 8 test-crypto-tlssession /qcrypto/tlssession/altname5
==8852==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8852==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffec042000; bottom 0x7f4e8a324000; size: 0x00b161d1e000 (761850355712)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
PASS 9 test-crypto-tlssession /qcrypto/tlssession/altname6
PASS 10 test-crypto-tlssession /qcrypto/tlssession/wildcard1
==8858==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8858==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd34417000; bottom 0x7f1e837fe000; size: 0x00deb0c19000 (956448215040)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
PASS 11 test-crypto-tlssession /qcrypto/tlssession/wildcard2
==8864==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8864==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe536e2000; bottom 0x7f05293fe000; size: 0x00f92a2e4000 (1070154530816)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 test-crypto-tlssession /qcrypto/tlssession/wildcard3
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
==8870==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
PASS 13 test-crypto-tlssession /qcrypto/tlssession/wildcard4
==8876==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
PASS 14 test-crypto-tlssession /qcrypto/tlssession/wildcard5
==8882==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8888==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8888==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcb5ec9000; bottom 0x7ff925ffe000; size: 0x00038fecb000 (15299555328)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 15 test-crypto-tlssession /qcrypto/tlssession/wildcard6
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
PASS 16 test-crypto-tlssession /qcrypto/tlssession/cachain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qga -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qga" 
==8894==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8894==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe2cb19000; bottom 0x7fa2debfe000; size: 0x005b4df1b000 (392149708800)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8908==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8908==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffeb9c23000; bottom 0x7fed4f1fe000; size: 0x00116aa25000 (74803466240)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-qga /qga/sync-delimited
---
PASS 15 test-qga /qga/invalid-cmd
PASS 16 test-qga /qga/invalid-args
PASS 17 test-qga /qga/fsfreeze-status
==8914==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8914==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc7410c000; bottom 0x7fa96b5fe000; size: 0x005308b0e000 (356628094976)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
---
PASS 19 test-qga /qga/config
PASS 20 test-qga /qga/guest-exec
PASS 21 test-qga /qga/guest-exec-invalid
==8923==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8923==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff6d798000; bottom 0x7f07861fe000; size: 0x00f7e759a000 (1064738332672)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
---
PASS 23 test-qga /qga/guest-get-host-name
PASS 24 test-qga /qga/guest-get-timezone
PASS 25 test-qga /qga/guest-get-users
==8941==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-timed-average -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-timed-average" 
PASS 1 test-timed-average /timed-average/average
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-util-filemonitor -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-util-filemonitor" 
==8941==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefe5c2000; bottom 0x7f2099bfe000; size: 0x00de649c4000 (955170701312)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-util-filemonitor /util/filemonitor
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-simple -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-simple" 
PASS 1 test-authz-simple /authz/simple
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-list" 
==8958==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-authz-list /auth/list/complex
PASS 2 test-authz-list /auth/list/add-remove
PASS 3 test-authz-list /auth/list/default/deny
---
PASS 5 test-authz-list /auth/list/explicit/deny
PASS 6 test-authz-list /auth/list/explicit/allow
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-listfile -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-listfile" 
==8958==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc17513000; bottom 0x7f3b7bb7c000; size: 0x00c09b997000 (827244244992)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-authz-listfile /auth/list/complex
---
PASS 4 test-io-channel-file /io/channel/pipe/sync
PASS 5 test-io-channel-file /io/channel/pipe/async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-tls -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-tls" 
==9025==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9025==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc54176000; bottom 0x7fdfd8d7c000; size: 0x001c7b3fa000 (122326851584)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==9043==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-io-channel-tls /qio/channel/tls/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-command -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-command" 
==9043==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcb53be000; bottom 0x7f86a8dfe000; size: 0x00760c5c0000 (507013496832)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-io-channel-command /io/channel/command/fifo/sync
---
PASS 17 test-crypto-pbkdf /crypto/pbkdf/nonrfc/sha384/iter1200
PASS 18 test-crypto-pbkdf /crypto/pbkdf/nonrfc/ripemd160/iter1200
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-ivgen -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-ivgen" 
==9061==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-ivgen /crypto/ivgen/plain/1
PASS 2 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c
PASS 3 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c5b6a7988
---
PASS 17 test-crypto-xts /crypto/xts/t-21-key-32-ptx-31/basic
PASS 18 test-crypto-xts /crypto/xts/t-21-key-32-ptx-31/unaligned
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-block -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-block" 
==9082==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-block /crypto/block/qcow
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-logging -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-logging" 
PASS 1 test-logging /logging/parse_range
---
PASS 4 test-logging /logging/logfile_lock_path
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-replication -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-replication" 
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
==9101==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9103==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-replication /replication/primary/read
PASS 2 test-replication /replication/primary/write
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==9111==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-replication /replication/primary/start
PASS 4 test-replication /replication/primary/stop
PASS 5 test-replication /replication/primary/do_checkpoint
PASS 6 test-replication /replication/primary/get_error_all
PASS 33 ahci-test /x86_64/ahci/io/dma/lba28/fragmented
==9117==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 test-replication /replication/secondary/read
PASS 34 ahci-test /x86_64/ahci/io/dma/lba28/retry
==9123==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 test-replication /replication/secondary/write
PASS 35 ahci-test /x86_64/ahci/io/dma/lba28/simple/zero
==9129==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
==9135==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
==9101==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd103dc000; bottom 0x7f8b5e4c8000; size: 0x0071b1f14000 (488316682240)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
==9165==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-replication /replication/secondary/start
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
==9171==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
==9177==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
PASS 10 test-replication /replication/secondary/stop
==9183==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9183==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff81f54000; bottom 0x7f89ff3fd000; size: 0x007582b57000 (504704102400)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
==9190==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9190==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc562b6000; bottom 0x7f73b9d23000; size: 0x00889c593000 (586738642944)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
==9197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9197==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd86101000; bottom 0x7f678157b000; size: 0x009604b86000 (644324286464)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 test-replication /replication/secondary/continuous_replication
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
==9204==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
==9210==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
==9216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-replication /replication/secondary/do_checkpoint
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
==9222==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 test-replication /replication/secondary/get_error_all
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bufferiszero -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bufferiszero" 
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
==9231==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
==9237==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
==9243==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
==9249==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
==9255==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
==9261==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9261==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffef4f72000; bottom 0x7fa48f9fd000; size: 0x005a65575000 (388247277568)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero
==9268==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9268==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff08fba000; bottom 0x7f231977b000; size: 0x00dbef83f000 (944616239104)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 54 ahci-test /x86_64/ahci/io/dma/lba48/long/low
==9275==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9275==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff06efc000; bottom 0x7f8eac77b000; size: 0x00705a781000 (482554155008)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high
==9282==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 56 ahci-test /x86_64/ahci/io/dma/lba48/short/zero
==9288==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 57 ahci-test /x86_64/ahci/io/dma/lba48/short/low
==9294==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
==9300==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
==9306==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 60 ahci-test /x86_64/ahci/io/ncq/retry
==9312==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 61 ahci-test /x86_64/ahci/flush/simple
==9318==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
==9324==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9330==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 63 ahci-test /x86_64/ahci/flush/migrate
==9338==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9344==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==9352==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9358==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==9366==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9372==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bufferiszero /cutils/bufferiszero
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-uuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-uuid" 
PASS 1 test-uuid /uuid/is_null
---
PASS 21 test-qgraph /qgraph/test_two_test_same_interface
PASS 22 test-qgraph /qgraph/test_test_in_path
PASS 23 test-qgraph /qgraph/test_double_edge
==9390==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9399==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==9407==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9413==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==9421==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==9426==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==9432==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==9438==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==9444==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9444==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc51ac5000; bottom 0x7fa285dcc000; size: 0x0059cbcf9000 (385671467008)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==9450==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" 
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==9464==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==9470==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==9476==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==9482==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==9488==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
==9494==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba
==9500==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==9506==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==9511==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
==9517==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9521==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9525==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9529==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9533==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9537==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9541==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9545==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9548==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 hd-geo-test /x86_64/hd-geo/override/ide
==9555==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9559==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9563==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9567==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9571==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9575==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9579==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9583==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9586==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 hd-geo-test /x86_64/hd-geo/override/scsi
==9593==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9597==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9601==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9605==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9609==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9613==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9617==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9621==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9624==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 hd-geo-test /x86_64/hd-geo/override/scsi_2_controllers
==9631==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9635==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9639==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9643==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9646==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 hd-geo-test /x86_64/hd-geo/override/virtio_blk
==9653==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9657==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9660==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 hd-geo-test /x86_64/hd-geo/override/zero_chs
==9667==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9671==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9675==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9679==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9682==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 16 hd-geo-test /x86_64/hd-geo/override/scsi_hot_unplug
==9689==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9693==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9697==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9701==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9704==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 17 hd-geo-test /x86_64/hd-geo/override/virtio_hot_unplug
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" 
PASS 1 boot-order-test /x86_64/boot-order/pc
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9773==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP'
Using expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9779==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP'
Using expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9785==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.bridge'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9791==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.ipmikcs'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9797==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.cphp'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9804==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.memhp'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9810==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.numamem'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9816==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.dimmpxm'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9825==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/pc/FACP.acpihmat'
Looking for expected file 'tests/data/acpi/pc/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9832==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.bridge'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9838==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.mmio64'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9844==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.ipmibt'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9850==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.cphp'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9857==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.memhp'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9863==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.numamem'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9869==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.dimmpxm'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
qemu-system-x86_64: falling back to tcg
==9878==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

Looking for expected file 'tests/data/acpi/q35/FACP.acpihmat'
Looking for expected file 'tests/data/acpi/q35/FACP'
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==9970==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" 
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qtest/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" 
PASS 1 drive_del-test /x86_64/drive_del/without-dev
**
ERROR:/tmp/qemu-test/src/tests/qtest/drive_del-test.c:25:drive_add: assertion failed (resp == "OK\r\n"): ("Duplicate ID 'drive0' for drive\r\n" == "OK\r\n")
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/drive_del-test.c:25:drive_add: assertion failed (resp == "OK\r\n"): ("Duplicate ID 'drive0' for drive\r\n" == "OK\r\n")
make: *** [/tmp/qemu-test/src/tests/Makefile.include:636: check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4107873e2dbe4110b794b0faef4efa39', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-kj0i4fue/src/docker-src.2020-04-16-17.04.53.12420:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4107873e2dbe4110b794b0faef4efa39
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-kj0i4fue/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    30m5.527s
user    0m9.442s


The full log is available at
http://patchew.org/logs/20200416203624.32366-1-mlevitsk@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
                   ` (5 preceding siblings ...)
  2020-04-16 21:35 ` no-reply
@ 2020-04-16 21:47 ` no-reply
  2020-05-04 10:59 ` Stefan Hajnoczi
  7 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2020-04-16 21:47 UTC (permalink / raw)
  To: mlevitsk; +Cc: fam, berrange, ehabkost, mst, qemu-devel, mlevitsk, pbonzini

Patchew URL: https://patchew.org/QEMU/20200416203624.32366-1-mlevitsk@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TEST    check-qtest-x86_64: tests/qtest/device-plug-test
  TEST    check-qtest-x86_64: tests/qtest/drive_del-test
**
ERROR:/tmp/qemu-test/src/tests/qtest/drive_del-test.c:25:drive_add: assertion failed (resp == "OK\r\n"): ("Duplicate ID 'drive0' for drive\r\n" == "OK\r\n")
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/drive_del-test.c:25:drive_add: assertion failed (resp == "OK\r\n"): ("Duplicate ID 'drive0' for drive\r\n" == "OK\r\n")
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 040
qemu-system-aarch64: -accel kvm: invalid accelerator kvm
---
  TEST    check-qtest-aarch64: tests/qtest/cdrom-test
  TEST    check-qtest-aarch64: tests/qtest/device-introspect-test
**
ERROR:/tmp/qemu-test/src/qom/object.c:1124:object_unref: assertion failed: (obj->ref > 0)
Broken pipe
/tmp/qemu-test/src/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
ERROR - too few tests run (expected 6, got 5)
make: *** [check-qtest-aarch64] Error 1
  TEST    iotest-qcow2: 154
  TEST    iotest-qcow2: 156
  TEST    iotest-qcow2: 158
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f01c53d3dbfc4338a717907bbf724fe3', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-1p2vlvi4/src/docker-src.2020-04-16-17.33.43.25485:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=f01c53d3dbfc4338a717907bbf724fe3
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-1p2vlvi4/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    14m7.554s
user    0m9.291s


The full log is available at
http://patchew.org/logs/20200416203624.32366-1-mlevitsk@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PATCH 2/4] device-core: use RCU for list of childs of a bus
  2020-04-16 20:36 ` [PATCH 2/4] device-core: use RCU for list of childs of a bus Maxim Levitsky
@ 2020-05-04 10:41   ` Stefan Hajnoczi
  2020-05-11  8:48     ` Maxim Levitsky
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2020-05-04 10:41 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 4058 bytes --]

On Thu, Apr 16, 2020 at 11:36:22PM +0300, Maxim Levitsky wrote:
> @@ -90,9 +92,13 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
>      BusState *bus = BUS(obj);
>      BusChild *kid;
>  
> -    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> +    rcu_read_lock();
> +
> +    QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
>          cb(OBJECT(kid->child), opaque, type);
>      }
> +
> +    rcu_read_unlock();
>  }
>  
>  static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
> @@ -138,10 +144,15 @@ static void bus_unparent(Object *obj)
>      /* Only the main system bus has no parent, and that bus is never freed */
>      assert(bus->parent);
>  
> -    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> +    rcu_read_lock();
> +
> +    while ((kid = QTAILQ_FIRST_RCU(&bus->children)) != NULL) {
>          DeviceState *dev = kid->child;
>          object_unparent(OBJECT(dev));
>      }
> +
> +    rcu_read_unlock();

rcu_read_lock() is called but this looks like a list write operation.
If I understand correctly bus->children list writes can only be called
with the QEMU global mutex and therefore rcu_read_lock() is not required
here?

> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 85f062def7..f0c87e582e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -50,26 +50,37 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
>      return dc->vmsd;
>  }
>  
> +static void bus_free_bus_child(BusChild *kid)
> +{
> +    object_unref(OBJECT(kid->child));

Users like scsi_device_find() do not take a refcount on the child.  If
the device is removed then bus_free_bus_child may call object_unref()
while another thread is still accessing the child.

Maybe I'm missing something that prevents this scenario?

If not, then another patch is necessary first that introduces stricter
refcount discipline across the codebase. This applies both to users who
directly access bus->children as well as to those who call walk() and
stash child pointers in their callback function.

> +    g_free(kid);
> +}
> +
>  static void bus_remove_child(BusState *bus, DeviceState *child)
>  {
>      BusChild *kid;
>  
> -    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> +    rcu_read_lock();

List write under rcu_read_lock().

> @@ -82,7 +93,9 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>      kid->child = child;
>      object_ref(OBJECT(kid->child));
>  
> -    QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
> +    rcu_read_lock();
> +    QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
> +    rcu_read_unlock();

List write under rcu_read_lock().

> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 472bbd233b..b0f4a35f81 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
>      case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
>          target = req->req.tmf.lun[1];
>          s->resetting++;
> -        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> +
> +        rcu_read_lock();
> +        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {

We need a QTAILQ_FOREACH_WITH_RCU_READ_LOCK() macro that combines
WITH_RCU_READ_LOCK() and QTAILQ_FOREACH_RCU(). :-)

> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 38c9399cd4..58733f28e2 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -128,8 +128,11 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
>  static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
>  {
>      BusState *qbus = &bus->parent_obj;
> -    BusChild *kid = QTAILQ_FIRST(&qbus->children);
> -    DeviceState *qdev = kid ? kid->child : NULL;
> +    BusChild *kid;
> +    DeviceState *qdev;
> +
> +    kid = QTAILQ_FIRST(&qbus->children);

QTAILQ_FIRST_RCU()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-04-16 20:36 ` [PATCH 3/4] device-core: use atomic_set on .realized property Maxim Levitsky
@ 2020-05-04 10:45   ` Stefan Hajnoczi
  2020-05-04 11:22     ` Paolo Bonzini
  0 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2020-05-04 10:45 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

On Thu, Apr 16, 2020 at 11:36:23PM +0300, Maxim Levitsky wrote:
> Some code might race with placement of new devices on a bus.
> We currently first place a (unrealized) device on the bus
> and then realize it.
> 
> As a workaround, users that scan the child device list, can
> check the realized property to see if it is safe to access such a device.
> Use an atomic write here too to aid with this.
> 
> A separate discussion is what to do with devices that are unrealized:
> It looks like for this case we only call the hotplug handler's unplug
> callback and its up to it to unrealize the device.
> An atomic operation doesn't cause harm for this code path though.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  hw/core/qdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index f0c87e582e..bbb1ae3eb3 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
>      }
>  
>      assert(local_err == NULL);
> -    dev->realized = value;
> +    atomic_set(&dev->realized, value);

A memory barrier is probably needed so that the atomic_read() thread
sees up-to-date dev fields.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
                   ` (6 preceding siblings ...)
  2020-04-16 21:47 ` no-reply
@ 2020-05-04 10:59 ` Stefan Hajnoczi
  2020-05-04 11:38   ` Paolo Bonzini
  7 siblings, 1 reply; 21+ messages in thread
From: Stefan Hajnoczi @ 2020-05-04 10:59 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 3202 bytes --]

On Thu, Apr 16, 2020 at 11:36:20PM +0300, Maxim Levitsky wrote:
> Hi!
> 
> This is a patch series that is a result of my discussion with Paulo on
> how to correctly fix the root cause of the BZ #1812399.
> 
> The root cause of this bug is the fact that IO thread is running mostly
> unlocked versus main thread on which device hotplug is done.
> 
> qdev_device_add first creates the device object, then places it on the bus,
> and only then realizes it.
> 
> However some drivers and currently only virtio-scsi enumerate its child bus
> devices on each request that is received from the guest and that can happen on the IO
> thread.
> 
> Thus we have a window when new device is on the bus but not realized and can be accessed
> by the virtio-scsi driver in that state.
> 
> Fix that by doing two things:
> 
> 1. Add partial RCU protection to the list of a bus's child devices.
> This allows the scsi IO thread to safely enumerate the child devices
> while it races with the hotplug placing the device on the bus.
> 
> 2. Make the virtio-scsi driver check .realized property of the scsi device
> and avoid touching the device if it isn't
> 
> I don't think that this is very pretty way to solve this, we discussed this
> with Paulo and it kind of looks like the lesser evil. I am open to your thoughts about this.
> 
> Note that this patch series doesn't pass some unit tests and in particular qtest 'drive_del-test'
> I did some light debug of this test and I see that the reason for this is that now child device deletion
> can be delayed due to RCU. This is also something I would like to discuss in this RFC.
> 
> Note also that I might have some code style errors and bugs in this since I haven't
> tested the code in depth yet, because I am not yet sure that this is the right way
> to fix that bug
> 
> Also note that in the particular bug report the issue wasn't a race but rather due
> to combination of things, the .realize code in the middle managed to trigger IO on the virtqueue
> which caused the virtio-scsi driver to access the half realized device. However
> since this can happen as well with real IO thread, this patch series was done,
> which fixes this as well.

Nice approach! Thanks for solving this at the hw/core/bus.c level so
that other devices can benefit too.

Regarding drive_del, I guess the issue here is that this HMP command's
semantics need to include not synchronize_rcu() but some kind of
drain_call_rcu() operation as well that ensures deletion has completed?

This also suggests that all code that removes bus children needs to be
audited since the actual object unref is now deferred to a later point
in time. There could be other cases besides drive_del that don't yet
handle this.

drain_call_rcu() can be implemented by invoking call_rcu(temp,
drain_call_rcu_cb, rcu) where drain_call_rcu_cb() sets a QemuEvent that
the caller is waiting on. This way the caller can be sure that all
previously queued call_rcu() callbacks have completed. call_rcu_thread()
needs to be tweaked to avoid g_usleep() and instead use a timed wait so
that drain_call_rcu() can immediately wake up the thread.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-05-04 10:45   ` Stefan Hajnoczi
@ 2020-05-04 11:22     ` Paolo Bonzini
  2020-05-04 11:36       ` Maxim Levitsky
  2020-05-11 11:00       ` Maxim Levitsky
  0 siblings, 2 replies; 21+ messages in thread
From: Paolo Bonzini @ 2020-05-04 11:22 UTC (permalink / raw)
  To: Stefan Hajnoczi, Maxim Levitsky
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost


[-- Attachment #1.1: Type: text/plain, Size: 500 bytes --]

On 04/05/20 12:45, Stefan Hajnoczi wrote:
>> @@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
>>      }
>>  
>>      assert(local_err == NULL);
>> -    dev->realized = value;
>> +    atomic_set(&dev->realized, value);
> A memory barrier is probably needed so that the atomic_read() thread
> sees up-to-date dev fields.

Yes, it should be a store-release for the false->true case.  The
true->false case probably doesn't matter as much.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized
  2020-04-16 20:36 ` [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized Maxim Levitsky
@ 2020-05-04 11:24   ` Paolo Bonzini
  2020-05-11 11:21     ` Maxim Levitsky
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2020-05-04 11:24 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé, Eduardo Habkost, Michael S. Tsirkin

On 16/04/20 22:36, Maxim Levitsky wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  hw/scsi/virtio-scsi.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index b0f4a35f81..e360b4e03e 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun)
>  
>  static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
>  {
> +    SCSIDevice *device = NULL;
> +
>      if (lun[0] != 1) {
>          return NULL;
>      }
>      if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) {
>          return NULL;
>      }
> -    return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
> +
> +    device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
> +
> +    /* This function might run on the IO thread and we might race against
> +     * main thread hot-plugging the device.
> +     *
> +     * We assume that as soon as .realized is set to true we can let
> +     * the user access the device.
> +     */

Likewise this needs to be load-acquire.

Paolo

> +    if (!device || !atomic_read(&device->qdev.realized)) {
> +        return NULL;
> +    }
> +
> +    return device;
>  }
>  
>  void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
> 



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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-05-04 11:22     ` Paolo Bonzini
@ 2020-05-04 11:36       ` Maxim Levitsky
  2020-05-11 11:00       ` Maxim Levitsky
  1 sibling, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-04 11:36 UTC (permalink / raw)
  To: Paolo Bonzini, Stefan Hajnoczi
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost

On Mon, 2020-05-04 at 13:22 +0200, Paolo Bonzini wrote:
> On 04/05/20 12:45, Stefan Hajnoczi wrote:
> > > @@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> > >      }
> > >  
> > >      assert(local_err == NULL);
> > > -    dev->realized = value;
> > > +    atomic_set(&dev->realized, value);
> > 
> > A memory barrier is probably needed so that the atomic_read() thread
> > sees up-to-date dev fields.
> 
> Yes, it should be a store-release for the false->true case.  The
> true->false case probably doesn't matter as much.
> 
> Paolo
> 
I was under impression that atomic_set implies a barrier, but now indeed it looks like it doesn't.
I''l read upon this a bit and then send an updated patch.


For RCU, sorry for not knowing the details yet, I was under impression that for reads you need the rcu read lock
and for writes you also need the RCU read lock, since I first would read then write the data, 
plus follow the RCU rule of the update (read, copy, update),
with an atomic swap of a pointer to point to the new copy, and finally register a callback with RCU so it frees the old
copy when all the readers of the old copy are guaranteed to be gone.

Best regards,
	Maxim Levitsky



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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-05-04 10:59 ` Stefan Hajnoczi
@ 2020-05-04 11:38   ` Paolo Bonzini
  2020-05-04 11:43     ` Maxim Levitsky
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2020-05-04 11:38 UTC (permalink / raw)
  To: Stefan Hajnoczi, Maxim Levitsky
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost


[-- Attachment #1.1: Type: text/plain, Size: 1364 bytes --]

On 04/05/20 12:59, Stefan Hajnoczi wrote:
> Regarding drive_del, I guess the issue here is that this HMP command's
> semantics need to include not synchronize_rcu() but some kind of
> drain_call_rcu() operation as well that ensures deletion has completed?

Good idea, this would be Linux's rcu_barrier().

It would be a pity though that we have to do this instead of just having
the test rely on the DEVICE_DELETED event.

> drain_call_rcu() can be implemented by invoking call_rcu(temp,
> drain_call_rcu_cb, rcu) where drain_call_rcu_cb() sets a QemuEvent that
> the caller is waiting on. This way the caller can be sure that all
> previously queued call_rcu() callbacks have completed. call_rcu_thread()
> needs to be tweaked to avoid g_usleep() and instead use a timed wait so
> that drain_call_rcu() can immediately wake up the thread.

This was actually intentional in order to let some RCU callbacks pile up
(based on the observation, or the hope, that RCU data structures are
written rarely).  But the overall delay would be 50 ms so I don't think
it's a big deal to keep the unconditional sleep. The synchronize_rcu()
call could be on the order of 50 ms if --enable-membarrier is in use.

Another thing to care about is that call_rcu needs the iothread lock, so
you need to release it around the qemu_event_wait() call.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread
  2020-05-04 11:38   ` Paolo Bonzini
@ 2020-05-04 11:43     ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-04 11:43 UTC (permalink / raw)
  To: Paolo Bonzini, Stefan Hajnoczi
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost

On Mon, 2020-05-04 at 13:38 +0200, Paolo Bonzini wrote:
> On 04/05/20 12:59, Stefan Hajnoczi wrote:
> > Regarding drive_del, I guess the issue here is that this HMP command's
> > semantics need to include not synchronize_rcu() but some kind of
> > drain_call_rcu() operation as well that ensures deletion has completed?
> 
> Good idea, this would be Linux's rcu_barrier().
> 
> It would be a pity though that we have to do this instead of just having
> the test rely on the DEVICE_DELETED event.
> 
> > drain_call_rcu() can be implemented by invoking call_rcu(temp,
> > drain_call_rcu_cb, rcu) where drain_call_rcu_cb() sets a QemuEvent that
> > the caller is waiting on. This way the caller can be sure that all
> > previously queued call_rcu() callbacks have completed. call_rcu_thread()
> > needs to be tweaked to avoid g_usleep() and instead use a timed wait so
> > that drain_call_rcu() can immediately wake up the thread.
> 
> This was actually intentional in order to let some RCU callbacks pile up
> (based on the observation, or the hope, that RCU data structures are
> written rarely).  But the overall delay would be 50 ms so I don't think
> it's a big deal to keep the unconditional sleep. The synchronize_rcu()
> call could be on the order of 50 ms if --enable-membarrier is in use.
> 
> Another thing to care about is that call_rcu needs the iothread lock, so
> you need to release it around the qemu_event_wait() call.
> 
> Paolo
> 
Thank a lot for the suggestions!
I'll try to implement this in the next version of these patches.

Best regards,
	Maxim Levitsky



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

* Re: [PATCH 2/4] device-core: use RCU for list of childs of a bus
  2020-05-04 10:41   ` Stefan Hajnoczi
@ 2020-05-11  8:48     ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-11  8:48 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Daniel P. Berrangé,
	Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini

On Mon, 2020-05-04 at 11:41 +0100, Stefan Hajnoczi wrote:
> On Thu, Apr 16, 2020 at 11:36:22PM +0300, Maxim Levitsky wrote:
> > @@ -90,9 +92,13 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
> >      BusState *bus = BUS(obj);
> >      BusChild *kid;
> >  
> > -    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> > +    rcu_read_lock();
> > +
> > +    QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
> >          cb(OBJECT(kid->child), opaque, type);
> >      }
> > +
> > +    rcu_read_unlock();
> >  }
> >  
> >  static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
> > @@ -138,10 +144,15 @@ static void bus_unparent(Object *obj)
> >      /* Only the main system bus has no parent, and that bus is never freed */
> >      assert(bus->parent);
> >  
> > -    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> > +    rcu_read_lock();
> > +
> > +    while ((kid = QTAILQ_FIRST_RCU(&bus->children)) != NULL) {
> >          DeviceState *dev = kid->child;
> >          object_unparent(OBJECT(dev));
> >      }
> > +
> > +    rcu_read_unlock();
> 
> rcu_read_lock() is called but this looks like a list write operation.
> If I understand correctly bus->children list writes can only be called
> with the QEMU global mutex and therefore rcu_read_lock() is not required
> here?

This is indeed write operation. Paulo helped me to finally understand
what RCU guarantees are, so now I understand.

About write locking here (which I also understand now that I need for RCU),
this is very good question if that can race as well:

It looks like qdev_unplug is what does the device removal, and it first
calls the hotplug handler which is supposed to unrealize the device,
in addition to whatever hot unplug actions are needed (like informing the guest),
and then it does object_unparent which removes the device from the bus.
Therefore as long as the .realized store is atomic and with proper barriers,
the scsi IO thread might be able to see the device but it will be unplugged by then.


There are plenty of object_unparent calls through the code base and I can only hope
that these are done with big qemu lock held.


> 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index 85f062def7..f0c87e582e 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -50,26 +50,37 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> >      return dc->vmsd;
> >  }
> >  
> > +static void bus_free_bus_child(BusChild *kid)
> > +{
> > +    object_unref(OBJECT(kid->child));
> 
> Users like scsi_device_find() do not take a refcount on the child.  If
> the device is removed then bus_free_bus_child may call object_unref()
> while another thread is still accessing the child.

I agree, however this is existing bug - bus_remove_child was dropping this
reference immediatly and I delayed it to RCU callback it now sets.
So I didn't made the situation worse.


> 
> Maybe I'm missing something that prevents this scenario?
> 
> If not, then another patch is necessary first that introduces stricter
> refcount discipline across the codebase. This applies both to users who
> directly access bus->children as well as to those who call walk() and
> stash child pointers in their callback function.

In scsi_device_find, as long as RCU read lock is held, no RCU reclaim should happen,
thus this code shouldn't have the child disapper under it.
However once scsi_device_find returns, indeed this can happen,

so scsi_device_find should indeed take a reference to the scsi device and the caller should
drop it when not needed. That should be done in a separate patch, and it
might open yet another can of worms.
While at it, it should be renamed scsi_device_get()
Maybe keep both scsi_device_find and scsi_device_get(), and let the legacy drivers
continue using the former one, while make virtio-scsi use the later? 


> 
> > +    g_free(kid);
> > +}
> > +
> >  static void bus_remove_child(BusState *bus, DeviceState *child)
> >  {
> >      BusChild *kid;
> >  
> > -    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> > +    rcu_read_lock();
> 
> List write under rcu_read_lock().
I removed the RCU read lock here, under assumption that
bus_remove_child will be called with BQL held.
I kept the _RCU version of the list removal, under assumption that
writer still needs it to avoid race vs readers.


> 
> > @@ -82,7 +93,9 @@ static void bus_add_child(BusState *bus, DeviceState *child)
> >      kid->child = child;
> >      object_ref(OBJECT(kid->child));
> >  
> > -    QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
> > +    rcu_read_lock();
> > +    QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
> > +    rcu_read_unlock();
> 
> List write under rcu_read_lock().
Same as above.

> 
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index 472bbd233b..b0f4a35f81 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> >      case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
> >          target = req->req.tmf.lun[1];
> >          s->resetting++;
> > -        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> > +
> > +        rcu_read_lock();
> > +        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
> 
> We need a QTAILQ_FOREACH_WITH_RCU_READ_LOCK() macro that combines
> WITH_RCU_READ_LOCK() and QTAILQ_FOREACH_RCU(). :-)
Assuming that you are not joking here, I'll add this in the new version of the patches.

> 
> > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> > index 38c9399cd4..58733f28e2 100644
> > --- a/include/hw/virtio/virtio-bus.h
> > +++ b/include/hw/virtio/virtio-bus.h
> > @@ -128,8 +128,11 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
> >  static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
> >  {
> >      BusState *qbus = &bus->parent_obj;
> > -    BusChild *kid = QTAILQ_FIRST(&qbus->children);
> > -    DeviceState *qdev = kid ? kid->child : NULL;
> > +    BusChild *kid;
> > +    DeviceState *qdev;
> > +
> > +    kid = QTAILQ_FIRST(&qbus->children);
> 
> QTAILQ_FIRST_RCU()

This is on purpose - I didn't convert to RCU most of the drivers
which don't have this race versus iothread after a discussion with Paulo,
and this is one of them. Virtio bus has only one device
and it is added right away on initialization and vise-versa of the parent
(aka virtio-pci/virtio-mmio) bus device.

I will revert the other cosmetic changes in this function which I did when
I wasn't sure if to use _RCU version here.

Best regards,
	Maxim Levitsky








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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-05-04 11:22     ` Paolo Bonzini
  2020-05-04 11:36       ` Maxim Levitsky
@ 2020-05-11 11:00       ` Maxim Levitsky
  2020-05-11 11:11         ` Paolo Bonzini
  1 sibling, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-11 11:00 UTC (permalink / raw)
  To: Paolo Bonzini, Stefan Hajnoczi
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost

On Mon, 2020-05-04 at 13:22 +0200, Paolo Bonzini wrote:
> On 04/05/20 12:45, Stefan Hajnoczi wrote:
> > > @@ -983,7 +983,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
> > >      }
> > >  
> > >      assert(local_err == NULL);
> > > -    dev->realized = value;
> > > +    atomic_set(&dev->realized, value);
> > 
> > A memory barrier is probably needed so that the atomic_read() thread
> > sees up-to-date dev fields.
> 
> Yes, it should be a store-release for the false->true case.  The
> true->false case probably doesn't matter as much.

On second thought, I think both cases matter, after I examined the device removal case.
In device removal case, the device is first un-realized and then removed from the bus,
so just like in device hotplug case, the scsi_device_find can give you an unrealized device.

I will change this patch to set .realized to false at the start (if needed) of the function and to true at the end (also if needed)
Will atomic_rcu_set work? or atomic_store_release?
(Both are the same thing, but former documents the purpose of using with RCU.

Best regards,
	Maxim Levitsky



> 
> Paolo
> 




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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-05-11 11:00       ` Maxim Levitsky
@ 2020-05-11 11:11         ` Paolo Bonzini
  2020-05-11 11:14           ` Maxim Levitsky
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2020-05-11 11:11 UTC (permalink / raw)
  To: Maxim Levitsky, Stefan Hajnoczi
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost

On 11/05/20 13:00, Maxim Levitsky wrote:
> On second thought, I think both cases matter, after I examined the device removal case.
> In device removal case, the device is first un-realized and then removed from the bus,
> so just like in device hotplug case, the scsi_device_find can give you an unrealized device.
> 
> I will change this patch to set .realized to false at the start (if needed) of the function and to true at the end (also if needed)
> Will atomic_rcu_set work? or atomic_store_release?
> (Both are the same thing, but former documents the purpose of using with RCU.

atomic_rcu_set is more to store pointers, in this case you want to store
the value after any other change to the struct so atomic_store_release
is more appropriate.

Paolo



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

* Re: [PATCH 3/4] device-core: use atomic_set on .realized property
  2020-05-11 11:11         ` Paolo Bonzini
@ 2020-05-11 11:14           ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-11 11:14 UTC (permalink / raw)
  To: Paolo Bonzini, Stefan Hajnoczi
  Cc: Fam Zheng, Michael S. Tsirkin, Daniel P. Berrangé,
	qemu-devel, Eduardo Habkost

On Mon, 2020-05-11 at 13:11 +0200, Paolo Bonzini wrote:
> On 11/05/20 13:00, Maxim Levitsky wrote:
> > On second thought, I think both cases matter, after I examined the device removal case.
> > In device removal case, the device is first un-realized and then removed from the bus,
> > so just like in device hotplug case, the scsi_device_find can give you an unrealized device.
> > 
> > I will change this patch to set .realized to false at the start (if needed) of the function and to true at the end (also if needed)
> > Will atomic_rcu_set work? or atomic_store_release?
> > (Both are the same thing, but former documents the purpose of using with RCU.
> 
> atomic_rcu_set is more to store pointers, in this case you want to store
> the value after any other change to the struct so atomic_store_release
> is more appropriate.
> 
> Paolo
> 
All right then.
Best regards,
	Maxim Levitsky



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

* Re: [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized
  2020-05-04 11:24   ` Paolo Bonzini
@ 2020-05-11 11:21     ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2020-05-11 11:21 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: Fam Zheng, Daniel P. Berrangé, Eduardo Habkost, Michael S. Tsirkin

On Mon, 2020-05-04 at 13:24 +0200, Paolo Bonzini wrote:
> On 16/04/20 22:36, Maxim Levitsky wrote:
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399
> > 
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  hw/scsi/virtio-scsi.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index b0f4a35f81..e360b4e03e 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun)
> >  
> >  static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
> >  {
> > +    SCSIDevice *device = NULL;
> > +
> >      if (lun[0] != 1) {
> >          return NULL;
> >      }
> >      if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) {
> >          return NULL;
> >      }
> > -    return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
> > +
> > +    device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
> > +
> > +    /* This function might run on the IO thread and we might race against
> > +     * main thread hot-plugging the device.
> > +     *
> > +     * We assume that as soon as .realized is set to true we can let
> > +     * the user access the device.
> > +     */
> 
> Likewise this needs to be load-acquire.

Done.

Best regards,
	Maxim Levitsky
> 
> Paolo
> 
> > +    if (!device || !atomic_read(&device->qdev.realized)) {
> > +        return NULL;
> > +    }
> > +
> > +    return device;
> >  }
> >  
> >  void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
> > 
> 
> 




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

end of thread, other threads:[~2020-05-11 11:38 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 20:36 [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
2020-04-16 20:36 ` [PATCH 1/4] scsi/scsi_bus: switch search direction in scsi_device_find Maxim Levitsky
2020-04-16 20:36 ` [PATCH 2/4] device-core: use RCU for list of childs of a bus Maxim Levitsky
2020-05-04 10:41   ` Stefan Hajnoczi
2020-05-11  8:48     ` Maxim Levitsky
2020-04-16 20:36 ` [PATCH 3/4] device-core: use atomic_set on .realized property Maxim Levitsky
2020-05-04 10:45   ` Stefan Hajnoczi
2020-05-04 11:22     ` Paolo Bonzini
2020-05-04 11:36       ` Maxim Levitsky
2020-05-11 11:00       ` Maxim Levitsky
2020-05-11 11:11         ` Paolo Bonzini
2020-05-11 11:14           ` Maxim Levitsky
2020-04-16 20:36 ` [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized Maxim Levitsky
2020-05-04 11:24   ` Paolo Bonzini
2020-05-11 11:21     ` Maxim Levitsky
2020-04-16 21:33 ` [PATCH 0/4] RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread no-reply
2020-04-16 21:35 ` no-reply
2020-04-16 21:47 ` no-reply
2020-05-04 10:59 ` Stefan Hajnoczi
2020-05-04 11:38   ` Paolo Bonzini
2020-05-04 11:43     ` Maxim Levitsky

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.