[libvirt] libxl: expose and control xen device model versions
Hi, These patches expose multiple xen device model options in the capabilities XML and allow the user to select a specific device model via the domain XML's <emulator> tag. It is important to control the device model per VM, since the default is changing from xen-4.2 "qemu traditional" to xen-4.3 "qemu upstream". In this proposal, the capabilities XML now has multiple <domain> elements per <arch> as follows: <arch name='i686'> <wordsize>32</wordsize> <machine>xenfv</machine> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-system-i386</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> </arch> -- is it valid/ sensible to have multiple <domain>'s with the same 'type'? Let me know if you can think of a better mapping. Comments, criticism welcome. Cheers, Dave
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386") The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture. Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h> #include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40 +enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +}; + +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386" + +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +}; struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j; if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL; - if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + } if (guest_archs[i].pae && virCapabilitiesAddGuestFeature(guest, @@ -163,7 +192,8 @@ libxlBuildCapabilities(virArch hostarch, static virCapsPtr libxlMakeCapabilitiesInternal(virArch hostarch, libxl_physinfo *phy_info, - char *capabilities) + char *capabilities, + int emulators_found[]) { char *str, *token; regmatch_t subs[4]; @@ -243,7 +273,7 @@ libxlMakeCapabilitiesInternal(virArch hostarch, continue; } - /* Search for existing matching (model,hvm) tuple */ + /* Search for existing matching (arch,hvm) tuple */ for (i = 0 ; i < nr_guest_archs ; i++) { if ((guest_archs[i].arch == arch) && guest_archs[i].hvm == hvm) { @@ -277,7 +307,8 @@ libxlMakeCapabilitiesInternal(virArch hostarch, if ((caps = libxlBuildCapabilities(hostarch, host_pae, guest_archs, - nr_guest_archs)) == NULL) + nr_guest_archs, + emulators_found)) == NULL) goto no_memory; return caps; @@ -756,9 +787,13 @@ error: virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx) { - int err; + int i, err; libxl_physinfo phy_info; const libxl_version_info *ver_info; + struct stat sbuf; + const char *path; + virArch hostarch; + int emulators_found[emulator_last]; err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED); if (err != 0) { @@ -782,9 +817,19 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; } - return libxlMakeCapabilitiesInternal(virArchFromHost(), + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; i++){ + path = (hostarch == VIR_ARCH_X86_64)? + emulator_lib64_path[i]: + emulator_lib32_path[i]; + emulators_found[i] = stat(path, &sbuf); + } + + return libxlMakeCapabilitiesInternal(hostarch, &phy_info, - ver_info->capabilities); + ver_info->capabilities, + emulators_found); } int -- 1.7.1
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like #define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g. if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } Regards, Jim
if (guest_archs[i].pae && virCapabilitiesAddGuestFeature(guest, @@ -163,7 +192,8 @@ libxlBuildCapabilities(virArch hostarch, static virCapsPtr libxlMakeCapabilitiesInternal(virArch hostarch, libxl_physinfo *phy_info, - char *capabilities) + char *capabilities, + int emulators_found[]) { char *str, *token; regmatch_t subs[4]; @@ -243,7 +273,7 @@ libxlMakeCapabilitiesInternal(virArch hostarch, continue; }
- /* Search for existing matching (model,hvm) tuple */ + /* Search for existing matching (arch,hvm) tuple */ for (i = 0 ; i < nr_guest_archs ; i++) { if ((guest_archs[i].arch == arch) && guest_archs[i].hvm == hvm) { @@ -277,7 +307,8 @@ libxlMakeCapabilitiesInternal(virArch hostarch, if ((caps = libxlBuildCapabilities(hostarch, host_pae, guest_archs, - nr_guest_archs)) == NULL) + nr_guest_archs, + emulators_found)) == NULL) goto no_memory;
return caps; @@ -756,9 +787,13 @@ error: virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx) { - int err; + int i, err; libxl_physinfo phy_info; const libxl_version_info *ver_info; + struct stat sbuf; + const char *path; + virArch hostarch; + int emulators_found[emulator_last];
err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED); if (err != 0) { @@ -782,9 +817,19 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; }
- return libxlMakeCapabilitiesInternal(virArchFromHost(), + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; i++){ + path = (hostarch == VIR_ARCH_X86_64)? + emulator_lib64_path[i]: + emulator_lib32_path[i]; + emulators_found[i] = stat(path, &sbuf); + } + + return libxlMakeCapabilitiesInternal(hostarch, &phy_info, - ver_info->capabilities); + ver_info->capabilities, + emulators_found); }
int
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; }
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple. The capabilities XML is about telling the app what the preferred emulator is for a (arch, domain, os_type) triple, not listing all possible emulators. So you really need to chose one, or the other, but not both. Apps are still free to provide emulator binaries that are not listed. 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 :|
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; }
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple.
Right. David mentioned in the cover letter that the resulting capabilities would looks something like <arch name='i686'> <wordsize>32</wordsize> <machine>xenfv</machine> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-system-i386</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> </arch>
The capabilities XML is about telling the app what the preferred emulator is for a (arch, domain, os_type) triple, not listing all possible emulators. So you really need to chose one, or the other, but not both.
I gave David this bogus info in response to a patch he posted on xen-devel while discussing another issue http://lists.xen.org/archives/html/xen-devel/2013-04/msg02905.html
Apps are still free to provide emulator binaries that are not listed.
Yes, I recall this now. I've even used it to call a wrapper around the installed binary! David, sorry for wasting your time with the capabilities nonsense :-/. I think something along the lines of your original patch will be sufficient, perhaps with checking that the requested emulator actually exists so an error can be given early. Regards, Jim
On Mon, Apr 29, 2013 at 12:54:55PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple.
Right. David mentioned in the cover letter that the resulting capabilities would looks something like
<arch name='i686'> <wordsize>32</wordsize> <machine>xenfv</machine> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> <domain type='xen'> <emulator>/usr/lib64/xen/bin/qemu-system-i386</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> </domain> </arch>
Yep, that would not be right, because there are two <domain> elements for type='xen' here. Each domain type is only allowed to exist once. So what you really want is something like <arch name='i686'> <emulator>/usr/lib64/xen/bin/qemu-dm</emulator> <loader>/usr/lib/xen/boot/hvmloader</loader> <wordsize>32</wordsize> <machine>xenfv</machine> <domain type='xen'> </domain> </arch> (Or choose the other qemu binary - which ever is declared as the "preferred" one). 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 :|
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; }
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple.
FYI, I advised David on this approach after observing the qemu driver behavior with both qemu and kvm packages installed. On this system, the capabilities for a x86-86, hvm guest contain <guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> <emulator>/usr/bin/qemu-kvm</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> </domain> </arch> <features> <cpuselection/> <deviceboot/> <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> </features> </guest> After a second look, the <domain type='kvm'> and <domain type='qemu'> elements and their subelements aren't symmetrical. Seems <domain type='qemu'> should include the emulator and supported machines, e.g. <domain type='qemu'> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> </domain> Regards, Jim
On Mon, Apr 29, 2013 at 11:01:04PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; }
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple.
FYI, I advised David on this approach after observing the qemu driver behavior with both qemu and kvm packages installed. On this system, the capabilities for a x86-86, hvm guest contain
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> <emulator>/usr/bin/qemu-kvm</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> </domain> </arch> <features> <cpuselection/> <deviceboot/> <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> </features> </guest>
After a second look, the <domain type='kvm'> and <domain type='qemu'> elements and their subelements aren't symmetrical. Seems <domain type='qemu'> should include the emulator and supported machines, e.g.
No, there's no need for that. The semantics are that at the <arch> level, we should the default machines for that (ostype,arch) combination. There are then one oro more <domain> sub-elements. The sub-elements only need to include the <machine> information, *if* they have a dedicated binary for that machine type. So for example, if the main /usr/bin/qemu-system-x86_64 supported KVM and QEMU, then the above capabilities would be <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> </domain> </arch> 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 :|
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 11:01:04PM -0600, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Mon, Apr 29, 2013 at 12:18:56PM -0600, Jim Fehlig wrote:
David Scott wrote:
libxl allows users to choose between two standard emulators: 1. (default in xen-4.2): qemu "traditional" (aka "qemu-dm") 2. (default in xen-4.3): qemu "upstream" (aka "qemu-system-i386")
The person who builds and packages xen gets to choose which emulators are built. We examine the filesystem for the emulators at runtime and expose them as separate "domains" within the same "guest" architecture.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 87 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7e0753a..472d116 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -29,6 +29,8 @@ #include <libxl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <unistd.h>
#include "internal.h" #include "virlog.h" @@ -50,6 +52,28 @@ /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */ #define LIBXL_X86_FEATURE_PAE_MASK 0x40
+enum emulator_type { + emulator_traditional = 0, + emulator_upstream = 1, + emulator_last = 2, + /* extend with specific qemu versions later */ +};
Do you think this will need to be extended in the future? As 'qemu-traditional' goes by way of the Dodo, this won't be needed right?
+ +#define EMULATOR_LIB64 "/usr/lib64/xen/bin/" +#define EMULATOR_LIB32 "/usr/lib/xen/bin/" + +#define EMULATOR_TRADITIONAL "qemu-dm" +#define EMULATOR_UPSTREAM "qemu-system-i386"
I think this could be made quite a bit simpler with something like
#define LIBXL_EMULATOR_TRADITIONAL_PATH LIBDIR "/xen/bin/qemu-dm" #define LIBXL_EMULATOR_UPSTREAM_PATH LIBDIR "/xen/bin/qemu-sytstem-i386"
+ +static const char* emulator_lib64_path [] = { + EMULATOR_LIB64 EMULATOR_TRADITIONAL, + EMULATOR_LIB64 EMULATOR_UPSTREAM, +}; + +static const char* emulator_lib32_path [] = { + EMULATOR_LIB32 EMULATOR_TRADITIONAL, + EMULATOR_LIB32 EMULATOR_UPSTREAM, +};
struct guest_arch { virArch arch; @@ -68,10 +92,11 @@ static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, struct guest_arch *guest_archs, - int nr_guest_archs) + int nr_guest_archs, + int emulators_found[]) { virCapsPtr caps; - int i; + int i, j;
if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; @@ -91,12 +116,8 @@ libxlBuildCapabilities(virArch hostarch, if ((guest = virCapabilitiesAddGuest(caps, guest_archs[i].hvm ? "hvm" : "xen", guest_archs[i].arch, - ((hostarch == VIR_ARCH_X86_64) ? - "/usr/lib64/xen/bin/qemu-dm" : - "/usr/lib/xen/bin/qemu-dm"), - (guest_archs[i].hvm ? - "/usr/lib/xen/boot/hvmloader" : - NULL), + NULL, + NULL, 1, machines)) == NULL) { virCapabilitiesFreeMachines(machines, 1); @@ -104,13 +125,21 @@ libxlBuildCapabilities(virArch hostarch, } machines = NULL;
- if (virCapabilitiesAddGuestDomain(guest, - "xen", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; + for (j = 0; j < emulator_last; ++j) { + if (emulators_found[j] == -1) /* failure from stat(2) */ + continue; + if (virCapabilitiesAddGuestDomain(guest, + "xen", + ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[j] : + emulator_lib32_path[j]), + (guest_archs[i].hvm ? + "/usr/lib/xen/boot/hvmloader" : + NULL), + 0, + NULL) == NULL) + goto no_memory; + }
and then just add the emulators here. E.g.
if (virFileExists(LIBXL_EMULATOR_TRADITIONAL_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_TRADITIONAL_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; } if (virFileExists(LIBXL_EMULATOR_UPSTREAM_PATH) { if (virCapabilitiesAddGuestDomain(guest, "xen", LIBXL_EMULATOR_UPSTREAM_PATH (guest_archs[i].hvm ? "/usr/lib/xen/boot/hvmloader" : NULL), 0, NULL) == NULL) goto no_memory; }
NB, for any single (arch, domain, os_type) triple, we should only report one <guest> in the capabilities XML. IIUC, your code above report cause us to have two entries for the same triple.
FYI, I advised David on this approach after observing the qemu driver behavior with both qemu and kvm packages installed. On this system, the capabilities for a x86-86, hvm guest contain
<guest> <os_type>hvm</os_type> <arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> <emulator>/usr/bin/qemu-kvm</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> </domain> </arch> <features> <cpuselection/> <deviceboot/> <acpi default='on' toggle='yes'/> <apic default='on' toggle='no'/> </features> </guest>
After a second look, the <domain type='kvm'> and <domain type='qemu'> elements and their subelements aren't symmetrical. Seems <domain type='qemu'> should include the emulator and supported machines, e.g.
No, there's no need for that. The semantics are that at the <arch> level, we should the default machines for that (ostype,arch) combination. There are then one oro more <domain> sub-elements. The sub-elements only need to include the <machine> information, *if* they have a dedicated binary for that machine type. So for example, if the main /usr/bin/qemu-system-x86_64 supported KVM and QEMU, then the above capabilities would be
<arch name='x86_64'> <wordsize>64</wordsize> <emulator>/usr/bin/qemu-system-x86_64</emulator> <machine>pc-1.1</machine> <machine canonical='pc-1.1'>pc</machine> <machine>pc-1.0</machine> <machine>pc-0.15</machine> <machine>pc-0.14</machine> <machine>pc-0.13</machine> <machine>pc-0.12</machine> <machine>pc-0.11</machine> <machine>pc-0.10</machine> <machine>isapc</machine> <domain type='qemu'> </domain> <domain type='kvm'> </domain> </arch>
Ah, nice! Thanks for the clarification. Regards, Jim
We cross-check the given path against the capabilties, and translate it into a libxl_device_model_version. Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 472d116..868d0cf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = { EMULATOR_LIB32 EMULATOR_UPSTREAM, }; +static const libxl_device_model_version emulator_to_device_model [] = { + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL, + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN, +}; + struct guest_arch { virArch arch; int bits; @@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx) } int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virArch hostarch; + const char *path; + int i; + + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; ++i) { + path = ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[i] : + emulator_lib32_path[i]); + if (STREQ(path, def->emulator)) { + d_config->b_info.device_model_version = + emulator_to_device_model[i]; + return 0; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight doesn't support emulator '%s'"), + def->emulator); + return -1; +} + + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) { @@ -856,6 +893,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
On Mon, Apr 29, 2013 at 03:49:41PM +0100, David Scott wrote:
We cross-check the given path against the capabilties, and translate it into a libxl_device_model_version.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 472d116..868d0cf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = { EMULATOR_LIB32 EMULATOR_UPSTREAM, };
+static const libxl_device_model_version emulator_to_device_model [] = { + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL, + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN, +}; + struct guest_arch { virArch arch; int bits; @@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx) }
int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virArch hostarch; + const char *path; + int i; + + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; ++i) { + path = ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[i] : + emulator_lib32_path[i]); + if (STREQ(path, def->emulator)) { + d_config->b_info.device_model_version = + emulator_to_device_model[i]; + return 0; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight doesn't support emulator '%s'"), + def->emulator); + return -1;
This check is bogus. The capabilities XML is only intended to list the recommended default emulator. It is perfectly aceptable for apps to pass emulator paths that are not present in the capabilities. 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 :|
David Scott wrote:
We cross-check the given path against the capabilties, and translate it into a libxl_device_model_version.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 472d116..868d0cf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = { EMULATOR_LIB32 EMULATOR_UPSTREAM, };
+static const libxl_device_model_version emulator_to_device_model [] = { + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL, + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN, +}; + struct guest_arch { virArch arch; int bits; @@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx) }
int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virArch hostarch; + const char *path; + int i; + + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; ++i) { + path = ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[i] : + emulator_lib32_path[i]); + if (STREQ(path, def->emulator)) {
I thought there was a virCapabilitiesSupportsGuestEmulator() or similar, but I don't see it in src/conf/capabilities.c. I think it makes sense to add such a function to the capabilities and then just call it here, passing the requested emulator. Perhaps other libvirt developers can comment on the usefulness of virCapabilitiesSupportGuestEmulator().
+ d_config->b_info.device_model_version = + emulator_to_device_model[i]; + return 0; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight doesn't support emulator '%s'"), + def->emulator); + return -1; +} + + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) { @@ -856,6 +893,10 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver, goto error; }
+ if (libxlMakeEmulator(def, d_config) < 0) {
The capabilities created when the libxl driver is loaded are available in libxlDriverPrivatePtr and could be passed to libxlMakeEmulator() libxlMakeEmulator(driver->caps, def, d_config) Regards, Jim
+ goto error; + } + d_config->on_reboot = def->onReboot; d_config->on_poweroff = def->onPoweroff; d_config->on_crash = def->onCrash;
Jim Fehlig wrote:
David Scott wrote:
We cross-check the given path against the capabilties, and translate it into a libxl_device_model_version.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> --- src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 472d116..868d0cf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -75,6 +75,11 @@ static const char* emulator_lib32_path [] = { EMULATOR_LIB32 EMULATOR_UPSTREAM, };
+static const libxl_device_model_version emulator_to_device_model [] = { + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL, + LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN, +}; + struct guest_arch { virArch arch; int bits; @@ -833,6 +838,38 @@ libxlMakeCapabilities(libxl_ctx *ctx) }
int +libxlMakeEmulator(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virArch hostarch; + const char *path; + int i; + + /* No explicit override means use the default */ + if (!def->emulator) { + return 0; + } + + hostarch = virArchFromHost(); + + for (i = 0; i < emulator_last; ++i) { + path = ((hostarch == VIR_ARCH_X86_64) ? + emulator_lib64_path[i] : + emulator_lib32_path[i]); + if (STREQ(path, def->emulator)) {
I thought there was a virCapabilitiesSupportsGuestEmulator() or similar, but I don't see it in src/conf/capabilities.c. I think it makes sense to add such a function to the capabilities and then just call it here, passing the requested emulator. Perhaps other libvirt developers can comment on the usefulness of virCapabilitiesSupportGuestEmulator().
Daniel already clarified this, so no need to do even more work David :). As mentioned in my other response [1], your original patch (with a check to verify the requested emulator exists) should be sufficient. Regards, Jim [1] https://www.redhat.com/archives/libvir-list/2013-April/msg02087.html
Jim, Daniel, thanks for all the feedback! On 29/04/13 20:02, Jim Fehlig wrote: [ snip ]
As mentioned in my other response [1], your original patch (with a check to verify the requested emulator exists) should be sufficient.
OK, I'll send an emulator-only patch shortly. Cheers, Dave
participants (3)
-
Daniel P. Berrange -
David Scott -
Jim Fehlig