All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/5] migration fixes
@ 2016-05-23 17:19 Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 1/5] migration: Move qjson.[ch] to migration/ Amit Shah
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

The following changes since commit 65603e2fc18b48e6e55a3dd693669413141694ec:

  tci: do not include exec/exec-all.h (2016-05-20 15:07:46 +0100)

are available in the git repository at:

  https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-2.7-1

for you to fetch changes up to fe904ea8242cbae2d7e69c052c754b8f5f1ba1d6:

  migration: regain control of images when migration fails to complete (2016-05-23 22:19:36 +0530)

----------------------------------------------------------------
migration fixes:

- ensure src block devices continue fine after a failed migration
- fail on migration blockers; helps 9p savevm/loadvm
- move autoconverge commands out of experimental state
- move the migration-specific qjson in migration/

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


Greg Kurz (2):
  savevm: fail if migration blockers are present
  migration: regain control of images when migration fails to complete

Jason J. Herne (1):
  migration: Promote improved autoconverge commands out of experimental
    state

Markus Armbruster (2):
  migration: Move qjson.[ch] to migration/
  migration/qjson: Drop gratuitous use of QOM

 Makefile.objs                 |   1 -
 hmp.c                         |  28 ++++-----
 include/migration/migration.h |   1 +
 include/migration/qjson.h     |  29 ++++++++++
 include/migration/vmstate.h   |   2 +-
 include/qjson.h               |  29 ----------
 migration/Makefile.objs       |   1 +
 migration/migration.c         |  94 ++++++++++++++++++------------
 migration/qjson.c             | 113 ++++++++++++++++++++++++++++++++++++
 migration/ram.c               |   4 +-
 migration/savevm.c            |   4 +-
 migration/vmstate.c           |   1 -
 qapi-schema.json              |  54 +++++++++---------
 qjson.c                       | 129 ------------------------------------------
 qmp-commands.hx               |  22 +++----
 tests/Makefile                |   2 +-
 16 files changed, 260 insertions(+), 254 deletions(-)
 create mode 100644 include/migration/qjson.h
 delete mode 100644 include/qjson.h
 create mode 100644 migration/qjson.c
 delete mode 100644 qjson.c

-- 
2.5.5

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

* [Qemu-devel] [PULL 1/5] migration: Move qjson.[ch] to migration/
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
@ 2016-05-23 17:19 ` Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 2/5] migration/qjson: Drop gratuitous use of QOM Amit Shah
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

From: Markus Armbruster <armbru@redhat.com>

Type QJSON lets you build JSON text.  Its interface mirrors (a subset
of) abstract JSON syntax.

QAPI output visitors also produce JSON text.  They assert their
preconditions and invariants, and therefore abort on incorrect use.

Contrastingly, QJSON does *not* detect incorrect use.  It happily
produces invalid JSON then.  This is what migration wants.

QJSON was designed for migration, and migration is its only user.
Move it to migration/ for proper coverage by MAINTAINERS, and to deter
accidental use outside migration.

[Pointed out by Eric: QJSON was added in commits 0457d07..b174257
 -- Amit]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1462380558-2030-2-git-send-email-armbru@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 Makefile.objs               |   1 -
 include/migration/qjson.h   |  29 +++++++++
 include/migration/vmstate.h |   2 +-
 include/qjson.h             |  29 ---------
 migration/Makefile.objs     |   1 +
 migration/qjson.c           | 140 ++++++++++++++++++++++++++++++++++++++++++++
 migration/vmstate.c         |   1 -
 qjson.c                     | 129 ----------------------------------------
 tests/Makefile              |   2 +-
 9 files changed, 172 insertions(+), 162 deletions(-)
 create mode 100644 include/migration/qjson.h
 delete mode 100644 include/qjson.h
 create mode 100644 migration/qjson.c
 delete mode 100644 qjson.c

diff --git a/Makefile.objs b/Makefile.objs
index 8f705f6..da49b71 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -52,7 +52,6 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 common-obj-y += migration/
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += page_cache.o
-common-obj-y += qjson.o
 
 common-obj-$(CONFIG_SPICE) += spice-qemu-char.o
 
diff --git a/include/migration/qjson.h b/include/migration/qjson.h
new file mode 100644
index 0000000..7c54fdf
--- /dev/null
+++ b/include/migration/qjson.h
@@ -0,0 +1,29 @@
+/*
+ * QEMU JSON writer
+ *
+ * Copyright Alexander Graf
+ *
+ * Authors:
+ *  Alexander Graf <agraf@suse.de>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+#ifndef QEMU_QJSON_H
+#define QEMU_QJSON_H
+
+#define TYPE_QJSON "QJSON"
+typedef struct QJSON QJSON;
+
+QJSON *qjson_new(void);
+void json_prop_str(QJSON *json, const char *name, const char *str);
+void json_prop_int(QJSON *json, const char *name, int64_t val);
+void json_end_array(QJSON *json);
+void json_start_array(QJSON *json, const char *name);
+void json_end_object(QJSON *json);
+void json_start_object(QJSON *json, const char *name);
+const char *qjson_get_str(QJSON *json);
+void qjson_finish(QJSON *json);
+
+#endif /* QEMU_QJSON_H */
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 84ee355..30ecc44 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -29,7 +29,7 @@
 #ifndef CONFIG_USER_ONLY
 #include <migration/qemu-file.h>
 #endif
-#include <qjson.h>
+#include "migration/qjson.h"
 
 typedef void SaveStateHandler(QEMUFile *f, void *opaque);
 typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
diff --git a/include/qjson.h b/include/qjson.h
deleted file mode 100644
index 7c54fdf..0000000
--- a/include/qjson.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * QEMU JSON writer
- *
- * Copyright Alexander Graf
- *
- * Authors:
- *  Alexander Graf <agraf@suse.de>
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-#ifndef QEMU_QJSON_H
-#define QEMU_QJSON_H
-
-#define TYPE_QJSON "QJSON"
-typedef struct QJSON QJSON;
-
-QJSON *qjson_new(void);
-void json_prop_str(QJSON *json, const char *name, const char *str);
-void json_prop_int(QJSON *json, const char *name, int64_t val);
-void json_end_array(QJSON *json);
-void json_start_array(QJSON *json, const char *name);
-void json_end_object(QJSON *json);
-void json_start_object(QJSON *json, const char *name);
-const char *qjson_get_str(QJSON *json);
-void qjson_finish(QJSON *json);
-
-#endif /* QEMU_QJSON_H */
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0cac6d7..d25ff48 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-y += migration.o tcp.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += xbzrle.o postcopy-ram.o
+common-obj-y += qjson.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
 common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o
diff --git a/migration/qjson.c b/migration/qjson.c
new file mode 100644
index 0000000..cb479fe
--- /dev/null
+++ b/migration/qjson.c
@@ -0,0 +1,140 @@
+/*
+ * A simple JSON writer
+ *
+ * Copyright Alexander Graf
+ *
+ * Authors:
+ *  Alexander Graf <agraf@suse.de>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+/*
+ * Type QJSON lets you build JSON text.  Its interface mirrors (a
+ * subset of) abstract JSON syntax.
+ *
+ * It does *not* detect incorrect use.  It happily produces invalid
+ * JSON then.  This is what migration wants.
+ *
+ * QAPI output visitors also produce JSON text.  However, they do
+ * assert their preconditions and invariants, and therefore abort on
+ * incorrect use.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qmp/qstring.h"
+#include "migration/qjson.h"
+#include "qemu/module.h"
+#include "qom/object.h"
+
+struct QJSON {
+    Object obj;
+    QString *str;
+    bool omit_comma;
+};
+
+#define QJSON(obj) OBJECT_CHECK(QJSON, (obj), TYPE_QJSON)
+
+static void json_emit_element(QJSON *json, const char *name)
+{
+    /* Check whether we need to print a , before an element */
+    if (json->omit_comma) {
+        json->omit_comma = false;
+    } else {
+        qstring_append(json->str, ", ");
+    }
+
+    if (name) {
+        qstring_append(json->str, "\"");
+        qstring_append(json->str, name);
+        qstring_append(json->str, "\" : ");
+    }
+}
+
+void json_start_object(QJSON *json, const char *name)
+{
+    json_emit_element(json, name);
+    qstring_append(json->str, "{ ");
+    json->omit_comma = true;
+}
+
+void json_end_object(QJSON *json)
+{
+    qstring_append(json->str, " }");
+    json->omit_comma = false;
+}
+
+void json_start_array(QJSON *json, const char *name)
+{
+    json_emit_element(json, name);
+    qstring_append(json->str, "[ ");
+    json->omit_comma = true;
+}
+
+void json_end_array(QJSON *json)
+{
+    qstring_append(json->str, " ]");
+    json->omit_comma = false;
+}
+
+void json_prop_int(QJSON *json, const char *name, int64_t val)
+{
+    json_emit_element(json, name);
+    qstring_append_int(json->str, val);
+}
+
+void json_prop_str(QJSON *json, const char *name, const char *str)
+{
+    json_emit_element(json, name);
+    qstring_append_chr(json->str, '"');
+    qstring_append(json->str, str);
+    qstring_append_chr(json->str, '"');
+}
+
+const char *qjson_get_str(QJSON *json)
+{
+    return qstring_get_str(json->str);
+}
+
+QJSON *qjson_new(void)
+{
+    QJSON *json = QJSON(object_new(TYPE_QJSON));
+    return json;
+}
+
+void qjson_finish(QJSON *json)
+{
+    json_end_object(json);
+}
+
+static void qjson_initfn(Object *obj)
+{
+    QJSON *json = QJSON(obj);
+
+    json->str = qstring_from_str("{ ");
+    json->omit_comma = true;
+}
+
+static void qjson_finalizefn(Object *obj)
+{
+    QJSON *json = QJSON(obj);
+
+    qobject_decref(QOBJECT(json->str));
+}
+
+static const TypeInfo qjson_type_info = {
+    .name = TYPE_QJSON,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(QJSON),
+    .instance_init = qjson_initfn,
+    .instance_finalize = qjson_finalizefn,
+};
+
+static void qjson_register_types(void)
+{
+    type_register_static(&qjson_type_info);
+}
+
+type_init(qjson_register_types)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bf3d5db..46dc55e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -6,7 +6,6 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "trace.h"
-#include "qjson.h"
 
 static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                     void *opaque, QJSON *vmdesc);
diff --git a/qjson.c b/qjson.c
deleted file mode 100644
index b65ca6e..0000000
--- a/qjson.c
+++ /dev/null
@@ -1,129 +0,0 @@
-/*
- * QEMU JSON writer
- *
- * Copyright Alexander Graf
- *
- * Authors:
- *  Alexander Graf <agraf@suse.de>
- *
- * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
- * See the COPYING.LIB file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include <qapi/qmp/qstring.h>
-#include <glib.h>
-#include <qjson.h>
-#include <qemu/module.h>
-#include <qom/object.h>
-
-struct QJSON {
-    Object obj;
-    QString *str;
-    bool omit_comma;
-};
-
-#define QJSON(obj) OBJECT_CHECK(QJSON, (obj), TYPE_QJSON)
-
-static void json_emit_element(QJSON *json, const char *name)
-{
-    /* Check whether we need to print a , before an element */
-    if (json->omit_comma) {
-        json->omit_comma = false;
-    } else {
-        qstring_append(json->str, ", ");
-    }
-
-    if (name) {
-        qstring_append(json->str, "\"");
-        qstring_append(json->str, name);
-        qstring_append(json->str, "\" : ");
-    }
-}
-
-void json_start_object(QJSON *json, const char *name)
-{
-    json_emit_element(json, name);
-    qstring_append(json->str, "{ ");
-    json->omit_comma = true;
-}
-
-void json_end_object(QJSON *json)
-{
-    qstring_append(json->str, " }");
-    json->omit_comma = false;
-}
-
-void json_start_array(QJSON *json, const char *name)
-{
-    json_emit_element(json, name);
-    qstring_append(json->str, "[ ");
-    json->omit_comma = true;
-}
-
-void json_end_array(QJSON *json)
-{
-    qstring_append(json->str, " ]");
-    json->omit_comma = false;
-}
-
-void json_prop_int(QJSON *json, const char *name, int64_t val)
-{
-    json_emit_element(json, name);
-    qstring_append_int(json->str, val);
-}
-
-void json_prop_str(QJSON *json, const char *name, const char *str)
-{
-    json_emit_element(json, name);
-    qstring_append_chr(json->str, '"');
-    qstring_append(json->str, str);
-    qstring_append_chr(json->str, '"');
-}
-
-const char *qjson_get_str(QJSON *json)
-{
-    return qstring_get_str(json->str);
-}
-
-QJSON *qjson_new(void)
-{
-    QJSON *json = QJSON(object_new(TYPE_QJSON));
-    return json;
-}
-
-void qjson_finish(QJSON *json)
-{
-    json_end_object(json);
-}
-
-static void qjson_initfn(Object *obj)
-{
-    QJSON *json = QJSON(obj);
-
-    json->str = qstring_from_str("{ ");
-    json->omit_comma = true;
-}
-
-static void qjson_finalizefn(Object *obj)
-{
-    QJSON *json = QJSON(obj);
-
-    qobject_decref(QOBJECT(json->str));
-}
-
-static const TypeInfo qjson_type_info = {
-    .name = TYPE_QJSON,
-    .parent = TYPE_OBJECT,
-    .instance_size = sizeof(QJSON),
-    .instance_init = qjson_initfn,
-    .instance_finalize = qjson_finalizefn,
-};
-
-static void qjson_register_types(void)
-{
-    type_register_static(&qjson_type_info);
-}
-
-type_init(qjson_register_types)
diff --git a/tests/Makefile b/tests/Makefile
index 9dddde6..1bbd1ca 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -439,7 +439,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	$(test-qapi-obj-y)
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 	migration/vmstate.o migration/qemu-file.o migration/qemu-file-buf.o \
-        migration/qemu-file-unix.o qjson.o \
+        migration/qemu-file-unix.o migration/qjson.o \
 	$(test-qom-obj-y)
 tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
 	$(test-util-obj-y)
-- 
2.5.5

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

* [Qemu-devel] [PULL 2/5] migration/qjson: Drop gratuitous use of QOM
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 1/5] migration: Move qjson.[ch] to migration/ Amit Shah
@ 2016-05-23 17:19 ` Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 3/5] migration: Promote improved autoconverge commands out of experimental state Amit Shah
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

From: Markus Armbruster <armbru@redhat.com>

All the use of QOM buys us here is the ability to destroy the thing
with object_unref(OBJECT(vmdesc)).  Not worth the notational overhead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1462380558-2030-3-git-send-email-armbru@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qjson.h |  2 +-
 migration/qjson.c         | 39 ++++++---------------------------------
 migration/savevm.c        |  2 +-
 3 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/include/migration/qjson.h b/include/migration/qjson.h
index 7c54fdf..2978b5f 100644
--- a/include/migration/qjson.h
+++ b/include/migration/qjson.h
@@ -13,10 +13,10 @@
 #ifndef QEMU_QJSON_H
 #define QEMU_QJSON_H
 
-#define TYPE_QJSON "QJSON"
 typedef struct QJSON QJSON;
 
 QJSON *qjson_new(void);
+void qjson_destroy(QJSON *json);
 void json_prop_str(QJSON *json, const char *name, const char *str);
 void json_prop_int(QJSON *json, const char *name, int64_t val);
 void json_end_array(QJSON *json);
diff --git a/migration/qjson.c b/migration/qjson.c
index cb479fe..5cae55a 100644
--- a/migration/qjson.c
+++ b/migration/qjson.c
@@ -26,17 +26,12 @@
 #include "qemu/osdep.h"
 #include "qapi/qmp/qstring.h"
 #include "migration/qjson.h"
-#include "qemu/module.h"
-#include "qom/object.h"
 
 struct QJSON {
-    Object obj;
     QString *str;
     bool omit_comma;
 };
 
-#define QJSON(obj) OBJECT_CHECK(QJSON, (obj), TYPE_QJSON)
-
 static void json_emit_element(QJSON *json, const char *name)
 {
     /* Check whether we need to print a , before an element */
@@ -100,41 +95,19 @@ const char *qjson_get_str(QJSON *json)
 
 QJSON *qjson_new(void)
 {
-    QJSON *json = QJSON(object_new(TYPE_QJSON));
-    return json;
-}
-
-void qjson_finish(QJSON *json)
-{
-    json_end_object(json);
-}
-
-static void qjson_initfn(Object *obj)
-{
-    QJSON *json = QJSON(obj);
+    QJSON *json = g_new0(QJSON, 1);
 
     json->str = qstring_from_str("{ ");
     json->omit_comma = true;
+    return json;
 }
 
-static void qjson_finalizefn(Object *obj)
+void qjson_finish(QJSON *json)
 {
-    QJSON *json = QJSON(obj);
-
-    qobject_decref(QOBJECT(json->str));
+    json_end_object(json);
 }
 
-static const TypeInfo qjson_type_info = {
-    .name = TYPE_QJSON,
-    .parent = TYPE_OBJECT,
-    .instance_size = sizeof(QJSON),
-    .instance_init = qjson_initfn,
-    .instance_finalize = qjson_finalizefn,
-};
-
-static void qjson_register_types(void)
+void qjson_destroy(QJSON *json)
 {
-    type_register_static(&qjson_type_info);
+    g_free(json);
 }
-
-type_init(qjson_register_types)
diff --git a/migration/savevm.c b/migration/savevm.c
index bfb3d91..8546fdf 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1115,7 +1115,7 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
         qemu_put_be32(f, vmdesc_len);
         qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
     }
-    object_unref(OBJECT(vmdesc));
+    qjson_destroy(vmdesc);
 
     qemu_fflush(f);
 }
-- 
2.5.5

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

* [Qemu-devel] [PULL 3/5] migration: Promote improved autoconverge commands out of experimental state
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 1/5] migration: Move qjson.[ch] to migration/ Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 2/5] migration/qjson: Drop gratuitous use of QOM Amit Shah
@ 2016-05-23 17:19 ` Amit Shah
  2016-05-23 17:19 ` [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present Amit Shah
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

The new autoconverge throttling commands have been tested for a release now. It
is time to move them out of the experimental state.

Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Message-Id: <1461262038-8197-1-git-send-email-jjherne@linux.vnet.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hmp.c                 | 28 +++++++++++++-------------
 migration/migration.c | 56 +++++++++++++++++++++++++--------------------------
 migration/ram.c       |  4 ++--
 qapi-schema.json      | 54 ++++++++++++++++++++++++-------------------------
 qmp-commands.hx       | 22 ++++++++++----------
 5 files changed, 82 insertions(+), 82 deletions(-)

diff --git a/hmp.c b/hmp.c
index d510236..9f9bcf9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -235,9 +235,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->xbzrle_cache->overflow);
     }
 
-    if (info->has_x_cpu_throttle_percentage) {
+    if (info->has_cpu_throttle_percentage) {
         monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
-                       info->x_cpu_throttle_percentage);
+                       info->cpu_throttle_percentage);
     }
 
     qapi_free_MigrationInfo(info);
@@ -281,11 +281,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
             MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
             params->decompress_threads);
         monitor_printf(mon, " %s: %" PRId64,
-            MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL],
-            params->x_cpu_throttle_initial);
+            MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL],
+            params->cpu_throttle_initial);
         monitor_printf(mon, " %s: %" PRId64,
-            MigrationParameter_lookup[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT],
-            params->x_cpu_throttle_increment);
+            MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT],
+            params->cpu_throttle_increment);
         monitor_printf(mon, "\n");
     }
 
@@ -1240,8 +1240,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     bool has_compress_level = false;
     bool has_compress_threads = false;
     bool has_decompress_threads = false;
-    bool has_x_cpu_throttle_initial = false;
-    bool has_x_cpu_throttle_increment = false;
+    bool has_cpu_throttle_initial = false;
+    bool has_cpu_throttle_increment = false;
     int i;
 
     for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
@@ -1256,18 +1256,18 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
             case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
                 has_decompress_threads = true;
                 break;
-            case MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL:
-                has_x_cpu_throttle_initial = true;
+            case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
+                has_cpu_throttle_initial = true;
                 break;
-            case MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT:
-                has_x_cpu_throttle_increment = true;
+            case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
+                has_cpu_throttle_increment = true;
                 break;
             }
             qmp_migrate_set_parameters(has_compress_level, value,
                                        has_compress_threads, value,
                                        has_decompress_threads, value,
-                                       has_x_cpu_throttle_initial, value,
-                                       has_x_cpu_throttle_increment, value,
+                                       has_cpu_throttle_initial, value,
+                                       has_cpu_throttle_increment, value,
                                        &err);
             break;
         }
diff --git a/migration/migration.c b/migration/migration.c
index c08d9a6..12dbf5b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -50,8 +50,8 @@
 /*0: means nocompress, 1: best speed, ... 9: best compress ratio */
 #define DEFAULT_MIGRATE_COMPRESS_LEVEL 1
 /* Define default autoconverge cpu throttle migration parameters */
-#define DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL 20
-#define DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT 10
+#define DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL 20
+#define DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT 10
 
 /* Migration XBZRLE default cache size */
 #define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
@@ -87,10 +87,10 @@ MigrationState *migrate_get_current(void)
                 DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
         .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
                 DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
-        .parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
-                DEFAULT_MIGRATE_X_CPU_THROTTLE_INITIAL,
-        .parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
-                DEFAULT_MIGRATE_X_CPU_THROTTLE_INCREMENT,
+        .parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL] =
+                DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
+        .parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT] =
+                DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
     };
 
     if (!once) {
@@ -521,10 +521,10 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
             s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
     params->decompress_threads =
             s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
-    params->x_cpu_throttle_initial =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
-    params->x_cpu_throttle_increment =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
+    params->cpu_throttle_initial =
+            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL];
+    params->cpu_throttle_increment =
+            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT];
 
     return params;
 }
@@ -607,8 +607,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         }
 
         if (cpu_throttle_active()) {
-            info->has_x_cpu_throttle_percentage = true;
-            info->x_cpu_throttle_percentage = cpu_throttle_get_percentage();
+            info->has_cpu_throttle_percentage = true;
+            info->cpu_throttle_percentage = cpu_throttle_get_percentage();
         }
 
         get_xbzrle_cache_stats(info);
@@ -718,10 +718,10 @@ void qmp_migrate_set_parameters(bool has_compress_level,
                                 int64_t compress_threads,
                                 bool has_decompress_threads,
                                 int64_t decompress_threads,
-                                bool has_x_cpu_throttle_initial,
-                                int64_t x_cpu_throttle_initial,
-                                bool has_x_cpu_throttle_increment,
-                                int64_t x_cpu_throttle_increment, Error **errp)
+                                bool has_cpu_throttle_initial,
+                                int64_t cpu_throttle_initial,
+                                bool has_cpu_throttle_increment,
+                                int64_t cpu_throttle_increment, Error **errp)
 {
     MigrationState *s = migrate_get_current();
 
@@ -744,16 +744,16 @@ void qmp_migrate_set_parameters(bool has_compress_level,
                    "is invalid, it should be in the range of 1 to 255");
         return;
     }
-    if (has_x_cpu_throttle_initial &&
-            (x_cpu_throttle_initial < 1 || x_cpu_throttle_initial > 99)) {
+    if (has_cpu_throttle_initial &&
+            (cpu_throttle_initial < 1 || cpu_throttle_initial > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "x_cpu_throttle_initial",
+                   "cpu_throttle_initial",
                    "an integer in the range of 1 to 99");
     }
-    if (has_x_cpu_throttle_increment &&
-            (x_cpu_throttle_increment < 1 || x_cpu_throttle_increment > 99)) {
+    if (has_cpu_throttle_increment &&
+            (cpu_throttle_increment < 1 || cpu_throttle_increment > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "x_cpu_throttle_increment",
+                   "cpu_throttle_increment",
                    "an integer in the range of 1 to 99");
     }
 
@@ -767,14 +767,14 @@ void qmp_migrate_set_parameters(bool has_compress_level,
         s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
                                                     decompress_threads;
     }
-    if (has_x_cpu_throttle_initial) {
-        s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL] =
-                                                    x_cpu_throttle_initial;
+    if (has_cpu_throttle_initial) {
+        s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL] =
+                                                    cpu_throttle_initial;
     }
 
-    if (has_x_cpu_throttle_increment) {
-        s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT] =
-                                                    x_cpu_throttle_increment;
+    if (has_cpu_throttle_increment) {
+        s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT] =
+                                                    cpu_throttle_increment;
     }
 }
 
diff --git a/migration/ram.c b/migration/ram.c
index 5e88080..d1ecb99 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -430,9 +430,9 @@ static void mig_throttle_guest_down(void)
 {
     MigrationState *s = migrate_get_current();
     uint64_t pct_initial =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
+            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL];
     uint64_t pct_icrement =
-            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
+            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT];
 
     /* We have not started throttling yet. Let's start it. */
     if (!cpu_throttle_active()) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 54634c4..9a322d1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -480,9 +480,9 @@
 #        may be expensive, but do not actually occur during the iterative
 #        migration rounds themselves. (since 1.6)
 #
-# @x-cpu-throttle-percentage: #optional percentage of time guest cpus are being
-#       throttled during auto-converge. This is only present when auto-converge
-#       has started throttling guest cpus. (Since 2.5)
+# @cpu-throttle-percentage: #optional percentage of time guest cpus are being
+#        throttled during auto-converge. This is only present when auto-converge
+#        has started throttling guest cpus. (Since 2.7)
 #
 # Since: 0.14.0
 ##
@@ -494,7 +494,7 @@
            '*expected-downtime': 'int',
            '*downtime': 'int',
            '*setup-time': 'int',
-           '*x-cpu-throttle-percentage': 'int'} }
+           '*cpu-throttle-percentage': 'int'} }
 
 ##
 # @query-migrate
@@ -605,18 +605,18 @@
 #          compression, so set the decompress-threads to the number about 1/4
 #          of compress-threads is adequate.
 #
-# @x-cpu-throttle-initial: Initial percentage of time guest cpus are throttled
-#                          when migration auto-converge is activated. The
-#                          default value is 20. (Since 2.5)
+# @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
+#                        when migration auto-converge is activated. The
+#                        default value is 20. (Since 2.7)
 #
-# @x-cpu-throttle-increment: throttle percentage increase each time
-#                            auto-converge detects that migration is not making
-#                            progress. The default value is 10. (Since 2.5)
+# @cpu-throttle-increment: throttle percentage increase each time
+#                          auto-converge detects that migration is not making
+#                          progress. The default value is 10. (Since 2.7)
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
   'data': ['compress-level', 'compress-threads', 'decompress-threads',
-           'x-cpu-throttle-initial', 'x-cpu-throttle-increment'] }
+           'cpu-throttle-initial', 'cpu-throttle-increment'] }
 
 #
 # @migrate-set-parameters
@@ -629,21 +629,21 @@
 #
 # @decompress-threads: decompression thread count
 #
-# @x-cpu-throttle-initial: Initial percentage of time guest cpus are throttled
-#                          when migration auto-converge is activated. The
-#                          default value is 20. (Since 2.5)
+# @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
+#                        when migration auto-converge is activated. The
+#                        default value is 20. (Since 2.7)
 #
-# @x-cpu-throttle-increment: throttle percentage increase each time
-#                            auto-converge detects that migration is not making
-#                            progress. The default value is 10. (Since 2.5)
+# @cpu-throttle-increment: throttle percentage increase each time
+#                          auto-converge detects that migration is not making
+#                          progress. The default value is 10. (Since 2.7)
 # Since: 2.4
 ##
 { 'command': 'migrate-set-parameters',
   'data': { '*compress-level': 'int',
             '*compress-threads': 'int',
             '*decompress-threads': 'int',
-            '*x-cpu-throttle-initial': 'int',
-            '*x-cpu-throttle-increment': 'int'} }
+            '*cpu-throttle-initial': 'int',
+            '*cpu-throttle-increment': 'int'} }
 
 #
 # @MigrationParameters
@@ -654,13 +654,13 @@
 #
 # @decompress-threads: decompression thread count
 #
-# @x-cpu-throttle-initial: Initial percentage of time guest cpus are throttled
-#                          when migration auto-converge is activated. The
-#                          default value is 20. (Since 2.5)
+# @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
+#                        when migration auto-converge is activated. The
+#                        default value is 20. (Since 2.7)
 #
-# @x-cpu-throttle-increment: throttle percentage increase each time
-#                            auto-converge detects that migration is not making
-#                            progress. The default value is 10. (Since 2.5)
+# @cpu-throttle-increment: throttle percentage increase each time
+#                          auto-converge detects that migration is not making
+#                          progress. The default value is 10. (Since 2.7)
 #
 # Since: 2.4
 ##
@@ -668,8 +668,8 @@
   'data': { 'compress-level': 'int',
             'compress-threads': 'int',
             'decompress-threads': 'int',
-            'x-cpu-throttle-initial': 'int',
-            'x-cpu-throttle-increment': 'int'} }
+            'cpu-throttle-initial': 'int',
+            'cpu-throttle-increment': 'int'} }
 ##
 # @query-migrate-parameters
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 94847e5..28801a2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3747,10 +3747,10 @@ Set migration parameters
 - "compress-level": set compression level during migration (json-int)
 - "compress-threads": set compression thread count for migration (json-int)
 - "decompress-threads": set decompression thread count for migration (json-int)
-- "x-cpu-throttle-initial": set initial percentage of time guest cpus are
-                           throttled for auto-converge (json-int)
-- "x-cpu-throttle-increment": set throttle increasing percentage for
-                             auto-converge (json-int)
+- "cpu-throttle-initial": set initial percentage of time guest cpus are
+                          throttled for auto-converge (json-int)
+- "cpu-throttle-increment": set throttle increasing percentage for
+                            auto-converge (json-int)
 
 Arguments:
 
@@ -3764,7 +3764,7 @@ EQMP
     {
         .name       = "migrate-set-parameters",
         .args_type  =
-            "compress-level:i?,compress-threads:i?,decompress-threads:i?,x-cpu-throttle-initial:i?,x-cpu-throttle-increment:i?",
+            "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?",
         .mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
     },
 SQMP
@@ -3777,10 +3777,10 @@ Query current migration parameters
          - "compress-level" : compression level value (json-int)
          - "compress-threads" : compression thread count value (json-int)
          - "decompress-threads" : decompression thread count value (json-int)
-         - "x-cpu-throttle-initial" : initial percentage of time guest cpus are
-                                      throttled (json-int)
-         - "x-cpu-throttle-increment" : throttle increasing percentage for
-                                        auto-converge (json-int)
+         - "cpu-throttle-initial" : initial percentage of time guest cpus are
+                                    throttled (json-int)
+         - "cpu-throttle-increment" : throttle increasing percentage for
+                                      auto-converge (json-int)
 
 Arguments:
 
@@ -3790,10 +3790,10 @@ Example:
 <- {
       "return": {
          "decompress-threads": 2,
-         "x-cpu-throttle-increment": 10,
+         "cpu-throttle-increment": 10,
          "compress-threads": 8,
          "compress-level": 1,
-         "x-cpu-throttle-initial": 20
+         "cpu-throttle-initial": 20
       }
    }
 
-- 
2.5.5

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

* [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
                   ` (2 preceding siblings ...)
  2016-05-23 17:19 ` [Qemu-devel] [PULL 3/5] migration: Promote improved autoconverge commands out of experimental state Amit Shah
@ 2016-05-23 17:19 ` Amit Shah
  2016-05-26  9:46   ` Greg Kurz
  2016-05-23 17:19 ` [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete Amit Shah
  2016-05-24 12:05 ` [Qemu-devel] [PULL 0/5] migration fixes Peter Maydell
  5 siblings, 1 reply; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

From: Greg Kurz <gkurz@linux.vnet.ibm.com>

QEMU has currently two ways to prevent migration to occur:
- migration blocker when it depends on runtime state
- VMStateDescription.unmigratable when migration is not supported at all

This patch gathers all the logic into a single function to be called from
both the savevm and the migrate paths.

This fixes a bug with 9p, at least, where savevm would succeed and the
following would happen in the guest after loadvm:

$ ls /host
ls: cannot access /host: Protocol error

With this patch:

(qemu) savevm foo
Migration is disabled when VirtFS export path '/' is mounted in the guest
using mount_tag 'host'

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <146239057139.11271.9011797645454781543.stgit@bahia.huguette.org>

[Update subject according to Paolo's suggestion - Amit]

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/migration.h |  1 +
 migration/migration.c         | 21 +++++++++++++++------
 migration/savevm.c            |  2 +-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index ac2c12c..9e36a97 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -210,6 +210,7 @@ int migrate_fd_close(MigrationState *s);
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 MigrationState *migrate_init(const MigrationParams *params);
+bool migration_is_blocked(Error **errp);
 bool migration_in_setup(MigrationState *);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
diff --git a/migration/migration.c b/migration/migration.c
index 12dbf5b..721d010 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -992,6 +992,20 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
     once = false;
 }
 
+bool migration_is_blocked(Error **errp)
+{
+    if (qemu_savevm_state_blocked(errp)) {
+        return true;
+    }
+
+    if (migration_blockers) {
+        *errp = error_copy(migration_blockers->data);
+        return true;
+    }
+
+    return false;
+}
+
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
                  Error **errp)
@@ -1014,12 +1028,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
-    if (qemu_savevm_state_blocked(errp)) {
-        return;
-    }
-
-    if (migration_blockers) {
-        *errp = error_copy(migration_blockers->data);
+    if (migration_is_blocked(errp)) {
         return;
     }
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 8546fdf..f8450fd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1170,7 +1170,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     MigrationState *ms = migrate_init(&params);
     ms->to_dst_file = f;
 
-    if (qemu_savevm_state_blocked(errp)) {
+    if (migration_is_blocked(errp)) {
         return -EINVAL;
     }
 
-- 
2.5.5

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

* [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
                   ` (3 preceding siblings ...)
  2016-05-23 17:19 ` [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present Amit Shah
@ 2016-05-23 17:19 ` Amit Shah
  2016-05-26  9:47   ` Greg Kurz
  2016-05-24 12:05 ` [Qemu-devel] [PULL 0/5] migration fixes Peter Maydell
  5 siblings, 1 reply; 9+ messages in thread
From: Amit Shah @ 2016-05-23 17:19 UTC (permalink / raw
  To: gkurz, Markus Armbruster, jjherne, Peter Maydell
  Cc: qemu list, Juan Quintela, Dr. David Alan Gilbert, Amit Shah

From: Greg Kurz <gkurz@linux.vnet.ibm.com>

We currently have an error path during migration that can cause
the source QEMU to abort:

migration_thread()
  migration_completion()
    runstate_is_running() ----------------> true if guest is running
    bdrv_inactivate_all() ----------------> inactivate images
    qemu_savevm_state_complete_precopy()
     ... qemu_fflush()
           socket_writev_buffer() --------> error because destination fails
         qemu_fflush() -------------------> set error on migration stream
  migration_completion() -----------------> set migrate state to FAILED
migration_thread() -----------------------> break migration loop
  vm_start() -----------------------------> restart guest with inactive
                                            images

and you get:

qemu-system-ppc64: socket_writev_buffer: Got err=104 for (32768/18446744073709551615)
qemu-system-ppc64: /home/greg/Work/qemu/qemu-master/block/io.c:1342:bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
Aborted (core dumped)

If we try postcopy with a similar scenario, we also get the writev error
message but QEMU leaves the guest paused because entered_postcopy is true.

We could possibly do the same with precopy and leave the guest paused.
But since the historical default for migration errors is to restart the
source, this patch adds a call to bdrv_invalidate_cache_all() instead.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Message-Id: <146357896785.6003.11983081732454362715.stgit@bahia.huguette.org>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/migration.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 721d010..f5327e8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1606,19 +1606,32 @@ static void migration_completion(MigrationState *s, int current_active_state,
         rp_error = await_return_path_close_on_source(s);
         trace_migration_completion_postcopy_end_after_rp(rp_error);
         if (rp_error) {
-            goto fail;
+            goto fail_invalidate;
         }
     }
 
     if (qemu_file_get_error(s->to_dst_file)) {
         trace_migration_completion_file_err();
-        goto fail;
+        goto fail_invalidate;
     }
 
     migrate_set_state(&s->state, current_active_state,
                       MIGRATION_STATUS_COMPLETED);
     return;
 
+fail_invalidate:
+    /* If not doing postcopy, vm_start() will be called: let's regain
+     * control on images.
+     */
+    if (s->state == MIGRATION_STATUS_ACTIVE) {
+        Error *local_err = NULL;
+
+        bdrv_invalidate_cache_all(&local_err);
+        if (local_err) {
+            error_report_err(local_err);
+        }
+    }
+
 fail:
     migrate_set_state(&s->state, current_active_state,
                       MIGRATION_STATUS_FAILED);
-- 
2.5.5

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

* Re: [Qemu-devel] [PULL 0/5] migration fixes
  2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
                   ` (4 preceding siblings ...)
  2016-05-23 17:19 ` [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete Amit Shah
@ 2016-05-24 12:05 ` Peter Maydell
  5 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2016-05-24 12:05 UTC (permalink / raw
  To: Amit Shah
  Cc: Greg Kurz, Markus Armbruster, jjherne, qemu list, Juan Quintela,
	Dr. David Alan Gilbert

On 23 May 2016 at 18:19, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit 65603e2fc18b48e6e55a3dd693669413141694ec:
>
>   tci: do not include exec/exec-all.h (2016-05-20 15:07:46 +0100)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-2.7-1
>
> for you to fetch changes up to fe904ea8242cbae2d7e69c052c754b8f5f1ba1d6:
>
>   migration: regain control of images when migration fails to complete (2016-05-23 22:19:36 +0530)
>
> ----------------------------------------------------------------
> migration fixes:
>
> - ensure src block devices continue fine after a failed migration
> - fail on migration blockers; helps 9p savevm/loadvm
> - move autoconverge commands out of experimental state
> - move the migration-specific qjson in migration/
>

Applied, thanks.

-- PMM

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

* Re: [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present
  2016-05-23 17:19 ` [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present Amit Shah
@ 2016-05-26  9:46   ` Greg Kurz
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kurz @ 2016-05-26  9:46 UTC (permalink / raw
  To: Amit Shah
  Cc: Markus Armbruster, jjherne, Peter Maydell, qemu list,
	Dr. David Alan Gilbert, Juan Quintela, qemu-stable, mdroth

On Mon, 23 May 2016 22:49:46 +0530
Amit Shah <amit.shah@redhat.com> wrote:

> From: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> QEMU has currently two ways to prevent migration to occur:
> - migration blocker when it depends on runtime state
> - VMStateDescription.unmigratable when migration is not supported at all
> 
> This patch gathers all the logic into a single function to be called from
> both the savevm and the migrate paths.
> 
> This fixes a bug with 9p, at least, where savevm would succeed and the
> following would happen in the guest after loadvm:
> 
> $ ls /host
> ls: cannot access /host: Protocol error
> 
> With this patch:
> 
> (qemu) savevm foo
> Migration is disabled when VirtFS export path '/' is mounted in the guest
> using mount_tag 'host'
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <146239057139.11271.9011797645454781543.stgit@bahia.huguette.org>
> 
> [Update subject according to Paolo's suggestion - Amit]
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---

FWIW this patch also fixes the issue in QEMU 2.5.

>  include/migration/migration.h |  1 +
>  migration/migration.c         | 21 +++++++++++++++------rtl8139
>  migration/savevm.c            |  2 +-
>  3 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index ac2c12c..9e36a97 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -210,6 +210,7 @@ int migrate_fd_close(MigrationState *s);
>  void add_migration_state_change_notifier(Notifier *notify);
>  void remove_migration_state_change_notifier(Notifier *notify);
>  MigrationState *migrate_init(const MigrationParams *params);
> +bool migration_is_blocked(Error **errp);
>  bool migration_in_setup(MigrationState *);
>  bool migration_has_finished(MigrationState *);
>  bool migration_has_failed(MigrationState *);
> diff --git a/migration/migration.c b/migration/migration.c
> index 12dbf5b..721d010 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -992,6 +992,20 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
>      once = false;
>  }
> 
> +bool migration_is_blocked(Error **errp)
> +{
> +    if (qemu_savevm_state_blocked(errp)) {
> +        return true;
> +    }
> +
> +    if (migration_blockers) {
> +        *errp = error_copy(migration_blockers->data);
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
>  void qmp_migrate(const char *uri, bool has_blk, bool blk,
>                   bool has_inc, bool inc, bool has_detach, bool detach,
>                   Error **errp)
> @@ -1014,12 +1028,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          return;
>      }
> 
> -    if (qemu_savevm_state_blocked(errp)) {
> -        return;
> -    }
> -
> -    if (migration_blockers) {
> -        *errp = error_copy(migration_blockers->data);
> +    if (migration_is_blocked(errp)) {
>          return;
>      }
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 8546fdf..f8450fd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1170,7 +1170,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>      MigrationState *ms = migrate_init(&params);
>      ms->to_dst_file = f;
> 
> -    if (qemu_savevm_state_blocked(errp)) {
> +    if (migration_is_blocked(errp)) {
>          return -EINVAL;
>      }
> 

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

* Re: [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete
  2016-05-23 17:19 ` [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete Amit Shah
@ 2016-05-26  9:47   ` Greg Kurz
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kurz @ 2016-05-26  9:47 UTC (permalink / raw
  To: Amit Shah
  Cc: Markus Armbruster, jjherne, Peter Maydell, qemu list,
	Dr. David Alan Gilbert, Juan Quintela, qemu-stable, mdroth

On Mon, 23 May 2016 22:49:47 +0530
Amit Shah <amit.shah@redhat.com> wrote:

> From: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> We currently have an error path during migration that can cause
> the source QEMU to abort:
> 
> migration_thread()
>   migration_completion()
>     runstate_is_running() ----------------> true if guest is running
>     bdrv_inactivate_all() ----------------> inactivate images
>     qemu_savevm_state_complete_precopy()
>      ... qemu_fflush()
>            socket_writev_buffer() --------> error because destination fails
>          qemu_fflush() -------------------> set error on migration stream
>   migration_completion() -----------------> set migrate state to FAILED
> migration_thread() -----------------------> break migration loop
>   vm_start() -----------------------------> restart guest with inactive
>                                             images
> 
> and you get:
> 
> qemu-system-ppc64: socket_writev_buffer: Got err=104 for (32768/18446744073709551615)
> qemu-system-ppc64: /home/greg/Work/qemu/qemu-master/block/io.c:1342:bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> Aborted (core dumped)
> 
> If we try postcopy with a similar scenario, we also get the writev error
> message but QEMU leaves the guest paused because entered_postcopy is true.
> 
> We could possibly do the same with precopy and leave the guest paused.
> But since the historical default for migration errors is to restart the
> source, this patch adds a call to bdrv_invalidate_cache_all() instead.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Message-Id: <146357896785.6003.11983081732454362715.stgit@bahia.huguette.org>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---

FWIW this patch also fixes the issue in QEMU 2.5.

>  migration/migration.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 721d010..f5327e8 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1606,19 +1606,32 @@ static void migration_completion(MigrationState *s, int current_active_state,
>          rp_error = await_return_path_close_on_source(s);
>          trace_migration_completion_postcopy_end_after_rp(rp_error);
>          if (rp_error) {
> -            goto fail;
> +            goto fail_invalidate;
>          }
>      }
> 
>      if (qemu_file_get_error(s->to_dst_file)) {
>          trace_migration_completion_file_err();
> -        goto fail;
> +        goto fail_invalidate;
>      }
> 
>      migrate_set_state(&s->state, current_active_state,
>                        MIGRATION_STATUS_COMPLETED);
>      return;
> 
> +fail_invalidate:
> +    /* If not doing postcopy, vm_start() will be called: let's regain
> +     * control on images.
> +     */
> +    if (s->state == MIGRATION_STATUS_ACTIVE) {
> +        Error *local_err = NULL;
> +
> +        bdrv_invalidate_cache_all(&local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +        }
> +    }
> +
>  fail:
>      migrate_set_state(&s->state, current_active_state,
>                        MIGRATION_STATUS_FAILED);

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

end of thread, other threads:[~2016-05-26  9:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-23 17:19 [Qemu-devel] [PULL 0/5] migration fixes Amit Shah
2016-05-23 17:19 ` [Qemu-devel] [PULL 1/5] migration: Move qjson.[ch] to migration/ Amit Shah
2016-05-23 17:19 ` [Qemu-devel] [PULL 2/5] migration/qjson: Drop gratuitous use of QOM Amit Shah
2016-05-23 17:19 ` [Qemu-devel] [PULL 3/5] migration: Promote improved autoconverge commands out of experimental state Amit Shah
2016-05-23 17:19 ` [Qemu-devel] [PULL 4/5] savevm: fail if migration blockers are present Amit Shah
2016-05-26  9:46   ` Greg Kurz
2016-05-23 17:19 ` [Qemu-devel] [PULL 5/5] migration: regain control of images when migration fails to complete Amit Shah
2016-05-26  9:47   ` Greg Kurz
2016-05-24 12:05 ` [Qemu-devel] [PULL 0/5] migration fixes Peter Maydell

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.