On 07/20/2010 07:09 AM, Daniel P. Berrange wrote:
> From: Daniel P. Berrange <dan@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list