[PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu

[PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu

[Lukas Straub]

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

Hello Everyone,
In many cases, if qemu has a network connection (qmp, migration, chardev, etc.)
to some other server and that server dies or hangs, qemu hangs too.
These patches introduce the new 'yank' out-of-band qmp command to recover from
these kinds of hangs. The different subsystems register callbacks which get
executed with the yank command. For example the callback can shutdown() a
socket. This is intended for the colo use-case, but it can be used for other
things too of course.

Regards,
Lukas Straub

v2:
 -don't touch io/ code anymore
 -always register yank functions
 -'yank' now takes a list of instances to yank
 -'query-yank' returns a list of yankable instances

Lukas Straub (4):
  Introduce yank feature
  block/nbd.c: Add yank feature
  chardev/char-socket.c: Add yank feature
  migration: Add yank feature

 Makefile.objs                 |   1 +
 block/nbd.c                   | 101 ++++++++++++--------
 chardev/char-socket.c         |  24 +++++
 migration/migration.c         |   9 ++
 migration/qemu-file-channel.c |   6 ++
 migration/socket.c            |  11 +++
 qapi/misc.json                |  45 +++++++++
 softmmu/vl.c                  |   2 +
 yank.c                        | 174 ++++++++++++++++++++++++++++++++++
 yank.h                        |  69 ++++++++++++++
 10 files changed, 405 insertions(+), 37 deletions(-)
 create mode 100644 yank.c
 create mode 100644 yank.h

--
2.20.1

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

[PATCH v2 1/4] Introduce yank feature

[Lukas Straub]

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

The yank feature allows to recover from hanging qemu by "yanking"
at various parts. Other qemu systems can register themselves and
multiple yank functions. Then all yank functions for selected
instances can be called by the 'yank' out-of-band qmp command.
Available instances can be queried by a 'query-yank' oob command.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 qapi/misc.json |  45 +++++++++++++
 softmmu/vl.c   |   2 +
 yank.c         | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
 yank.h         |  69 ++++++++++++++++++++
 4 files changed, 290 insertions(+)
 create mode 100644 yank.c
 create mode 100644 yank.h

diff --git a/qapi/misc.json b/qapi/misc.json
index 99b90ac80b..f5228b2502 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1550,3 +1550,48 @@
 ##
 { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }

+##
+# @YankInstances:
+#
+# @instances: List of yank instances.
+#
+# Yank instances are named after the following schema:
+# "blockdev:<node-name>", "chardev:<chardev-name>" and "migration"
+#
+# Since: 5.1
+##
+{ 'struct': 'YankInstances', 'data': {'instances': ['str'] } }
+
+##
+# @yank:
+#
+# Recover from hanging qemu by yanking the specified instances.
+#
+# Takes @YankInstances as argument.
+#
+# Returns: nothing.
+#
+# Example:
+#
+# -> { "execute": "yank", "arguments": { "instances": ["blockdev:nbd0"] } }
+# <- { "return": {} }
+#
+# Since: 5.1
+##
+{ 'command': 'yank', 'data': 'YankInstances', 'allow-oob': true }
+
+##
+# @query-yank:
+#
+# Query yank instances.
+#
+# Returns: @YankInstances
+#
+# Example:
+#
+# -> { "execute": "query-yank" }
+# <- { "return": { "instances": ["blockdev:nbd0"] } }
+#
+# Since: 5.1
+##
+{ 'command': 'query-yank', 'returns': 'YankInstances', 'allow-oob': true }
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 32c0047889..5d99749d29 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -112,6 +112,7 @@
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
 #include "qemu/guest-random.h"
+#include "yank.h"

 #define MAX_VIRTIO_CONSOLES 1

@@ -2906,6 +2907,7 @@ void qemu_init(int argc, char **argv, char **envp)
     precopy_infrastructure_init();
     postcopy_infrastructure_init();
     monitor_init_globals();
+    yank_init();

     if (qcrypto_init(&err) < 0) {
         error_reportf_err(err, "cannot initialize crypto: ");
diff --git a/yank.c b/yank.c
new file mode 100644
index 0000000000..bfce19185e
--- /dev/null
+++ b/yank.c
@@ -0,0 +1,174 @@
+/*
+ * QEMU yank feature
+ *
+ * Copyright (c) Lukas Straub <lukasstraub2@web.de>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/thread.h"
+#include "qemu/queue.h"
+#include "qapi/qapi-commands-misc.h"
+#include "io/channel.h"
+#include "yank.h"
+
+struct YankFuncAndParam {
+    YankFn *func;
+    void *opaque;
+    QLIST_ENTRY(YankFuncAndParam) next;
+};
+
+struct YankInstance {
+    char *name;
+    QLIST_HEAD(, YankFuncAndParam) yankfns;
+    QLIST_ENTRY(YankInstance) next;
+};
+
+static QemuMutex lock;
+static QLIST_HEAD(yankinst_list, YankInstance) head
+    = QLIST_HEAD_INITIALIZER(head);
+
+static struct YankInstance *yank_find_instance(char *name)
+{
+    struct YankInstance *tmp, *instance;
+    instance = NULL;
+    QLIST_FOREACH(tmp, &head, next) {
+        if (!strcmp(tmp->name, name)) {
+            instance = tmp;
+        }
+    }
+    return instance;
+}
+
+void yank_register_instance(char *instance_name)
+{
+    struct YankInstance *instance;
+
+    qemu_mutex_lock(&lock);
+    assert(!yank_find_instance(instance_name));
+
+    instance = g_slice_new(struct YankInstance);
+    instance->name = g_strdup(instance_name);
+    QLIST_INIT(&instance->yankfns);
+    QLIST_INSERT_HEAD(&head, instance, next);
+
+    qemu_mutex_unlock(&lock);
+}
+
+void yank_unregister_instance(char *instance_name)
+{
+    struct YankInstance *instance;
+
+    qemu_mutex_lock(&lock);
+    instance = yank_find_instance(instance_name);
+    assert(instance);
+
+    assert(QLIST_EMPTY(&instance->yankfns));
+    QLIST_REMOVE(instance, next);
+    g_free(instance->name);
+    g_slice_free(struct YankInstance, instance);
+
+    qemu_mutex_unlock(&lock);
+}
+
+void yank_register_function(char *instance_name, YankFn *func, void *opaque)
+{
+    struct YankInstance *instance;
+    struct YankFuncAndParam *entry;
+
+    qemu_mutex_lock(&lock);
+    instance = yank_find_instance(instance_name);
+    assert(instance);
+
+    entry = g_slice_new(struct YankFuncAndParam);
+    entry->func = func;
+    entry->opaque = opaque;
+
+    QLIST_INSERT_HEAD(&instance->yankfns, entry, next);
+    qemu_mutex_unlock(&lock);
+}
+
+void yank_unregister_function(char *instance_name, YankFn *func, void *opaque)
+{
+    struct YankInstance *instance;
+    struct YankFuncAndParam *entry;
+
+    qemu_mutex_lock(&lock);
+    instance = yank_find_instance(instance_name);
+    assert(instance);
+
+    QLIST_FOREACH(entry, &instance->yankfns, next) {
+        if (entry->func == func && entry->opaque == opaque) {
+            QLIST_REMOVE(entry, next);
+            g_slice_free(struct YankFuncAndParam, entry);
+            qemu_mutex_unlock(&lock);
+            return;
+        }
+    }
+
+    abort();
+}
+
+void yank_generic_iochannel(void *opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+
+    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+
+void qmp_yank(strList *instances, Error **errp)
+{
+    strList *tmp;
+    struct YankInstance *instance;
+    struct YankFuncAndParam *entry;
+
+    qemu_mutex_lock(&lock);
+    tmp = instances;
+    for (; tmp; tmp = tmp->next) {
+        instance = yank_find_instance(tmp->value);
+        if (!instance) {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "Instance '%s' not found", tmp->value);
+            qemu_mutex_unlock(&lock);
+            return;
+        }
+    }
+    tmp = instances;
+    for (; tmp; tmp = tmp->next) {
+        instance = yank_find_instance(tmp->value);
+        assert(instance);
+        QLIST_FOREACH(entry, &instance->yankfns, next) {
+            entry->func(entry->opaque);
+        }
+    }
+    qemu_mutex_unlock(&lock);
+}
+
+YankInstances *qmp_query_yank(Error **errp)
+{
+    struct YankInstance *instance;
+    YankInstances *ret;
+
+    ret = g_new0(YankInstances, 1);
+    ret->instances = NULL;
+
+    qemu_mutex_lock(&lock);
+    QLIST_FOREACH(instance, &head, next) {
+        strList *entry;
+        entry = g_new0(strList, 1);
+        entry->value = g_strdup(instance->name);
+        entry->next = ret->instances;
+        ret->instances = entry;
+    }
+    qemu_mutex_unlock(&lock);
+
+    return ret;
+}
+
+void yank_init(void)
+{
+    qemu_mutex_init(&lock);
+}
diff --git a/yank.h b/yank.h
new file mode 100644
index 0000000000..f01b370999
--- /dev/null
+++ b/yank.h
@@ -0,0 +1,69 @@
+
+#ifndef YANK_H
+#define YANK_H
+
+typedef void (YankFn) (void *opaque);
+
+/**
+ * yank_register_instance: Register a new instance.
+ *
+ * This registers a new instance for yanking. Must be called before any yank
+ * function is registered for this instance.
+ *
+ * This function is thread-safe.
+ *
+ * @instance_name: The globally unique name of the instance.
+ */
+void yank_register_instance(char *instance_name);
+
+/**
+ * yank_unregister_instance: Unregister a instance.
+ *
+ * This unregisters a instance. Must be called only after every yank function
+ * of the instance has been unregistered.
+ *
+ * This function is thread-safe.
+ *
+ * @instance_name: The name of the instance.
+ */
+void yank_unregister_instance(char *instance_name);
+
+/**
+ * yank_register_function: Register a yank function
+ *
+ * This registers a yank function. All limitations of qmp oob commands apply
+ * to the yank function as well.
+ *
+ * This function is thread-safe.
+ *
+ * @instance_name: The name of the instance
+ * @func: The yank function
+ * @opaque: Will be passed to the yank function
+ */
+void yank_register_function(char *instance_name, YankFn *func, void *opaque);
+
+/**
+ * yank_unregister_function: Unregister a yank function
+ *
+ * This unregisters a yank function.
+ *
+ * This function is thread-safe.
+ *
+ * @instance_name: The name of the instance
+ * @func: func that was passed to yank_register_function
+ * @opaque: opaque that was passed to yank_register_function
+ */
+void yank_unregister_function(char *instance_name, YankFn *func, void *opaque);
+
+/**
+ * yank_unregister_function: Generic yank function for iochannel
+ *
+ * This is a generic yank function which will call qio_channel_shutdown on the
+ * provided QIOChannel.
+ *
+ * @opaque: QIOChannel to shutdown
+ */
+void yank_generic_iochannel(void *opaque);
+
+void yank_init(void);
+#endif
--
2.20.1


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

[PATCH v2 2/4] block/nbd.c: Add yank feature

[Lukas Straub]

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

Register a yank function which shuts down the socket and sets
s->state = NBD_CLIENT_QUIT. This is the same behaviour as if an
error occured.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 Makefile.objs |   1 +
 block/nbd.c   | 101 ++++++++++++++++++++++++++++++++------------------
 2 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index a7c967633a..8e403b81f3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -18,6 +18,7 @@ block-obj-y += block.o blockjob.o job.o
 block-obj-y += block/ scsi/
 block-obj-y += qemu-io-cmds.o
 block-obj-$(CONFIG_REPLICATION) += replication.o
+block-obj-y += yank.o

 block-obj-m = block/

diff --git a/block/nbd.c b/block/nbd.c
index 2160859f64..3a41749f1b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -35,6 +35,7 @@
 #include "qemu/option.h"
 #include "qemu/cutils.h"
 #include "qemu/main-loop.h"
+#include "qemu/atomic.h"

 #include "qapi/qapi-visit-sockets.h"
 #include "qapi/qmp/qstring.h"
@@ -43,6 +44,8 @@
 #include "block/nbd.h"
 #include "block/block_int.h"

+#include "yank.h"
+
 #define EN_OPTSTR ":exportname="
 #define MAX_NBD_REQUESTS    16

@@ -84,6 +87,8 @@ typedef struct BDRVNBDState {
     NBDReply reply;
     BlockDriverState *bs;

+    char *yank_name;
+
     /* Connection parameters */
     uint32_t reconnect_delay;
     SocketAddress *saddr;
@@ -94,6 +99,7 @@ typedef struct BDRVNBDState {
 } BDRVNBDState;

 static int nbd_client_connect(BlockDriverState *bs, Error **errp);
+static void nbd_yank(void *opaque);

 static void nbd_clear_bdrvstate(BDRVNBDState *s)
 {
@@ -106,17 +112,19 @@ static void nbd_clear_bdrvstate(BDRVNBDState *s)
     s->tlscredsid = NULL;
     g_free(s->x_dirty_bitmap);
     s->x_dirty_bitmap = NULL;
+    g_free(s->yank_name);
+    s->yank_name = NULL;
 }

 static void nbd_channel_error(BDRVNBDState *s, int ret)
 {
     if (ret == -EIO) {
-        if (s->state == NBD_CLIENT_CONNECTED) {
+        if (atomic_read(&s->state) == NBD_CLIENT_CONNECTED) {
             s->state = s->reconnect_delay ? NBD_CLIENT_CONNECTING_WAIT :
                                             NBD_CLIENT_CONNECTING_NOWAIT;
         }
     } else {
-        if (s->state == NBD_CLIENT_CONNECTED) {
+        if (atomic_read(&s->state) == NBD_CLIENT_CONNECTED) {
             qio_channel_shutdown(s->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
         }
         s->state = NBD_CLIENT_QUIT;
@@ -167,7 +175,7 @@ static void nbd_client_attach_aio_context(BlockDriverState *bs,
      * s->connection_co is either yielded from nbd_receive_reply or from
      * nbd_co_reconnect_loop()
      */
-    if (s->state == NBD_CLIENT_CONNECTED) {
+    if (atomic_read(&s->state) == NBD_CLIENT_CONNECTED) {
         qio_channel_attach_aio_context(QIO_CHANNEL(s->ioc), new_context);
     }

@@ -206,7 +214,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;

-    if (s->state == NBD_CLIENT_CONNECTED) {
+    if (atomic_read(&s->state) == NBD_CLIENT_CONNECTED) {
         /* finish any pending coroutines */
         assert(s->ioc);
         qio_channel_shutdown(s->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
@@ -230,13 +238,14 @@ static void nbd_teardown_connection(BlockDriverState *bs)

 static bool nbd_client_connecting(BDRVNBDState *s)
 {
-    return s->state == NBD_CLIENT_CONNECTING_WAIT ||
-        s->state == NBD_CLIENT_CONNECTING_NOWAIT;
+    NBDClientState state = atomic_read(&s->state);
+    return state == NBD_CLIENT_CONNECTING_WAIT ||
+        state == NBD_CLIENT_CONNECTING_NOWAIT;
 }

 static bool nbd_client_connecting_wait(BDRVNBDState *s)
 {
-    return s->state == NBD_CLIENT_CONNECTING_WAIT;
+    return atomic_read(&s->state) == NBD_CLIENT_CONNECTING_WAIT;
 }

 static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
@@ -274,6 +283,7 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
     /* Finalize previous connection if any */
     if (s->ioc) {
         nbd_client_detach_aio_context(s->bs);
+        yank_unregister_function(s->yank_name, nbd_yank, s->bs);
         object_unref(OBJECT(s->sioc));
         s->sioc = NULL;
         object_unref(OBJECT(s->ioc));
@@ -305,7 +315,7 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
     nbd_reconnect_attempt(s);

     while (nbd_client_connecting(s)) {
-        if (s->state == NBD_CLIENT_CONNECTING_WAIT &&
+        if (atomic_read(&s->state) == NBD_CLIENT_CONNECTING_WAIT &&
             qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start_time_ns > delay_ns)
         {
             s->state = NBD_CLIENT_CONNECTING_NOWAIT;
@@ -341,7 +351,7 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
     int ret = 0;
     Error *local_err = NULL;

-    while (s->state != NBD_CLIENT_QUIT) {
+    while (atomic_read(&s->state) != NBD_CLIENT_QUIT) {
         /*
          * The NBD client can only really be considered idle when it has
          * yielded from qio_channel_readv_all_eof(), waiting for data. This is
@@ -356,7 +366,7 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
             nbd_co_reconnect_loop(s);
         }

-        if (s->state != NBD_CLIENT_CONNECTED) {
+        if (atomic_read(&s->state) != NBD_CLIENT_CONNECTED) {
             continue;
         }

@@ -411,6 +421,7 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
     s->connection_co = NULL;
     if (s->ioc) {
         nbd_client_detach_aio_context(s->bs);
+        yank_unregister_function(s->yank_name, nbd_yank, s->bs);
         object_unref(OBJECT(s->sioc));
         s->sioc = NULL;
         object_unref(OBJECT(s->ioc));
@@ -435,7 +446,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
         qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
     }

-    if (s->state != NBD_CLIENT_CONNECTED) {
+    if (atomic_read(&s->state) != NBD_CLIENT_CONNECTED) {
         rc = -EIO;
         goto err;
     }
@@ -462,7 +473,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
     if (qiov) {
         qio_channel_set_cork(s->ioc, true);
         rc = nbd_send_request(s->ioc, request);
-        if (rc >= 0 && s->state == NBD_CLIENT_CONNECTED) {
+        if (rc >= 0 && atomic_read(&s->state) == NBD_CLIENT_CONNECTED) {
             if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
                                        NULL) < 0) {
                 rc = -EIO;
@@ -777,7 +788,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
     s->requests[i].receiving = true;
     qemu_coroutine_yield();
     s->requests[i].receiving = false;
-    if (s->state != NBD_CLIENT_CONNECTED) {
+    if (atomic_read(&s->state) != NBD_CLIENT_CONNECTED) {
         error_setg(errp, "Connection closed");
         return -EIO;
     }
@@ -936,7 +947,7 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
     NBDReply local_reply;
     NBDStructuredReplyChunk *chunk;
     Error *local_err = NULL;
-    if (s->state != NBD_CLIENT_CONNECTED) {
+    if (atomic_read(&s->state) != NBD_CLIENT_CONNECTED) {
         error_setg(&local_err, "Connection closed");
         nbd_iter_channel_error(iter, -EIO, &local_err);
         goto break_loop;
@@ -961,7 +972,8 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
     }

     /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
-    if (nbd_reply_is_simple(reply) || s->state != NBD_CLIENT_CONNECTED) {
+    if (nbd_reply_is_simple(reply) ||
+        atomic_read(&s->state) != NBD_CLIENT_CONNECTED) {
         goto break_loop;
     }

@@ -1395,6 +1407,15 @@ static int nbd_client_reopen_prepare(BDRVReopenState *state,
     return 0;
 }

+static void nbd_yank(void *opaque)
+{
+    BlockDriverState *bs = opaque;
+    BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+
+    qio_channel_shutdown(QIO_CHANNEL(s->sioc), QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+    atomic_set(&s->state, NBD_CLIENT_QUIT);
+}
+
 static void nbd_client_close(BlockDriverState *bs)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
@@ -1407,25 +1428,29 @@ static void nbd_client_close(BlockDriverState *bs)
     nbd_teardown_connection(bs);
 }

-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
-                                                  Error **errp)
+static int nbd_establish_connection(BlockDriverState *bs,
+                                    SocketAddress *saddr,
+                                    Error **errp)
 {
-    QIOChannelSocket *sioc;
+    BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
     Error *local_err = NULL;

-    sioc = qio_channel_socket_new();
-    qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
+    s->sioc = qio_channel_socket_new();
+    qio_channel_set_name(QIO_CHANNEL(s->sioc), "nbd-client");
+    yank_register_function(s->yank_name, nbd_yank, bs);

-    qio_channel_socket_connect_sync(sioc, saddr, &local_err);
+    qio_channel_socket_connect_sync(s->sioc, saddr, &local_err);
     if (local_err) {
-        object_unref(OBJECT(sioc));
+        yank_unregister_function(s->yank_name, nbd_yank, bs);
+        object_unref(OBJECT(s->sioc));
+        s->sioc = NULL;
         error_propagate(errp, local_err);
-        return NULL;
+        return -1;
     }

-    qio_channel_set_delay(QIO_CHANNEL(sioc), false);
+    qio_channel_set_delay(QIO_CHANNEL(s->sioc), false);

-    return sioc;
+    return 0;
 }

 static int nbd_client_connect(BlockDriverState *bs, Error **errp)
@@ -1438,28 +1463,27 @@ static int nbd_client_connect(BlockDriverState *bs, Error **errp)
      * establish TCP connection, return error if it fails
      * TODO: Configurable retry-until-timeout behaviour.
      */
-    QIOChannelSocket *sioc = nbd_establish_connection(s->saddr, errp);
-
-    if (!sioc) {
+    if (nbd_establish_connection(bs, s->saddr, errp) < 0) {
         return -ECONNREFUSED;
     }

     /* NBD handshake */
     trace_nbd_client_connect(s->export);
-    qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-    qio_channel_attach_aio_context(QIO_CHANNEL(sioc), aio_context);
+    qio_channel_set_blocking(QIO_CHANNEL(s->sioc), false, NULL);
+    qio_channel_attach_aio_context(QIO_CHANNEL(s->sioc), aio_context);

     s->info.request_sizes = true;
     s->info.structured_reply = true;
     s->info.base_allocation = true;
     s->info.x_dirty_bitmap = g_strdup(s->x_dirty_bitmap);
     s->info.name = g_strdup(s->export ?: "");
-    ret = nbd_receive_negotiate(aio_context, QIO_CHANNEL(sioc), s->tlscreds,
+    ret = nbd_receive_negotiate(aio_context, QIO_CHANNEL(s->sioc), s->tlscreds,
                                 s->hostname, &s->ioc, &s->info, errp);
     g_free(s->info.x_dirty_bitmap);
     g_free(s->info.name);
     if (ret < 0) {
-        object_unref(OBJECT(sioc));
+        yank_unregister_function(s->yank_name, nbd_yank, bs);
+        object_unref(OBJECT(s->sioc));
         return ret;
     }
     if (s->x_dirty_bitmap && !s->info.base_allocation) {
@@ -1485,10 +1509,8 @@ static int nbd_client_connect(BlockDriverState *bs, Error **errp)
         }
     }

-    s->sioc = sioc;
-
     if (!s->ioc) {
-        s->ioc = QIO_CHANNEL(sioc);
+        s->ioc = QIO_CHANNEL(s->sioc);
         object_ref(OBJECT(s->ioc));
     }

@@ -1504,9 +1526,10 @@ static int nbd_client_connect(BlockDriverState *bs, Error **errp)
     {
         NBDRequest request = { .type = NBD_CMD_DISC };

-        nbd_send_request(s->ioc ?: QIO_CHANNEL(sioc), &request);
+        nbd_send_request(s->ioc ?: QIO_CHANNEL(s->sioc), &request);

-        object_unref(OBJECT(sioc));
+        yank_unregister_function(s->yank_name, nbd_yank, bs);
+        object_unref(OBJECT(s->sioc));

         return ret;
     }
@@ -1913,6 +1936,9 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     qemu_co_mutex_init(&s->send_mutex);
     qemu_co_queue_init(&s->free_sema);

+    s->yank_name = g_strconcat("blockdev:", bs->node_name, NULL);
+    yank_register_instance(s->yank_name);
+
     ret = nbd_client_connect(bs, errp);
     if (ret < 0) {
         nbd_clear_bdrvstate(s);
@@ -1972,6 +1998,7 @@ static void nbd_close(BlockDriverState *bs)
     BDRVNBDState *s = bs->opaque;

     nbd_client_close(bs);
+    yank_unregister_instance(s->yank_name);
     nbd_clear_bdrvstate(s);
 }

--
2.20.1


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

[PATCH v2 3/4] chardev/char-socket.c: Add yank feature

[Lukas Straub]

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

Register a yank function to shutdown the socket on yank.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 chardev/char-socket.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 185fe38dda..d5c6cd2153 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -34,6 +34,7 @@
 #include "qapi/error.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-sockets.h"
+#include "yank.h"

 #include "chardev/char-io.h"

@@ -69,6 +70,7 @@ typedef struct {
     size_t read_msgfds_num;
     int *write_msgfds;
     size_t write_msgfds_num;
+    char *yank_name;

     SocketAddress *addr;
     bool is_listen;
@@ -409,6 +411,11 @@ static void tcp_chr_free_connection(Chardev *chr)

     tcp_set_msgfds(chr, NULL, 0);
     remove_fd_in_watch(chr);
+    if (s->state == TCP_CHARDEV_STATE_CONNECTING
+        || s->state == TCP_CHARDEV_STATE_CONNECTED) {
+        yank_unregister_function(s->yank_name, yank_generic_iochannel,
+                                 QIO_CHANNEL(s->sioc));
+    }
     object_unref(OBJECT(s->sioc));
     s->sioc = NULL;
     object_unref(OBJECT(s->ioc));
@@ -912,6 +919,8 @@ static int tcp_chr_add_client(Chardev *chr, int fd)
     }
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
     tcp_chr_set_client_ioc_name(chr, sioc);
+    yank_register_function(s->yank_name, yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     ret = tcp_chr_new_client(chr, sioc);
     object_unref(OBJECT(sioc));
     return ret;
@@ -926,6 +935,8 @@ static void tcp_chr_accept(QIONetListener *listener,

     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
     tcp_chr_set_client_ioc_name(chr, cioc);
+    yank_register_function(s->yank_name, yank_generic_iochannel,
+                           QIO_CHANNEL(cioc));
     tcp_chr_new_client(chr, cioc);
 }

@@ -941,6 +952,8 @@ static int tcp_chr_connect_client_sync(Chardev *chr, Error **errp)
         object_unref(OBJECT(sioc));
         return -1;
     }
+    yank_register_function(s->yank_name, yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     tcp_chr_new_client(chr, sioc);
     object_unref(OBJECT(sioc));
     return 0;
@@ -956,6 +969,8 @@ static void tcp_chr_accept_server_sync(Chardev *chr)
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
     sioc = qio_net_listener_wait_client(s->listener);
     tcp_chr_set_client_ioc_name(chr, sioc);
+    yank_register_function(s->yank_name, yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     tcp_chr_new_client(chr, sioc);
     object_unref(OBJECT(sioc));
 }
@@ -1066,6 +1081,8 @@ static void char_socket_finalize(Object *obj)
         object_unref(OBJECT(s->tls_creds));
     }
     g_free(s->tls_authz);
+    yank_unregister_instance(s->yank_name);
+    g_free(s->yank_name);

     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
@@ -1081,6 +1098,8 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)

     if (qio_task_propagate_error(task, &err)) {
         tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
+        yank_unregister_function(s->yank_name, yank_generic_iochannel,
+                                 QIO_CHANNEL(sioc));
         check_report_connect_error(chr, err);
         error_free(err);
         goto cleanup;
@@ -1115,6 +1134,8 @@ static void tcp_chr_connect_client_async(Chardev *chr)
     tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING);
     sioc = qio_channel_socket_new();
     tcp_chr_set_client_ioc_name(chr, sioc);
+    yank_register_function(s->yank_name, yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     /*
      * Normally code would use the qio_channel_socket_connect_async
      * method which uses a QIOTask + qio_task_set_error internally
@@ -1356,6 +1377,9 @@ static void qmp_chardev_open_socket(Chardev *chr,
         qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
     }

+    s->yank_name = g_strconcat("chardev:", chr->label, NULL);
+    yank_register_instance(s->yank_name);
+
     /* be isn't opened until we get a connection */
     *be_opened = false;

--
2.20.1


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

[PATCH v2 4/4] migration: Add yank feature

[Lukas Straub]

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

Register yank functions on sockets to shut them down.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 migration/migration.c         |  9 +++++++++
 migration/qemu-file-channel.c |  6 ++++++
 migration/socket.c            | 11 +++++++++++
 3 files changed, 26 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 187ac0410c..f89fcba198 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -54,6 +54,7 @@
 #include "net/announce.h"
 #include "qemu/queue.h"
 #include "multifd.h"
+#include "yank.h"

 #define MAX_THROTTLE  (32 << 20)      /* Migration transfer speed throttling */

@@ -231,6 +232,8 @@ void migration_incoming_state_destroy(void)
         qapi_free_SocketAddressList(mis->socket_address_list);
         mis->socket_address_list = NULL;
     }
+
+    yank_unregister_instance((char *) "migration");
 }

 static void migrate_generate_event(int new_state)
@@ -362,6 +365,7 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
     const char *p;

     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
+    yank_register_instance((char *) "migration");
     if (!strcmp(uri, "defer")) {
         deferred_incoming_migration(errp);
     } else if (strstart(uri, "tcp:", &p)) {
@@ -377,6 +381,7 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
     } else {
+        yank_unregister_instance((char *) "migration");
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
 }
@@ -1632,6 +1637,7 @@ static void migrate_fd_cleanup(MigrationState *s)
     }
     notifier_list_notify(&migration_state_notifiers, s);
     block_cleanup_parameters(s);
+    yank_unregister_instance((char *) "migration");
 }

 static void migrate_fd_cleanup_schedule(MigrationState *s)
@@ -2036,6 +2042,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }

+    yank_register_instance((char *) "migration");
     if (strstart(uri, "tcp:", &p)) {
         tcp_start_outgoing_migration(s, p, &local_err);
 #ifdef CONFIG_RDMA
@@ -2049,6 +2056,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_outgoing_migration(s, p, &local_err);
     } else {
+        yank_unregister_instance((char *) "migration");
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
                    "a valid migration protocol");
         migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
@@ -2058,6 +2066,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     }

     if (local_err) {
+        yank_unregister_instance((char *) "migration");
         migrate_fd_error(s, local_err);
         error_propagate(errp, local_err);
         return;
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index d2ce32f4b9..6224bda029 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -27,6 +27,7 @@
 #include "qemu-file.h"
 #include "io/channel-socket.h"
 #include "qemu/iov.h"
+#include "yank.h"


 static ssize_t channel_writev_buffer(void *opaque,
@@ -104,6 +105,11 @@ static int channel_close(void *opaque, Error **errp)
     int ret;
     QIOChannel *ioc = QIO_CHANNEL(opaque);
     ret = qio_channel_close(ioc, errp);
+    if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET)
+        && OBJECT(ioc)->ref == 2) {
+        yank_unregister_function((char *) "migration", yank_generic_iochannel,
+                                 QIO_CHANNEL(ioc));
+    }
     object_unref(OBJECT(ioc));
     return ret;
 }
diff --git a/migration/socket.c b/migration/socket.c
index 97c9efde59..bbca53cc49 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -26,6 +26,7 @@
 #include "io/channel-socket.h"
 #include "io/net-listener.h"
 #include "trace.h"
+#include "yank.h"


 struct SocketOutgoingArgs {
@@ -35,6 +36,8 @@ struct SocketOutgoingArgs {
 void socket_send_channel_create(QIOTaskFunc f, void *data)
 {
     QIOChannelSocket *sioc = qio_channel_socket_new();
+    yank_register_function((char *) "migration", yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     qio_channel_socket_connect_async(sioc, outgoing_args.saddr,
                                      f, data, NULL, NULL);
 }
@@ -42,6 +45,8 @@ void socket_send_channel_create(QIOTaskFunc f, void *data)
 int socket_send_channel_destroy(QIOChannel *send)
 {
     /* Remove channel */
+    yank_unregister_function((char *) "migration", yank_generic_iochannel,
+                             QIO_CHANNEL(send));
     object_unref(OBJECT(send));
     if (outgoing_args.saddr) {
         qapi_free_SocketAddress(outgoing_args.saddr);
@@ -101,6 +106,8 @@ static void socket_outgoing_migration(QIOTask *task,
     Error *err = NULL;

     if (qio_task_propagate_error(task, &err)) {
+        yank_unregister_function((char *) "migration", yank_generic_iochannel,
+                                 QIO_CHANNEL(sioc));
         trace_migration_socket_outgoing_error(error_get_pretty(err));
     } else {
         trace_migration_socket_outgoing_connected(data->hostname);
@@ -127,6 +134,8 @@ static void socket_start_outgoing_migration(MigrationState *s,
     }

     qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-outgoing");
+    yank_register_function((char *) "migration", yank_generic_iochannel,
+                           QIO_CHANNEL(sioc));
     qio_channel_socket_connect_async(sioc,
                                      saddr,
                                      socket_outgoing_migration,
@@ -163,6 +172,8 @@ static void socket_accept_incoming_migration(QIONetListener *listener,
     trace_migration_socket_incoming_accepted();

     qio_channel_set_name(QIO_CHANNEL(cioc), "migration-socket-incoming");
+    yank_register_function((char *) "migration", yank_generic_iochannel,
+                           QIO_CHANNEL(cioc));
     migration_channel_process_incoming(QIO_CHANNEL(cioc));

     if (migration_has_all_channels()) {
--
2.20.1

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

Re: [PATCH v2 1/4] Introduce yank feature

[Paolo Bonzini]

On 20/05/20 23:05, Lukas Straub wrote:
> +
> +void yank_init(void)
> +{
> +    qemu_mutex_init(&lock);
> +}

You can use __constructor__ for this to avoid the call in vl.c.  See
job.c for an example.

Thanks,

Paolo

Re: [PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu

[no-reply]

Patchew URL: https://patchew.org/QEMU/cover.1590008051.git.lukasstraub2@web.de/



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 ===

/tmp/qemu-test/src/chardev/char-socket.c:1381: undefined reference to `yank_register_instance'
chardev/char-socket.o: In function `char_socket_finalize':
/tmp/qemu-test/src/chardev/char-socket.c:1084: undefined reference to `yank_unregister_instance'
collect2: error: ld returned 1 exit status
  CC      s_normSubnormalF64Sig.o
  CC      s_roundPackToF64.o
  CC      s_normRoundPackToF64.o
make: *** [tests/test-char] Error 1
make: *** Waiting for unfinished jobs....
  CC      s_addMagsF64.o
  CC      s_subMagsF64.o
---
Not run: 259
Failures: 140 143 267
Failed 3 of 119 iotests
make: *** [check-tests/check-block.sh] Error 1
make: *** wait: No child processes.  Stop.
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=01104a80117541dfabf73e596a8c2328', '-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-1_i_19yu/src/docker-src.2020-05-20-19.03.34.15520:/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=01104a80117541dfabf73e596a8c2328
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-1_i_19yu/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    12m8.434s
user    0m8.504s


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

Re: [PATCH v2 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu

[no-reply]

Patchew URL: https://patchew.org/QEMU/cover.1590008051.git.lukasstraub2@web.de/



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 ===

/usr/bin/ld: /tmp/qemu-test/src/chardev/char-socket.c:1101: undefined reference to `yank_unregister_function'
clang++ -g  -Wl,--warn-common -fsanitize=undefined -fsanitize=address -Wl,-z,relro -Wl,-z,now -pie -m64  -fstack-protector-strong -o tests/check-qnull tests/check-qnull.o  libqemuutil.a   -lm -lz  -lgthread-2.0 -pthread -lglib-2.0  -lnettle  -lgnutls  -L/usr/lib -lzstd   -lrt -lz -lutil -lcap-ng 
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_addMagsF16.o -MF ./s_addMagsF16.d -g   -c -o s_addMagsF16.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_addMagsF16.c
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_subMagsF16.o -MF ./s_subMagsF16.d -g   -c -o s_subMagsF16.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_subMagsF16.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_mulAddF16.o -MF ./s_mulAddF16.d -g   -c -o s_mulAddF16.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_mulAddF16.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_normSubnormalF32Sig.o -MF ./s_normSubnormalF32Sig.d -g   -c -o s_normSubnormalF32Sig.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_normSubnormalF32Sig.c
---
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_roundPackToF64.o -MF ./s_roundPackToF64.d -g   -c -o s_roundPackToF64.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_roundPackToF64.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_normRoundPackToF64.o -MF ./s_normRoundPackToF64.d -g   -c -o s_normRoundPackToF64.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_normRoundPackToF64.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_addMagsF64.o -MF ./s_addMagsF64.d -g   -c -o s_addMagsF64.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_addMagsF64.c
make: *** [/tmp/qemu-test/src/rules.mak:124: tests/test-char] Error 1
make: *** Waiting for unfinished jobs....
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_subMagsF64.o -MF ./s_subMagsF64.d -g   -c -o s_subMagsF64.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_subMagsF64.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80 -Wno-missing-prototypes -Wno-redundant-decls -Wno-return-type -Wno-error -MMD -MP -MT s_mulAddF64.o -MF ./s_mulAddF64.d -g   -c -o s_mulAddF64.o /tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/s_mulAddF64.c
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=2789d6e2430144ccaaa2afa8f60d2912', '-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-4srx9vwp/src/docker-src.2020-05-20-19.16.23.17821:/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=2789d6e2430144ccaaa2afa8f60d2912
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4srx9vwp/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m52.381s
user    0m8.277s


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

Re: [PATCH v2 1/4] Introduce yank feature

[Stefan Hajnoczi]

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

On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:
> +void yank_generic_iochannel(void *opaque)
> +{
> +    QIOChannel *ioc = QIO_CHANNEL(opaque);
> +
> +    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> +}
> +
> +void qmp_yank(strList *instances, Error **errp)
> +{
> +    strList *tmp;
> +    struct YankInstance *instance;
> +    struct YankFuncAndParam *entry;
> +
> +    qemu_mutex_lock(&lock);
> +    tmp = instances;
> +    for (; tmp; tmp = tmp->next) {
> +        instance = yank_find_instance(tmp->value);
> +        if (!instance) {
> +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                      "Instance '%s' not found", tmp->value);
> +            qemu_mutex_unlock(&lock);
> +            return;
> +        }
> +    }
> +    tmp = instances;
> +    for (; tmp; tmp = tmp->next) {
> +        instance = yank_find_instance(tmp->value);
> +        assert(instance);
> +        QLIST_FOREACH(entry, &instance->yankfns, next) {
> +            entry->func(entry->opaque);
> +        }
> +    }
> +    qemu_mutex_unlock(&lock);
> +}

From docs/devel/qapi-code-gen.txt:

  An OOB-capable command handler must satisfy the following conditions:

  - It terminates quickly.
  - It does not invoke system calls that may block.
  - It does not access guest RAM that may block when userfaultfd is
    enabled for postcopy live migration.
  - It takes only "fast" locks, i.e. all critical sections protected by
    any lock it takes also satisfy the conditions for OOB command
    handler code.

This patch series violates these rules and calls existing functions that
were not designed for OOB execution.

Please explain why it is safe to do this.

Stefan

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

Re: [PATCH v2 1/4] Introduce yank feature

[Lukas Straub]

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

On Thu, 21 May 2020 16:03:35 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:
> > +void yank_generic_iochannel(void *opaque)
> > +{
> > +    QIOChannel *ioc = QIO_CHANNEL(opaque);
> > +
> > +    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > +}
> > +
> > +void qmp_yank(strList *instances, Error **errp)
> > +{
> > +    strList *tmp;
> > +    struct YankInstance *instance;
> > +    struct YankFuncAndParam *entry;
> > +
> > +    qemu_mutex_lock(&lock);
> > +    tmp = instances;
> > +    for (; tmp; tmp = tmp->next) {
> > +        instance = yank_find_instance(tmp->value);
> > +        if (!instance) {
> > +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > +                      "Instance '%s' not found", tmp->value);
> > +            qemu_mutex_unlock(&lock);
> > +            return;
> > +        }
> > +    }
> > +    tmp = instances;
> > +    for (; tmp; tmp = tmp->next) {
> > +        instance = yank_find_instance(tmp->value);
> > +        assert(instance);
> > +        QLIST_FOREACH(entry, &instance->yankfns, next) {
> > +            entry->func(entry->opaque);
> > +        }
> > +    }
> > +    qemu_mutex_unlock(&lock);
> > +}  
> 
> From docs/devel/qapi-code-gen.txt:
> 
>   An OOB-capable command handler must satisfy the following conditions:
> 
>   - It terminates quickly.
Check.

>   - It does not invoke system calls that may block.
brk/sbrk (malloc and friends):
The manpage doesn't say anything about blocking, but malloc is already used while handling the qmp command.

shutdown():
The manpage doesn't say anything about blocking, but this is already used in migration oob qmp commands.

There are no other syscalls involved to my knowledge.

>   - It does not access guest RAM that may block when userfaultfd is
>     enabled for postcopy live migration.
Check.

>   - It takes only "fast" locks, i.e. all critical sections protected by
>     any lock it takes also satisfy the conditions for OOB command
>     handler code.

The lock in yank.c satisfies this requirement.

qio_channel_shutdown doesn't take any locks.

Regards,
Lukas Straub

> This patch series violates these rules and calls existing functions that
> were not designed for OOB execution.
> 
> Please explain why it is safe to do this.
> 
> Stefan


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

Re: [PATCH v2 4/4] migration: Add yank feature

[Lukas Straub]

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

On Wed, 20 May 2020 23:05:50 +0200
Lukas Straub <lukasstraub2@web.de> wrote:

> Register yank functions on sockets to shut them down.
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>

Don't review this commit for now, I'll have to revamp it anyway.

Regards,
Lukas Straub

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

Re: [PATCH v2 1/4] Introduce yank feature

[Daniel P. Berrangé]

On Thu, May 21, 2020 at 05:42:41PM +0200, Lukas Straub wrote:
> On Thu, 21 May 2020 16:03:35 +0100
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
> 
> > On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:
> > > +void yank_generic_iochannel(void *opaque)
> > > +{
> > > +    QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > +
> > > +    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > > +}
> > > +
> > > +void qmp_yank(strList *instances, Error **errp)
> > > +{
> > > +    strList *tmp;
> > > +    struct YankInstance *instance;
> > > +    struct YankFuncAndParam *entry;
> > > +
> > > +    qemu_mutex_lock(&lock);
> > > +    tmp = instances;
> > > +    for (; tmp; tmp = tmp->next) {
> > > +        instance = yank_find_instance(tmp->value);
> > > +        if (!instance) {
> > > +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > > +                      "Instance '%s' not found", tmp->value);
> > > +            qemu_mutex_unlock(&lock);
> > > +            return;
> > > +        }
> > > +    }
> > > +    tmp = instances;
> > > +    for (; tmp; tmp = tmp->next) {
> > > +        instance = yank_find_instance(tmp->value);
> > > +        assert(instance);
> > > +        QLIST_FOREACH(entry, &instance->yankfns, next) {
> > > +            entry->func(entry->opaque);
> > > +        }
> > > +    }
> > > +    qemu_mutex_unlock(&lock);
> > > +}  
> > 
> > From docs/devel/qapi-code-gen.txt:
> > 
> >   An OOB-capable command handler must satisfy the following conditions:
> > 
> >   - It terminates quickly.
> Check.
> 
> >   - It does not invoke system calls that may block.
> brk/sbrk (malloc and friends):
> The manpage doesn't say anything about blocking, but malloc is already used while handling the qmp command.
> 
> shutdown():
> The manpage doesn't say anything about blocking, but this is already used in migration oob qmp commands.

It just marks the socket state in local kernel side. It doesn't involve
any blocking roundtrips over the wire, so this is fine.

> 
> There are no other syscalls involved to my knowledge.
> 
> >   - It does not access guest RAM that may block when userfaultfd is
> >     enabled for postcopy live migration.
> Check.
> 
> >   - It takes only "fast" locks, i.e. all critical sections protected by
> >     any lock it takes also satisfy the conditions for OOB command
> >     handler code.
> 
> The lock in yank.c satisfies this requirement.
> 
> qio_channel_shutdown doesn't take any locks.

Agreed, I think the yank code is compliant with all the points
listed above.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/4] Introduce yank feature

[Stefan Hajnoczi]

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

On Thu, May 21, 2020 at 04:48:06PM +0100, Daniel P. Berrangé wrote:
> On Thu, May 21, 2020 at 05:42:41PM +0200, Lukas Straub wrote:
> > On Thu, 21 May 2020 16:03:35 +0100
> > Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > 
> > > On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:
> > > > +void yank_generic_iochannel(void *opaque)
> > > > +{
> > > > +    QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > > +
> > > > +    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > > > +}
> > > > +
> > > > +void qmp_yank(strList *instances, Error **errp)
> > > > +{
> > > > +    strList *tmp;
> > > > +    struct YankInstance *instance;
> > > > +    struct YankFuncAndParam *entry;
> > > > +
> > > > +    qemu_mutex_lock(&lock);
> > > > +    tmp = instances;
> > > > +    for (; tmp; tmp = tmp->next) {
> > > > +        instance = yank_find_instance(tmp->value);
> > > > +        if (!instance) {
> > > > +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > > > +                      "Instance '%s' not found", tmp->value);
> > > > +            qemu_mutex_unlock(&lock);
> > > > +            return;
> > > > +        }
> > > > +    }
> > > > +    tmp = instances;
> > > > +    for (; tmp; tmp = tmp->next) {
> > > > +        instance = yank_find_instance(tmp->value);
> > > > +        assert(instance);
> > > > +        QLIST_FOREACH(entry, &instance->yankfns, next) {
> > > > +            entry->func(entry->opaque);
> > > > +        }
> > > > +    }
> > > > +    qemu_mutex_unlock(&lock);
> > > > +}  
> > > 
> > > From docs/devel/qapi-code-gen.txt:
> > > 
> > >   An OOB-capable command handler must satisfy the following conditions:
> > > 
> > >   - It terminates quickly.
> > Check.
> > 
> > >   - It does not invoke system calls that may block.
> > brk/sbrk (malloc and friends):
> > The manpage doesn't say anything about blocking, but malloc is already used while handling the qmp command.
> > 
> > shutdown():
> > The manpage doesn't say anything about blocking, but this is already used in migration oob qmp commands.
> 
> It just marks the socket state in local kernel side. It doesn't involve
> any blocking roundtrips over the wire, so this is fine.
> 
> > 
> > There are no other syscalls involved to my knowledge.
> > 
> > >   - It does not access guest RAM that may block when userfaultfd is
> > >     enabled for postcopy live migration.
> > Check.
> > 
> > >   - It takes only "fast" locks, i.e. all critical sections protected by
> > >     any lock it takes also satisfy the conditions for OOB command
> > >     handler code.
> > 
> > The lock in yank.c satisfies this requirement.
> > 
> > qio_channel_shutdown doesn't take any locks.
> 
> Agreed, I think the yank code is compliant with all the points
> listed above.

The code does not document this in any way. In fact, it's an interface
for registering arbitrary callback functions which execute in this
special environment.

If you don't document this explicitly it will break when someone changes
the code, even if it works right now.

Please document this in the yank APIs and also in the implementation.

For example, QemuMutex yank has the priority inversion problem: no other
function may violate the oob rules while holding the mutex, otherwise
the oob function will hang while waiting for the lock when the other
function is blocked.

Stefan

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

Re: [PATCH v2 1/4] Introduce yank feature

[Lukas Straub]

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

On Wed, 17 Jun 2020 15:39:36 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Thu, May 21, 2020 at 04:48:06PM +0100, Daniel P. Berrangé wrote:
> > On Thu, May 21, 2020 at 05:42:41PM +0200, Lukas Straub wrote:  
> > > On Thu, 21 May 2020 16:03:35 +0100
> > > Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > >   
> > > > On Wed, May 20, 2020 at 11:05:39PM +0200, Lukas Straub wrote:  
> > > > > +void yank_generic_iochannel(void *opaque)
> > > > > +{
> > > > > +    QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > > > +
> > > > > +    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > > > > +}
> > > > > +
> > > > > +void qmp_yank(strList *instances, Error **errp)
> > > > > +{
> > > > > +    strList *tmp;
> > > > > +    struct YankInstance *instance;
> > > > > +    struct YankFuncAndParam *entry;
> > > > > +
> > > > > +    qemu_mutex_lock(&lock);
> > > > > +    tmp = instances;
> > > > > +    for (; tmp; tmp = tmp->next) {
> > > > > +        instance = yank_find_instance(tmp->value);
> > > > > +        if (!instance) {
> > > > > +            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> > > > > +                      "Instance '%s' not found", tmp->value);
> > > > > +            qemu_mutex_unlock(&lock);
> > > > > +            return;
> > > > > +        }
> > > > > +    }
> > > > > +    tmp = instances;
> > > > > +    for (; tmp; tmp = tmp->next) {
> > > > > +        instance = yank_find_instance(tmp->value);
> > > > > +        assert(instance);
> > > > > +        QLIST_FOREACH(entry, &instance->yankfns, next) {
> > > > > +            entry->func(entry->opaque);
> > > > > +        }
> > > > > +    }
> > > > > +    qemu_mutex_unlock(&lock);
> > > > > +}    
> > > > 
> > > > From docs/devel/qapi-code-gen.txt:
> > > > 
> > > >   An OOB-capable command handler must satisfy the following conditions:
> > > > 
> > > >   - It terminates quickly.  
> > > Check.
> > >   
> > > >   - It does not invoke system calls that may block.  
> > > brk/sbrk (malloc and friends):
> > > The manpage doesn't say anything about blocking, but malloc is already used while handling the qmp command.
> > > 
> > > shutdown():
> > > The manpage doesn't say anything about blocking, but this is already used in migration oob qmp commands.  
> > 
> > It just marks the socket state in local kernel side. It doesn't involve
> > any blocking roundtrips over the wire, so this is fine.
> >   
> > > 
> > > There are no other syscalls involved to my knowledge.
> > >   
> > > >   - It does not access guest RAM that may block when userfaultfd is
> > > >     enabled for postcopy live migration.  
> > > Check.
> > >   
> > > >   - It takes only "fast" locks, i.e. all critical sections protected by
> > > >     any lock it takes also satisfy the conditions for OOB command
> > > >     handler code.  
> > > 
> > > The lock in yank.c satisfies this requirement.
> > > 
> > > qio_channel_shutdown doesn't take any locks.  
> > 
> > Agreed, I think the yank code is compliant with all the points
> > listed above.  
> 
> The code does not document this in any way. In fact, it's an interface
> for registering arbitrary callback functions which execute in this
> special environment.
> 
> If you don't document this explicitly it will break when someone changes
> the code, even if it works right now.
> 
> Please document this in the yank APIs and also in the implementation.

Hi,
It is documented in yank.h:

/**
 * yank_register_function: Register a yank function
 *
 * This registers a yank function. All limitations of qmp oob commands apply
 * to the yank function as well.
 *
 * This function is thread-safe.
 *
 * @instance_name: The name of the instance
 * @func: The yank function
 * @opaque: Will be passed to the yank function
 */

Thanks,
Lukas Straub

> For example, QemuMutex yank has the priority inversion problem: no other
> function may violate the oob rules while holding the mutex, otherwise
> the oob function will hang while waiting for the lock when the other
> function is blocked.
> 
> Stefan


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