[PATCH RESEND v1] xen: recognize device_model_override
Since Xen 4.2 libxl expects device_model_override="/path" instead of device_model="/path". Adjust the code to parse this as <emulator>. While libxl also recognizes device_model_version="", this knob is not supported by libvirt. A runtime detection exists to select either "qemu-xen" or "qemu-xen-traditional". Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/libxl/xen_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index c82e487d80..88a784ed04 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1508,7 +1508,7 @@ xenParseConfigCommon(virConfPtr conf, if (xenParseTimeOffset(conf, def) < 0) return -1; - if (xenConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0) + if (xenConfigCopyStringOpt(conf, "device_model_override", &def->emulator) < 0) return -1; if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { @@ -2242,7 +2242,7 @@ static int xenFormatEmulator(virConfPtr conf, virDomainDefPtr def) { if (def->emulator && - xenConfigSetString(conf, "device_model", def->emulator) < 0) + xenConfigSetString(conf, "device_model_override", def->emulator) < 0) return -1; return 0;
On 11/12/20 11:03 AM, Olaf Hering wrote:
Since Xen 4.2 libxl expects device_model_override="/path" instead of device_model="/path".
This has been on my todo list but never got the priority it deserved. Thanks for taking it on. Without your patch, using 'xl create' on xl.cfg converted from domXML by the libxl driver results in WARNING: ignoring device_model directive. WARNING: Use "device_model_override" instead if you really want a non-default device_model
Adjust the code to parse this as <emulator>.
You also have to adjust all the failing {xl,xm}configtests.
While libxl also recognizes device_model_version="", this knob is not supported by libvirt. A runtime detection exists to select either "qemu-xen" or "qemu-xen-traditional".
Correct. It is selected based on the specified <emulator> and not modeled in domXML. Does it need to be exposed to the user? AFAIK those are the only two values we care about, and it is possible to determine which to use based on the <emulator>. Regards, Jim
Signed-off-by: Olaf Hering <olaf@aepfle.de> --- src/libxl/xen_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index c82e487d80..88a784ed04 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1508,7 +1508,7 @@ xenParseConfigCommon(virConfPtr conf, if (xenParseTimeOffset(conf, def) < 0) return -1;
- if (xenConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0) + if (xenConfigCopyStringOpt(conf, "device_model_override", &def->emulator) < 0) return -1;
if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { @@ -2242,7 +2242,7 @@ static int xenFormatEmulator(virConfPtr conf, virDomainDefPtr def) { if (def->emulator && - xenConfigSetString(conf, "device_model", def->emulator) < 0) + xenConfigSetString(conf, "device_model_override", def->emulator) < 0) return -1;
return 0;
Am Mon, 16 Nov 2020 19:48:28 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Adjust the code to parse this as <emulator>. You also have to adjust all the failing {xl,xm}configtests.
It seems one has to run 'ninja test'. Can this be done via _multibuild in a libvirt-testsuite pkg in an isolated VM?
Correct. It is selected based on the specified <emulator> and not modeled in domXML. Does it need to be exposed to the user? AFAIK those are the only two values we care about, and it is possible to determine which to use based on the <emulator>.
I think 'device_model_version' can be ignored. In current xen.git support for "qemu-xen-traditional" was declared for stubdom only. It is unrealistic that some HVM domUs with qemu-dm are still running out there. In case they really exist, they were likely not started with libvirt. Olaf
On 11/19/20 12:58 PM, Olaf Hering wrote:
Am Mon, 16 Nov 2020 19:48:28 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Adjust the code to parse this as <emulator>. You also have to adjust all the failing {xl,xm}configtests.
It seems one has to run 'ninja test'.
Yes. See https://libvirt.org/hacking.html
Can this be done via _multibuild in a libvirt-testsuite pkg in an isolated VM?
I suppose. Currently it is done in the downstream OBS package in %check VIR_TEST_DEBUG=1 %meson_test -t 5 --no-suite syntax-check
Correct. It is selected based on the specified <emulator> and not modeled in domXML. Does it need to be exposed to the user? AFAIK those are the only two values we care about, and it is possible to determine which to use based on the <emulator>.
I think 'device_model_version' can be ignored.
I think we need to take a closer look. It is not explicitly set in libxl_domain_build_info_init(), hence would be set to LIBXL_DEVICE_MODEL_VERSION_UNKNOWN. The only thing interesting from a quick grep in $xensrc/tools/libxl/ is libxl_domain.c: assert(version != LIBXL_DEVICE_MODEL_VERSION_UNKNOWN);
In current xen.git support for "qemu-xen-traditional" was declared for stubdom only. It is unrealistic that some HVM domUs with qemu-dm are still running out there. In case they really exist, they were likely not started with libvirt.
Or by "ignored" do you mean removing the logic and unconditionally setting it to LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN? Regards, Jim
Am Thu, 19 Nov 2020 16:11:34 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
I suppose. Currently it is done in the downstream OBS package in %check VIR_TEST_DEBUG=1 %meson_test -t 5 --no-suite syntax-check
I had to run the build in a chroot in a dom0, not sure why it happens to work elsewhere. With v2 of the patch all checks pass again. Will take a closer look at the device_model_version= handling next week. Olaf
Am Thu, 19 Nov 2020 16:11:34 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Am Mon, 16 Nov 2020 19:48:28 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Correct. It is selected based on the specified <emulator> and not modeled in domXML. Does it need to be exposed to the user? AFAIK those are the only two values we care about, and it is possible to determine which to use based on the <emulator>. I think 'device_model_version' can be ignored. I think we need to take a closer look. It is not explicitly set in
On 11/19/20 12:58 PM, Olaf Hering wrote: libxl_domain_build_info_init(), hence would be set to LIBXL_DEVICE_MODEL_VERSION_UNKNOWN.
libxl__domain_build_info_setdefault sets it, if needed. libxl_domain_create_new do_domain_create initiate_domain_create libxl__domain_config_setdefault libxl__domain_build_info_setdefault Otherwise 'xl create' would fail as well if device_model_version= is missing domU.cfg. Olaf
On 12/7/20 10:15 AM, Olaf Hering wrote:
Am Thu, 19 Nov 2020 16:11:34 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Am Mon, 16 Nov 2020 19:48:28 -0700 schrieb Jim Fehlig <jfehlig@suse.com>:
Correct. It is selected based on the specified <emulator> and not modeled in domXML. Does it need to be exposed to the user? AFAIK those are the only two values we care about, and it is possible to determine which to use based on the <emulator>. I think 'device_model_version' can be ignored. I think we need to take a closer look. It is not explicitly set in
On 11/19/20 12:58 PM, Olaf Hering wrote: libxl_domain_build_info_init(), hence would be set to LIBXL_DEVICE_MODEL_VERSION_UNKNOWN.
libxl__domain_build_info_setdefault sets it, if needed.
libxl_domain_create_new do_domain_create initiate_domain_create libxl__domain_config_setdefault libxl__domain_build_info_setdefault
Which, if needed, calls libxl__default_device_model in the various libxl_<os-flavor>.c files.
Otherwise 'xl create' would fail as well if device_model_version= is missing domU.cfg.
Yes, good point! Jim
participants (2)
-
Jim Fehlig -
Olaf Hering