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

Re: [libvirt] [PATCH 3/4] Explicitly represent balloon device in XML and handle PCI address



On Tue, Jul 20, 2010 at 02:09:31PM +0100, Daniel P. Berrange wrote:
> From: Daniel P. Berrange <dan berrange com>
> 
> To allow compatability with older QEMU PCI device slot assignment
> it is neccessary to explicitly track the balloon device in the
> XML. This introduces a new device
> 
>    <memballoon model='virtio|xen'/>

  Hum ... so this is a feature of the hypervisor emulation
but that we need to list in the domain devices. It's a bit funky,

> It can also have a PCI address, auto-assigned if neccessary.
> 
> The memballoon will be automatically added to all Xen and QEMU
> guests by default.

  hum, that's not ideal, but I don't see any workaround ...

> * docs/schemas/domain.rng: Add <memballoon> element
> * src/conf/domain_conf.c, src/conf/domain_conf.h: parsing
>   and formatting for memballoon device. Always add a memory
>   balloon device to Xen/QEMU if none exists in XML
> * src/libvirt_private.syms: Export memballoon model APIs
> * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Honour the
>   PCI device address in memory balloon device
> * tests/*: Update to test new functionality
[...]

> @@ -4634,6 +4685,41 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          VIR_FREE(nodes);
>      }
>  
> +    /* analysis of the memballoon devices */
> +    def->memballoon = NULL;
> +    if ((n = virXPathNodeSet("./devices/memballoon", ctxt, &nodes)) < 0) {
> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                             "%s", _("cannot extract memballoon devices"));
> +        goto error;
> +    }
> +    if (n > 1) {
> +        virDomainReportError (VIR_ERR_INTERNAL_ERROR,
> +                              "%s", _("only a single memballoon device is supported"));
> +        goto error;
> +    }
> +    if (n > 0) {
> +        virDomainMemballoonDefPtr memballoon =
> +            virDomainMemballoonDefParseXML(nodes[0], flags);
> +        if (!memballoon)
> +            goto error;
> +
> +        def->memballoon = memballoon;
> +        VIR_FREE(nodes);
> +    } else {
> +        if (def->virtType == VIR_DOMAIN_VIRT_XEN ||
> +            def->virtType == VIR_DOMAIN_VIRT_QEMU ||
> +            def->virtType == VIR_DOMAIN_VIRT_KQEMU ||
> +            def->virtType == VIR_DOMAIN_VIRT_KVM) {
> +            virDomainMemballoonDefPtr memballoon;
> +            if (VIR_ALLOC(memballoon) < 0)
> +                goto no_memory;
> +            memballoon->model = def->virtType == VIR_DOMAIN_VIRT_XEN ?
> +                VIR_DOMAIN_MEMBALLOON_MODEL_XEN :
> +                VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO;
> +            def->memballoon = memballoon;
> +        }
> +    }

  okay so we auto-add there.

[...]
> +    if (def->memballoon) {
> +        if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
> +            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                            _("Memory balloon device type '%s' is not supported by this version of qemu"),
> +                            virDomainMemballoonModelTypeToString(def->memballoon->model));
> +            goto error;

  Hum, this is likely to lead to a printf( "%s", NULL) isn't it ?
is that dangerous report or useful report ? 
[...]

  BTW we auto add the baloon for Xen, by symetry with QEmu but we never
  use that information in the Xen code. should we just ignore it for Xen
  right now to aoid introducing a format which may not work okay if Xen
  evolves, since we don't use it in practice right now ?

But overall ACK, this is not ideal, but I don't see a way around...

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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