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

2013-04-30 Thread David Scott 
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.sc...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2013-04-30 Thread Jim Fehlig 
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.sc...@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;
   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2013-04-30 Thread David Scott 

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list