* [PATCH v2] libxl: use qemu-xen (upstream QEMU) as device model by default
@ 2012-12-04 12:58 Stefano Stabellini
2013-01-24 11:43 ` Ian Campbell
0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2012-12-04 12:58 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Stefano Stabellini
Changes in v2:
- update the xl man page;
- write a small helper function in libxl_{linux,netbsd}.c to set the
default device_model, so that NetBSD can keep using the old version.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index fe4fac9..69a38b9 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1132,15 +1132,15 @@ guest. Valid values are:
=over 4
-=item B<qemu-xen-traditional>
+=item B<qemu-xen>
-Use the device-model based upon the historical Xen fork of Qemu. This
-device-model is currently the default.
+use the device-model merged into the upstream QEMU project.
+This device-model is the default for Linux dom0.
-=item B<qemu-xen>
+=item B<qemu-xen-traditional>
-use the device-model merged into the upstream QEMU project. This
-device-model will become the default in a future version of Xen.
+Use the device-model based upon the historical Xen fork of Qemu.
+This device-model is still the default for NetBSD dom0.
=back
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9d20086..6ec543a 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -143,8 +143,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (!b_info->device_model_version) {
if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ b_info->device_model_version = libxl__default_device_model(gc);
else {
const char *dm;
int rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index cba3616..0ea11d1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1557,6 +1557,10 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
/* Based on /local/domain/$domid/dm-version xenstore key
* default is qemu xen traditional */
_hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
+ /* Return the system-wide default device model:
+ * qemu-xen for Linux, qemu-xen-traditional for NetBSD.
+ */
+_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
/* Check how executes hotplug script currently */
int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 1fed3cd..409e9f2 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -266,3 +266,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
out:
return rc;
}
+
+libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 9587833..aa04969 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -94,3 +94,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
out:
return rc;
}
+
+libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] libxl: use qemu-xen (upstream QEMU) as device model by default
2012-12-04 12:58 [PATCH v2] libxl: use qemu-xen (upstream QEMU) as device model by default Stefano Stabellini
@ 2013-01-24 11:43 ` Ian Campbell
2013-03-01 17:41 ` [PATCH v3 0/2] libxl: change to qemu-xen (upstream) " Ian Jackson
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Ian Campbell @ 2013-01-24 11:43 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Ian Jackson
On Tue, 2012-12-04 at 12:58 +0000, Stefano Stabellini wrote:
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index fe4fac9..69a38b9 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1132,15 +1132,15 @@ guest. Valid values are:
>
> =over 4
>
> -=item B<qemu-xen-traditional>
> +=item B<qemu-xen>
>
> -Use the device-model based upon the historical Xen fork of Qemu. This
> -device-model is currently the default.
> +use the device-model merged into the upstream QEMU project.
Can you correct to "Use" as you move it please.
> +This device-model is the default for Linux dom0.
>
> -=item B<qemu-xen>
> +=item B<qemu-xen-traditional>
>
> -use the device-model merged into the upstream QEMU project. This
> -device-model will become the default in a future version of Xen.
> +Use the device-model based upon the historical Xen fork of Qemu.
> +This device-model is still the default for NetBSD dom0.
Does that look like changing for 4.3?
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 9d20086..6ec543a 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -143,8 +143,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>
> if (!b_info->device_model_version) {
> if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + b_info->device_model_version = libxl__default_device_model(gc);
For PV we stat() the executable before using it -- I don't remember why
but should we be doing the same here?
Also, will this not break things for people with
"device_model_stubdomain_override = 1" (but nothing else) in their
configuration?
I think the logic needs to be (perhaps in Linux's
libxl__default_device_model(gc) only)
if (libxl_defbool_val(b_info->device_model_stubdomain)
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
else
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
? At least until we have upstream stubdoms in some form.
> else {
> const char *dm;
> int rc;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index cba3616..0ea11d1 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1557,6 +1557,10 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
> /* Based on /local/domain/$domid/dm-version xenstore key
> * default is qemu xen traditional */
> _hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
> + /* Return the system-wide default device model:
> + * qemu-xen for Linux, qemu-xen-traditional for NetBSD.
I bet you someone forgets to update this when NetBSD changes ;-)
> + */
> +_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
>
> /* Check how executes hotplug script currently */
> int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 0/2] libxl: change to qemu-xen (upstream) by default
2013-01-24 11:43 ` Ian Campbell
@ 2013-03-01 17:41 ` Ian Jackson
2013-03-01 17:41 ` [PATCH 1/2] libxl: move check for existence of qemuu device model Ian Jackson
2013-03-01 17:41 ` [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default Ian Jackson
2 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2013-03-01 17:41 UTC (permalink / raw)
To: xen-devel
Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: use qemu-xen (upstream QEMU) as device model by default"):
> On Tue, 2012-12-04 at 12:58 +0000, Stefano Stabellini wrote:
> > -Use the device-model based upon the historical Xen fork of Qemu. This
> > -device-model is currently the default.
> > +use the device-model merged into the upstream QEMU project.
>
> Can you correct to "Use" as you move it please.
Fixed.
> > -use the device-model merged into the upstream QEMU project. This
> > -device-model will become the default in a future version of Xen.
> > +Use the device-model based upon the historical Xen fork of Qemu.
> > +This device-model is still the default for NetBSD dom0.
>
> Does that look like changing for 4.3?
I guess not but I don't think that should block this change.
> > - b_info->device_model_version =
> > - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> > + b_info->device_model_version = libxl__default_device_model(gc);
>
> For PV we stat() the executable before using it -- I don't remember why
> but should we be doing the same here?
Presumably in case the upstream build was disabled. I think the same
logic applies so I have arranged to do the same.
> I think the logic needs to be (perhaps in Linux's
> libxl__default_device_model(gc) only)
> if (libxl_defbool_val(b_info->device_model_stubdomain)
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> else
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
Yes, done.
> > + /* Return the system-wide default device model:
> > + * qemu-xen for Linux, qemu-xen-traditional for NetBSD.
>
> I bet you someone forgets to update this when NetBSD changes ;-)
That remark just needs to go IMO.
So we now have:
[PATCH 1/2] libxl: move check for existence of qemuu device model
[PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-01-24 11:43 ` Ian Campbell
2013-03-01 17:41 ` [PATCH v3 0/2] libxl: change to qemu-xen (upstream) " Ian Jackson
@ 2013-03-01 17:41 ` Ian Jackson
2013-03-02 3:56 ` Ian Campbell
2013-03-01 17:41 ` [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default Ian Jackson
2 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2013-03-01 17:41 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
The stat in libxl__domain_build_info_setdefault's default device model
logic works to fall back to qemu-xen-traditional whenever the
executable for qemu-xen is not found.
We are going to use qemu-xen-traditional in more cases, so break this
check out into its own if statement.
Also add a pair of braces to make the if() statement symmetrical.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
--
New in v3 of series.
---
tools/libxl/libxl_create.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index efeebf2..04bf4a5 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -105,22 +105,25 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
if (!b_info->device_model_version) {
- if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
+ if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
b_info->device_model_version =
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- else {
+ } 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;
int rc;
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
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) {
LIBXL__LOG_ERRNO(CTX, XTL_VERBOSE, "qemu-xen is unavailable"
- ", use qemu-xen-traditional instead");
+ ", use qemu-xen-traditional instead");
b_info->device_model_version =
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
} else {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default
2013-01-24 11:43 ` Ian Campbell
2013-03-01 17:41 ` [PATCH v3 0/2] libxl: change to qemu-xen (upstream) " Ian Jackson
2013-03-01 17:41 ` [PATCH 1/2] libxl: move check for existence of qemuu device model Ian Jackson
@ 2013-03-01 17:41 ` Ian Jackson
2013-03-03 5:07 ` Stefano Stabellini
2 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2013-03-01 17:41 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Stefano Stabellini
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
--
Changes in v3:
- Fix "use" to "Use" in documentation part
- Do not try to use qemu-xen (upstream) for stubdoms
- Remove a hostage-to-fortune remark in a comment about OS defaults
Changes in v2:
- update the xl man page;
- write a small helper function in libxl_{linux,netbsd}.c to set the
default device_model, so that NetBSD can keep using the old version.
---
docs/man/xl.cfg.pod.5 | 12 ++++++------
tools/libxl/libxl_create.c | 10 +++++++---
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_linux.c | 5 +++++
tools/libxl/libxl_netbsd.c | 5 +++++
5 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 25523c9..8db24d7 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1176,15 +1176,15 @@ guest. Valid values are:
=over 4
-=item B<qemu-xen-traditional>
+=item B<qemu-xen>
-Use the device-model based upon the historical Xen fork of Qemu. This
-device-model is currently the default.
+Use the device-model merged into the upstream QEMU project.
+This device-model is the default for Linux dom0.
-=item B<qemu-xen>
+=item B<qemu-xen-traditional>
-use the device-model merged into the upstream QEMU project. This
-device-model will become the default in a future version of Xen.
+Use the device-model based upon the historical Xen fork of Qemu.
+This device-model is still the default for NetBSD dom0.
=back
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 04bf4a5..7ec8c2b 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -105,9 +105,13 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
if (!b_info->device_model_version) {
- if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- b_info->device_model_version =
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
+ if (libxl_defbool_val(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;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1567b4b..25172f0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1561,6 +1561,8 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
/* Based on /local/domain/$domid/dm-version xenstore key
* default is qemu xen traditional */
_hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
+ /* Return the system-wide default device model */
+_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
/* Check how executes hotplug script currently */
int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 1fed3cd..409e9f2 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -266,3 +266,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
out:
return rc;
}
+
+libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 9587833..aa04969 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -94,3 +94,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
out:
return rc;
}
+
+libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-01 17:41 ` [PATCH 1/2] libxl: move check for existence of qemuu device model Ian Jackson
@ 2013-03-02 3:56 ` Ian Campbell
2013-03-04 11:42 ` Ian Jackson
0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-03-02 3:56 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Fri, 2013-03-01 at 17:41 +0000, Ian Jackson wrote:
> + if (b_info->device_model_version
> + == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> const char *dm;
> int rc;
>
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> 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) {
> LIBXL__LOG_ERRNO(CTX, XTL_VERBOSE, "qemu-xen is unavailable"
> - ", use qemu-xen-traditional instead");
> + ", use qemu-xen-traditional instead");
> b_info->device_model_version =
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
After the following change (in 2/2) won't this stat potentially be
checking for the qemu-xen-trad binaries, which makes the error message
somewhat confusing and also makes the fallback unhelpful.
On a related note we could do with stating the device model even in the
case where the user has specified device_model_version explicitly, to
give better error reporting in that case too.
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default
2013-03-01 17:41 ` [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default Ian Jackson
@ 2013-03-03 5:07 ` Stefano Stabellini
2013-03-13 15:51 ` Ian Campbell
0 siblings, 1 reply; 17+ messages in thread
From: Stefano Stabellini @ 2013-03-03 5:07 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini
On Fri, 1 Mar 2013, Ian Jackson wrote:
> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> --
> Changes in v3:
> - Fix "use" to "Use" in documentation part
> - Do not try to use qemu-xen (upstream) for stubdoms
> - Remove a hostage-to-fortune remark in a comment about OS defaults
>
> Changes in v2:
> - update the xl man page;
> - write a small helper function in libxl_{linux,netbsd}.c to set the
> default device_model, so that NetBSD can keep using the old version.
it looks OK to me, thanks for picking it up
> docs/man/xl.cfg.pod.5 | 12 ++++++------
> tools/libxl/libxl_create.c | 10 +++++++---
> tools/libxl/libxl_internal.h | 2 ++
> tools/libxl/libxl_linux.c | 5 +++++
> tools/libxl/libxl_netbsd.c | 5 +++++
> 5 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 25523c9..8db24d7 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1176,15 +1176,15 @@ guest. Valid values are:
>
> =over 4
>
> -=item B<qemu-xen-traditional>
> +=item B<qemu-xen>
>
> -Use the device-model based upon the historical Xen fork of Qemu. This
> -device-model is currently the default.
> +Use the device-model merged into the upstream QEMU project.
> +This device-model is the default for Linux dom0.
>
> -=item B<qemu-xen>
> +=item B<qemu-xen-traditional>
>
> -use the device-model merged into the upstream QEMU project. This
> -device-model will become the default in a future version of Xen.
> +Use the device-model based upon the historical Xen fork of Qemu.
> +This device-model is still the default for NetBSD dom0.
>
> =back
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 04bf4a5..7ec8c2b 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -105,9 +105,13 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
>
> if (!b_info->device_model_version) {
> - if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> - b_info->device_model_version =
> - LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> + if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
> + if (libxl_defbool_val(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;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 1567b4b..25172f0 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1561,6 +1561,8 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
> /* Based on /local/domain/$domid/dm-version xenstore key
> * default is qemu xen traditional */
> _hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
> + /* Return the system-wide default device model */
> +_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
>
> /* Check how executes hotplug script currently */
> int libxl__hotplug_settings(libxl__gc *gc, xs_transaction_t t);
> diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
> index 1fed3cd..409e9f2 100644
> --- a/tools/libxl/libxl_linux.c
> +++ b/tools/libxl/libxl_linux.c
> @@ -266,3 +266,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
> out:
> return rc;
> }
> +
> +libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
> +{
> + return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
> +}
> diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
> index 9587833..aa04969 100644
> --- a/tools/libxl/libxl_netbsd.c
> +++ b/tools/libxl/libxl_netbsd.c
> @@ -94,3 +94,8 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
> out:
> return rc;
> }
> +
> +libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
> +{
> + return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> +}
> --
> 1.7.2.5
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-02 3:56 ` Ian Campbell
@ 2013-03-04 11:42 ` Ian Jackson
2013-03-05 4:03 ` Ian Campbell
0 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2013-03-04 11:42 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> On Fri, 2013-03-01 at 17:41 +0000, Ian Jackson wrote:
> > + if (b_info->device_model_version
> > + == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > const char *dm;
> > int rc;
...
> > rc = access(dm, X_OK);
> > if (rc < 0) {
> > /* qemu-xen unavailable, use qemu-xen-traditional */
...
> After the following change (in 2/2) won't this stat potentially be
> checking for the qemu-xen-trad binaries, which makes the error message
> somewhat confusing and also makes the fallback unhelpful.
No, because the whole thing is in an if which applies only if we're
trying to use ..._QEMU_XEN.
> On a related note we could do with stating the device model even in the
> case where the user has specified device_model_version explicitly, to
> give better error reporting in that case too.
You mean logging it ?
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-04 11:42 ` Ian Jackson
@ 2013-03-05 4:03 ` Ian Campbell
2013-03-05 11:22 ` Ian Jackson
2013-03-13 15:51 ` Ian Campbell
0 siblings, 2 replies; 17+ messages in thread
From: Ian Campbell @ 2013-03-05 4:03 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Mon, 2013-03-04 at 11:42 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> > On Fri, 2013-03-01 at 17:41 +0000, Ian Jackson wrote:
> > > + if (b_info->device_model_version
> > > + == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > > const char *dm;
> > > int rc;
> ...
> > > rc = access(dm, X_OK);
> > > if (rc < 0) {
> > > /* qemu-xen unavailable, use qemu-xen-traditional */
> ...
> > After the following change (in 2/2) won't this stat potentially be
> > checking for the qemu-xen-trad binaries, which makes the error message
> > somewhat confusing and also makes the fallback unhelpful.
>
> No, because the whole thing is in an if which applies only if we're
> trying to use ..._QEMU_XEN.
Oh, right. I combined the tow patches wrongly in my head.
Don't we also want to check for the presence of the trad binary though?
> > On a related note we could do with stating the device model even in the
> > case where the user has specified device_model_version explicitly, to
> > give better error reporting in that case too.
>
> You mean logging it ?
Actually I meant "not trying to start the guest regardless and returning
success", which AIUI is what happens... Logging would be nice too
though.
>
> Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 4:03 ` Ian Campbell
@ 2013-03-05 11:22 ` Ian Jackson
2013-03-05 16:03 ` Ian Campbell
2013-03-13 15:51 ` Ian Campbell
1 sibling, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2013-03-05 11:22 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Stefano.Stabellini
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> On Mon, 2013-03-04 at 11:42 +0000, Ian Jackson wrote:
> > No, because the whole thing is in an if which applies only if we're
> > trying to use ..._QEMU_XEN.
>
> Oh, right. I combined the tow patches wrongly in my head.
>
> Don't we also want to check for the presence of the trad binary though?
AIUI the trad binary is always built ? AIUI there is a possibility
that the new one isn't ? And that was the rationale for this check ?
CCing Stefano.
> > > On a related note we could do with stating the device model even in the
> > > case where the user has specified device_model_version explicitly, to
> > > give better error reporting in that case too.
> >
> > You mean logging it ?
>
> Actually I meant "not trying to start the guest regardless and returning
> success", which AIUI is what happens... Logging would be nice too
> though.
Oh I see. TBH once we start to think about these kinds of
installation problems it's difficult to see where we'll stop.
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 11:22 ` Ian Jackson
@ 2013-03-05 16:03 ` Ian Campbell
2013-03-05 16:10 ` Ian Jackson
0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-03-05 16:03 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini
On Tue, 2013-03-05 at 11:22 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> > On Mon, 2013-03-04 at 11:42 +0000, Ian Jackson wrote:
> > > No, because the whole thing is in an if which applies only if we're
> > > trying to use ..._QEMU_XEN.
> >
> > Oh, right. I combined the tow patches wrongly in my head.
> >
> > Don't we also want to check for the presence of the trad binary though?
>
> AIUI the trad binary is always built ?
By us it is, but e.g. Debian's packaging has been known to omit it, or
the user may have overridden the path but made a typo.
> AIUI there is a possibility
> that the new one isn't ? And that was the rationale for this check ?
>
> CCing Stefano.
>
> > > > On a related note we could do with stating the device model even in the
> > > > case where the user has specified device_model_version explicitly, to
> > > > give better error reporting in that case too.
> > >
> > > You mean logging it ?
> >
> > Actually I meant "not trying to start the guest regardless and returning
> > success", which AIUI is what happens... Logging would be nice too
> > though.
>
> Oh I see. TBH once we start to think about these kinds of
> installation problems it's difficult to see where we'll stop.
This is one which has been tripping people up in practice.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 16:03 ` Ian Campbell
@ 2013-03-05 16:10 ` Ian Jackson
2013-03-05 16:30 ` Ian Campbell
0 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2013-03-05 16:10 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Stefano Stabellini
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> On Tue, 2013-03-05 at 11:22 +0000, Ian Jackson wrote:
> > Oh I see. TBH once we start to think about these kinds of
> > installation problems it's difficult to see where we'll stop.
>
> This is one which has been tripping people up in practice.
OK then.
Should we stat whichever one we intend to use and fall back to the
other with a warning, then ?
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 16:10 ` Ian Jackson
@ 2013-03-05 16:30 ` Ian Campbell
2013-03-05 17:09 ` Ian Jackson
0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-03-05 16:30 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Stefano Stabellini
On Tue, 2013-03-05 at 16:10 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> > On Tue, 2013-03-05 at 11:22 +0000, Ian Jackson wrote:
> > > Oh I see. TBH once we start to think about these kinds of
> > > installation problems it's difficult to see where we'll stop.
> >
> > This is one which has been tripping people up in practice.
>
> OK then.
>
> Should we stat whichever one we intend to use and fall back to the
> other with a warning, then ?
>
Could do that if libxl is the one doing the choosing, I suppose. I'd
also be happy with failing explicitly with an appropriate message.
In the case where the user gave an explicit path or version we should
obviously use it or fail.
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 16:30 ` Ian Campbell
@ 2013-03-05 17:09 ` Ian Jackson
0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2013-03-05 17:09 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Stefano Stabellini
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> Could do that if libxl is the one doing the choosing, I suppose. I'd
> also be happy with failing explicitly with an appropriate message.
Right.
> In the case where the user gave an explicit path or version we should
> obviously use it or fail.
Indeed.
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] libxl: move check for existence of qemuu device model
2013-03-05 4:03 ` Ian Campbell
2013-03-05 11:22 ` Ian Jackson
@ 2013-03-13 15:51 ` Ian Campbell
1 sibling, 0 replies; 17+ messages in thread
From: Ian Campbell @ 2013-03-13 15:51 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Tue, 2013-03-05 at 04:03 +0000, Ian Campbell wrote:
> On Mon, 2013-03-04 at 11:42 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: move check for existence of qemuu device model"):
> > > On Fri, 2013-03-01 at 17:41 +0000, Ian Jackson wrote:
> > > > + if (b_info->device_model_version
> > > > + == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> > > > const char *dm;
> > > > int rc;
> > ...
> > > > rc = access(dm, X_OK);
> > > > if (rc < 0) {
> > > > /* qemu-xen unavailable, use qemu-xen-traditional */
> > ...
> > > After the following change (in 2/2) won't this stat potentially be
> > > checking for the qemu-xen-trad binaries, which makes the error message
> > > somewhat confusing and also makes the fallback unhelpful.
> >
> > No, because the whole thing is in an if which applies only if we're
> > trying to use ..._QEMU_XEN.
>
> Oh, right. I combined the tow patches wrongly in my head.
So I think this patch is good as far as it goes and the rest of this
thread is really followup material.
So:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Don't we also want to check for the presence of the trad binary though?
>
> > > On a related note we could do with stating the device model even in the
> > > case where the user has specified device_model_version explicitly, to
> > > give better error reporting in that case too.
> >
> > You mean logging it ?
>
> Actually I meant "not trying to start the guest regardless and returning
> success", which AIUI is what happens... Logging would be nice too
> though.
>
> >
> > Ian.
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default
2013-03-03 5:07 ` Stefano Stabellini
@ 2013-03-13 15:51 ` Ian Campbell
2013-03-13 16:00 ` Ian Jackson
0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2013-03-13 15:51 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Ian Jackson
On Sun, 2013-03-03 at 05:07 +0000, Stefano Stabellini wrote:
> On Fri, 1 Mar 2013, Ian Jackson wrote:
> > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >
> > --
> > Changes in v3:
> > - Fix "use" to "Use" in documentation part
> > - Do not try to use qemu-xen (upstream) for stubdoms
> > - Remove a hostage-to-fortune remark in a comment about OS defaults
> >
> > Changes in v2:
> > - update the xl man page;
> > - write a small helper function in libxl_{linux,netbsd}.c to set the
> > default device_model, so that NetBSD can keep using the old version.
>
> it looks OK to me, thanks for picking it up
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default
2013-03-13 15:51 ` Ian Campbell
@ 2013-03-13 16:00 ` Ian Jackson
0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2013-03-13 16:00 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Stefano Stabellini
Ian Campbell writes ("Re: [Xen-devel] [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default"):
> On Sun, 2013-03-03 at 05:07 +0000, Stefano Stabellini wrote:
> > it looks OK to me, thanks for picking it up
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks, pushed both to staging.
Ian.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-03-13 16:00 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-04 12:58 [PATCH v2] libxl: use qemu-xen (upstream QEMU) as device model by default Stefano Stabellini
2013-01-24 11:43 ` Ian Campbell
2013-03-01 17:41 ` [PATCH v3 0/2] libxl: change to qemu-xen (upstream) " Ian Jackson
2013-03-01 17:41 ` [PATCH 1/2] libxl: move check for existence of qemuu device model Ian Jackson
2013-03-02 3:56 ` Ian Campbell
2013-03-04 11:42 ` Ian Jackson
2013-03-05 4:03 ` Ian Campbell
2013-03-05 11:22 ` Ian Jackson
2013-03-05 16:03 ` Ian Campbell
2013-03-05 16:10 ` Ian Jackson
2013-03-05 16:30 ` Ian Campbell
2013-03-05 17:09 ` Ian Jackson
2013-03-13 15:51 ` Ian Campbell
2013-03-01 17:41 ` [PATCH 2/2] libxl: use qemu-xen (upstream QEMU) as device model by default Ian Jackson
2013-03-03 5:07 ` Stefano Stabellini
2013-03-13 15:51 ` Ian Campbell
2013-03-13 16:00 ` Ian Jackson
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.