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 :|