- Devel - libvirt List Archives

[libvirt] [PATCH] Fix race condition when destroying guests
by Daniel P. Berrange 19 Jan '13

19 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When running virDomainDestroy, we need to make sure that no other background thread cleans up the domain while we're doing our work. This can happen if we release the domain object while in the middle of work, because the monitor might detect EOF in this window. For this reason we have a 'beingDestroyed' flag to stop the monitor from doing its normal cleanup. Unfortunately this flag was only being used to protect qemuDomainBeginJob, and not qemuProcessKill This left open a race condition where either libvirtd could crash, or alternatively report bogus error messages about the domain already having been destroyed to the caller Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_driver.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c28c223..2a6f381 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2121,24 +2121,29 @@ qemuDomainDestroyFlags(virDomainPtr dom, qemuDomainSetFakeReboot(driver, vm, false); + + /* We need to prevent monitor EOF callback from doing our work (and sending + * misleading events) while the vm is unlocked inside BeginJob/ProcessKill API + */ + priv->beingDestroyed = true; + /* Although qemuProcessStop does this already, there may * be an outstanding job active. We want to make sure we * can kill the process even if a job is active. Killing * it now means the job will be released */ if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) { - if (qemuProcessKill(driver, vm, 0) < 0) + if (qemuProcessKill(driver, vm, 0) < 0) { + priv->beingDestroyed = false; goto cleanup; + } } else { - if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0) + if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0) { + priv->beingDestroyed = false; goto cleanup; + } } - /* We need to prevent monitor EOF callback from doing our work (and sending - * misleading events) while the vm is unlocked inside BeginJob API - */ - priv->beingDestroyed = true; - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; -- 1.8.0.2

5 6

[libvirt] RFC: libxl race fixes
by Jim Fehlig 19 Jan '13

19 Jan '13

I've been investigating some races in the libxl driver and would like to get comments on some potential solutions. The first race is in the fd/timeout event handling code, which maps libxl's osevent interface to libvirt's event loop interface. This mapping opens the possibility for libvirt's event loop to invoke event callbacks after libxl has already deregistered the event, potentially accessing an event object that has already been freed. One solution to this race I've found successful is reference counting the objects associated with the events. When libxl registers an event, an object encapsulating the event is created and it's reference count is set to 1. When the event is injected into libvirt's event loop, another reference is taken on the object. When libxl deregisters the event, it's reference count is decremented. Once the event is removed from libvirt's event loop, the final reference is decremented and the object is disposed. This approach ensures the object is not disposed until it is removed from libvirt's event loop *and* libxl had explicitly deregistered the event. The notion of an event being 'disabled' found in libvirt's event loop impl also had to be added to the libxl event object, to ensure the driver doesn't call into libxl for a previously deregistered event. The second race is between destroying a vm (i.e., calling privateDataFreeFunc, which frees the libxl ctx) and deregistration/cleanup of all events that have been registered by libxl. One solution for this race is to convert libxlDomainObjPrivate to a virObject and increment its reference for each fd and timeout registration. Only when all fds and timeouts are deregistered and destroyed will the libxlDomainObjPrivate be destroyed. One downside to this approach is that an API to "cleanup" the libxl ctx is needed. Without such an API, some fds are not deregistered until calling libxl_ctx_free. But since the fd events have references on the libxlDomainObjPrivate, libxl_ctx_free is never called. (BTW I have a patch adding libxl_ctx_quiesce() to libxl, which upstream xen folks seem receptive to, including backporting to Xen 4.2 branch, but I don't think this is ideal.) An alternate solution that can be used to address both of these races is to maintain a list of the fd/timeout registrations in the libxlDomainObjPrivate object, and take a more brute force approach to managing the registrations Other solutions that I might be overlooking are certainly appreciated :). Regards, Jim

2 3

[libvirt] [PATCH] qemu: Support ram bar size for qxl devices
by Alon Levy 19 Jan '13

19 Jan '13

Adds a qxl-ram attribute globaly to the video.model element, that changes the resulting qemu command line only if video.type == "qxl". That attribute gets a default value of 64*1024 only if model.type is "qxl". In effect not changing any xml or argv for non qxl devices. For qxl devices a new property is set: -global qxl-vga.ram_size=<ram>*1024 or -global qxl.ram_size=<ram>*1024 For the main and secondary qxl devices respectively. The default for the qxl ram bar is the same as the default for the qxl vram bar, 64*1024. --- I've added a qxl-ram attribute. There is no precedent for adding am attribute prefixed like this, so I'm open for any other suggestion on how to do it. docs/schemas/domaincommon.rng | 9 +++- src/conf/domain_conf.c | 19 ++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 58 ++++++++++++++++++---- .../qemuxml2argv-graphics-spice-compression.args | 2 +- .../qemuxml2argv-graphics-spice-compression.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 2 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 +- .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 4 +- .../qemuxml2argv-video-device-pciaddr-default.args | 6 +-- 11 files changed, 86 insertions(+), 25 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67ae864..50fc834 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2251,7 +2251,9 @@ </define>  <define name="video"> <element name="video"> @@ -2268,6 +2270,11 @@ </choice> </attribute> <optional> + <attribute name="qxl-ram"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> <attribute name="vram"> <ref name="unsignedInt"/> </attribute> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 13c14e9..a16f1b5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7391,6 +7391,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *qxl_ram = NULL; char *primary = NULL; if (VIR_ALLOC(def) < 0) { @@ -7401,9 +7402,10 @@ virDomainVideoDefParseXML(const xmlNodePtr node, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if (!type && !vram && !heads && + if (!type && !vram && !qxl_ram && !heads && xmlStrEqual(cur->name, BAD_CAST "model")) { type = virXMLPropString(cur, "type"); + qxl_ram = virXMLPropString(cur, "qxl-ram"); vram = virXMLPropString(cur, "vram"); heads = virXMLPropString(cur, "heads"); @@ -7431,6 +7433,18 @@ virDomainVideoDefParseXML(const xmlNodePtr node, } } + if (qxl_ram) { + if (virStrToLong_ui(qxl_ram, NULL, 10, &def->qxl_ram) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse video qxl-ram '%s'"), qxl_ram); + goto error; + } + } else { + if (def->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + def->qxl_ram = virDomainVideoDefaultRAM(dom, def->type); + } + } + if (vram) { if (virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -7455,6 +7469,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, goto error; VIR_FREE(type); + VIR_FREE(qxl_ram); VIR_FREE(vram); VIR_FREE(heads); @@ -13383,6 +13398,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, " <video>\n"); virBufferAsprintf(buf, " <model type='%s'", model); + if (def->qxl_ram && def->type == VIR_DOMAIN_VIDEO_TYPE_QXL) + virBufferAsprintf(buf, " qxl-ram='%u'", def->qxl_ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ce36003..9333d26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1157,6 +1157,7 @@ struct _virDomainVideoAccelDef { struct _virDomainVideoDef { int type; + unsigned int qxl_ram; unsigned int vram; unsigned int heads; bool primary; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 981c692..05f656a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3563,6 +3563,15 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video, UINT_MAX / 1024); goto error; } + if (video->qxl_ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("value for 'qxl-ram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + + /* QEMU accepts bytes for ram_size. */ + virBufferAsprintf(&buf, ",ram_size=%u", video->qxl_ram * 1024); /* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); @@ -6569,23 +6578,48 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-vga", vgastr, NULL); if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { - if (def->videos[0]->vram && + if ((def->videos[0]->vram || def->videos[0]->qxl_ram) && qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - if (def->videos[0]->vram > (UINT_MAX / 1024)) { + int qxl_ram = def->videos[0]->qxl_ram; + int vram = def->videos[0]->vram; + if (vram > (UINT_MAX / 1024)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("value for 'vram' must be less than '%u'"), + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + if (qxl_ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("value for 'qxl-ram' must be less than '%u'"), UINT_MAX / 1024); goto error; } - virCommandAddArg(cmd, "-global"); - - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA)) - virCommandAddArgFormat(cmd, "qxl-vga.vram_size=%u", - def->videos[0]->vram * 1024); - else - virCommandAddArgFormat(cmd, "qxl.vram_size=%u", - def->videos[0]->vram * 1024); + if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA)) { + if (qxl_ram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, + "qxl-vga.ram_size=%u", + qxl_ram * 1024); + } + if (vram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, + "qxl-vga.vram_size=%u", + vram * 1024); + } + } else { + if (qxl_ram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "qxl.ram_size=%u", + qxl_ram * 1024); + } + if (vram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "qxl.vram_size=%u", + vram * 1024); + } + } } } } @@ -9247,6 +9281,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, else vid->type = video; vid->vram = virDomainVideoDefaultRAM(def, vid->type); + vid->qxl_ram = vid->type == VIR_DOMAIN_VIDEO_TYPE_QXL ? + virDomainVideoDefaultRAM(def, vid->type) : 0; vid->heads = 1; if (VIR_REALLOC_N(def->videos, def->nvideos+1) < 0) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args index 5c5912b..2f537df 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args @@ -5,5 +5,5 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ x509-dir=/etc/pki/libvirt-spice,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter -vga \ -qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 -device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml index 52eb5b9..32e4cc8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml @@ -31,10 +31,10 @@ <streaming mode='filter'/> </graphics> <video> - <model type='qxl' vram='18432' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='18432' heads='1'/> </video> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='32768' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args index 3954c03..007dd3f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args @@ -3,5 +3,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs -vga \ -qxl -global qxl-vga.vram_size=33554432 -device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x4 \ +qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 -device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml index 49cb8cc..ab40c7b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml @@ -28,10 +28,10 @@ <channel name='inputs' mode='insecure'/> </graphics> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='32768' heads='1'/> </video> <video> - <model type='qxl' vram='65536' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='65536' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index 854e723..030e1f2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -5,5 +5,5 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,plaintext-channel=inputs,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter,disable-copy-paste -vga \ -qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 -device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index d4939a4..b540915 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -34,10 +34,10 @@ <clipboard copypaste='no'/> </graphics> <video> - <model type='qxl' vram='18432' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='18432' heads='1'/> </video> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' qxl-ram='65536' vram='32768' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args index 9ce852f..4abd7c2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args @@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ -usb \ -hda /var/lib/libvirt/images/QEMUGuest1 -vnc 127.0.0.1:-5900 \ --device qxl-vga,id=video0,vram_size=67108864,bus=pci.0,addr=0x3 \ --device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x4 \ --device qxl,id=video2,vram_size=67108864,bus=pci.0,addr=0x5 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x3 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ +-device qxl,id=video2,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x5 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 -- 1.8.0.1

6 10

[libvirt] [PATCH] Log flags passed to qemuMigrationPrepare{Tunnel, Direct} APIs
by Michal Privoznik 19 Jan '13

19 Jan '13

We are already logging other arguments passed, however, @flags were missing there. --- Pushed as these chunks were ACKed and are not really part of my NBD patchset: https://www.redhat.com/archives/libvir-list/2013-January/msg01306.html src/qemu/qemu_migration.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 465dbc1..d03e361 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1787,9 +1787,10 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, int ret; VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " - "cookieout=%p, cookieoutlen=%p, st=%p, dname=%s, dom_xml=%s", + "cookieout=%p, cookieoutlen=%p, st=%p, dname=%s, dom_xml=%s " + "flags=%lx", driver, dconn, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, st, NULLSTR(dname), dom_xml); + cookieout, cookieoutlen, st, NULLSTR(dname), dom_xml, flags); /* QEMU will be started with -incoming stdio (which qemu_command might * convert to exec:cat or fd:n) @@ -1823,10 +1824,10 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, VIR_DEBUG("driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, " - "dname=%s, dom_xml=%s", + "dname=%s, dom_xml=%s flags=%lx", driver, dconn, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, NULLSTR(uri_in), uri_out, - NULLSTR(dname), dom_xml); + NULLSTR(dname), dom_xml, flags); /* The URI passed in may be NULL or a string "tcp://somehostname:port". * -- 1.8.0.2

1 0

[libvirt] [PATCH] build: Fix RPM build errors related to libvirt-lxc API
by Viktor Mihajlovski 19 Jan '13

19 Jan '13

Added missing entries to makefile and spec. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- docs/Makefile.am | 2 +- libvirt.spec.in | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index b1fa77a..c95497f 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -120,7 +120,7 @@ EXTRA_DIST= \ hacking1.xsl hacking2.xsl wrapstring.xsl \ $(dot_html) $(dot_html_in) $(gif) $(apihtml) $(apipng) \ $(devhelphtml) $(devhelppng) $(devhelpcss) $(devhelpxsl) \ - $(xml) $(qemu_xml) $(fig) $(png) $(css) \ + $(xml) $(qemu_xml) $(lxc_xml) $(fig) $(png) $(css) \ $(patches) $(dot_php_in) $(dot_php_code_in) $(dot_php)\ $(internals_html_in) $(internals_html) \ sitemap.html.in \ diff --git a/libvirt.spec.in b/libvirt.spec.in index dca5053..3486d29 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1983,6 +1983,7 @@ fi %doc AUTHORS NEWS README COPYING.LIB %{_libdir}/python*/site-packages/libvirt.py* %{_libdir}/python*/site-packages/libvirt_qemu.py* +%{_libdir}/python*/site-packages/libvirt_lxc.py* %{_libdir}/python*/site-packages/libvirtmod* %doc python/tests/*.py %doc python/TODO -- 1.7.9.5

2 1

[libvirt] Coverity update
by John Ferlan 18 Jan '13

18 Jan '13

Taking Eric's suggestion from: https://www.redhat.com/archives/libvir-list/2013-January/msg01050.html There's two changes that have yet to be reviewed: https://www.redhat.com/archives/libvir-list/2013-January/msg00537.html https://www.redhat.com/archives/libvir-list/2013-January/msg00541.html After this recent set of 10 patches there are about 20 patches remaining. With the 2 above and the 10 from yesterday, the coverity defect list stands at 236 (down from 297). Of those, about 50 are related to a sdt.h macro issue, 50 are false positives requiring a comment change, and another 95 can be fixed/resolved by a very simple (but yet unnecessary) change to src/esx/esx_vi.c and src/esx/esx_vi_types.c. Each module as a __TEMPLATE__FREE() macro which does the following: item=*ptrptr; \ \ _body \ \ VIR_FREE(*ptrptr); \ Coverity has a hard time with the VIR_FREE(*ptrptr) here and in other modules, thus it decides we must be leaking. No amount of tagging completely removes all the warnings (I tried adding them everywhere and only reduced the count, not eliminated). There is a solution, but I really don't like it. If the VIR_FREE(*ptrptr) is changed to: VIR_FREE(item); \ *ptrptr = NULL; \ Coverity is happy. I asked about this on a Coverity community message board, but no one has an answer. I even generated a small program which replicated the issue. In a way I think it could be a Coverity bug, but I'm not sure I want to make that claim yet. John

1 0

[libvirt] [PATCH] qemu: Double mutex unlock in qemuDomainModifyDeviceFlags
by Viktor Mihajlovski 18 Jan '13

18 Jan '13

The driver mutex was unlocked in qemuDomainModifyDeviceFlags before entering qemuDomainObjBeginJobWithDriver where it will be unlocked once more leaving it in an undefined state. The result was that two threads were simultaneously looking up the domain hash table during multiple parallel device attach/detach operations. Luckily this triggered a virHashIterationError. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8c39864..c28c223 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6443,7 +6443,6 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); -- 1.7.9.5

3 4

Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by Carlos O'Donell 18 Jan '13

18 Jan '13

On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger <vapier(a)gentoo.org> wrote: > On Wednesday 16 January 2013 22:15:38 David Miller wrote: >> From: Carlos O'Donell <carlos(a)systemhalted.org> >> Date: Wed, 16 Jan 2013 21:15:03 -0500 >> >> > +/* If a glibc-based userspace has already included in.h, then we will >> > not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. >> > The + * ABI used by the kernel and by glibc match exactly. Neither the >> > kernel + * nor glibc should break this ABI without coordination. >> > + */ >> > +#ifndef _NETINET_IN_H >> > + >> >> I think we should shoot for a non-glibc-centric solution. >> >> I can't imagine that other libc's won't have the same exact problem >> with their netinet/in.h conflicting with the kernel's, redefining >> structures like in6_addr, that we'd want to provide a protection >> scheme for here as well. > > yes, the kernel's use of __GLIBC__ in exported headers has already caused > problems in the past. fortunately, it's been reduced down to just one case > now (stat.h). let's not balloon it back up. > -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. Cheers, Carlos.

3 5

[libvirt] [PATCH 00/12] Introduce public APIs for dealing with virTypedParameters
by Jiri Denemark 18 Jan '13

18 Jan '13

Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it. Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API python: Avoid freeing uninitialized new_params pointer daemon/remote.c | 34 +- include/libvirt/libvirt.h.in | 95 ++++++ python/generator.py | 20 ++ python/libvirt-override.c | 73 ++-- src/libvirt_private.syms | 1 - src/libvirt_public.syms | 18 + src/remote/remote_driver.c | 4 +- src/rpc/gendispatch.pl | 3 +- src/util/virtypedparam.c | 782 ++++++++++++++++++++++++++++++++++++++++++- src/util/virtypedparam.h | 2 - tools/virsh-domain-monitor.c | 5 +- tools/virsh-domain.c | 542 +++++++++++------------------- tools/virsh-host.c | 78 ++--- tools/virsh.c | 20 -- 14 files changed, 1171 insertions(+), 506 deletions(-) -- 1.8.1.1

3 17

[libvirt] [PATCH] storage: Fix lvcreate parameter for backingStore.
by Atsushi Kumagai 18 Jan '13

18 Jan '13

When virStorageBackendLogicalCreateVol() creates a snapshot for a logical volume with backingStore element, it fails with the message below: 2013-01-17 03:10:18.869+0000: 1967: error : virCommandWait:2345 : internal error Child process (/sbin/lvcreate --name lvm-snapshot -L 51200K -s=/dev/lvm-pool/lvm-volume) unexpected exit status 3: /sbin/lvcreate: invalid option -- '=' Error during parsing of command line. This is because virCommandAddArgPair() uses '=' to connect the two parameters, it's unsuitable for -s option of the lvcreate. Signed-off-by: Atsushi Kumagai <kumagai-atsushi(a)mxc.nes.nec.co.jp> --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 91db3fd..bb709df 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -736,7 +736,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->capacity, 1024)); if (vol->backingStore.path) - virCommandAddArgPair(cmd, "-s", vol->backingStore.path); + virCommandAddArgList(cmd, "-s", vol->backingStore.path, NULL); else virCommandAddArg(cmd, pool->def->source.name); -- 1.7.1

2 1

Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by David Miller 18 Jan '13

18 Jan '13

From: Mike Frysinger <vapier(a)gentoo.org> Date: Thu, 17 Jan 2013 23:14:31 -0500 > the kernel already exports many types with a __kernel_ prefix. i changed the > kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do the same > thing to pretty much all the networking headers. a few packages broke, but > the number was low, and trivial to fix (a sed would do it most of the time). > > it's also trivial for userland packages to detect that they need to do this > sort of thing in a local header by using linux/version.h and a set of defines > to redirect the new structure name back to the old one. > > would be a lot cleaner to just break it and be done. We are not at liberty to break something that has legitimately compiled successfully for two decades. Your __kernel_ prefix idea breaks compilation just as equally. One thing you certainly don't have to be is happy about this header file situation, but you cannot use that dissatisfaction as justification to make the situation worse by breaking the build for people outside of the world you directly control.

1 0

Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
by David Miller 18 Jan '13

18 Jan '13

From: Mike Frysinger <vapier(a)gentoo.org> Date: Wed, 16 Jan 2013 12:04:56 -0500 > certainly true, but the current expectation is that you don't mix your ABIs. > if you're programming with the C library API, then use the C library headers. > if you're banging directly on the kernel, then use the kernel headers. not > saying it's a perfect solution, but it works for the vast majority of use > cases. This isn't how real life works. GLIBC itself brings in some of the kernel headers, as do various library headers for libraries other than glibc. So you can get these conflicting headers included indirectly, and it is of no fault of any of the various parties involved. We have to make them work when included at the same time somehow, and this is totally unavoidable.

3 5

[libvirt] [PATCH] build: fix build on BSD
by Eric Blake 18 Jan '13

18 Jan '13

A build on FreeBSD failed with: util/virportallocator.c:108: error: storage size of 'addr' isn't known util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function) It turns out that while POSIX allows sockaddr_in to leak in through other headers (the way Linux does it), conforming applications are required to get it through netinet/in.h. * src/util/virportallocator.c: Include header for struct sockaddr_in. --- Pushing under the build-breaker rule. src/util/virportallocator.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 35f2157..590bb57 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -23,6 +23,7 @@ #include <sys/socket.h> #include <arpa/inet.h> +#include <netinet/in.h> #include "viralloc.h" #include "virbitmap.h" -- 1.8.0.2

1 0

[libvirt] [PATCH v2.1 RESEND 00/11] Rework storage migration
by Michal Privoznik 18 Jan '13

18 Jan '13

This patch set re-implements migration with storage for enough new qemu. Currently, you can migrate a domain to a host without need for shared storage. This is done by setting 'blk' or 'inc' attribute (representing VIR_MIGRATE_NON_SHARED_DISK and VIR_MIGRATE_NON_SHARED_INC flags respectively) of 'migrate' monitor command. However, the qemu implementation is buggy and applications are advised to switch to new impementation which, moreover, offers some nice features, like migrating only explicitly specified disks. The new functionality is controlled via 'nbd-server-*' and 'drive-mirror' commands. The flow is meant to look like this: 1) User invokes libvirt's migrate functionality. 2) libvirt checks that no block jobs are active on the source. 3) libvirt starts the destination QEMU and sets up the NBD server using the nbd-server-start and nbd-server-add commands. 4) libvirt starts drive-mirror with a destination pointing to the remote NBD server, for example nbd:host:port:exportname=diskname (where diskname is the -drive id specified on the destination). 5) once all mirroring jobs reach steady state, libvirt invokes the migrate command. 6) once migration completed, libvirt invokes the nbd-server-stop command on the destination QEMU. If we just skip the 2nd step and there is an active block-job, qemu will fail in step 4. No big deal. Since we try to NOT break migration and keep things compatible, this feature is enabled iff both sides support it. Since there's obvious need for some data transfer between src and dst, I've put it into qemuCookieMigration: 1) src -> dest: (QEMU_MIGRATION_PHASE_BEGIN3 -> QEMU_MIGRATION_PHASE_PREPARE) <nbd> <disk size='17179869184'/> </nbd> Hey destination, I know how to use this cool new feature. Moreover, these are the disks I'll send you. Each one of them is X bytes big. It's one of the prerequisite - the file (disk->src) on dst exists and has at least the same size as on dst. 2) dst -> src: (QEMU_MIGRATION_PHASE_PREPARE -> QEMU_MIGRATION_PHASE_PERFORM3) <nbd port='X'/> Okay, I (destination) support this feature as well. I've created all files as you (src) told me to and you can start rolling data. I am listening on port X. 3) src -> dst: (QEMU_MIGRATION_PHASE_PERFORM3 -> QEMU_MIGRATION_PHASE_FINISH3) <nbd port='-1'/> Migration completed, destination, you may shut the NBD server down. If either src or dst doesn't support NBD, it is not used and whole process fall backs to old implementation. diff to v1: -Eric's and Daniel's suggestions worked in. To point out the bigger ones: don't do NBD style when TUNNELLED requested, added 'b:writable' to 'nbd-server-add' -drop '/qemu-migration/nbd/disk/@src' attribute from migration cookie. As pointed out by Jirka, disk->src can be changed during migration (e.g. by migration hook or by passed xml). So I've tried (as suggested on the list) passing disk alias. However, since qemu hasn't been started on destination yet, the aliases hasn't been generated yet. So we have to rely on ordering completely. diff to v2: -rebase to reflect changes made by offline migration patch -send initial nbd cookie only if needed diff to previous v2.1: -rebase to current HEAD The patches 1,3 and 5 has been ACKed already. Maybe, I'll need to drop 7/11 patch completely, once Dan's patches [1] are in. 1: https://www.redhat.com/archives/libvir-list/2013-January/msg00742.html Michal Privoznik (11): qemu: Introduce NBD_SERVER capability Introduce NBD migration cookie qemu: Introduce nbd-server-start command qemu: Introduce nbd-server-add command qemu: Introduce nbd-server-stop command qemu_migration: Introduce qemuMigrationStartNBDServer qemu_migration: Move port allocation to a separate func qemu_migration: Implement qemuMigrationStartNBDServer() qemu_migration: Implement qemuMigrationDriveMirror qemu_migration: Check size prerequisites qemu_migration: Stop NBD server at Finish phase src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_migration.c | 624 ++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 63 +++++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 95 +++++++ src/qemu/qemu_monitor_json.h | 7 + 7 files changed, 768 insertions(+), 33 deletions(-) -- 1.8.0.2

2 23

[libvirt] [RFC] Data in the <topology> element in the capabilities XML
by Peter Krempa 18 Jan '13

18 Jan '13

Hi everybody, a while ago there was a discussion about changing the data that is returned in the <topology> sub-element: <capabilities> <host> <cpu> <arch>x86_64</arch> <model>SandyBridge</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='2'/> The data provided here is as of today taken from the nodeinfo detection code and thus is really wrong when the fallback mechanisms are used. To get a useful count, the user has to multiply the data by the number of NUMA nodes in the host. With the fallback detection code used for nodeinfo the NUMA node count used to get the CPU count should be 1 instead of the actual number. As Jiri proposed, I think we should change this output to separate detection code that will not take into account NUMA nodes for this output and will rather provide data as the "lspci" command does. This change will make the data provided by the element standalone and also usable in guest XMLs to mirror host's topology. The meaning of the attributes of that element isn't really documented anywhere, so as an additional precaution we should document that. ( http://libvirt.org/formatcaps.html ) For some additional background, please refer to the original discussion here: https://www.redhat.com/archives/libvir-list/2012-March/msg01123.html Thanks in advance for your comments and suggestions. Peter

3 16

[libvirt] [PATCH v2] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 18 Jan '13

18 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 6 +++--- tests/virportallocatortest.c | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..d80347a 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; @@ -168,7 +168,7 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectLock(pa); if (port < pa->start || - port >= pa->end) { + port > pa->end) { virReportInvalidArg(port, "port %d must be in range (%d, %d)", port, pa->start, pa->end); goto cleanup; diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 93577d7..3f6edcc 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -59,7 +59,7 @@ int bind(int sockfd ATTRIBUTE_UNUSED, static int testAllocAll(const void *args ATTRIBUTE_UNUSED) { - virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5910); + virPortAllocatorPtr alloc = virPortAllocatorNew(5900, 5909); int ret = -1; unsigned short p1, p2, p3, p4, p5, p6, p7; -- 1.8.0.2

3 2

[libvirt] [PATCH] Avoid integer wrap on remotePortMax in QEMU driver
by Daniel P. Berrange 18 Jan '13

18 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU driver default max port is 65535, but it then increments this by 1 to 65536. This maps to 0 in an unsigned short :-( This was apparently done so that for() loops could use "< max" instead of "<= max". Remove this insanity and just make the loop do the right thing. --- src/qemu/qemu_conf.c | 4 ---- src/util/virportallocator.c | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 680e8fb..b21392e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -233,10 +233,6 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, filename, QEMU_REMOTE_PORT_MAX); goto cleanup; } - /* increasing the value by 1 makes all the loops going through - the bitmap (i = remotePortMin; i < remotePortMax; i++), work as - expected. */ - driver->remotePortMax++; if (driver->remotePortMin > driver->remotePortMax) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 033aee4..5d07dd0 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -84,7 +84,7 @@ virPortAllocatorPtr virPortAllocatorNew(unsigned short start, pa->start = start; pa->end = end; - if (!(pa->bitmap = virBitmapNew(end-start))) { + if (!(pa->bitmap = virBitmapNew((end-start)+1))) { virReportOOMError(); virObjectUnref(pa); return NULL; @@ -103,7 +103,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, *port = 0; virObjectLock(pa); - for (i = pa->start ; i < pa->end && !*port; i++) { + for (i = pa->start ; i <= pa->end && !*port; i++) { int reuse = 1; struct sockaddr_in addr; bool used = false; -- 1.8.0.2

3 2

[libvirt] [PATCH 0/5] Resolve REVERSE_INULL errors found by Coverity
by John Ferlan 18 Jan '13

18 Jan '13

This patch will resolve a set of "REVERSE_INULL (CWE-476):" errors found by Coverity. John Ferlan (5): network: Resolve some issues around vlan copying xen: Remove extraneous checks in xenStoreGetDomainInfo() interface: Need to check ifacedef->mac not just ifacedef after strdup() xen: Need to check validity of domain->conn before deref privateData openvz: Need to check 'vm' first before dereferencing 'def' src/interface/interface_backend_udev.c | 2 +- src/network/bridge_driver.c | 16 +++++++++++++--- src/openvz/openvz_driver.c | 2 +- src/xen/xen_hypervisor.c | 3 ++- src/xen/xs_internal.c | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) -- 1.7.11.7

5 15

[libvirt] [PATCH] network: use bandwidth from portgroup when appropriate
by Laine Stump 18 Jan '13

18 Jan '13

The bandwidth plug and unplug functions were assuming that an interface's bandwidth setting was always specified directly in the domain's <interface> definition, but that's not necessarily true - it could have been obtained from a <portgroup> definition in the network definition. This patch fixes those functions to use virDomainNetGetActualBandwidth(), which gets the bandwidth pointer from iface->data.network.actual if it exists, otherwise returns iface->bandwidth. --- src/network/bridge_driver.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c3cb63d..6f3c839 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4518,7 +4518,7 @@ networkCheckBandwidth(virNetworkObjPtr net, { int ret = -1; virNetDevBandwidthPtr netBand = net->def->bandwidth; - virNetDevBandwidthPtr ifaceBand = iface->bandwidth; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); unsigned long long tmp_floor_sum = net->floor_sum; unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; @@ -4597,6 +4597,7 @@ networkPlugBandwidth(virNetworkObjPtr net, unsigned long long new_rate = 0; ssize_t class_id = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) { /* helper reported error */ @@ -4625,11 +4626,8 @@ networkPlugBandwidth(virNetworkObjPtr net, goto cleanup; } - plug_ret = virNetDevBandwidthPlug(net->def->bridge, - net->def->bandwidth, - &iface->mac, - iface->bandwidth, - class_id); + plug_ret = virNetDevBandwidthPlug(net->def->bridge, net->def->bandwidth, + &iface->mac, ifaceBand, class_id); if (plug_ret < 0) { ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4638,11 +4636,11 @@ networkPlugBandwidth(virNetworkObjPtr net, /* QoS was set, generate new class ID */ iface->data.network.actual->class_id = class_id; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { ignore_value(virBitmapClearBit(net->class_id, class_id)); - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; iface->data.network.actual->class_id = 0; ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); goto cleanup; @@ -4666,6 +4664,7 @@ networkUnplugBandwidth(virNetworkObjPtr net, { int ret = 0; unsigned long long new_rate; + virNetDevBandwidthPtr ifaceBand = virDomainNetGetActualBandwidth(iface); if (iface->data.network.actual && iface->data.network.actual->class_id) { @@ -4680,13 +4679,13 @@ networkUnplugBandwidth(virNetworkObjPtr net, if (ret < 0) goto cleanup; /* update sum of 'floor'-s of attached NICs */ - net->floor_sum -= iface->bandwidth->in->floor; + net->floor_sum -= ifaceBand->in->floor; /* return class ID */ ignore_value(virBitmapClearBit(net->class_id, iface->data.network.actual->class_id)); /* update status file */ if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) { - net->floor_sum += iface->bandwidth->in->floor; + net->floor_sum += ifaceBand->in->floor; ignore_value(virBitmapSetBit(net->class_id, iface->data.network.actual->class_id)); goto cleanup; -- 1.7.11.7

2 2

[libvirt] [PATCH] conf: don't fail to parse <boot> element when parsing a single device
by Laine Stump 18 Jan '13

18 Jan '13

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=895294 The symptom was that attempts to modify a network device using virDomainUpdateDeviceFlags() would fail if the original device had a <boot> element (e.g. "<boot order='1'/>"), even if the updated device had the same <boot> element. Instead, the following error would be logged: cannot modify network device boot index setting It's true that it's not possible to change boot order (internally known as bootIndex) of a live device; qemuDomainChangeNet checks for that, but the problem was that the information it was checking was incorrect. Explanation: When a complete domain is parsed, a global (to the domain) "bootMap" is passed down to the parse for each device; the bootMap is used to make sure that devices don't have conflicting settings for their boot orders. When a single device is parsed by itself (as in the case of virDomainUpdateDeviceFlags), there is no global bootMap that would be appropriate to send, so NULL is sent instead. However, although the lowest level function that parses just the boot order *does* simply skip the sanity check in that case, the next higher level "virDomainDeviceInfoParseXML" function refuses to call down to the lower "virDomainDeviceBootParseXML" if bootMap is NULL. So, the boot order is never set in the "new" device object, and when it is compared to the original (which does have a boot order), they don't match. The fix is to patch virDomainDeviceInfoParseXML to not care about bootMap, and just always call virDomainDeviceInfoBootParseXML whenever there is a <boot> element. When we are only parsing a single device, we don't care whether or not any specified boot order is consistent with the rest of the domain; we will always do this check later (in the current case, we do it by verifying that the net bootIndex exactly matches the old bootIndex). --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06b9c25..e4babaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1,7 +1,7 @@ /* * domain_conf.c: domain XML processing * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2624,7 +2624,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, } else if (master == NULL && xmlStrEqual(cur->name, BAD_CAST "master")) { master = cur; - } else if (boot == NULL && bootMap && + } else if (boot == NULL && (flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) && xmlStrEqual(cur->name, BAD_CAST "boot")) { boot = cur; -- 1.7.11.7

2 2

[libvirt] [PATCH] lock_daemon: Simplify error handling
by Peter Krempa 17 Jan '13

17 Jan '13

No need to check the run_dir variable twice for NULL. --- src/locking/lock_daemon.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f0eba48..97e5d74 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1311,19 +1311,16 @@ int main(int argc, char **argv) { /* Ensure the rundir exists (on tmpfs on some systems) */ if (privileged) { - run_dir = strdup(LOCALSTATEDIR "/run/libvirt"); + if (!(run_dir = strdup(LOCALSTATEDIR "/run/libvirt"))) { + virReportOOMError(); + goto cleanup; + } } else { - run_dir = virGetUserRuntimeDirectory(); - - if (!run_dir) { + if (!(run_dir = virGetUserRuntimeDirectory())) { VIR_ERROR(_("Can't determine user directory")); goto cleanup; } } - if (!run_dir) { - virReportOOMError(); - goto cleanup; - } if (privileged) old_umask = umask(022); -- 1.8.1.1

2 2

[libvirt] [PATCH 0/2 v2] Colorize HTML documentation
by Claudio Bley 17 Jan '13

17 Jan '13

The only difference to v1 is that I fixed the CSS pseudo elements adding some vital whitespaces. (I tried to be extra smart and removed some whitespace without further testing, just to discover that it did not work anymore *after* sending the patcheset). Claudio Bley (2): docs: Assign classes to documentation elements docs: Add some style and color to the HTML documentation docs/generic.css | 4 ++ docs/libvirt.css | 56 +++++++++++++++- docs/newapi.xsl | 187 +++++++++++++++++++++++++++++++----------------------- 3 files changed, 166 insertions(+), 81 deletions(-) -- 1.7.9.5

2 6

[libvirt] [PATCH 00/12] virsh: Partially convert to c99 struct initialization
by Michal Privoznik 17 Jan '13

17 Jan '13

For now, we have to suffice with vshCmdOptDef. Others coming soon. Changes are pretty mechanical, so if you find a mistake there, blame my vim macro :) Michal Privoznik (12): virsh.c: Switch to C99 initialization of vshCmdOptDef virsh-domain.c: Switch to C99 initialization of vshCmdOptDef virsh-domain-monitor.c: Switch to C99 initialization of vshCmdOptDef virsh-host.c: Switch to C99 initialization of vshCmdOptDef virsh-interface.c: Switch to C99 initialization of vshCmdOptDef virsh-network.c: Switch to C99 initialization of vshCmdOptDef virsh-nodedev.c: Switch to C99 initialization of vshCmdOptDef virsh-nwfilter.c: Switch to C99 initialization of vshCmdOptDef virsh-pool.c: Switch to C99 initialization of vshCmdOptDef virsh-secret.c: Switch to C99 initialization of vshCmdOptDef virsh-snapshot.c: Switch to C99 initialization of vshCmdOptDef virsh-volume.c: Switch to C99 initialization of vshCmdOptDef tools/virsh-domain-monitor.c | 279 +++-- tools/virsh-domain.c | 2299 +++++++++++++++++++++++++++++++++--------- tools/virsh-host.c | 102 +- tools/virsh-interface.c | 138 ++- tools/virsh-network.c | 193 +++- tools/virsh-nodedev.c | 64 +- tools/virsh-nwfilter.c | 34 +- tools/virsh-pool.c | 286 ++++-- tools/virsh-secret.c | 72 +- tools/virsh-snapshot.c | 452 +++++++-- tools/virsh-volume.c | 368 +++++-- tools/virsh.c | 62 +- 12 files changed, 3406 insertions(+), 943 deletions(-) -- 1.8.0.2

3 28

[libvirt] [PATCH] libvirt.c: fix typo in comment
by Claudio Bley 17 Jan '13

17 Jan '13

Signed-off-by: Claudio Bley <cbley(a)av-test.de> --- Pushing under the trivial rule. src/libvirt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 7db677f..d5d561c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17181,7 +17181,7 @@ error: * encrypted, or running over a channel which is not exposed * to eavesdropping (eg a UNIX domain socket, or pipe) * - * Returns 1 if secure, 0 if secure, -1 on error + * Returns 1 if secure, 0 if not secure, -1 on error */ int virConnectIsSecure(virConnectPtr conn) { -- 1.7.9.5

1 0

[libvirt] [PATCH] Move comment after enum members
by Claudio Bley 17 Jan '13

17 Jan '13

The api builder always associates comments to the last member it read, not to the current member even if there was a comment for the previous member and a comma was already seen. This has the effect that the comment for the previous member gets overwritten and the current member has no comment at all. Signed-off-by: Claudio Bley <cbley(a)av-test.de> --- include/libvirt/libvirt.h.in | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 09c89c5..9110fcf 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -127,12 +127,12 @@ typedef enum { power management */ #ifdef VIR_ENUM_SENTINELS - /* - * NB: this enum value will increase over time as new events are - * added to the libvirt API. It reflects the last state supported - * by this version of the libvirt API. - */ VIR_DOMAIN_LAST + /* + * NB: this enum value will increase over time as new events are + * added to the libvirt API. It reflects the last state supported + * by this version of the libvirt API. + */ #endif } virDomainState; @@ -2742,12 +2742,12 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ #ifdef VIR_ENUM_SENTINELS + VIR_STORAGE_VOL_WIPE_ALG_LAST /* * NB: this enum value will increase over time as new algorithms are * added to the libvirt API. It reflects the last algorithm supported * by this version of the libvirt API. */ - VIR_STORAGE_VOL_WIPE_ALG_LAST #endif } virStorageVolWipeAlgorithm; @@ -2974,12 +2974,12 @@ typedef enum { VIR_KEYCODE_SET_RFB = 9, #ifdef VIR_ENUM_SENTINELS + VIR_KEYCODE_SET_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last keycode set supported * by this version of the libvirt API. */ - VIR_KEYCODE_SET_LAST #endif } virKeycodeSet; @@ -3533,12 +3533,12 @@ typedef enum { VIR_SECRET_USAGE_TYPE_CEPH = 2, #ifdef VIR_ENUM_SENTINELS + VIR_SECRET_USAGE_TYPE_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last secret owner ID * supported by this version of the libvirt API. */ - VIR_SECRET_USAGE_TYPE_LAST #endif } virSecretUsageType; @@ -4443,12 +4443,12 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ #ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_EVENT_ID_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last event ID supported * by this version of the libvirt API. */ - VIR_DOMAIN_EVENT_ID_LAST #endif } virDomainEventID; -- 1.7.9.5

2 6