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(a)eu.citrix.com&gt; --- 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

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(a)eu.citrix.com&gt; >> --- >> 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.

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(a)eu.citrix.com&gt; >> --- >> 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.

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(a)eu.citrix.com&gt; >>>> --- >>>> 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>

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(a)eu.citrix.com&gt; --- 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;

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(a)eu.citrix.com&gt; > --- > 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().