[libvirt] [PATCH] libxl: allow an <emulator> to be selected in the domain config XML

The emulator path supplied can be any valid path on the system. Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either ...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory. Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..2aa5a62 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -788,6 +788,46 @@ libxlMakeCapabilities(libxl_ctx *ctx) } int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + if (!virFileExists(def->emulator)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("emulator '%s' not found"), + def->emulator); + return -1; + } + + VIR_FREE(d_config->b_info.device_model); + if ((d_config->b_info.device_model = strdup(def->emulator)) == NULL) { + virReportOOMError(); + return -1; + } + + /* N.B. from xen/tools/libxl/libxl_types.idl: + * "If setting device_model you must set device_model_version too." + * + * The xen-4.3 and later default is "upstream qemu" (QEMU_XEN) + * so we make that the default and special-case the old-style + * "traditional qemu" (QEMU_XEN_TRADITIONAL) + */ + + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; + + if (STREQ(basename(def->emulator), "qemu-dm")) + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL; + + return 0; +} + + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) { @@ -811,6 +851,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, goto error; } + if (libxlMakeEmulator(def, d_config) < 0) { + goto error; + } + d_config->on_reboot = def->onReboot; d_config->on_poweroff = def->onPoweroff; d_config->on_crash = def->onCrash; -- 1.7.1

David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm? Regards, Jim
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..2aa5a62 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -788,6 +788,46 @@ libxlMakeCapabilities(libxl_ctx *ctx) }
int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + if (!virFileExists(def->emulator)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("emulator '%s' not found"), + def->emulator); + return -1; + } + + VIR_FREE(d_config->b_info.device_model); + if ((d_config->b_info.device_model = strdup(def->emulator)) == NULL) { + virReportOOMError(); + return -1; + } + + /* N.B. from xen/tools/libxl/libxl_types.idl: + * "If setting device_model you must set device_model_version too." + * + * The xen-4.3 and later default is "upstream qemu" (QEMU_XEN) + * so we make that the default and special-case the old-style + * "traditional qemu" (QEMU_XEN_TRADITIONAL) + */ + + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; + + if (STREQ(basename(def->emulator), "qemu-dm")) + d_config->b_info.device_model_version = + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL; + + return 0; +} + + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) { @@ -811,6 +851,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, goto error; }
+ if (libxlMakeEmulator(def, d_config) < 0) { + goto error; + } + d_config->on_reboot = def->onReboot; d_config->on_poweroff = def->onPoweroff; d_config->on_crash = def->onCrash;

Hi, [added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.] On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think? The other options I can think of are: 1. weaken the test so we interpret any filename containing the substring "qemu-dm" as traditional-- this would catch your case at least 2. flip the default around so that if an <emulator> is provided we assume traditional unless the filename is "qemu-system-i386" (or maybe just "contains qemu-system-i386" or "contains qemu-system") 3. add libxl driver-specific XML (is that possible?) to allow the user to override a libvirt default. It would be a shame to expose the complexity to the libvirt client though. What do you think? Cheers, Dave

David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The other options I can think of are:
1. weaken the test so we interpret any filename containing the substring "qemu-dm" as traditional-- this would catch your case at least
Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component.
2. flip the default around so that if an <emulator> is provided we assume traditional unless the filename is "qemu-system-i386" (or maybe just "contains qemu-system-i386" or "contains qemu-system")
How is this handled in xl? There is certainly a lot of xm config out there with device_model="/usr/lib/xen/bin/qemu-dm" Are users expected to add device_model_version="qemu-traditional" when migrating to xl/libxl? I suppose xl handles this (should just look at the code), but it seems an implementation detail that shouldn't be exposed to 3rd party apps.
3. add libxl driver-specific XML (is that possible?) to allow the user to override a libvirt default. It would be a shame to expose the complexity to the libvirt client though.
There is such functionality for qemu, but I would prefer to avoid it for this case, particularly since it works in the legacy xen driver. Regards, Jim

On Tue, Apr 30, 2013 at 09:42:59PM -0600, Jim Fehlig wrote:
David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The other options I can think of are:
1. weaken the test so we interpret any filename containing the substring "qemu-dm" as traditional-- this would catch your case at least
Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component.
2. flip the default around so that if an <emulator> is provided we assume traditional unless the filename is "qemu-system-i386" (or maybe just "contains qemu-system-i386" or "contains qemu-system")
How is this handled in xl? There is certainly a lot of xm config out there with
device_model="/usr/lib/xen/bin/qemu-dm"
Are users expected to add
device_model_version="qemu-traditional"
when migrating to xl/libxl? I suppose xl handles this (should just look at the code), but it seems an implementation detail that shouldn't be exposed to 3rd party apps.
Sigh, is there really no way for Xen to figure this out automatically, or for QEMU itself to tell Xen what type of device model it is. Pushing it onto the user is a really horrible design decision IMHO.
3. add libxl driver-specific XML (is that possible?) to allow the user to override a libvirt default. It would be a shame to expose the complexity to the libvirt client though.
There is such functionality for qemu, but I would prefer to avoid it for this case, particularly since it works in the legacy xen driver.
The QEMU scenario is different to this too. QEMU has the optional to allow arbitrary command line args to be passed through, but this is declared to be feature without any formal support. The intent is that everything be modelled in the generic XML if it is to be supported. If this device model version is something that is mandatory to make it work, then it is not viable to consider a xen specific XML extension for it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote:
David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary. Most users won't even do that and will just want to accept the default device model version. Only users with preexisting VMs which are reliant on the older device model for some reason (e.g. Windows "Genuine Advantage" or whatever it is called) or, historically, people who wanted to try out the new device model before it became the default would normally want to specify the device model version at all. Only advanced users should want to override the actual path to use a specific device model binary. The _override suffix on device_model_override is intended to imply "advanced use, you better know what you are doing", and that includes knowing which version it is (since there is no obvious way to detect, other than the sorts of heuristics you are discussing here). This is probably best documented in the xl.cfg(5) manpage rather than in anything libxl-ish (sorry): =item B<device_model_version="DEVICE-MODEL"> Selects which variant of the device-model should be used for this guest. Valid values are: ... the obvious list ... =back It is recommended to accept the default value for new guests. If you have existing guests then, depending on the nature of the guest Operating System, you may wish to force them to use the device model which they were installed with. ... =item B<device_model_override="PATH"> Override the path to the binary to be used as the device-model. The binary provided here MUST be consistent with the `device_model_version` which you have specified. You should not normally need to specify this option.
The other options I can think of are:
1. weaken the test so we interpret any filename containing the substring "qemu-dm" as traditional-- this would catch your case at least
Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component.
2. flip the default around so that if an <emulator> is provided we assume traditional unless the filename is "qemu-system-i386" (or maybe just "contains qemu-system-i386" or "contains qemu-system")
How is this handled in xl? There is certainly a lot of xm config out there with
device_model="/usr/lib/xen/bin/qemu-dm"
xl will ignore this and print a warning, fprintf(stderr, "WARNING: ignoring device_model directive.\n" "WARNING: Use \"device_model_override\" instead if you" " really want a non-default device_model\n"); This is because we think the majority of users should not need to care about the path to the device model binary.
Are users expected to add
device_model_version="qemu-traditional"
Not unless they care about the particular device model they are using. I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you. If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn. Ian.

On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-) Slightly tangentially, do you anticipate the list of valid LIBXL_DEVICE_MODEL_VERSIONs expanding in future? For example, will it be used to differentiate between different major versions of qemu? Cheers, Dave

On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path. IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly. -- Best Regards, Marek Marczykowski Invisible Things Lab

Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO. https://www.redhat.com/archives/libvir-list/2013-May/msg01994.html Regards, Jim

On Thu, 2013-05-30 at 11:53 -0600, Jim Fehlig wrote:
Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO.
The default is handled in libxl, so you don't need to do anything if you don't want to. It's up to you if you want to provide an override facility in libvirt. Personally I'd do it with a specific XML property/key/whatever rather than magic emulator paths... Ian.

On 31.05.2013 10:25, Ian Campbell wrote:
On Thu, 2013-05-30 at 11:53 -0600, Jim Fehlig wrote:
Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path. Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO.
The default is handled in libxl, so you don't need to do anything if you don't want to. It's up to you if you want to provide an override facility in libvirt. Personally I'd do it with a specific XML property/key/whatever rather than magic emulator paths...
Optional attribute for <emulator> tag? Maybe sth like: <emulator type="stubdom">/usr/lib/xen/boot/ioemu-stubdom.gz</emulator> ? -- Best Regards, Marek Marczykowski Invisible Things Lab

On Fri, 2013-05-31 at 12:46 +0200, Marek Marczykowski wrote:
On 31.05.2013 10:25, Ian Campbell wrote:
On Thu, 2013-05-30 at 11:53 -0600, Jim Fehlig wrote:
Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
> I would suggest that libvirt+libxl expose the version as the "emulator" > option and not the path. Just leave the path as the default in the > normal case. You may also want to provide an extra > emulator-path-override tag/attribute/XML for advanced users, but that's > up to you. > > If you need to support upgrade from xend then you could perhaps treat > <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION > string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a > qemu-xen-traditional device model -- no xend user can possibly have been > using the new device model with xend. Or you could take the approach > that xl does and just warn. > This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO.
The default is handled in libxl, so you don't need to do anything if you don't want to. It's up to you if you want to provide an override facility in libvirt. Personally I'd do it with a specific XML property/key/whatever rather than magic emulator paths...
Optional attribute for <emulator> tag? Maybe sth like: <emulator type="stubdom">/usr/lib/xen/boot/ioemu-stubdom.gz</emulator> ?
FWIW the path can be omitted and libxl will DTRT. Ian.

On 31.05.2013 12:55, Ian Campbell wrote:
On Fri, 2013-05-31 at 12:46 +0200, Marek Marczykowski wrote:
On 31.05.2013 10:25, Ian Campbell wrote:
On Thu, 2013-05-30 at 11:53 -0600, Jim Fehlig wrote:
Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
> On 01/05/13 09:46, Ian Campbell wrote: > >> I would suggest that libvirt+libxl expose the version as the "emulator" >> option and not the path. Just leave the path as the default in the >> normal case. You may also want to provide an extra >> emulator-path-override tag/attribute/XML for advanced users, but that's >> up to you. >> >> If you need to support upgrade from xend then you could perhaps treat >> <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION >> string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a >> qemu-xen-traditional device model -- no xend user can possibly have been >> using the new device model with xend. Or you could take the approach >> that xl does and just warn. >> > This would work for me: it doesn't seem too bad to consider > "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's > a useful observation that xend never supported the new device model. > Jim: should I cook up patch v3? :-) > No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO.
The default is handled in libxl, so you don't need to do anything if you don't want to. It's up to you if you want to provide an override facility in libvirt. Personally I'd do it with a specific XML property/key/whatever rather than magic emulator paths...
Optional attribute for <emulator> tag? Maybe sth like: <emulator type="stubdom">/usr/lib/xen/boot/ioemu-stubdom.gz</emulator> ?
FWIW the path can be omitted and libxl will DTRT.
Indeed. It looks like libxl even doesn't allow custom stubdom path - always uses hardcoded default. libxl_dm.c: stubdom_state->pv_kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path()); stubdom_state->pv_cmdline = libxl__sprintf(gc, " -d %d", guest_domid); stubdom_state->pv_ramdisk.path = ""; IMO it is much more useful to specify device model type (qemu upstream, qemu traditional, qemu traditional in stubdom) than explicit qemu path, but libvirt config syntax currently allow only the later one. -- Best Regards, Marek Marczykowski Invisible Things Lab

On Fri, 2013-05-31 at 13:14 +0200, Marek Marczykowski wrote:
Optional attribute for <emulator> tag? Maybe sth like: <emulator type="stubdom">/usr/lib/xen/boot/ioemu-stubdom.gz</emulator> ?
FWIW the path can be omitted and libxl will DTRT.
Indeed. It looks like libxl even doesn't allow custom stubdom path - always uses hardcoded default. libxl_dm.c: stubdom_state->pv_kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path()); stubdom_state->pv_cmdline = libxl__sprintf(gc, " -d %d", guest_domid); stubdom_state->pv_ramdisk.path = "";
So it does. FWIW given libxl allows an override for the process type of qemu I can't think why we wouldn't allow an override here too.
IMO it is much more useful to specify device model type (qemu upstream, qemu traditional, qemu traditional in stubdom) than explicit qemu path,
I agree. Users should not generally need to care about paths to things. Ian.

Marek Marczykowski wrote:
On 31.05.2013 10:25, Ian Campbell wrote:
On Thu, 2013-05-30 at 11:53 -0600, Jim Fehlig wrote:
Marek Marczykowski wrote:
On 01.05.2013 16:11, Daniel P. Berrange wrote:
On Wed, May 01, 2013 at 02:44:11PM +0100, David Scott wrote:
On 01/05/13 09:46, Ian Campbell wrote:
> I would suggest that libvirt+libxl expose the version as the "emulator" > option and not the path. Just leave the path as the default in the > normal case. You may also want to provide an extra > emulator-path-override tag/attribute/XML for advanced users, but that's > up to you. > > If you need to support upgrade from xend then you could perhaps treat > <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION > string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a > qemu-xen-traditional device model -- no xend user can possibly have been > using the new device model with xend. Or you could take the approach > that xl does and just warn. > > This would work for me: it doesn't seem too bad to consider "qemu-xen" and "qemu-xen-traditional" as special virtual paths. It's a useful observation that xend never supported the new device model. Jim: should I cook up patch v3? :-)
No, that really doesn't fly. The <emlator> element must always point to a qualfied binary path.
IMHO, libvirt should just default to the new QEMU binary and if people want to use the old one, they can configure the emulator path for it.
What about stubdom? Any idea how to do it better than special paths? Even if given path will point to stubdom binary, I don't know how libvirt shoud distinguish it from standalone qemu and set b_info->device_model_stubdomain accordingly.
As mentioned in my reply to your stubdom patch, this should be handled in libxl IMO.
The default is handled in libxl, so you don't need to do anything if you don't want to. It's up to you if you want to provide an override facility in libvirt. Personally I'd do it with a specific XML property/key/whatever rather than magic emulator paths...
Optional attribute for <emulator> tag? Maybe sth like: <emulator type="stubdom">/usr/lib/xen/boot/ioemu-stubdom.gz</emulator> ?
That sounds like a reasonable compromise to me. Regards, Jim

Ian Campbell wrote:
On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote:
David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary.
That doesn't seem right to me. In a few years time who will care about "qemu-traditional" at all? Seems more useful for me to be able to say use '/usr/bin/qemu-system-i386', '/usr/bin/qemu-system-arm', '/usr/lib/xen/bin/qemu-dm', '/usr/local/bin/qemu-add-my-args', etc. And if I don't specify an emulator, then pick a sane default.
Most users won't even do that and will just want to accept the default device model version. Only users with preexisting VMs which are reliant on the older device model for some reason (e.g. Windows "Genuine Advantage" or whatever it is called) or, historically, people who wanted to try out the new device model before it became the default would normally want to specify the device model version at all.
Only advanced users should want to override the actual path to use a specific device model binary. The _override suffix on device_model_override is intended to imply "advanced use, you better know what you are doing", and that includes knowing which version it is (since there is no obvious way to detect, other than the sorts of heuristics you are discussing here).
This is probably best documented in the xl.cfg(5) manpage rather than in anything libxl-ish (sorry):
=item B<device_model_version="DEVICE-MODEL">
Selects which variant of the device-model should be used for this guest. Valid values are:
... the obvious list ...
=back
It is recommended to accept the default value for new guests. If you have existing guests then, depending on the nature of the guest Operating System, you may wish to force them to use the device model which they were installed with.
...
=item B<device_model_override="PATH">
Override the path to the binary to be used as the device-model. The binary provided here MUST be consistent with the `device_model_version` which you have specified. You should not normally need to specify this option.
The other options I can think of are:
1. weaken the test so we interpret any filename containing the substring "qemu-dm" as traditional-- this would catch your case at least
Right, it would probably catch a lot of cases. But users are free to have names with no 'qemu-dm' component.
2. flip the default around so that if an <emulator> is provided we assume traditional unless the filename is "qemu-system-i386" (or maybe just "contains qemu-system-i386" or "contains qemu-system")
How is this handled in xl? There is certainly a lot of xm config out there with
device_model="/usr/lib/xen/bin/qemu-dm"
xl will ignore this and print a warning, fprintf(stderr, "WARNING: ignoring device_model directive.\n" "WARNING: Use \"device_model_override\" instead if you" " really want a non-default device_model\n"); This is because we think the majority of users should not need to care about the path to the device model binary.
Are users expected to add
device_model_version="qemu-traditional"
Not unless they care about the particular device model they are using.
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path.
That doesn't fit well with the <emulator> documentation |emulator| || ||The contents of the |emulator| element specify the fully qualified path to the device model emulator binary. The capabilities XML specifies the recommended default emulator to use for each particular domain type / architecture combination. Regards, Jim
Just leave the path as the default in the normal case. You may also want to provide an extra emulator-path-override tag/attribute/XML for advanced users, but that's up to you.
If you need to support upgrade from xend then you could perhaps treat <emulator> values not in the set of valid LIBXL_DEVICE_MODEL_VERSION string (currently "qemu-xen" and "qemu-xen-traditional") as a path to a qemu-xen-traditional device model -- no xend user can possibly have been using the new device model with xend. Or you could take the approach that xl does and just warn.
Ian.
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel

On Wed, 2013-05-01 at 15:31 +0100, Jim Fehlig wrote:
Ian Campbell wrote:
On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote:
David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
The emulator path supplied can be any valid path on the system.
Note that when setting a device_model, libxl needs us to set the device_model_version too. The device_model_version can be either
...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards ...QEMU_XEN_TRADITIONAL: the old xen-specific fork
We detect the device_model_version by examining the qemu filename: if it is "qemu-dm" then it's the old xen-specific fork. If anything else then we assume "upstream qemu" (whose filename may change in future). Note that if you are using a wrapper script to (eg) adjust the arguments of the old qemu during development, you will have to ensure the wrapper script also has the name "qemu-dm", by placing it in a separate directory.
That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary.
That doesn't seem right to me. In a few years time who will care about "qemu-traditional" at all?
People who installed Windows with it and are therefore stuck with it for the lifetime of the VM. We expect it to eventually go away but not as quickly as you might expect, for this reason.
Seems more useful for me to be able to say use '/usr/bin/qemu-system-i386', '/usr/bin/qemu-system-arm', '/usr/lib/xen/bin/qemu-dm', '/usr/local/bin/qemu-add-my-args', etc. And if I don't specify an emulator, then pick a sane default.
This is still all advanced usage, the majority of users should specify neither the version nor the path, libxl will just do the right thing for them. If the libvirt bindings is trying to insert a path in the "pick a sane default" case then it should stop trying to do this and just let libxl DTRT. BTW "qemu-add-my-args" can be achieved via the libxl API which has a field for extra arguments to pass the device model. [...]
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path.
That doesn't fit well with the <emulator> documentation
That's a shame.
|emulator| || ||The contents of the |emulator| element specify the fully qualified path to the device model emulator binary. The capabilities XML specifies the recommended default emulator to use for each particular domain type / architecture combination.
Ian

Ian Campbell wrote:
On Wed, 2013-05-01 at 15:31 +0100, Jim Fehlig wrote:
Ian Campbell wrote:
On Wed, 2013-05-01 at 04:42 +0100, Jim Fehlig wrote:
David Scott wrote:
Hi,
[added xen-devel: FYI this is about how to properly set the libxl device_model_version when the user has provided a manual device_model override (aka a path to a qemu) in the libvirt domain XML.]
On 30/04/13 16:10, Jim Fehlig wrote:
David Scott wrote:
> The emulator path supplied can be any valid path on the system. > > Note that when setting a device_model, libxl needs us to set the > device_model_version too. The device_model_version can be either > > ...QEMU_XEN: meaning "upstream qemu", the default in xen-4.3 onwards > ...QEMU_XEN_TRADITIONAL: the old xen-specific fork > > We detect the device_model_version by examining the qemu filename: > if it is "qemu-dm" then it's the old xen-specific fork. If anything > else then we assume "upstream qemu" (whose filename may change > in future). Note that if you are using a wrapper script to (eg) > adjust the arguments of the old qemu during development, you will > have to ensure the wrapper script also has the name "qemu-dm", by > placing it in a separate directory. > > > That is unfortunate. Users could have existing config with <emulator>/usr/bin/my-qemu-dm</emulator> which works with the legacy stack but not with libxl right? Is it possible to safely query the binary to determine if it is qemu-dm?
From my reading of libxl, it doesn't seem to have any way to detect the type of a given qemu binary (or at least I couldn't spot it). I think that if we were to write some detection code we should probably add it to libxl rather than libvirt -- what do you think?
I tend to agree. Why should apps have to specify device_model_version? I think it is sufficient to say here's an emulator, please use it.
The intended usage model is the other way around, we expect normal users to just ask for a specific device model version and for them not to care about the specific path to the device model binary.
That doesn't seem right to me. In a few years time who will care about "qemu-traditional" at all?
People who installed Windows with it and are therefore stuck with it for the lifetime of the VM. We expect it to eventually go away but not as quickly as you might expect, for this reason.
These people should use <emulator>/usr/lib/xen/bin/qemu-dm</emulator> IMO.
Seems more useful for me to be able to say use '/usr/bin/qemu-system-i386', '/usr/bin/qemu-system-arm', '/usr/lib/xen/bin/qemu-dm', '/usr/local/bin/qemu-add-my-args', etc. And if I don't specify an emulator, then pick a sane default.
This is still all advanced usage, the majority of users should specify neither the version nor the path, libxl will just do the right thing for them. If the libvirt bindings is trying to insert a path in the "pick a sane default" case then it should stop trying to do this and just let libxl DTRT.
Yes, I agree. But if a user specifies <emulator>/bla/bla</emulator>, how should libvirt populate device_model_version? Is it expected that qemu-xen-traditional will always be the 0.10.2 version? If so, libxl could DTRT by setting device_model_version to qemu-xen-traditional when detecting the the requested emulator version in 0.10.2.
BTW "qemu-add-my-args" can be achieved via the libxl API which has a field for extra arguments to pass the device model.
Good to know, thanks. Regards, Jim
[...]
I would suggest that libvirt+libxl expose the version as the "emulator" option and not the path.
That doesn't fit well with the <emulator> documentation
That's a shame.
|emulator| || ||The contents of the |emulator| element specify the fully qualified path to the device model emulator binary. The capabilities XML specifies the recommended default emulator to use for each particular domain type / architecture combination.
Ian
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel

On Wed, 2013-05-01 at 16:40 +0100, Jim Fehlig wrote:
This is still all advanced usage, the majority of users should specify neither the version nor the path, libxl will just do the right thing for them. If the libvirt bindings is trying to insert a path in the "pick a sane default" case then it should stop trying to do this and just let libxl DTRT.
Yes, I agree. But if a user specifies <emulator>/bla/bla</emulator>, how should libvirt populate device_model_version? Is it expected that qemu-xen-traditional will always be the 0.10.2 version? If so, libxl could DTRT by setting device_model_version to qemu-xen-traditional when detecting the the requested emulator version in 0.10.2.
qemu-xen-traditional certainly won't be getting any version bumps (certainly not for the first two numbers, I suppose there is a vanishingly small chance for the third one), so if that is the version it reports then that might work. Ian.
participants (5)
-
Daniel P. Berrange
-
David Scott
-
Ian Campbell
-
Jim Fehlig
-
Marek Marczykowski