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

[Eric Blake]

On 07/20/2010 07:09 AM, Daniel P. Berrange wrote:
> From: Daniel P. Berrange <dan at berrange.com>
> 
> To allow compatability with older QEMU PCI device slot assignment

s/compatability/compatibility/

> it is neccessary to explicitly track the balloon device in the

s/neccessary/necessary/

> XML. This introduces a new device
> 
>    <memballoon model='virtio|xen'/>
> 
> It can also have a PCI address, auto-assigned if neccessary.

s/neccessary/necessary/

> 
> The memballoon will be automatically added to all Xen and QEMU
> guests by default.
> 
> * 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
> ---
>  docs/schemas/domain.rng                            |   16 +++

No change to docs/formatdomain.html.in?

>  
> +static virDomainMemballoonDefPtr
> +virDomainMemballoonDefParseXML(const xmlNodePtr node,
> +                               int flags)
> +{
> +    char *model;
> +    virDomainMemballoonDefPtr def;
> +
> +    if (VIR_ALLOC(def) < 0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    model = virXMLPropString(node, "model");
> +    if ((def->model = virDomainMemballoonModelTypeFromString(model)) < 0) {
> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                             _("unknown memory baloon model '%s'"), model);

s/baloon/balloon/

>  
> +    /* 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"));

Do we want to s/memballoon/memory balloon/ in user-visible messages?

>  /*
>   * This entire method assumes that PCI devices in 'info pci'
>   * match ordering of devices specified on the command line
> @@ -2634,7 +2655,7 @@ qemuDetectPCIAddresses(virDomainObjPtr vm,
>              continue;
>  
>          if (qemuAssignNextPCIAddress(&(vm->def->sounds[i]->info),
> -                                     vendor, product,
> +                                    vendor, product,

Why the whitespace change?

>                                       addrs,  naddrs) < 0) {
>              qemuReportError(VIR_ERR_INTERNAL_ERROR,
>                              _("cannot find PCI address for sound adapter %s"),
> @@ -2656,6 +2677,18 @@ qemuDetectPCIAddresses(virDomainObjPtr vm,
>          }
>      }
>  
> +    if (vm->def->memballoon &&
> +        qemuGetPCIMemballoonVendorProduct(vm->def->memballoon, &vendor, &product) == 0) {
> +        if (qemuAssignNextPCIAddress(&(vm->def->memballoon->info),
> +                                     vendor, product,
> +                                     addrs,  naddrs) < 0) {

s/,  /, /

> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            _("cannot find PCI address for watchdog %s"),

s/watchdog/memory balloon/

> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-balloon-device.args
> @@ -0,0 +1 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x12

Email may truncate these long lines, but from what I can tell, they
should be okay.

> +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-device.xml
> @@ -19,5 +19,6 @@
>        <target dev='hda' bus='ide'/>
>      </disk>
>      <watchdog model='ib700' action='poweroff'/>
> +    <memballoon model='virtio'/>
>    </devices>
>  </domain>

I didn't see any tests of model='xen' instead of model='virtio'; do
there need to be any?

Technically sound, but quite a few nits.  I'd really like to get in the
habit of touching up formatdomain.html.in before giving the ACK.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100720/e81d16a5/attachment-0001.sig>