- Devel - libvirt List Archives

[libvirt] [PATCH] Don't start a nested job in qemuMigrationPrepareAny
by Ján Tomko 22 Nov '13

22 Nov '13

This nested job is canceled by the first ExitMonitor call (even though it was not created by the corresponding EnterMonitor call), and again in qemuMigrationPrepareAny if qemuProcessStart failed. This can lead to a crash if the vm object was disposed of before calling qemuDomainRemoveInactive: 0 ..62bc in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7ffce4cdd270) at util/virobject.c:166 1 ..6666 in virObjectIsClass at util/virobject.c:362 2 ..66b4 in virObjectLock at util/virobject.c:314 3 ..477e in virDomainObjListRemove at conf/domain_conf.c:2359 4 ..7a64 in qemuDomainRemoveInactive at qemu/qemu_domain.c:2087 5 ..956c in qemuMigrationPrepareAny at qemu/qemu_migration.c:2469 This was added by commit e4e2822, exposed by 5a4c237 and c7ac251. https://bugzilla.redhat.com/show_bug.cgi?id=1018267 --- src/qemu/qemu_migration.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e87ea85..ef6f1c5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2358,10 +2358,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto endjob; } - if (qemuDomainObjBeginNestedJob(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - goto endjob; - /* Start the QEMU daemon, with the same command-line arguments plus * -incoming $migrateFrom */ @@ -2370,8 +2366,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) { virDomainAuditStart(vm, "migrated", false); - if (!qemuDomainObjEndJob(driver, vm)) - vm = NULL; goto endjob; } @@ -2474,7 +2468,7 @@ stop: qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0); endjob: - if (vm && !qemuMigrationJobFinish(driver, vm)) { + if (!qemuMigrationJobFinish(driver, vm)) { vm = NULL; } goto cleanup; -- 1.8.3.2

2 2

[libvirt] [PATCH] util: Make port allocator data more up-to-date
by Martin Kletzander 22 Nov '13

22 Nov '13

There are few places where we know about port getting opened even though it was not allocated on any virPortAllocator (or we know it was reserved like this). We cannot notify the virPortAllocator that the port is open because of that (since the port may be out of range) and in that case, we are stuck with old data. This also fixes the issue that if you have autoport='no' and port='-1', the port never gets returned (introduced by 7b4a630484); the case when we don't know whether to release the port or not. There may be also other places where the port is being released even though it might have been set statically out of range by the user. In that case we report an error, but that shouldn't stop APIs releasing ports (e.g. destroy). Moreover, the return value is not checked sometimes and we're stuck with an error in the logs and also set as a lastError. To make this behave the best I can imagine, we'd have to have a single virPortAllocator with configurable ranges (ports shared between allocators), but that's an idea for another day. In this patch, I'm modifyping the virPortAllocatorRelease() function to be safe for all kinds of parameters, making it a no-op in case of out-of-range ports and warning in case virBitmapClearBit() returns an error (which is just a sanity code in combination with the range check). Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/libxl/libxl_driver.c | 13 ++++--------- src/qemu/qemu_process.c | 32 +++++++++++++++++++++----------- src/util/virportallocator.c | 33 +++++++++------------------------ src/util/virportallocator.h | 4 ++-- tests/virportallocatortest.c | 5 +---- 5 files changed, 37 insertions(+), 50 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1b42f14..26e2d3a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -279,15 +279,10 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - vnc_port = vm->def->graphics[0]->data.vnc.port; - if (vnc_port >= LIBXL_VNC_PORT_MIN) { - if (virPortAllocatorRelease(driver->reservedVNCPorts, - vnc_port) < 0) - VIR_DEBUG("Could not mark port %d as unused", vnc_port); - } + if (vm->def->ngraphics == 1 && + vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virPortAllocatorRelease(driver->reservedVNCPorts, + vm->def->graphics[0]->data.vnc.port); } /* Remove any cputune settings */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a26c079..dfaf95f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4331,25 +4331,35 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC and Spice ports from port reservation bitmap, but only if - they were reserved by the driver (autoport=yes) - */ + /* Remove remote ports from all port allocators, even when + * autoport isn't set, because they might have been auto-allocated + * anyway. It also helps keeping the allocator data a tad more + * updated. + */ for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - if (graphics->data.vnc.autoport) { - virPortAllocatorRelease(driver->remotePorts, - graphics->data.vnc.port); - } + switch ((enum virDomainGraphicsType) graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + virPortAllocatorRelease(driver->remotePorts, + graphics->data.vnc.port); virPortAllocatorRelease(driver->webSocketPorts, graphics->data.vnc.websocket); - } - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->data.spice.autoport) { + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: virPortAllocatorRelease(driver->remotePorts, graphics->data.spice.port); virPortAllocatorRelease(driver->remotePorts, graphics->data.spice.tlsPort); + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + /* Nothing has been allocated */ + + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; } } diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 694f191..f7f3755 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -27,11 +27,12 @@ #include "viralloc.h" #include "virbitmap.h" -#include "virportallocator.h" -#include "virthread.h" #include "virerror.h" #include "virfile.h" +#include "virlog.h" +#include "virportallocator.h" #include "virstring.h" +#include "virthread.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -172,33 +173,17 @@ cleanup: return ret; } -int virPortAllocatorRelease(virPortAllocatorPtr pa, - unsigned short port) +void virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port) { - int ret = -1; - if (!port) - return 0; + return; virObjectLock(pa); - if (port < pa->start || - port > pa->end) { - virReportInvalidArg(port, "port %d must be in range (%d, %d)", - port, pa->start, pa->end); - goto cleanup; - } - - if (virBitmapClearBit(pa->bitmap, - port - pa->start) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to release port %d"), - port); - goto cleanup; - } + if (port >= pa->start && port <= pa->end && + virBitmapClearBit(pa->bitmap, port - pa->start) < 0) + VIR_WARN("Problem releasing port %d from range '%s'", port, pa->name); - ret = 0; -cleanup: virObjectUnlock(pa); - return ret; } diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..ba7c504 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -35,7 +35,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name, int virPortAllocatorAcquire(virPortAllocatorPtr pa, unsigned short *port); -int virPortAllocatorRelease(virPortAllocatorPtr pa, - unsigned short port); +void virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port); #endif /* __VIR_PORT_ALLOCATOR_H__ */ diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 721356e..0d41c98 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -165,10 +165,7 @@ static int testAllocReuse(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - - if (virPortAllocatorRelease(alloc, p2) < 0) - goto cleanup; - + virPortAllocatorRelease(alloc, p2); if (virPortAllocatorAcquire(alloc, &p4) < 0) goto cleanup; if (p4 != 5902) { -- 1.8.4.3

3 3

[libvirt] [PATCH] spec: Don't save/restore running VMs on libvirt-client update
by Cole Robinson 22 Nov '13

22 Nov '13

Restarting an active libvirt-guests.service is the equivalent of doing: /usr/libexec/libvirt-guests.sh stop /usr/libexec/libvirt-guests.sh start Which in a default configuration will managedsave every running VM, and then restore them. Certainly not something we should do every time the libvirt-client RPM is updated. Just drop the try-restart attempt, I don't know what purpose it serves anyways. https://bugzilla.redhat.com/show_bug.cgi?id=962225 --- libvirt.spec.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a5b01df..0d50ccd 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1714,9 +1714,8 @@ fi # If the package is allowed to autostart: /bin/systemctl --no-reload enable libvirt-guests.service >/dev/null 2>&1 ||: -# Run these because the SysV package being removed won't do them +# Run this because the SysV package being removed won't do them /sbin/chkconfig --del libvirt-guests >/dev/null 2>&1 || : -/bin/systemctl try-restart libvirt-guests.service >/dev/null 2>&1 || : %endif %if %{with_sanlock} -- 1.8.4.2

2 2

[libvirt] [PATCH] spec: Restrict virt-login-shell usage
by Jiri Denemark 22 Nov '13

22 Nov '13

https://bugzilla.redhat.com/show_bug.cgi?id=1033614 As virt-login-shell is an SUID binary, we should restrict its usage to just the users chosen by an administrator to use virt-login-shell as their login shell. This can easily be done by making the binary executable only by users from a new virtlogin group. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- libvirt.spec.in | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a5b01df..864fbf4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1727,6 +1727,12 @@ if getent group sanlock > /dev/null ; then fi %endif +%if %{with_lxc} +%pre login-shell +getent group virtlogin >/dev/null || groupadd -r virtlogin +exit 0 +%endif + %files %defattr(-, root, root) @@ -2072,7 +2078,7 @@ fi %if %{with_lxc} %files login-shell -%attr(4755, root, root) %{_bindir}/virt-login-shell +%attr(4750, root, virtlogin) %{_bindir}/virt-login-shell %config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf %{_mandir}/man1/virt-login-shell.1* %endif -- 1.8.4.4

2 2

[libvirt] [PATCH 0/2] Network events feature
by Cédric Bosdonnat 22 Nov '13

22 Nov '13

These changes are all about bringing events for network object like the ones existing for domains. This feature is needed for virt-manager to refresh its UI when networks are started/destroyed/defined/undefined. The first commit is a refactoring of the domain events internal functions to extract the reusable parts of them. The second commit adds network events based on those now-common event functions. The network events are implemented in the test and bridge network drivers ATM. Cédric Bosdonnat (2): virDomainEvent refactoring to separate generic things from domain ones Added network events API and unit tests .gitignore | 2 + daemon/libvirtd.h | 1 + daemon/remote.c | 175 +++- include/libvirt/libvirt.h.in | 85 ++ python/generator.py | 2 + python/libvirt-override-virConnect.py | 34 + python/libvirt-override.c | 150 ++++ src/Makefile.am | 6 + src/conf/domain_event.c | 1431 ++++++--------------------------- src/conf/domain_event.h | 134 ++- src/conf/object_event.c | 1008 +++++++++++++++++++++++ src/conf/object_event.h | 114 +++ src/conf/object_event_private.h | 167 ++++ src/driver.h | 14 + src/libvirt.c | 133 +++ src/libvirt_private.syms | 18 +- src/libvirt_public.syms | 6 + src/libxl/libxl_conf.h | 2 +- src/libxl/libxl_driver.c | 28 +- src/lxc/lxc_conf.h | 2 +- src/lxc/lxc_driver.c | 38 +- src/lxc/lxc_process.c | 12 +- src/network/bridge_driver.c | 91 ++- src/network/bridge_driver_platform.h | 3 + src/parallels/parallels_utils.h | 2 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_domain.c | 6 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 50 +- src/qemu/qemu_hotplug.c | 10 +- src/qemu/qemu_migration.c | 12 +- src/qemu/qemu_process.c | 48 +- src/remote/remote_driver.c | 173 +++- src/remote/remote_protocol.x | 46 +- src/test/test_driver.c | 166 ++-- src/uml/uml_conf.h | 2 +- src/uml/uml_driver.c | 26 +- src/vbox/vbox_tmpl.c | 20 +- src/xen/xen_driver.c | 10 +- src/xen/xen_driver.h | 4 +- src/xen/xen_inotify.c | 8 +- src/xen/xs_internal.c | 4 +- tests/Makefile.am | 7 + tests/objecteventtest.c | 228 ++++++ tests/qemuhotplugtest.c | 2 +- 45 files changed, 2974 insertions(+), 1510 deletions(-) create mode 100644 src/conf/object_event.c create mode 100644 src/conf/object_event.h create mode 100644 src/conf/object_event_private.h create mode 100644 tests/objecteventtest.c -- 1.8.4.2

3 7

[libvirt] [PATCH v2] sasl: Fix authentication when using PLAIN mechanism
by Christophe Fergeau 22 Nov '13

22 Nov '13

With some authentication mechanism (PLAIN for example), sasl_client_start() can return SASL_OK, which translates to virNetSASLSessionClientStart() returning VIR_NET_SASL_COMPLETE. cyrus-sasl documentation is a bit vague as to what to do in such situation, but upstream clarified this a bit in http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&ms… When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and if the remote also tells us that authentication is complete, then we should end the authentication procedure rather than forcing a call to virNetSASLSessionClientStep(). Without this patch, when trying to use SASL PLAIN, I get: error :authentication failed : Failed to step SASL negotiation: -1 (SASL(-1): generic failure: Unable to find a callback: 32775) This patch is based on a spice-gtk patch by Dietmar Maurer. --- Changes since v1: - adjust comment before the for(;;) code block - add comment before the if (complete && ...) test as this is what is done for these tests in this block. src/remote/remote_driver.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7181949..38da119 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4122,9 +4122,16 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, complete, serverinlen, serverin); /* Loop-the-loop... - * Even if the server has completed, the client must *always* do at least one step - * in this loop to verify the server isn't lying about something. Mutual auth */ + * Even if the server has completed, the client must loop until sasl_client_start() or + * sasl_client_step() return SASL_OK to verify the server isn't lying + * about something. Mutual auth + * */ for (;;) { + /* Previous server call showed completion & sasl_client_start() + * told us we are locally complete too */ + if (complete && err == VIR_NET_SASL_COMPLETE) + break; + restep: if ((err = virNetSASLSessionClientStep(sasl, serverin, -- 1.8.4.2

2 1

[libvirt] [PATCH] m4: use -Wl,-pie instead of -pie for clang
by Ryota Ozaki 22 Nov '13

22 Nov '13

clang doesn't support -pie option (as of clang 3.3): clang: error: argument unused during compilation: '-pie' [-Werror,-Wunused-command-line-argument] We have to use -Wl,-pie instead. (ref. https://bugs.ruby-lang.org/issues/5697) The new code checks -pie first, and if it fails, checks -Wl,-pie next. By doing so, we can handle both cases. The patch is tested on Mac OS X 10.8.5 and Fedora 19. Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com> --- m4/virt-compile-pie.m4 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/m4/virt-compile-pie.m4 b/m4/virt-compile-pie.m4 index cceb495..11cf0af 100644 --- a/m4/virt-compile-pie.m4 +++ b/m4/virt-compile-pie.m4 @@ -27,7 +27,14 @@ AC_DEFUN([LIBVIRT_COMPILE_PIE],[ *) gl_COMPILER_OPTION_IF([-fPIE -DPIE], [ PIE_CFLAGS="-fPIE -DPIE" - PIE_LDFLAGS="-pie" + gl_COMPILER_OPTION_IF(["-pie"], [ + PIE_LDFLAGS="-pie" + ], [ + dnl clang requires -Wl,-pie instead of -pie + gl_COMPILER_OPTION_IF(["-Wl,-pie"], [ + PIE_LDFLAGS="-Wl,-pie" + ]) + ]) ]) esac AC_SUBST([PIE_CFLAGS]) -- 1.8.4

1 0

[libvirt] [PATCH] sasl: Fix authentication when using PLAIN mechanism
by Christophe Fergeau 22 Nov '13

22 Nov '13

With some authentication mechanism (PLAIN for example), sasl_client_start() can return SASL_OK, which translates to virNetSASLSessionClientStart() returning VIR_NET_SASL_COMPLETE. cyrus-sasl documentation is a bit vague as to what to do in such situation, but upstream clarified this a bit in http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&ms… When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and if the remote also tells us that authentication is complete, then we should end the authentication procedure rather than forcing a call to virNetSASLSessionClientStep(). Without this patch, when trying to use SASL PLAIN, I get: error :authentication failed : Failed to step SASL negotiation: -1 (SASL(-1): generic failure: Unable to find a callback: 32775) This patch is based on a spice-gtk patch by Dietmar Maurer. --- src/remote/remote_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7181949..e15eeaf 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4125,6 +4125,9 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, * Even if the server has completed, the client must *always* do at least one step * in this loop to verify the server isn't lying about something. Mutual auth */ for (;;) { + if (complete && err == VIR_NET_SASL_COMPLETE) + break; + restep: if ((err = virNetSASLSessionClientStep(sasl, serverin, -- 1.8.4.2

2 3

[libvirt] [PATCH] build: Don't fail on '< ' or '> ' with old xmllint
by Martin Kletzander 22 Nov '13

22 Nov '13

Older xmllint version don't allow such characters in datatype anyURI. In order not to change too much, I'm suggesting making a choice of anyURI or 'absPathName' which should be fine (checked with upstream and that old xmllint, both work fine). Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: Pushed as a build-breaker docs/schemas/storagevol.rng | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8bc5907..5da8e1f 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -105,7 +105,10 @@ <element name='target'> <optional> <element name='path'> - <data type='anyURI'/> + <choice> + <data type='anyURI'/> + <ref name='absFilePath'/> + </choice> </element> </optional> <ref name='format'/> -- 1.8.4.3

1 0

[libvirt] [PATCH] storage: use valid XML for awkward volume names
by Eric Blake 22 Nov '13

22 Nov '13

$ touch /var/lib/libvirt/images/'a<b>c' $ virsh pool-refresh default $ virsh vol-dumpxml 'a<b>c' default | head -n2 <volume> <name>a<b>c</name> Oops. That's not valid XML. And when we fix the XML generation, it fails RelaxNG validation. I'm also tired of seeing <key>(null)</key> in the example output for volume xml; while we used NULLSTR() to avoid a NULL deref rather than relying on glibc's printf extension behavior, it's even better if we avoid the issue in the first place. Note that this patch allows pretty much any volume name that can appear in a directory (excluding . and .. because those are special), but does nothing to change the current (unenforced) RelaxNG claim that pool names will consist only of letters, numbers, _, -, and +. Tightening the C code to match RelaxNG patterns and/or relaxing the grammar to match the C code for pool names is a task for another day (but remember, we DID recently tighten C code for domain names to exclude a leading '.'). * src/conf/storage_conf.c (virStoragePoolSourceFormat) (virStoragePoolDefFormat, virStorageVolTargetDefFormat) (virStorageVolDefFormat): Escape user-controlled strings. (virStorageVolDefParseXML): Parse key, for use in unit tests. * docs/schemas/basictypes.rng (volName): Relax definition. * tests/storagepoolxml2xmltest.c (mymain): Test it. * tests/storagevolxml2xmltest.c (mymain): Likewise. * tests/storagepoolxml2xmlin/pool-dir-naming.xml: New file. * tests/storagepoolxml2xmlout/pool-dir-naming.xml: Likewise. * tests/storagevolxml2xmlin/vol-file-naming.xml: Likewise. * tests/storagevolxml2xmlout/vol-file-naming.xml: Likewise. * tests/storagevolxml2xmlout/vol-*.xml: Fix fallout. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- docs/schemas/basictypes.rng | 9 ++- src/conf/storage_conf.c | 72 ++++++++++------------ tests/storagepoolxml2xmlin/pool-dir-naming.xml | 18 ++++++ tests/storagepoolxml2xmlout/pool-dir-naming.xml | 18 ++++++ tests/storagepoolxml2xmltest.c | 1 + tests/storagevolxml2xmlin/vol-file-backing.xml | 1 + tests/storagevolxml2xmlin/vol-file-naming.xml | 20 ++++++ tests/storagevolxml2xmlout/vol-file-backing.xml | 2 +- tests/storagevolxml2xmlout/vol-file-naming.xml | 17 +++++ tests/storagevolxml2xmlout/vol-file.xml | 1 - tests/storagevolxml2xmlout/vol-logical-backing.xml | 2 +- tests/storagevolxml2xmlout/vol-logical.xml | 2 +- tests/storagevolxml2xmlout/vol-partition.xml | 2 +- tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml | 2 +- tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 +- tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 +- tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml | 2 +- tests/storagevolxml2xmlout/vol-qcow2.xml | 2 +- tests/storagevolxml2xmlout/vol-sheepdog.xml | 1 - tests/storagevolxml2xmltest.c | 1 + 20 files changed, 125 insertions(+), 52 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-dir-naming.xml create mode 100644 tests/storagepoolxml2xmlout/pool-dir-naming.xml create mode 100644 tests/storagevolxml2xmlin/vol-file-naming.xml create mode 100644 tests/storagevolxml2xmlout/vol-file-naming.xml diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 7fbfa53..268bc5a 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -291,8 +291,15 @@ </define> <define name='volName'> +  <data type='string'> - <param name="pattern">[a-zA-Z0-9_\+\-\.]+</param> + <param name="pattern">[^/]+</param> + <except> + <choice> + <value>.</value> + <value>..</value> + </choice> + </except> </data> </define> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 33e4caf..8b378c2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1056,7 +1056,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, virBufferAddLit(buf, " <source>\n"); if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && src->nhost) { for (i = 0; i < src->nhost; i++) { - virBufferAsprintf(buf, " <host name='%s'", src->hosts[i].name); + virBufferEscapeString(buf, " <host name='%s'", + src->hosts[i].name); if (src->hosts[i].port) virBufferAsprintf(buf, " port='%d'", src->hosts[i].port); virBufferAddLit(buf, "/>\n"); @@ -1067,8 +1068,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, src->ndevice) { for (i = 0; i < src->ndevice; i++) { if (src->devices[i].nfreeExtent) { - virBufferAsprintf(buf, " <device path='%s'>\n", - src->devices[i].path); + virBufferEscapeString(buf, " <device path='%s'>\n", + src->devices[i].path); for (j = 0; j < src->devices[i].nfreeExtent; j++) { virBufferAsprintf(buf, " <freeExtent start='%llu' end='%llu'/>\n", src->devices[i].freeExtents[j].start, @@ -1076,15 +1077,14 @@ virStoragePoolSourceFormat(virBufferPtr buf, } virBufferAddLit(buf, " </device>\n"); } else { - virBufferAsprintf(buf, " <device path='%s'/>\n", - src->devices[i].path); + virBufferEscapeString(buf, " <device path='%s'/>\n", + src->devices[i].path); } } } - if ((options->flags & VIR_STORAGE_POOL_SOURCE_DIR) && - src->dir) - virBufferAsprintf(buf, " <dir path='%s'/>\n", src->dir); + if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) + virBufferEscapeString(buf, " <dir path='%s'/>\n", src->dir); if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) { if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST || @@ -1104,9 +1104,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, } } - if ((options->flags & VIR_STORAGE_POOL_SOURCE_NAME) && - src->name) - virBufferAsprintf(buf, " <name>%s</name>\n", src->name); + if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) + virBufferEscapeString(buf, " <name>%s</name>\n", src->name); if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) && src->initiator.iqn) { @@ -1129,11 +1128,12 @@ virStoragePoolSourceFormat(virBufferPtr buf, if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP || src->authType == VIR_STORAGE_POOL_AUTH_CEPHX) { - virBufferAsprintf(buf, " <auth type='%s' username='%s'>\n", - virStoragePoolAuthTypeTypeToString(src->authType), - (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ? - src->auth.chap.username : - src->auth.cephx.username)); + virBufferAsprintf(buf, " <auth type='%s' ", + virStoragePoolAuthTypeTypeToString(src->authType)); + virBufferEscapeString(buf, "username='%s'>\n", + (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ? + src->auth.chap.username : + src->auth.cephx.username)); virBufferAddLit(buf, " <secret"); if (src->auth.cephx.secret.uuidUsable) { @@ -1149,13 +1149,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, virBufferAddLit(buf, " </auth>\n"); } - if (src->vendor != NULL) { - virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor); - } - - if (src->product != NULL) { - virBufferEscapeString(buf, " <product name='%s'/>\n", src->product); - } + virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor); + virBufferEscapeString(buf, " <product name='%s'/>\n", src->product); virBufferAddLit(buf, " </source>\n"); @@ -1182,7 +1177,7 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) goto cleanup; } virBufferAsprintf(&buf, "<pool type='%s'>\n", type); - virBufferAsprintf(&buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); virUUIDFormat(def->uuid, uuid); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuid); @@ -1203,8 +1198,7 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) def->type != VIR_STORAGE_POOL_SHEEPDOG) { virBufferAddLit(&buf, " <target>\n"); - if (def->target.path) - virBufferAsprintf(&buf, " <path>%s</path>\n", def->target.path); + virBufferEscapeString(&buf, " <path>%s</path>\n", def->target.path); virBufferAddLit(&buf, " <permissions>\n"); virBufferAsprintf(&buf, " <mode>0%o</mode>\n", @@ -1214,9 +1208,8 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) virBufferAsprintf(&buf, " <group>%d</group>\n", (int) def->target.perms.gid); - if (def->target.perms.label) - virBufferAsprintf(&buf, " <label>%s</label>\n", - def->target.perms.label); + virBufferEscapeString(&buf, " <label>%s</label>\n", + def->target.perms.label); virBufferAddLit(&buf, " </permissions>\n"); virBufferAddLit(&buf, " </target>\n"); @@ -1282,8 +1275,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, goto error; } - /* Auto-generated so deliberately ignore */ - /* ret->key = virXPathString("string(./key)", ctxt); */ + /* Normally generated by pool refresh, but useful for unit tests */ + ret->key = virXPathString("string(./key)", ctxt); capacity = virXPathString("string(./capacity)", ctxt); unit = virXPathString("string(./capacity/@unit)", ctxt); @@ -1485,11 +1478,11 @@ static int virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferPtr buf, virStorageVolTargetPtr def, - const char *type) { + const char *type) +{ virBufferAsprintf(buf, " <%s>\n", type); - if (def->path) - virBufferAsprintf(buf, " <path>%s</path>\n", def->path); + virBufferEscapeString(buf, " <path>%s</path>\n", def->path); if (options->formatToString) { const char *format = (options->formatToString)(def->format); @@ -1511,8 +1504,7 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, (unsigned int) def->perms.gid); - if (def->perms.label) - virBufferAsprintf(buf, " <label>%s</label>\n", + virBufferEscapeString(buf, " <label>%s</label>\n", def->perms.label); virBufferAddLit(buf, " </permissions>\n"); @@ -1572,8 +1564,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, return NULL; virBufferAddLit(&buf, "<volume>\n"); - virBufferAsprintf(&buf, " <name>%s</name>\n", def->name); - virBufferAsprintf(&buf, " <key>%s</key>\n", NULLSTR(def->key)); + virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(&buf, " <key>%s</key>\n", def->key); virBufferAddLit(&buf, " <source>\n"); if (def->source.nextent) { @@ -1585,8 +1577,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, if (thispath != NULL) virBufferAddLit(&buf, " </device>\n"); - virBufferAsprintf(&buf, " <device path='%s'>\n", - def->source.extents[i].path); + virBufferEscapeString(&buf, " <device path='%s'>\n", + def->source.extents[i].path); } virBufferAsprintf(&buf, diff --git a/tests/storagepoolxml2xmlin/pool-dir-naming.xml b/tests/storagepoolxml2xmlin/pool-dir-naming.xml new file mode 100644 index 0000000..aa043be --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-dir-naming.xml @@ -0,0 +1,18 @@ +<pool type='dir'> + <name>virtimages</name> + <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid> + <capacity>0</capacity> + <allocation>0</allocation> + <available>0</available> + <source> + </source> + <target> + <path>///var/////lib/libvirt/<images>//</path> + <permissions> + <mode>0700</mode> + <owner>-1</owner> + <group>-1</group> + <label>some_label_t</label> + </permissions> + </target> +</pool> diff --git a/tests/storagepoolxml2xmlout/pool-dir-naming.xml b/tests/storagepoolxml2xmlout/pool-dir-naming.xml new file mode 100644 index 0000000..536f58c --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-dir-naming.xml @@ -0,0 +1,18 @@ +<pool type='dir'> + <name>virtimages</name> + <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <available unit='bytes'>0</available> + <source> + </source> + <target> + <path>/var/lib/libvirt/<images></path> + <permissions> + <mode>0700</mode> + <owner>-1</owner> + <group>-1</group> + <label>some_label_t</label> + </permissions> + </target> +</pool> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 743e1cb..a81cf99 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -85,6 +85,7 @@ mymain(void) ret = -1 DO_TEST("pool-dir"); + DO_TEST("pool-dir-naming"); DO_TEST("pool-fs"); DO_TEST("pool-logical"); DO_TEST("pool-logical-nopath"); diff --git a/tests/storagevolxml2xmlin/vol-file-backing.xml b/tests/storagevolxml2xmlin/vol-file-backing.xml index d23349e..73e7f28 100644 --- a/tests/storagevolxml2xmlin/vol-file-backing.xml +++ b/tests/storagevolxml2xmlin/vol-file-backing.xml @@ -1,5 +1,6 @@ <volume> <name>sparse.img</name> + <key>/var/lib/libvirt/images/sparse.img</key> <source/> <capacity unit='GB'>10</capacity> <allocation unit='MiB'>0</allocation> diff --git a/tests/storagevolxml2xmlin/vol-file-naming.xml b/tests/storagevolxml2xmlin/vol-file-naming.xml new file mode 100644 index 0000000..9a33e2b --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-file-naming.xml @@ -0,0 +1,20 @@ +<volume> + <name><sparse>.img</name> + <source/> + <capacity unit="TiB">1</capacity> + <allocation unit="bytes">0</allocation> + <target> + <path>/var/lib/libvirt/images/<sparse>.img</path> + <permissions> + <mode>0</mode> + <owner>0744</owner> + <group>0</group> + <label>virt_image_t</label> + </permissions> + <timestamps> + <atime>1341933637.273190990</atime> + <mtime>1341930622.047245868</mtime> + <ctime>1341930622.047245868</ctime> + </timestamps> + </target> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-file-backing.xml b/tests/storagevolxml2xmlout/vol-file-backing.xml index c0f152e..8d2fb57 100644 --- a/tests/storagevolxml2xmlout/vol-file-backing.xml +++ b/tests/storagevolxml2xmlout/vol-file-backing.xml @@ -1,6 +1,6 @@ <volume> <name>sparse.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/sparse.img</key> <source> </source> <capacity unit='bytes'>10000000000</capacity> diff --git a/tests/storagevolxml2xmlout/vol-file-naming.xml b/tests/storagevolxml2xmlout/vol-file-naming.xml new file mode 100644 index 0000000..7022b02 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-file-naming.xml @@ -0,0 +1,17 @@ +<volume> + <name><sparse>.img</name> + <source> + </source> + <capacity unit='bytes'>1099511627776</capacity> + <allocation unit='bytes'>0</allocation> + <target> + <path>/var/lib/libvirt/images/<sparse>.img</path> + <format type='raw'/> + <permissions> + <mode>00</mode> + <owner>744</owner> + <group>0</group> + <label>virt_image_t</label> + </permissions> + </target> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-file.xml b/tests/storagevolxml2xmlout/vol-file.xml index a3d6473..b97dd50 100644 --- a/tests/storagevolxml2xmlout/vol-file.xml +++ b/tests/storagevolxml2xmlout/vol-file.xml @@ -1,6 +1,5 @@ <volume> <name>sparse.img</name> - <key>(null)</key> <source> </source> <capacity unit='bytes'>1099511627776</capacity> diff --git a/tests/storagevolxml2xmlout/vol-logical-backing.xml b/tests/storagevolxml2xmlout/vol-logical-backing.xml index 6b010e3..bf34b08 100644 --- a/tests/storagevolxml2xmlout/vol-logical-backing.xml +++ b/tests/storagevolxml2xmlout/vol-logical-backing.xml @@ -1,6 +1,6 @@ <volume> <name>Swap</name> - <key>(null)</key> + <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key> <source> </source> <capacity unit='bytes'>2080374784</capacity> diff --git a/tests/storagevolxml2xmlout/vol-logical.xml b/tests/storagevolxml2xmlout/vol-logical.xml index 7bf309e..e9b4e4b 100644 --- a/tests/storagevolxml2xmlout/vol-logical.xml +++ b/tests/storagevolxml2xmlout/vol-logical.xml @@ -1,6 +1,6 @@ <volume> <name>Swap</name> - <key>(null)</key> + <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key> <source> </source> <capacity unit='bytes'>2080374784</capacity> diff --git a/tests/storagevolxml2xmlout/vol-partition.xml b/tests/storagevolxml2xmlout/vol-partition.xml index 271964f..9be1cf1 100644 --- a/tests/storagevolxml2xmlout/vol-partition.xml +++ b/tests/storagevolxml2xmlout/vol-partition.xml @@ -1,6 +1,6 @@ <volume> <name>sda1</name> - <key>(null)</key> + <key>/dev/sda1</key> <source> </source> <capacity unit='bytes'>106896384</capacity> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml b/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml index a7b5fed..fd3d606 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml @@ -1,6 +1,6 @@ <volume> <name>OtherDemo.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> <capacity unit='bytes'>5368709120</capacity> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml index b7df8a6..99fb5ac 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml @@ -1,6 +1,6 @@ <volume> <name>OtherDemo.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> <capacity unit='bytes'>5368709120</capacity> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml index 92b7875..3708ea7 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml @@ -1,6 +1,6 @@ <volume> <name>OtherDemo.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> <capacity unit='bytes'>5368709120</capacity> diff --git a/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml b/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml index e2da702..f6a2e21 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml @@ -1,6 +1,6 @@ <volume> <name>OtherDemo.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> <capacity unit='bytes'>5368709120</capacity> diff --git a/tests/storagevolxml2xmlout/vol-qcow2.xml b/tests/storagevolxml2xmlout/vol-qcow2.xml index f931a62..b9adcb4 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2.xml @@ -1,6 +1,6 @@ <volume> <name>OtherDemo.img</name> - <key>(null)</key> + <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> <capacity unit='bytes'>5368709120</capacity> diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml index 2f19af8..bd5d6d8 100644 --- a/tests/storagevolxml2xmlout/vol-sheepdog.xml +++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml @@ -1,6 +1,5 @@ <volume> <name>test2</name> - <key>(null)</key> <source> </source> <capacity unit='bytes'>1024</capacity> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 1fd01f1..e1db465 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -110,6 +110,7 @@ mymain(void) while (0); DO_TEST("pool-dir", "vol-file"); + DO_TEST("pool-dir", "vol-file-naming"); DO_TEST("pool-dir", "vol-file-backing"); DO_TEST("pool-dir", "vol-qcow2"); DO_TEST("pool-dir", "vol-qcow2-1.1"); -- 1.8.3.1

2 3