* [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
@ 2019-05-10 15:20 Olaf Hering
2019-05-10 15:20 ` [Xen-devel] " Olaf Hering
2019-05-13 14:37 ` Roger Pau Monné
0 siblings, 2 replies; 10+ messages in thread
From: Olaf Hering @ 2019-05-10 15:20 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Wei Liu, Olaf Hering, Ian Jackson, Roger Pau Monné
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() hardcodes
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.
Create a new function to set device_model_version. Move existing code
from libxl__domain_build_info_setdefault() to cover the HVM case. Add
new code to cover non-HVM case, use libxl__need_xenpv_qemu() to set
device_model_version.
Move also initialization for device_model_stubdomain to the new function.
Update libxl__spawn_stub_dm() and initiate_domain_create() to call the
new function prior libxl__domain_build_info_setdefault() because
device_mode_version is expected to be initialzed.
libxl_domain_need_memory() needs no update because it does not have a
d_config available anyway, and the callers provide a populated b_info.
Introduce LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED for PV and PVH that
have no need for a device_model to make the state explicit.
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_create.c | 99 ++++++++++++++++++++++++++---------------
tools/libxl/libxl_dm.c | 2 +
tools/libxl/libxl_dom_suspend.c | 8 +++-
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_types.idl | 1 +
5 files changed, 75 insertions(+), 37 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 89fe80fc9c..bc4613a296 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -27,6 +27,64 @@
#include <xen-xsm/flask/flask.h>
+int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config)
+{
+ libxl_domain_build_info *b_info = &d_config->b_info;
+ int ret;
+
+ libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
+
+ if (b_info->device_model_version)
+ return 0;
+
+ switch (b_info->type) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+ b_info->device_model_version =
+ LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ } else {
+ b_info->device_model_version = libxl__default_device_model(gc);
+ }
+ break;
+ default:
+ 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_REQUIRED;
+ break;
+ default:
+ LOGE(ERROR, "Unable to determine QEMU requisite");
+ return ERROR_FAIL;
+ }
+ }
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+ const char *dm;
+
+ dm = libxl__domain_device_model(gc, b_info);
+ ret = access(dm, X_OK);
+ if (ret < 0) {
+ /* qemu-xen unavailable, use qemu-xen-traditional */
+ if (errno == ENOENT) {
+ LOGE(INFO, "qemu-xen is unavailable"
+ ", using qemu-xen-traditional instead");
+ b_info->device_model_version =
+ LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ } else {
+ LOGE(ERROR, "qemu-xen access error");
+ return ERROR_FAIL;
+ }
+ }
+ }
+
+ return 0;
+}
+
int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info)
{
@@ -80,45 +138,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
return rc;
}
- libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
-
if (libxl_defbool_val(b_info->device_model_stubdomain) &&
!b_info->device_model_ssidref)
b_info->device_model_ssidref = SECINITSID_DOMDM;
- if (!b_info->device_model_version) {
- if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- if (libxl_defbool_val(b_info->device_model_stubdomain)) {
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- } else {
- b_info->device_model_version = libxl__default_device_model(gc);
- }
- } else {
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
- }
- if (b_info->device_model_version
- == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
- const char *dm;
-
- dm = libxl__domain_device_model(gc, b_info);
- rc = access(dm, X_OK);
- if (rc < 0) {
- /* qemu-xen unavailable, use qemu-xen-traditional */
- if (errno == ENOENT) {
- LOGE(INFO, "qemu-xen is unavailable"
- ", using qemu-xen-traditional instead");
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- } else {
- LOGE(ERROR, "qemu-xen access error");
- return ERROR_FAIL;
- }
- }
- }
- }
-
if (b_info->blkdev_start == NULL)
b_info->blkdev_start = libxl__strdup(NOGC, "xvda");
@@ -938,6 +961,12 @@ static void initiate_domain_create(libxl__egc *egc,
goto error_out;
}
+ ret = libxl__domain_set_device_model(gc, d_config);
+ if (ret) {
+ LOGD(ERROR, domid, "Unable to set domain device model");
+ goto error_out;
+ }
+
ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
if (ret) {
LOGD(ERROR, domid, "Unable to set domain create info defaults");
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2f19786bdd..086e566311 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2168,6 +2168,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->c_info.run_hotplug_scripts =
guest_config->c_info.run_hotplug_scripts;
+ ret = libxl__domain_set_device_model(gc, dm_config);
+ if (ret) goto out;
ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
if (ret) goto out;
ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
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_internal.h b/tools/libxl/libxl_internal.h
index 44e0221284..25f113404b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1374,6 +1374,8 @@ _hidden int libxl__device_nextid(libxl__gc *gc, uint32_t domid,
_hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
uint32_t *domid);
+_hidden int libxl__domain_set_device_model(libxl__gc *gc,
+ libxl_domain_config *d_config);
/*
* For each aggregate type which can be used as an input we provide:
*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cb4702fd7a..7d75bd3850 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_REQUIRED"),
])
libxl_console_type = Enumeration("console_type", [
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-10 15:20 [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu Olaf Hering
@ 2019-05-10 15:20 ` Olaf Hering
2019-05-13 14:37 ` Roger Pau Monné
1 sibling, 0 replies; 10+ messages in thread
From: Olaf Hering @ 2019-05-10 15:20 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Wei Liu, Olaf Hering, Ian Jackson, Roger Pau Monné
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() hardcodes
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.
Create a new function to set device_model_version. Move existing code
from libxl__domain_build_info_setdefault() to cover the HVM case. Add
new code to cover non-HVM case, use libxl__need_xenpv_qemu() to set
device_model_version.
Move also initialization for device_model_stubdomain to the new function.
Update libxl__spawn_stub_dm() and initiate_domain_create() to call the
new function prior libxl__domain_build_info_setdefault() because
device_mode_version is expected to be initialzed.
libxl_domain_need_memory() needs no update because it does not have a
d_config available anyway, and the callers provide a populated b_info.
Introduce LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED for PV and PVH that
have no need for a device_model to make the state explicit.
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_create.c | 99 ++++++++++++++++++++++++++---------------
tools/libxl/libxl_dm.c | 2 +
tools/libxl/libxl_dom_suspend.c | 8 +++-
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_types.idl | 1 +
5 files changed, 75 insertions(+), 37 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 89fe80fc9c..bc4613a296 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -27,6 +27,64 @@
#include <xen-xsm/flask/flask.h>
+int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config)
+{
+ libxl_domain_build_info *b_info = &d_config->b_info;
+ int ret;
+
+ libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
+
+ if (b_info->device_model_version)
+ return 0;
+
+ switch (b_info->type) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+ b_info->device_model_version =
+ LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ } else {
+ b_info->device_model_version = libxl__default_device_model(gc);
+ }
+ break;
+ default:
+ 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_REQUIRED;
+ break;
+ default:
+ LOGE(ERROR, "Unable to determine QEMU requisite");
+ return ERROR_FAIL;
+ }
+ }
+
+ if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+ const char *dm;
+
+ dm = libxl__domain_device_model(gc, b_info);
+ ret = access(dm, X_OK);
+ if (ret < 0) {
+ /* qemu-xen unavailable, use qemu-xen-traditional */
+ if (errno == ENOENT) {
+ LOGE(INFO, "qemu-xen is unavailable"
+ ", using qemu-xen-traditional instead");
+ b_info->device_model_version =
+ LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ } else {
+ LOGE(ERROR, "qemu-xen access error");
+ return ERROR_FAIL;
+ }
+ }
+ }
+
+ return 0;
+}
+
int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info)
{
@@ -80,45 +138,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
return rc;
}
- libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
-
if (libxl_defbool_val(b_info->device_model_stubdomain) &&
!b_info->device_model_ssidref)
b_info->device_model_ssidref = SECINITSID_DOMDM;
- if (!b_info->device_model_version) {
- if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- if (libxl_defbool_val(b_info->device_model_stubdomain)) {
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- } else {
- b_info->device_model_version = libxl__default_device_model(gc);
- }
- } else {
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
- }
- if (b_info->device_model_version
- == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
- const char *dm;
-
- dm = libxl__domain_device_model(gc, b_info);
- rc = access(dm, X_OK);
- if (rc < 0) {
- /* qemu-xen unavailable, use qemu-xen-traditional */
- if (errno == ENOENT) {
- LOGE(INFO, "qemu-xen is unavailable"
- ", using qemu-xen-traditional instead");
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- } else {
- LOGE(ERROR, "qemu-xen access error");
- return ERROR_FAIL;
- }
- }
- }
- }
-
if (b_info->blkdev_start == NULL)
b_info->blkdev_start = libxl__strdup(NOGC, "xvda");
@@ -938,6 +961,12 @@ static void initiate_domain_create(libxl__egc *egc,
goto error_out;
}
+ ret = libxl__domain_set_device_model(gc, d_config);
+ if (ret) {
+ LOGD(ERROR, domid, "Unable to set domain device model");
+ goto error_out;
+ }
+
ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
if (ret) {
LOGD(ERROR, domid, "Unable to set domain create info defaults");
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 2f19786bdd..086e566311 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2168,6 +2168,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->c_info.run_hotplug_scripts =
guest_config->c_info.run_hotplug_scripts;
+ ret = libxl__domain_set_device_model(gc, dm_config);
+ if (ret) goto out;
ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
if (ret) goto out;
ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
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_internal.h b/tools/libxl/libxl_internal.h
index 44e0221284..25f113404b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1374,6 +1374,8 @@ _hidden int libxl__device_nextid(libxl__gc *gc, uint32_t domid,
_hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
uint32_t *domid);
+_hidden int libxl__domain_set_device_model(libxl__gc *gc,
+ libxl_domain_config *d_config);
/*
* For each aggregate type which can be used as an input we provide:
*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cb4702fd7a..7d75bd3850 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_REQUIRED"),
])
libxl_console_type = Enumeration("console_type", [
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-10 15:20 [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu Olaf Hering
2019-05-10 15:20 ` [Xen-devel] " Olaf Hering
@ 2019-05-13 14:37 ` Roger Pau Monné
2019-05-13 14:37 ` [Xen-devel] " Roger Pau Monné
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Roger Pau Monné @ 2019-05-13 14:37 UTC (permalink / raw)
To: Olaf Hering; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
On Fri, May 10, 2019 at 05:20:47PM +0200, Olaf Hering wrote:
> 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() hardcodes
> 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.
>
> Create a new function to set device_model_version. Move existing code
> from libxl__domain_build_info_setdefault() to cover the HVM case. Add
> new code to cover non-HVM case, use libxl__need_xenpv_qemu() to set
> device_model_version.
> Move also initialization for device_model_stubdomain to the new function.
>
> Update libxl__spawn_stub_dm() and initiate_domain_create() to call the
> new function prior libxl__domain_build_info_setdefault() because
> device_mode_version is expected to be initialzed.
> libxl_domain_need_memory() needs no update because it does not have a
> d_config available anyway, and the callers provide a populated b_info.
>
> Introduce LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED for PV and PVH that
> have no need for a device_model to make the state explicit.
>
> 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>
LGTM:
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> tools/libxl/libxl_create.c | 99 ++++++++++++++++++++++++++---------------
> tools/libxl/libxl_dm.c | 2 +
> tools/libxl/libxl_dom_suspend.c | 8 +++-
> tools/libxl/libxl_internal.h | 2 +
> tools/libxl/libxl_types.idl | 1 +
> 5 files changed, 75 insertions(+), 37 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 89fe80fc9c..bc4613a296 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -27,6 +27,64 @@
>
> #include <xen-xsm/flask/flask.h>
>
> +int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config)
> +{
> + libxl_domain_build_info *b_info = &d_config->b_info;
> + int ret;
> +
> + libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
> +
> + if (b_info->device_model_version)
> + return 0;
> +
> + switch (b_info->type) {
> + case LIBXL_DOMAIN_TYPE_HVM:
> + if (libxl_defbool_val(b_info->device_model_stubdomain)) {
> + b_info->device_model_version =
> + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + } else {
> + b_info->device_model_version = libxl__default_device_model(gc);
> + }
> + break;
> + default:
> + 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_REQUIRED;
> + break;
> + default:
> + LOGE(ERROR, "Unable to determine QEMU requisite");
> + return ERROR_FAIL;
You could just return ret and propagate the error from the previous
function.
> + }
> + }
> +
> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> + const char *dm;
> +
> + dm = libxl__domain_device_model(gc, b_info);
> + ret = access(dm, X_OK);
> + if (ret < 0) {
> + /* qemu-xen unavailable, use qemu-xen-traditional */
> + if (errno == ENOENT) {
> + LOGE(INFO, "qemu-xen is unavailable"
> + ", using qemu-xen-traditional instead");
> + b_info->device_model_version =
> + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + } else {
> + LOGE(ERROR, "qemu-xen access error");
> + return ERROR_FAIL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> int libxl__domain_create_info_setdefault(libxl__gc *gc,
> libxl_domain_create_info *c_info)
> {
> @@ -80,45 +138,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> return rc;
> }
>
> - libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
> -
> if (libxl_defbool_val(b_info->device_model_stubdomain) &&
> !b_info->device_model_ssidref)
> b_info->device_model_ssidref = SECINITSID_DOMDM;
>
> - if (!b_info->device_model_version) {
> - if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> - if (libxl_defbool_val(b_info->device_model_stubdomain)) {
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> - } else {
> - b_info->device_model_version = libxl__default_device_model(gc);
> - }
> - } else {
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> - }
> - if (b_info->device_model_version
> - == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> - const char *dm;
> -
> - dm = libxl__domain_device_model(gc, b_info);
> - rc = access(dm, X_OK);
> - if (rc < 0) {
> - /* qemu-xen unavailable, use qemu-xen-traditional */
> - if (errno == ENOENT) {
> - LOGE(INFO, "qemu-xen is unavailable"
> - ", using qemu-xen-traditional instead");
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> - } else {
> - LOGE(ERROR, "qemu-xen access error");
> - return ERROR_FAIL;
> - }
> - }
> - }
> - }
FTR I would have preferred a pre-patch that did the move of this chunk
of code into libxl__domain_set_device_model without any functional
change, and then a second patch that introduces the new functionality.
> -
> if (b_info->blkdev_start == NULL)
> b_info->blkdev_start = libxl__strdup(NOGC, "xvda");
>
> @@ -938,6 +961,12 @@ static void initiate_domain_create(libxl__egc *egc,
> goto error_out;
> }
>
> + ret = libxl__domain_set_device_model(gc, d_config);
> + if (ret) {
> + LOGD(ERROR, domid, "Unable to set domain device model");
> + goto error_out;
> + }
> +
> ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
> if (ret) {
> LOGD(ERROR, domid, "Unable to set domain create info defaults");
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 2f19786bdd..086e566311 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2168,6 +2168,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
> dm_config->c_info.run_hotplug_scripts =
> guest_config->c_info.run_hotplug_scripts;
>
> + ret = libxl__domain_set_device_model(gc, dm_config);
> + if (ret) goto out;
> ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
> if (ret) goto out;
> ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
> 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_internal.h b/tools/libxl/libxl_internal.h
> index 44e0221284..25f113404b 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1374,6 +1374,8 @@ _hidden int libxl__device_nextid(libxl__gc *gc, uint32_t domid,
> _hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
> uint32_t *domid);
>
> +_hidden int libxl__domain_set_device_model(libxl__gc *gc,
> + libxl_domain_config *d_config);
> /*
> * For each aggregate type which can be used as an input we provide:
> *
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index cb4702fd7a..7d75bd3850 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_REQUIRED"),
Plain "NONE" would also be fine here IMO.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:37 ` Roger Pau Monné
@ 2019-05-13 14:37 ` Roger Pau Monné
2019-05-13 14:42 ` Wei Liu
2019-05-13 14:45 ` Olaf Hering
2 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monné @ 2019-05-13 14:37 UTC (permalink / raw)
To: Olaf Hering; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
On Fri, May 10, 2019 at 05:20:47PM +0200, Olaf Hering wrote:
> 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() hardcodes
> 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.
>
> Create a new function to set device_model_version. Move existing code
> from libxl__domain_build_info_setdefault() to cover the HVM case. Add
> new code to cover non-HVM case, use libxl__need_xenpv_qemu() to set
> device_model_version.
> Move also initialization for device_model_stubdomain to the new function.
>
> Update libxl__spawn_stub_dm() and initiate_domain_create() to call the
> new function prior libxl__domain_build_info_setdefault() because
> device_mode_version is expected to be initialzed.
> libxl_domain_need_memory() needs no update because it does not have a
> d_config available anyway, and the callers provide a populated b_info.
>
> Introduce LIBXL_DEVICE_MODEL_VERSION_NONE_REQUIRED for PV and PVH that
> have no need for a device_model to make the state explicit.
>
> 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>
LGTM:
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> tools/libxl/libxl_create.c | 99 ++++++++++++++++++++++++++---------------
> tools/libxl/libxl_dm.c | 2 +
> tools/libxl/libxl_dom_suspend.c | 8 +++-
> tools/libxl/libxl_internal.h | 2 +
> tools/libxl/libxl_types.idl | 1 +
> 5 files changed, 75 insertions(+), 37 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 89fe80fc9c..bc4613a296 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -27,6 +27,64 @@
>
> #include <xen-xsm/flask/flask.h>
>
> +int libxl__domain_set_device_model(libxl__gc *gc, libxl_domain_config *d_config)
> +{
> + libxl_domain_build_info *b_info = &d_config->b_info;
> + int ret;
> +
> + libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
> +
> + if (b_info->device_model_version)
> + return 0;
> +
> + switch (b_info->type) {
> + case LIBXL_DOMAIN_TYPE_HVM:
> + if (libxl_defbool_val(b_info->device_model_stubdomain)) {
> + b_info->device_model_version =
> + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + } else {
> + b_info->device_model_version = libxl__default_device_model(gc);
> + }
> + break;
> + default:
> + 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_REQUIRED;
> + break;
> + default:
> + LOGE(ERROR, "Unable to determine QEMU requisite");
> + return ERROR_FAIL;
You could just return ret and propagate the error from the previous
function.
> + }
> + }
> +
> + if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> + const char *dm;
> +
> + dm = libxl__domain_device_model(gc, b_info);
> + ret = access(dm, X_OK);
> + if (ret < 0) {
> + /* qemu-xen unavailable, use qemu-xen-traditional */
> + if (errno == ENOENT) {
> + LOGE(INFO, "qemu-xen is unavailable"
> + ", using qemu-xen-traditional instead");
> + b_info->device_model_version =
> + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + } else {
> + LOGE(ERROR, "qemu-xen access error");
> + return ERROR_FAIL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> int libxl__domain_create_info_setdefault(libxl__gc *gc,
> libxl_domain_create_info *c_info)
> {
> @@ -80,45 +138,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> return rc;
> }
>
> - libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
> -
> if (libxl_defbool_val(b_info->device_model_stubdomain) &&
> !b_info->device_model_ssidref)
> b_info->device_model_ssidref = SECINITSID_DOMDM;
>
> - if (!b_info->device_model_version) {
> - if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> - if (libxl_defbool_val(b_info->device_model_stubdomain)) {
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> - } else {
> - b_info->device_model_version = libxl__default_device_model(gc);
> - }
> - } else {
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> - }
> - if (b_info->device_model_version
> - == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> - const char *dm;
> -
> - dm = libxl__domain_device_model(gc, b_info);
> - rc = access(dm, X_OK);
> - if (rc < 0) {
> - /* qemu-xen unavailable, use qemu-xen-traditional */
> - if (errno == ENOENT) {
> - LOGE(INFO, "qemu-xen is unavailable"
> - ", using qemu-xen-traditional instead");
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> - } else {
> - LOGE(ERROR, "qemu-xen access error");
> - return ERROR_FAIL;
> - }
> - }
> - }
> - }
FTR I would have preferred a pre-patch that did the move of this chunk
of code into libxl__domain_set_device_model without any functional
change, and then a second patch that introduces the new functionality.
> -
> if (b_info->blkdev_start == NULL)
> b_info->blkdev_start = libxl__strdup(NOGC, "xvda");
>
> @@ -938,6 +961,12 @@ static void initiate_domain_create(libxl__egc *egc,
> goto error_out;
> }
>
> + ret = libxl__domain_set_device_model(gc, d_config);
> + if (ret) {
> + LOGD(ERROR, domid, "Unable to set domain device model");
> + goto error_out;
> + }
> +
> ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
> if (ret) {
> LOGD(ERROR, domid, "Unable to set domain create info defaults");
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 2f19786bdd..086e566311 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2168,6 +2168,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
> dm_config->c_info.run_hotplug_scripts =
> guest_config->c_info.run_hotplug_scripts;
>
> + ret = libxl__domain_set_device_model(gc, dm_config);
> + if (ret) goto out;
> ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info);
> if (ret) goto out;
> ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
> 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_internal.h b/tools/libxl/libxl_internal.h
> index 44e0221284..25f113404b 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1374,6 +1374,8 @@ _hidden int libxl__device_nextid(libxl__gc *gc, uint32_t domid,
> _hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
> uint32_t *domid);
>
> +_hidden int libxl__domain_set_device_model(libxl__gc *gc,
> + libxl_domain_config *d_config);
> /*
> * For each aggregate type which can be used as an input we provide:
> *
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index cb4702fd7a..7d75bd3850 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_REQUIRED"),
Plain "NONE" would also be fine here IMO.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:37 ` Roger Pau Monné
2019-05-13 14:37 ` [Xen-devel] " Roger Pau Monné
@ 2019-05-13 14:42 ` Wei Liu
2019-05-13 14:42 ` [Xen-devel] " Wei Liu
2019-05-13 14:45 ` Olaf Hering
2 siblings, 1 reply; 10+ messages in thread
From: Wei Liu @ 2019-05-13 14:42 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Anthony PERARD, xen-devel, Olaf Hering, Ian Jackson, Wei Liu
On Mon, May 13, 2019 at 04:37:33PM +0200, Roger Pau Monné wrote:
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index cb4702fd7a..7d75bd3850 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_REQUIRED"),
>
> Plain "NONE" would also be fine here IMO.
Also please add a LIBXL_HAVE macro to libxl.h
Roger, thanks for reviewing this patch.
Wei.
>
> Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:42 ` Wei Liu
@ 2019-05-13 14:42 ` Wei Liu
0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2019-05-13 14:42 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Anthony PERARD, xen-devel, Olaf Hering, Ian Jackson, Wei Liu
On Mon, May 13, 2019 at 04:37:33PM +0200, Roger Pau Monné wrote:
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index cb4702fd7a..7d75bd3850 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_REQUIRED"),
>
> Plain "NONE" would also be fine here IMO.
Also please add a LIBXL_HAVE macro to libxl.h
Roger, thanks for reviewing this patch.
Wei.
>
> Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:37 ` Roger Pau Monné
2019-05-13 14:37 ` [Xen-devel] " Roger Pau Monné
2019-05-13 14:42 ` Wei Liu
@ 2019-05-13 14:45 ` Olaf Hering
2019-05-13 14:45 ` [Xen-devel] " Olaf Hering
2019-05-13 15:00 ` Roger Pau Monné
2 siblings, 2 replies; 10+ messages in thread
From: Olaf Hering @ 2019-05-13 14:45 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
[-- Attachment #1.1: Type: text/plain, Size: 503 bytes --]
Am Mon, 13 May 2019 16:37:33 +0200
schrieb Roger Pau Monné <roger.pau@citrix.com>:
> FTR I would have preferred a pre-patch that did the move of this chunk
> of code into libxl__domain_set_device_model without any functional
> change, and then a second patch that introduces the new functionality.
If that needs to be done, I can do it.
What could be done in addition is to add an assert to
libxl__domain_build_info_setdefault to check if device_model_version
is really set.
Olaf
[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:45 ` Olaf Hering
@ 2019-05-13 14:45 ` Olaf Hering
2019-05-13 15:00 ` Roger Pau Monné
1 sibling, 0 replies; 10+ messages in thread
From: Olaf Hering @ 2019-05-13 14:45 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
[-- Attachment #1.1: Type: text/plain, Size: 503 bytes --]
Am Mon, 13 May 2019 16:37:33 +0200
schrieb Roger Pau Monné <roger.pau@citrix.com>:
> FTR I would have preferred a pre-patch that did the move of this chunk
> of code into libxl__domain_set_device_model without any functional
> change, and then a second patch that introduces the new functionality.
If that needs to be done, I can do it.
What could be done in addition is to add an assert to
libxl__domain_build_info_setdefault to check if device_model_version
is really set.
Olaf
[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 14:45 ` Olaf Hering
2019-05-13 14:45 ` [Xen-devel] " Olaf Hering
@ 2019-05-13 15:00 ` Roger Pau Monné
2019-05-13 15:00 ` [Xen-devel] " Roger Pau Monné
1 sibling, 1 reply; 10+ messages in thread
From: Roger Pau Monné @ 2019-05-13 15:00 UTC (permalink / raw)
To: Olaf Hering; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
On Mon, May 13, 2019 at 04:45:21PM +0200, Olaf Hering wrote:
> Am Mon, 13 May 2019 16:37:33 +0200
> schrieb Roger Pau Monné <roger.pau@citrix.com>:
>
> > FTR I would have preferred a pre-patch that did the move of this chunk
> > of code into libxl__domain_set_device_model without any functional
> > change, and then a second patch that introduces the new functionality.
>
> If that needs to be done, I can do it.
Iff you have to resend anyway, and it's not too much fuss then I would
say yes.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu
2019-05-13 15:00 ` Roger Pau Monné
@ 2019-05-13 15:00 ` Roger Pau Monné
0 siblings, 0 replies; 10+ messages in thread
From: Roger Pau Monné @ 2019-05-13 15:00 UTC (permalink / raw)
To: Olaf Hering; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu
On Mon, May 13, 2019 at 04:45:21PM +0200, Olaf Hering wrote:
> Am Mon, 13 May 2019 16:37:33 +0200
> schrieb Roger Pau Monné <roger.pau@citrix.com>:
>
> > FTR I would have preferred a pre-patch that did the move of this chunk
> > of code into libxl__domain_set_device_model without any functional
> > change, and then a second patch that introduces the new functionality.
>
> If that needs to be done, I can do it.
Iff you have to resend anyway, and it's not too much fuss then I would
say yes.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-13 15:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10 15:20 [PATCH v4] libxl: fix migration of PV and PVH domUs with and without qemu Olaf Hering
2019-05-10 15:20 ` [Xen-devel] " Olaf Hering
2019-05-13 14:37 ` Roger Pau Monné
2019-05-13 14:37 ` [Xen-devel] " Roger Pau Monné
2019-05-13 14:42 ` Wei Liu
2019-05-13 14:42 ` [Xen-devel] " Wei Liu
2019-05-13 14:45 ` Olaf Hering
2019-05-13 14:45 ` [Xen-devel] " Olaf Hering
2019-05-13 15:00 ` Roger Pau Monné
2019-05-13 15:00 ` [Xen-devel] " Roger Pau Monné
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).