From: Olaf Hering <olaf@aepfle.de> To: xen-devel@lists.xenproject.org Cc: "Anthony PERARD" <anthony.perard@citrix.com>, "Wei Liu" <wei.liu2@citrix.com>, "Olaf Hering" <olaf@aepfle.de>, "Ian Jackson" <ian.jackson@eu.citrix.com>, "Roger Pau Monné" <roger.pau@citrix.com> Subject: [PATCH v5] libxl: fix migration of PV and PVH domUs with and without qemu Date: Tue, 14 May 2019 10:05:58 +0200 [thread overview] Message-ID: <20190514080558.14540-1-olaf@aepfle.de> (raw) If a domU has a qemu-xen instance attached, it is required to call qemus "xen-save-devices-state" method. Without it, the receiving side of a PV or PVH migration may be unable to lock the image: xen be: qdisk-51712: xen be: qdisk-51712: error: Failed to get "write" lock error: Failed to get "write" lock xen be: qdisk-51712: xen be: qdisk-51712: initialise() failed initialise() failed To fix this bug, libxl__domain_suspend_device_model() and libxl__domain_resume_device_model() have to be called not only for HVM, but also if the active device_model is QEMU_XEN. Unfortunately, libxl__domain_build_info_setdefault() used to hardcode b_info->device_model_version to QEMU_XEN if it does not know it any better. As a result libxl__device_model_version_running() will return incorrect values. This breaks domUs without a device_model. libxl__qmp_stop() would wait 10 seconds in qmp_open() for a qemu that will never appear. During this long timeframe the domU remains in state paused on the sending side. As a result network connections may be dropped. Once this bug is fixed as well, by just removing the assumption that every domU has a QEMU_XEN, there is no code to actually initialise b_info->device_model_version. There is a helper function libxl__need_xenpv_qemu(), which is used in various places to decide if a device_model has to be spawned. This function can not be used as is, just to fill device_model_version, because store_libxl_entry() was already called earlier. Introduce LIBXL_DEVICE_MODEL_VERSION_NONE for PV and PVH that have no need for a device_model to make the state explicit. Indicate this new state via LIBXL_HAVE macro in libxl.h. v05: - move initialization of device_model_version to extra commit - return error from libxl__need_xenpv_qemu - add LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE v04: - make sure device_model_stubdomain is initialized v03: - rearrange code to make sure device_model_version is initialized before store_libxl_entry() is called v02: - update wording in a comment - remove stale goto in domcreate_launch_dm - initialize ret in libxl__need_xenpv_qemu Signed-off-by: Olaf Hering <olaf@aepfle.de> Cc: Roger Pau Monné <roger.pau@citrix.com> Cc: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl.h | 7 +++++++ tools/libxl/libxl_create.c | 17 ++++++++++++++--- tools/libxl/libxl_dom_suspend.c | 8 ++++++-- tools/libxl/libxl_types.idl | 1 + 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 482499a6c0..e0f6916b66 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1182,6 +1182,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src); */ #define LIBXL_HAVE_PVCALLS 1 +/* + * LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE + * + * If this is defined, libxl will only run a device-model if required. + */ +#define LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE 1 + typedef char **libxl_string_list; void libxl_string_list_dispose(libxl_string_list *sl); int libxl_string_list_length(const libxl_string_list *sl); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 3f0431cc84..64336b0d29 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -47,9 +47,20 @@ int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config) } break; default: - b_info->device_model_version = - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL; - break; + ret = libxl__need_xenpv_qemu(gc, d_config); + switch (ret) { + case 1: + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; + break; + case 0: + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_NONE; + break; + default: + LOGE(ERROR, "Unable to determine QEMU requisite"); + return ret; + } } if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c index d1af3a6573..c492fe5dd1 100644 --- a/tools/libxl/libxl_dom_suspend.c +++ b/tools/libxl/libxl_dom_suspend.c @@ -379,7 +379,9 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc, libxl__ev_xswatch_deregister(gc, &dsps->guest_watch); libxl__ev_time_deregister(gc, &dsps->guest_timeout); - if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) { + if (dsps->type == LIBXL_DOMAIN_TYPE_HVM || + libxl__device_model_version_running(gc, dsps->domid) == + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { dsps->callback_device_model_done = domain_suspend_common_done; libxl__domain_suspend_device_model(egc, dsps); /* must be last */ return; @@ -459,7 +461,9 @@ int libxl__domain_resume(libxl__gc *gc, uint32_t domid, int suspend_cancel) goto out; } - if (type == LIBXL_DOMAIN_TYPE_HVM) { + if (type == LIBXL_DOMAIN_TYPE_HVM || + libxl__device_model_version_running(gc, domid) == + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { rc = libxl__domain_resume_device_model(gc, domid); if (rc) { LOGD(ERROR, domid, "failed to resume device model:%d", rc); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index cb4702fd7a..75bde095bc 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -106,6 +106,7 @@ libxl_device_model_version = Enumeration("device_model_version", [ (0, "UNKNOWN"), (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm) (2, "QEMU_XEN"), # Upstream based qemu-xen device model + (3, "NONE"), ]) libxl_console_type = Enumeration("console_type", [ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Olaf Hering <olaf@aepfle.de> To: xen-devel@lists.xenproject.org Cc: "Anthony PERARD" <anthony.perard@citrix.com>, "Wei Liu" <wei.liu2@citrix.com>, "Olaf Hering" <olaf@aepfle.de>, "Ian Jackson" <ian.jackson@eu.citrix.com>, "Roger Pau Monné" <roger.pau@citrix.com> Subject: [Xen-devel] [PATCH v5] libxl: fix migration of PV and PVH domUs with and without qemu Date: Tue, 14 May 2019 10:05:58 +0200 [thread overview] Message-ID: <20190514080558.14540-1-olaf@aepfle.de> (raw) Message-ID: <20190514080558.5h97a59c8Uc4dRnBD0-xX9WSF1RpGWhofku27XyN66s@z> (raw) If a domU has a qemu-xen instance attached, it is required to call qemus "xen-save-devices-state" method. Without it, the receiving side of a PV or PVH migration may be unable to lock the image: xen be: qdisk-51712: xen be: qdisk-51712: error: Failed to get "write" lock error: Failed to get "write" lock xen be: qdisk-51712: xen be: qdisk-51712: initialise() failed initialise() failed To fix this bug, libxl__domain_suspend_device_model() and libxl__domain_resume_device_model() have to be called not only for HVM, but also if the active device_model is QEMU_XEN. Unfortunately, libxl__domain_build_info_setdefault() used to hardcode b_info->device_model_version to QEMU_XEN if it does not know it any better. As a result libxl__device_model_version_running() will return incorrect values. This breaks domUs without a device_model. libxl__qmp_stop() would wait 10 seconds in qmp_open() for a qemu that will never appear. During this long timeframe the domU remains in state paused on the sending side. As a result network connections may be dropped. Once this bug is fixed as well, by just removing the assumption that every domU has a QEMU_XEN, there is no code to actually initialise b_info->device_model_version. There is a helper function libxl__need_xenpv_qemu(), which is used in various places to decide if a device_model has to be spawned. This function can not be used as is, just to fill device_model_version, because store_libxl_entry() was already called earlier. Introduce LIBXL_DEVICE_MODEL_VERSION_NONE for PV and PVH that have no need for a device_model to make the state explicit. Indicate this new state via LIBXL_HAVE macro in libxl.h. v05: - move initialization of device_model_version to extra commit - return error from libxl__need_xenpv_qemu - add LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE v04: - make sure device_model_stubdomain is initialized v03: - rearrange code to make sure device_model_version is initialized before store_libxl_entry() is called v02: - update wording in a comment - remove stale goto in domcreate_launch_dm - initialize ret in libxl__need_xenpv_qemu Signed-off-by: Olaf Hering <olaf@aepfle.de> Cc: Roger Pau Monné <roger.pau@citrix.com> Cc: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl.h | 7 +++++++ tools/libxl/libxl_create.c | 17 ++++++++++++++--- tools/libxl/libxl_dom_suspend.c | 8 ++++++-- tools/libxl/libxl_types.idl | 1 + 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 482499a6c0..e0f6916b66 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1182,6 +1182,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src); */ #define LIBXL_HAVE_PVCALLS 1 +/* + * LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE + * + * If this is defined, libxl will only run a device-model if required. + */ +#define LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE 1 + typedef char **libxl_string_list; void libxl_string_list_dispose(libxl_string_list *sl); int libxl_string_list_length(const libxl_string_list *sl); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 3f0431cc84..64336b0d29 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -47,9 +47,20 @@ int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config) } break; default: - b_info->device_model_version = - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL; - break; + ret = libxl__need_xenpv_qemu(gc, d_config); + switch (ret) { + case 1: + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; + break; + case 0: + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_NONE; + break; + default: + LOGE(ERROR, "Unable to determine QEMU requisite"); + return ret; + } } if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c index d1af3a6573..c492fe5dd1 100644 --- a/tools/libxl/libxl_dom_suspend.c +++ b/tools/libxl/libxl_dom_suspend.c @@ -379,7 +379,9 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc, libxl__ev_xswatch_deregister(gc, &dsps->guest_watch); libxl__ev_time_deregister(gc, &dsps->guest_timeout); - if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) { + if (dsps->type == LIBXL_DOMAIN_TYPE_HVM || + libxl__device_model_version_running(gc, dsps->domid) == + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { dsps->callback_device_model_done = domain_suspend_common_done; libxl__domain_suspend_device_model(egc, dsps); /* must be last */ return; @@ -459,7 +461,9 @@ int libxl__domain_resume(libxl__gc *gc, uint32_t domid, int suspend_cancel) goto out; } - if (type == LIBXL_DOMAIN_TYPE_HVM) { + if (type == LIBXL_DOMAIN_TYPE_HVM || + libxl__device_model_version_running(gc, domid) == + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { rc = libxl__domain_resume_device_model(gc, domid); if (rc) { LOGD(ERROR, domid, "failed to resume device model:%d", rc); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index cb4702fd7a..75bde095bc 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -106,6 +106,7 @@ libxl_device_model_version = Enumeration("device_model_version", [ (0, "UNKNOWN"), (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm) (2, "QEMU_XEN"), # Upstream based qemu-xen device model + (3, "NONE"), ]) libxl_console_type = Enumeration("console_type", [ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next reply other threads:[~2019-05-14 8:06 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-14 8:05 Olaf Hering [this message] 2019-05-14 8:05 ` [Xen-devel] [PATCH v5] libxl: fix migration of PV and PVH domUs with and without qemu Olaf Hering 2019-05-14 8:14 ` Olaf Hering 2019-05-14 8:14 ` [Xen-devel] " Olaf Hering 2019-05-14 14:38 ` Wei Liu 2019-05-14 14:38 ` [Xen-devel] " Wei Liu 2019-05-14 14:44 ` Olaf Hering 2019-05-14 14:44 ` [Xen-devel] " Olaf Hering 2019-05-14 15:42 ` Wei Liu 2019-05-14 15:42 ` [Xen-devel] " Wei Liu 2019-05-14 11:19 ` Roger Pau Monné 2019-05-14 11:19 ` [Xen-devel] " Roger Pau Monné
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190514080558.14540-1-olaf@aepfle.de \ --to=olaf@aepfle.de \ --cc=anthony.perard@citrix.com \ --cc=ian.jackson@eu.citrix.com \ --cc=roger.pau@citrix.com \ --cc=wei.liu2@citrix.com \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).