[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] libxl: expose multiple emulators per guest in the capabilities XML



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]