[libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML
Jim Fehlig
jfehlig at suse.com
Mon Apr 29 18:54:55 UTC 2013
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 at 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
More information about the libvir-list
mailing list