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@xxxxxxxxxxxxx>
>>>>> ---
>>>>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list