]> xenbits.xensource.com Git - libvirt.git/log
libvirt.git
11 years agoFix memory leak in cmdAttachDisk
Hongwei Bi [Sat, 31 Aug 2013 03:39:35 +0000 (11:39 +0800)]
Fix memory leak in cmdAttachDisk

When virBufferError is ok in cmdAttachDisk, the latter
should 'goto cleanup', instead of returning a false to
prevent memory leaking.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agobuild: fix virtlockd file distribution
Eric Blake [Sat, 31 Aug 2013 02:16:06 +0000 (20:16 -0600)]
build: fix virtlockd file distribution

Since virtlockd is only built when libvirtd is built, we should
not install its auxiliary files unconditionally.  This solves
two failures.  1. 'make distcheck' complains:

rm -f Makefile
ERROR: files left in build directory after distclean:
./src/virtlockd.8

2. './autobuild.sh' complains:

Checking for unpackaged file(s): /usr/lib/rpm/check-files
/home/eblake/rpmbuild/BUILDROOT/mingw-libvirt-1.1.1-1.fc19.eblake1377879911.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/i686-w64-mingw32/sys-root/mingw/etc/libvirt/virtlockd.conf

/usr/i686-w64-mingw32/sys-root/mingw/share/augeas/lenses/tests/test_virtlockd.aug
   /usr/i686-w64-mingw32/sys-root/mingw/share/augeas/lenses/virtlockd.aug
   /usr/i686-w64-mingw32/sys-root/mingw/share/man/man8/virtlockd.8
   /usr/x86_64-w64-mingw32/sys-root/mingw/etc/libvirt/virtlockd.conf

/usr/x86_64-w64-mingw32/sys-root/mingw/share/augeas/lenses/tests/test_virtlockd.aug
   /usr/x86_64-w64-mingw32/sys-root/mingw/share/augeas/lenses/virtlockd.aug
   /usr/x86_64-w64-mingw32/sys-root/mingw/share/man/man8/virtlockd.8

* src/Makefile.am (CLEANFILES): Add virtlockd.8.
(man8_MANS, conf_DATA, augeas_DATA, augeastest_DATA): Only install
virtlockd files when daemon is built.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agobuild: shipped files must not depend on BUILT_SOURCES
Eric Blake [Fri, 30 Aug 2013 22:05:43 +0000 (16:05 -0600)]
build: shipped files must not depend on BUILT_SOURCES

'make distcheck' was failing with:
make[3]: Entering directory `/home/eblake/libvirt-tmp2/libvirt-1.1.1/_build/docs'
perl ../../docs/genaclperms.pl ../../src/access/viraccessperm.h > ../../docs/aclperms.htmlinc
/bin/sh: ../../docs/aclperms.htmlinc: Permission denied

when simulating the case of a user doing a VPATH build from a
read-only source tree.  The culprit?  BUILT_SOURCES are _always_
built, and so must NOT be built into srcdir and need not be part
of the tarball.  On the other hand, shipped files must never
depend on files in the builddir.  While it would be possible to
fix the problem by generating aclperms.htmlinc into builddir,
we then have the problem that we ship acl.html - we'd have to
rejigger a lot of things to not ship pre-built html.  So this
patch goes the other direction - we don't need BUILT_SOURCES,
but instead ensure that we have proper dependencies so that
all files in srcdir are up-to-date at the time the tarball is
created.  And because we ship html files in the tarball, that
implies we don't expect users to be able to rebuild them, so
we must not clean any files that would trigger a rebuild except
under the maintainer rules.

* docs/Makefile.am (BUILT_SOURCES): Delete.
(CLEANFILES): Downgrade aclperms.htmlinc cleanup...
(maintainer-clean-local): ...and move hvsupport.html.in...
(MAINTAINERCLEANFILES): ...to a maintainer action.
(hvsupport.html.in): Write into srcdir.
(hvsupport.html): Ensure files are built in order.
(aclperms.htmlinc): Honor silent make.
(EXTRA_DIST): Ship aclperms.htmlinc.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agobuild: fix 'make distcheck' out of the box
Eric Blake [Fri, 30 Aug 2013 21:03:52 +0000 (15:03 -0600)]
build: fix 'make distcheck' out of the box

With the 1.1.1 tarball, if a user does 'make && make distcheck',
things pass, but if they do 'make distcheck' after 'make clean',
there is an odd failure:

  GEN      ../../docs/devhelp/index.html
I/O error : Permission denied
I/O error : Permission denied
runtime error: file ../../docs/devhelp/devhelp.xsl line 43 element document
xsltDocumentElem: unable to save to ../../docs/devhelp/libvirt-virterror.html
I/O error : Permission denied
I/O error : Permission denied

This implies that the rules for 'make dist' are missing a
dependency - the generated documentation needs to be up-to-date
before creating the tarball, or else the tarball will be missing
files, where the end user will end up trying to rebuild files in
srcdir, and that fails when srcdir is read-only.

1.1.1 plus this patch now works without issues (other issues have
crept in to 1.1.2-rc1 that prevent 'make distcheck' from working,
but those will be cleaned up in later patches).

* docs/Makefile.am (dist-local): New dependency.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agobuild: only create virt-login-shell for lxc builds
Eric Blake [Fri, 30 Aug 2013 19:58:59 +0000 (13:58 -0600)]
build: only create virt-login-shell for lxc builds

I noticed from an ./autobuild.sh run that we were installing a
virt-login-shell.exe binary when cross-building for mingw,
even though such a binary is necessarily worthless since the
code depends on lxc which is a Linux-only concept.

* tools/Makefile.am (conf_DATA, bin_PROGRAMS, dist_man1_MANS):
Make virt-login-shell installation conditional.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agoqemu: Only setup vhost if virtType == "kvm"
Cole Robinson [Thu, 1 Aug 2013 01:37:40 +0000 (21:37 -0400)]
qemu: Only setup vhost if virtType == "kvm"

vhost only works in KVM mode at the moment, and is infact compiled
out if the emulator is built for non-native architecture. While it
may work at some point in the future for plain qemu, for now it's
just noise on the command line (and which contributes to arm cli
breakage).

11 years agoProcess virtlockd.conf instead of libvirtd.conf
Guido Günther [Fri, 30 Aug 2013 10:58:08 +0000 (12:58 +0200)]
Process virtlockd.conf instead of libvirtd.conf

11 years agoChange way we fake dbus method calls
Daniel P. Berrange [Fri, 30 Aug 2013 13:10:52 +0000 (14:10 +0100)]
Change way we fake dbus method calls

Ubuntu libdbus.so links with -Bsymbolic-functions, which means
that we can only LD_PRELOAD functions that we directly call.
Functions which libdbus.so calls internally can not be replaced.
Thus we cannot use dbus_message_new_error or dbus_message_new_method_return

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agorandom: don't mix RAND_MAX with random_r
Eric Blake [Thu, 29 Aug 2013 23:03:34 +0000 (17:03 -0600)]
random: don't mix RAND_MAX with random_r

FreeBSD 10 recently changed their definition of RAND_MAX, to try
and cover the fact that their evenly distributed results of rand()
really are a smaller range than a full power of 2.  As a result,
I did some investigation, and learned:

1. POSIX requires random() to be evenly distributed across exactly
31 bits.  glibc also guarantees this for rand(), but the two are
unrelated, and POSIX only associates RAND_MAX with rand().
Avoiding RAND_MAX altogether thus avoids a build failure on
FreeBSD 10.

2. Concatenating random bits from a PRNG will NOT provide uniform
coverage over the larger value UNLESS the period of the original
PRNG is at least as large as the number of bits being concatenated.
Simple example: suppose that RAND_MAX were 1 with a period of 2**1
(which means that the PRNG merely alternates between 0 and 1).
Concatenating two successive rand() calls would then invariably
result in 01 or 10, which is a rather non-uniform distribution
(00 and 11 are impossible) and an even worse period (2**0, since
our second attempt will get the same number as our first attempt).
But a RAND_MAX of 1 with a period of 2**2 (alternating between
0, 1, 1, 0) provides sane coverage of all four values, if properly
tempered.  (Back-to-back calls would still only see half the values
if we don't do some tempering).  We therefore want to guarantee a
period of at least 2**64, preferably larger (as a tempering factor);
POSIX only makes this guarantee for random() with 256 bytes of info.

* src/util/virrandom.c (virRandomBits): Use constants that are
accurate for the PRNG we are using, not an unrelated PRNG.
(randomState): Ensure the period of our PRNG exceeds our usage.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agovirsh-domain: rename print_job_progress to vshPrintJobProgress
Peter Krempa [Mon, 26 Aug 2013 10:31:51 +0000 (12:31 +0200)]
virsh-domain: rename print_job_progress to vshPrintJobProgress

11 years agosecurity: provide supplemental groups even when parsing label (CVE-2013-4291)
Eric Blake [Fri, 23 Aug 2013 15:30:42 +0000 (09:30 -0600)]
security: provide supplemental groups even when parsing label (CVE-2013-4291)

Commit 29fe5d7 (released in 1.1.1) introduced a latent problem
for any caller of virSecurityManagerSetProcessLabel and where
the domain already had a uid:gid label to be parsed.  Such a
setup would collect the list of supplementary groups during
virSecurityManagerPreFork, but then ignores that information,
and thus fails to call setgroups() to adjust the supplementary
groups of the process.

Upstream does not use virSecurityManagerSetProcessLabel for
qemu (it uses virSecurityManagerSetChildProcessLabel instead),
so this problem remained latent until backporting the initial
commit into v0.10.2-maint (commit c061ff5, released in 0.10.2.7),
where virSecurityManagerSetChildProcessLabel has not been
backported.  As a result of using a different code path in the
backport, attempts to start a qemu domain that runs as qemu:qemu
will end up with supplementary groups unchanged from the libvirtd
parent process, rather than the desired supplementary groups of
the qemu user.  This can lead to failure to start a domain
(typical Fedora setup assigns user 107 'qemu' to both group 107
'qemu' and group 36 'kvm', so a disk image that is only readable
under kvm group rights is locked out).  Worse, it is a security
hole (the qemu process will inherit supplemental group rights
from the parent libvirtd process, which means it has access
rights to files owned by group 0 even when such files should
not normally be visible to user qemu).

LXC does not use the DAC security driver, so it is not vulnerable
at this time.  Still, it is better to plug the latent hole on
the master branch first, before cherry-picking it to the only
vulnerable branch v0.10.2-maint.

* src/security/security_dac.c (virSecurityDACGetIds): Always populate
groups and ngroups, rather than only when no label is parsed.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agoProhibit unbounded arrays in XDR protocols
Daniel P. Berrange [Mon, 19 Aug 2013 14:17:20 +0000 (15:17 +0100)]
Prohibit unbounded arrays in XDR protocols

The use of <> is a security issue for RPC parameters, since a
malicious client can set a huge array length causing arbitrary
memory allocation in the daemon.

It is also a robustness issue for RPC return values, because if
the stream is corrupted, it can cause the client to also allocate
arbitrary memory.

Use a syntax-check rule to prohibit any use of <>

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllSecrets RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:49:57 +0000 (14:49 +0100)]
Add bounds checking on virConnectListAllSecrets RPC call

The return values for the virConnectListAllSecrets call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllNWFilters RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:47:22 +0000 (14:47 +0100)]
Add bounds checking on virConnectListAllNWFilters RPC call

The return values for the virConnectListAllNWFilters call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllNodeDevices RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:44:52 +0000 (14:44 +0100)]
Add bounds checking on virConnectListAllNodeDevices RPC call

The return values for the virConnectListAllNodeDevices call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllInterfaces RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:41:56 +0000 (14:41 +0100)]
Add bounds checking on virConnectListAllInterfaces RPC call

The return values for the virConnectListAllInterfaces call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllNetworks RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:37:29 +0000 (14:37 +0100)]
Add bounds checking on virConnectListAllNetworks RPC call

The return values for the virConnectListAllNetworks call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virStoragePoolListAllVolumes RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:33:58 +0000 (14:33 +0100)]
Add bounds checking on virStoragePoolListAllVolumes RPC call

The return values for the virStoragePoolListAllVolumes call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllStoragePools RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:27:56 +0000 (14:27 +0100)]
Add bounds checking on virConnectListAllStoragePools RPC call

The return values for the virConnectListAllStoragePools call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virConnectListAllDomains RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 13:23:31 +0000 (14:23 +0100)]
Add bounds checking on virConnectListAllDomains RPC call

The return values for the virConnectListAllDomains call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virDomain{SnapshotListAllChildren,ListAllSnapshots} RPC calls
Daniel P. Berrange [Mon, 19 Aug 2013 11:55:53 +0000 (12:55 +0100)]
Add bounds checking on virDomain{SnapshotListAllChildren,ListAllSnapshots} RPC calls

The return values for the virDomain{SnapshotListAllChildren,ListAllSnapshots}
calls were not bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virDomainGetJobStats RPC call
Daniel P. Berrange [Mon, 19 Aug 2013 11:42:31 +0000 (12:42 +0100)]
Add bounds checking on virDomainGetJobStats RPC call

The return values for the virDomainGetJobStats call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoAdd bounds checking on virDomainMigrate*Params RPC calls (CVE-2013-4292)
Daniel P. Berrange [Mon, 19 Aug 2013 13:55:21 +0000 (14:55 +0100)]
Add bounds checking on virDomainMigrate*Params RPC calls (CVE-2013-4292)

The parameters for the virDomainMigrate*Params RPC calls were
not bounds checks, meaning a malicious client can cause libvirtd
to consume arbitrary memory

This issue was introduced in the 1.1.0 release of libvirt

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agopython: Fix a PyList usage mistake
Guan Qiang [Thu, 29 Aug 2013 11:02:25 +0000 (19:02 +0800)]
python: Fix a PyList usage mistake

Fix PyList usage mistake in Function libvirt_lxc_virDomainLxcOpenNamespace.

https://bugzilla.redhat.com/show_bug.cgi?id=1002383

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agoautogen.sh: Correctly detect .git as a file
Michal Privoznik [Thu, 29 Aug 2013 11:19:45 +0000 (13:19 +0200)]
autogen.sh: Correctly detect .git as a file

One of my previous patches 5cfe0d37cd0be tried to handle the case when
libvirt is a submodule of another project. In that case, the .git is
just a link to the parent .git directory (which the autogen.sh script
didn't count on). The fix was missing 'test' though.

11 years agobridge_driver: Introduce networkObjFromNetwork
Michal Privoznik [Wed, 28 Aug 2013 12:34:34 +0000 (14:34 +0200)]
bridge_driver: Introduce networkObjFromNetwork

Similarly to qemu_driver.c, we can join often repeating code of looking
up network into one function: networkObjFromNetwork.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
11 years agoqemu_hotplug: Fix whitespace around addition in argument
Peter Krempa [Wed, 28 Aug 2013 12:56:21 +0000 (14:56 +0200)]
qemu_hotplug: Fix whitespace around addition in argument

11 years agoqemu: Remove hostdev entry when freeing the depending network entry
Peter Krempa [Tue, 27 Aug 2013 17:06:18 +0000 (19:06 +0200)]
qemu: Remove hostdev entry when freeing the depending network entry

When using a <interface type="network"> that points to a network with
hostdev forwarding mode a hostdev alias is created for the network. This
allias is inserted into the hostdev list, but is backed with a part of
the network object that it is connected to.

When a VM is being stopped qemuProcessStop() calls
networkReleaseActualDevice() which eventually frees the memory for the
hostdev object. Afterwards when the domain definition is being freed by
virDomainDefFree() an invalid pointer is accessed by
virDomainHostdevDefFree() and may cause a crash of the daemon.

This patch removes the entry in the hostdev list before freeing the
depending memory to avoid this issue.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1000973

11 years agovirsh: detect programming errors with option parsing
Eric Blake [Fri, 16 Aug 2013 22:07:31 +0000 (16:07 -0600)]
virsh: detect programming errors with option parsing

Noticed while reviewing another patch that had an accidental
mismatch due to refactoring.  An audit of the code showed that
very few callers of vshCommandOpt were expecting a return of
-2, indicating programmer error, and of those that DID check,
they just propagated that status to yet another caller that
did not check.  Fix this by making the code blatantly warn
the programmer, rather than silently ignoring it and possibly
doing the wrong thing downstream.

I know that we frown on assert()/abort() inside libvirtd
(libraries should NEVER kill the program that linked them),
but as virsh is an app rather than the library, and as this
is not the first use of assert() in virsh, I think this
approach is okay.

* tools/virsh.h (vshCommandOpt): Drop declaration.
* tools/virsh.c (vshCommandOpt): Make static, and add a
parameter.  Abort on programmer errors rather than making callers
repeat that logic.
(vshCommandOptInt, vshCommandOptUInt, vshCommandOptUL)
(vshCommandOptString, vshCommandOptStringReq)
(vshCommandOptLongLong, vshCommandOptULongLong)
(vshCommandOptBool): Adjust callers.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agovirt-sanlock-cleanup; Fix augtool usage
Jiri Denemark [Wed, 28 Aug 2013 11:50:10 +0000 (13:50 +0200)]
virt-sanlock-cleanup; Fix augtool usage

Surprisingly, augtool get (or print) returns "path = value" while we are
only interested in the value. We need to remove the "path = " part from
the augtool's output. The following is an example of the augtool command
as used in virt-sanlock-cleanup script:

$ augtool get /files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir
/files/etc/libvirt/qemu-sanlock.conf/disk_lease_dir = /var/lib/libvirt/sanlock

11 years agovirsh: Fix debugging
Martin Kletzander [Tue, 27 Aug 2013 11:19:24 +0000 (13:19 +0200)]
virsh: Fix debugging

Commit a0b6a36f "fixed" what abfff210 broke (URI precedence), but
there was still one more thing missing to fix.  When using virsh
parameters to setup debugging, those weren't honored, because at the
time debugging was initializing, arguments weren't parsed yet.  To
make ewerything work as expected, we need to initialize the debugging
twice, once before debugging (so we can debug option parsing properly)
and then again after these options are parsed.

As a side effect, this patch also fixes a leak when virsh is ran with
multiple '-l' parameters.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
11 years agovirsh-pool.c: Don't jump over variable declaration
Michal Privoznik [Wed, 28 Aug 2013 07:25:59 +0000 (09:25 +0200)]
virsh-pool.c: Don't jump over variable declaration

Since 785ff34bf8 we are using the outputStr variable in cleanup label.
However, there is a possibility to jump to the label before the variable
has been declared:

virsh-pool.c: In function 'cmdPoolList':
virsh-pool.c:1121:25: error: jump skips variable initialization [-Werror=jump-misses-init]
                         goto asprintf_failure;
                         ^
virsh-pool.c:1308:1: note: label 'asprintf_failure' defined here
 asprintf_failure:
 ^
virsh-pool.c:1267:11: note: 'outputStr' declared here
     char *outputStr = NULL;

11 years agovirsh: free the caps list properly if one of them is invalid
Ján Tomko [Tue, 27 Aug 2013 11:47:57 +0000 (13:47 +0200)]
virsh: free the caps list properly if one of them is invalid

VIR_FREE(caps) is not enough to free an array allocated
by vshStringToArray.

==17== 4 bytes in 1 blocks are definitely lost in loss record 4 of 728
==17==    by 0x4EFFC44: virStrdup (virstring.c:554)
==17==    by 0x128B10: _vshStrdup (virsh.c:125)
==17==    by 0x129164: vshStringToArray (virsh.c:218)
==17==    by 0x157BB3: cmdNodeListDevices (virsh-nodedev.c:409)

https://bugzilla.redhat.com/show_bug.cgi?id=1001536

11 years agovirsh: free the formatting string when listing pool details
Ján Tomko [Tue, 27 Aug 2013 11:34:09 +0000 (13:34 +0200)]
virsh: free the formatting string when listing pool details

==23== 41 bytes in 1 blocks are definitely lost in loss record 626 of 727
==23==    by 0x4F0099F: virAsprintfInternal (virstring.c:358)
==23==    by 0x15D2C9: cmdPoolList (virsh-pool.c:1268)

https://bugzilla.redhat.com/show_bug.cgi?id=1001536

11 years agovirsh: free the list from ListAll APIs even for 0 items
Ján Tomko [Tue, 27 Aug 2013 11:27:50 +0000 (13:27 +0200)]
virsh: free the list from ListAll APIs even for 0 items

virsh secret-list leak when no secrets are defined:

==27== 8 bytes in 1 blocks are definitely lost in loss record 6 of 726
==27==    by 0x4E941DD: virAllocN (viralloc.c:183)
==27==    by 0x5037F1A: remoteConnectListAllSecrets (remote_driver.c:3076)
==27==    by 0x5004EC6: virConnectListAllSecrets (libvirt.c:16298)
==27==    by 0x15F813: vshSecretListCollect (virsh-secret.c:397)
==27==    by 0x15F0E1: cmdSecretList (virsh-secret.c:532)

And so do some other *-list commands.

https://bugzilla.redhat.com/show_bug.cgi?id=1001536

11 years agovirsh: free messages after logging them to a file
Ján Tomko [Tue, 27 Aug 2013 11:07:27 +0000 (13:07 +0200)]
virsh: free messages after logging them to a file

The messages were only freed on error.

==12== 1,100 bytes in 1 blocks are definitely lost in loss record 698 of 729
==12==    by 0x4E98C22: virBufferAsprintf (virbuffer.c:294)
==12==    by 0x12C950: vshOutputLogFile (virsh.c:2440)
==12==    by 0x12880B: vshError (virsh.c:2254)
==12==    by 0x131957: vshCommandOptDomainBy (virsh-domain.c:109)
==12==    by 0x14253E: cmdStart (virsh-domain.c:3333)

https://bugzilla.redhat.com/show_bug.cgi?id=1001536

11 years agoTest network update XML parsing
Ján Tomko [Mon, 29 Jul 2013 15:17:47 +0000 (17:17 +0200)]
Test network update XML parsing

Add checks for updating sections of network definition via
virNetworkDefUpdateSection.

https://bugzilla.redhat.com/show_bug.cgi?id=989569

11 years agoRemove the space before the slash in network XML
Ján Tomko [Tue, 30 Jul 2013 12:36:08 +0000 (14:36 +0200)]
Remove the space before the slash in network XML

This matches the style we use elsewhere and allows
nat-network-dns-srv-record{,-minimal}.xml to be tested in
network XML -> XML test.

11 years agoBuild QEMU command line for pcihole64
Ján Tomko [Mon, 12 Aug 2013 11:48:34 +0000 (13:48 +0200)]
Build QEMU command line for pcihole64

QEMU commit 3984890 introduced the "pci-hole64-size" property,
to i440FX-pcihost and q35-pcihost with a default setting of 2 GB.

Translate <pcihole64>x<pcihole64/> to:
-global q35-pcihost.pci-hole64-size=x for q35 machines and
-global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines.

Error out on other machine types or if the size was specified
but the pcihost device lacks 'pci-hole64-size' property.

https://bugzilla.redhat.com/show_bug.cgi?id=990418

11 years agoAdd pcihole64 element to root PCI controllers
Ján Tomko [Mon, 12 Aug 2013 11:39:04 +0000 (13:39 +0200)]
Add pcihole64 element to root PCI controllers

<controller type='pci' index='0' model='pci-root'>
  <pcihole64 unit='KiB'>1048576</pcihole64>
</controller>

It can be used to adjust (or disable) the size of the 64-bit
PCI hole. The size attribute is in kilobytes (different unit
can be specified on input), but it gets rounded up to
the nearest GB by QEMU.

Disabling it will be needed for guests that crash with the
64-bit PCI hole (like Windows XP), see:
https://bugzilla.redhat.com/show_bug.cgi?id=990418

11 years agoAllow controller XML parsing to use XPath context
Ján Tomko [Tue, 13 Aug 2013 13:10:17 +0000 (15:10 +0200)]
Allow controller XML parsing to use XPath context

virDomainParseScaledValue requires it.

11 years agoMove virDomainParseScaledValue earlier
Ján Tomko [Tue, 13 Aug 2013 13:08:19 +0000 (15:08 +0200)]
Move virDomainParseScaledValue earlier

Let virDomainControllerDefParseXML use it without
a forward declaration.

11 years agoAdd ftp protocol support for cdrom disk
Aline Manera [Thu, 22 Aug 2013 19:03:08 +0000 (16:03 -0300)]
Add ftp protocol support for cdrom disk

The ftp protocol is already recognized by qemu/KVM so add this support to
libvirt as well.
The xml should be as following:

     <disk type='network' device='cdrom'>
       <source protocol='ftp' name='/url/path'>
         <host name='host.name' port='21'/>
       </source>
     </disk>

Signed-off-by: Aline Manera <alinefm@br.ibm.com>
11 years agoAdd http protocol support for cdrom disk
Aline Manera [Thu, 22 Aug 2013 19:03:07 +0000 (16:03 -0300)]
Add http protocol support for cdrom disk

QEMU/KVM already allows a HTTP URL for the cdrom ISO image so add this support
to libvirt as well.
The xml should be as following:

    <disk type='network' device='cdrom'>
      <source protocol='http' name='/url/path'>
        <host name='host.name' port='80'/>
      </source>
    </disk>

Signed-off-by: Aline Manera <alinefm@br.ibm.com>
11 years agoAlways specify qcow2 compat level on qemu-img command line
Ján Tomko [Tue, 20 Aug 2013 15:37:08 +0000 (17:37 +0200)]
Always specify qcow2 compat level on qemu-img command line

qemu-img is going to switch the default for QCOW2
to QCOW2v3 (compat=1.1)

Extend the probing for qemu-img command line options to check
if -o compat is supported. If the volume definition specifies
the qcow2 format but no compat level and -o compat is supported,
specify -o compat=0.10 to create a QCOW2v2 image.

https://bugzilla.redhat.com/show_bug.cgi?id=997977

11 years agovirsh: fix return value error of cpu-stats
Guannan Ren [Fri, 23 Aug 2013 10:17:25 +0000 (18:17 +0800)]
virsh: fix return value error of cpu-stats

virsh cpu-stats guest --start 0 --count 3
It outputs right but the return value is 1 rather than 0
echo $?
1

Found by running libvirt-autotest
./run -t libvirt --tests virsh_cpu_stats

11 years agovirsh: C99 style for info_domfstrim and opts_lxc_enter_namespace
Tomas Meszaros [Mon, 26 Aug 2013 12:36:52 +0000 (14:36 +0200)]
virsh: C99 style for info_domfstrim and opts_lxc_enter_namespace

Change info_domfstrim and opts_lxc_enter_namespace initialization style
to C99.

11 years agoqemuDomainAttachHostPciDevice: Fall back to mem balloon if there's no hard_limit
Michal Privoznik [Mon, 26 Aug 2013 14:59:03 +0000 (16:59 +0200)]
qemuDomainAttachHostPciDevice: Fall back to mem balloon if there's no hard_limit

If there's no hard_limit set and domain uses VFIO we still must lock
the guest memory (prerequisite from qemu). Hence, we should compute
the amount to be locked from max_balloon.

11 years agoqemuhotplugtest: Add tests for virtio SCSI disk hotplug
Jiri Denemark [Fri, 26 Jul 2013 13:28:33 +0000 (15:28 +0200)]
qemuhotplugtest: Add tests for virtio SCSI disk hotplug

11 years agoqemuhotplugtest: Add tests for USB disk hotplug
Jiri Denemark [Fri, 26 Jul 2013 12:44:52 +0000 (14:44 +0200)]
qemuhotplugtest: Add tests for USB disk hotplug

11 years agoqemuhotplugtest: Add tests for async virtio disk detach
Jiri Denemark [Fri, 19 Jul 2013 11:04:44 +0000 (13:04 +0200)]
qemuhotplugtest: Add tests for async virtio disk detach

11 years agoqemuhotplugtest: Add support for DEVICE_DELETED event
Jiri Denemark [Fri, 26 Jul 2013 12:22:10 +0000 (14:22 +0200)]
qemuhotplugtest: Add support for DEVICE_DELETED event

11 years agoqemu: Let tests override waiting time for device unplug
Jiri Denemark [Fri, 26 Jul 2013 10:18:01 +0000 (12:18 +0200)]
qemu: Let tests override waiting time for device unplug

We don't want tests to wait 5 seconds for an event which we know will
never come.

11 years agoqemu: Export qemuProcessHandleDeviceDeleted for tests
Jiri Denemark [Fri, 26 Jul 2013 12:24:55 +0000 (14:24 +0200)]
qemu: Export qemuProcessHandleDeviceDeleted for tests

11 years agotests: Add support for passing driver to qemu monitor
Jiri Denemark [Thu, 25 Jul 2013 17:28:51 +0000 (19:28 +0200)]
tests: Add support for passing driver to qemu monitor

The driver is then passed to monitor event handlers.

11 years agotests: Add support for passing vm to qemu monitor
Jiri Denemark [Thu, 25 Jul 2013 17:17:44 +0000 (19:17 +0200)]
tests: Add support for passing vm to qemu monitor

Some tests need the monitor to operate on an already existing VM object
rather than on a new mock-up the monitor test normally creates.

11 years agoqemuhotplugtest: Add tests for virtio disk hotplug
Jiri Denemark [Thu, 18 Jul 2013 09:21:34 +0000 (11:21 +0200)]
qemuhotplugtest: Add tests for virtio disk hotplug

11 years agoqemuxml2argvtest: Add XML for testing device hotplug
Jiri Denemark [Thu, 18 Jul 2013 09:19:23 +0000 (11:19 +0200)]
qemuxml2argvtest: Add XML for testing device hotplug

This is a generic XML usable for hotplugging various types of devices.

11 years agoqemuhotplugtest: Define QMP_OK for the most common reply
Jiri Denemark [Fri, 26 Jul 2013 13:06:37 +0000 (15:06 +0200)]
qemuhotplugtest: Define QMP_OK for the most common reply

11 years agoqemuhotplugtest: Compare domain XML after device hotplug
Jiri Denemark [Thu, 18 Jul 2013 14:46:13 +0000 (16:46 +0200)]
qemuhotplugtest: Compare domain XML after device hotplug

We need to make sure a device is properly added/removed (or not) to a
domain definition to check that a hotplug API did not lie to us.

11 years agoqemuhotplugtest: Generate better output
Jiri Denemark [Thu, 18 Jul 2013 09:07:21 +0000 (11:07 +0200)]
qemuhotplugtest: Generate better output

Each test case label now contains more data useful to identify the test.

11 years agoqemu: Move qemuDomainDetachDeviceDiskLive to qemu_hotplug.c
Jiri Denemark [Thu, 18 Jul 2013 09:00:13 +0000 (11:00 +0200)]
qemu: Move qemuDomainDetachDeviceDiskLive to qemu_hotplug.c

11 years agoqemu: Move qemuDomainAttachDeviceDiskLive to qemu_hotplug.c
Jiri Denemark [Thu, 18 Jul 2013 08:58:01 +0000 (10:58 +0200)]
qemu: Move qemuDomainAttachDeviceDiskLive to qemu_hotplug.c

11 years agoqemu: Avoid using global qemu_driver in event handlers
Jiri Denemark [Thu, 25 Jul 2013 17:26:15 +0000 (19:26 +0200)]
qemu: Avoid using global qemu_driver in event handlers

We will have to pass a mock-up of the driver when testing monitor
events.

11 years agoqemu: Typedef monitor callbacks
Jiri Denemark [Thu, 25 Jul 2013 15:27:52 +0000 (17:27 +0200)]
qemu: Typedef monitor callbacks

Otherwise defining variables that hold callbacks pointers is ugly and
several places have to be changed when new parameters are added.

11 years agoDon't free NULL network in cmdNetworkUpdate
Ján Tomko [Mon, 26 Aug 2013 10:51:08 +0000 (12:51 +0200)]
Don't free NULL network in cmdNetworkUpdate

If the network has not been found, virNetworkFree(NULL)
was called, resulting in an extra error:
error: invalid network pointer in virNetworkFree

https://bugzilla.redhat.com/show_bug.cgi?id=1001094

11 years agoschema: Allow dots in device aliases
Jiri Denemark [Fri, 23 Aug 2013 13:03:31 +0000 (15:03 +0200)]
schema: Allow dots in device aliases

Commit 01b88127 changed aliases for PCI controller devices to "pcie.0" or
"pci.%u". Thus device aliases may now contain dots.

11 years agoqemu: Don't update count of vCPUs if hot-plug fails silently
Peter Krempa [Mon, 26 Aug 2013 12:34:56 +0000 (14:34 +0200)]
qemu: Don't update count of vCPUs if hot-plug fails silently

When cpu hotplug fails without reporting an error, we would fail the
command but update the count of vCPUs anyways.

Commit 761fc481365703b861429d73a341bde352ba8d41 fixed the case when CPU
hot-unplug failed silently, but forgot to fix up the value in this case.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1000357

11 years agotests: skip schema validation tests if xmllint is missing
Eric Blake [Thu, 22 Aug 2013 15:30:50 +0000 (09:30 -0600)]
tests: skip schema validation tests if xmllint is missing

On IRC, someone complained that a system without xmllint installed
failed a number of tests.

* tests/schematestutils.sh: Probe for xmllint.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agopython: simplify complicated conditional assignment
Claudio Bley [Wed, 21 Aug 2013 13:37:42 +0000 (15:37 +0200)]
python: simplify complicated conditional assignment

11 years agoTest for object identity when checking for None in Python
Claudio Bley [Thu, 22 Aug 2013 09:16:03 +0000 (11:16 +0200)]
Test for object identity when checking for None in Python

Consistently use "is" or "is not" to compare variables to None,
because doing so is preferrable, as per PEP 8
(http://www.python.org/dev/peps/pep-0008/#programming-recommendations):

> Comparisons to singletons like None should always be done with is or
> is not, never the equality operators.

11 years agoqemuagenttest.c: Missing documentation (Timeout)
Nehal J Wani [Thu, 22 Aug 2013 19:43:57 +0000 (01:13 +0530)]
qemuagenttest.c: Missing documentation (Timeout)

In tests/qemuagenttest.c, the Timeout test should always be
called last. Any additional tests should come before this.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agopython: Use RELRO_LDFLAGS and NO_INDIRECT_LDFLAGS
Guido Günther [Wed, 21 Aug 2013 11:09:48 +0000 (13:09 +0200)]
python: Use RELRO_LDFLAGS and NO_INDIRECT_LDFLAGS

A readonly GOT and detecting indirect linkage is useful here too.

11 years agoCheck for --no-copy-dt-needed linker flag
Guido Günther [Tue, 13 Aug 2013 11:49:05 +0000 (13:49 +0200)]
Check for --no-copy-dt-needed linker flag

and use it when available

11 years agoSimplify RELRO_LDFLAGS
Guido Günther [Tue, 20 Aug 2013 09:20:46 +0000 (11:20 +0200)]
Simplify RELRO_LDFLAGS

by adding it to AM_LDFLAGS instead of every linking rule and
by avoiding a forked grep.

11 years agotests: Add URI precedence checking
Martin Kletzander [Thu, 22 Aug 2013 10:49:41 +0000 (12:49 +0200)]
tests: Add URI precedence checking

Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
to deal with this issue again, herec comes "virsh-uriprecedence".

11 years agoSet security label on FD for virDomainOpenGraphics
Daniel P. Berrange [Thu, 22 Aug 2013 11:38:26 +0000 (12:38 +0100)]
Set security label on FD for virDomainOpenGraphics

The virDomainOpenGraphics method accepts a UNIX socket FD from
the client app. It must set the label on this FD otherwise QEMU
will be prevented from receiving it with recvmsg.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agoqemuBuildNicDevStr: Add mq=on for multiqueue networking
Michal Privoznik [Thu, 22 Aug 2013 08:19:08 +0000 (10:19 +0200)]
qemuBuildNicDevStr: Add mq=on for multiqueue networking

If user requested multiqueue networking, beside multiple /dev/tap and
/dev/vhost-net openings, we forgot to pass mq=on onto the -device
virtio-net-pci command line. This is advised at:

  http://www.linux-kvm.org/page/Multiqueue#Enable_MQ_feature

11 years agodocs: Reformat <disk> attribute description in formatdomain
John Ferlan [Tue, 20 Aug 2013 19:37:00 +0000 (15:37 -0400)]
docs: Reformat <disk> attribute description in formatdomain

Reformat the description to more cleanly delineate the attributes
for a <disk> element.

11 years agovirBitmapParse: Fix behavior in case of error and fix up callers
Peter Krempa [Mon, 19 Aug 2013 14:08:24 +0000 (16:08 +0200)]
virBitmapParse: Fix behavior in case of error and fix up callers

Re-arrange the code so that the returned bitmap is always initialized to
NULL even on early failures and return an error message as some callers
are already expecting it. Fix up the rest not to shadow the error.

11 years agoFix URI connect precedence
Martin Kletzander [Wed, 21 Aug 2013 09:02:42 +0000 (11:02 +0200)]
Fix URI connect precedence

Commit abfff210 changed the order of vshParseArgv() and vshInit() in
order to make fix debugging of parameter parsing.  However, vshInit()
did a vshReconnect() even though ctl->name wasn't set according to the
'-c' parameter yet.  In order to keep both issues fixed, I've split
the vshInit() into vshInitDebug() and vshInit().

One simple memleak of ctl->name is fixed as a part of this patch,
since it is related to the issue it's fixing.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=999323

11 years agoVMX: Improve disk parse error for unknown values
Doug Goldstein [Mon, 12 Aug 2013 20:42:42 +0000 (15:42 -0500)]
VMX: Improve disk parse error for unknown values

Previously the error message showed the following:

error: internal error: Invalid or not yet handled value 'auto detect'
for VMX entry 'ide0:0.fileName'

This left the user unsure if it was a CD-ROM or a disk device that they
needed to fix. Now the error shows:

error: internal error: Invalid or not yet handled value 'auto detect'
for VMX entry 'ide0:0.fileName' for device type 'cdrom-raw'

Which should hopefully make it easier to see the issue with the VMX
configuration.

11 years agobridge driver: implement networkEnableIpForwarding for BSD
Roman Bogorodskiy [Sun, 11 Aug 2013 14:30:56 +0000 (18:30 +0400)]
bridge driver: implement networkEnableIpForwarding for BSD

Implement networkEnableIpForwarding() using BSD style sysctl.

11 years agoBSD: implement virNetDev(Set|Clear)IPv4Address
Roman Bogorodskiy [Sun, 11 Aug 2013 13:54:48 +0000 (17:54 +0400)]
BSD: implement virNetDev(Set|Clear)IPv4Address

Provide an implementation of virNetDev(Set|Clear)IPv4Address based on
BSD ifconfig tool in addition to 'ip' from Linux iproute2 package.

11 years agolibxl: fix libvirtd crash when reconnecting domains
Jim Fehlig [Wed, 21 Aug 2013 17:05:18 +0000 (11:05 -0600)]
libxl: fix libvirtd crash when reconnecting domains

More fallout from commit d72ef888.  When reconnecting to running
domains, the libxl_ctx in libxlDomainObjPrivate was used before
initializing it, causing a segfault in libxl and consequently
crashing libvirtd.

Initialize the libxlDomainObjPrivate libxl_ctx in libxlReconnectDomain,
and while at it use this ctx in libxlReconnectDomain instead of the
driver-wide ctx.

11 years agomigration: do not restore labels on failed migration
Eric Blake [Tue, 20 Aug 2013 22:39:16 +0000 (16:39 -0600)]
migration: do not restore labels on failed migration

https://bugzilla.redhat.com/show_bug.cgi?id=822052

When doing a live migration, if the destination fails for any
reason after the point in which files should be labeled, then
the cleanup of the destination would restore the labels to their
defaults, even though the source is still trying to continue
running with the image open.  Bug 822052 mentioned one source
of live migration failure - a mismatch in SELinux virt_use_nfs
settings (on for source, off for destination); but I found other
situations that would also trigger it (for example, having a
graphics device tied to port 5999 on the source, and a different
domain on the destination already using that port, so that the
destination cannot reuse the port).

In short, just as cleanup of the source on a successful migration
must not relabel files (because the destination would be crippled
by the relabel), cleanup of the destination on a failed migration
must not relabel files (because the source would be crippled).

* src/qemu/qemu_process.c (qemuProcessStart): Set flag to avoid
label restoration when cleaning up on failed migration.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agotests: fix building without xattr support
Claudio Bley [Wed, 21 Aug 2013 06:52:20 +0000 (08:52 +0200)]
tests: fix building without xattr support

Only compile securityselinuxhelper.c if xattr support was detected to
avoid this error:

securityselinuxhelper.c:34:24: fatal error: attr/xattr.h: No such file
or directory compilation terminated.

Since all SELinux tests depend upon the securityselinuxhelper library,
these test programs are now only build when xattr support is
available.

11 years agoTest handling of non-existent x509 certs
Daniel P. Berrange [Wed, 21 Aug 2013 11:48:58 +0000 (12:48 +0100)]
Test handling of non-existent x509 certs

In commit f905cc998449c89339d0e2894a71d9a9e45293e5 a use of
uninitialized data was fixed based on a coverity report. It
turns out it was possible to trigger this issue by pointing
libvirt at non-existent certificate files, typically causing
a crash.

This adds a test case for that scenario. With the above
commit reverted, this new test case will crash with a SEGV.
With the fix applied, it passes, reporting a normal libvirt
error to the caller.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
11 years agostorage: Fix the use-after-free memory bug
Osier Yang [Tue, 20 Aug 2013 09:08:49 +0000 (17:08 +0800)]
storage: Fix the use-after-free memory bug

Introduced by commit e0139e30444. virStorageVolDefFree free'ed the
pointers that are still used by the added volume object, this changes
it back to VIR_FREE.

11 years agostorage: Fix coverity warning
Osier Yang [Tue, 20 Aug 2013 15:28:28 +0000 (23:28 +0800)]
storage: Fix coverity warning

Introduced by commit e0139e30444:

1777      /* Updating pool metadata */

(40) Event var_deref_op: Dereferencing null pointer "newvol".
     Also see events: [assign_zero]

1778      pool->def->allocation += newvol->allocation;
1779      pool->def->available -= newvol->allocation;

11 years agodocs: Update iSCSI storage pool example
John Ferlan [Wed, 7 Aug 2013 13:32:54 +0000 (09:32 -0400)]
docs: Update iSCSI storage pool example

Update the iSCSI storage pool example to include the secret

11 years agodocs: Update formatsecrets to include more examples of each type
John Ferlan [Wed, 7 Aug 2013 13:07:28 +0000 (09:07 -0400)]
docs: Update formatsecrets to include more examples of each type

Update formatsecret docs to describe the various options and provide examples
in order to set up secrets for each type of secret.

11 years agodocs: Update the formatdomain disk examples
John Ferlan [Wed, 7 Aug 2013 13:05:43 +0000 (09:05 -0400)]
docs: Update the formatdomain disk examples

Add more iSCSI examples including having a secret attached. There are 4 new
examples; one for each way to have an iSCSI - a network disk using virtio,
a passthrough network lun using scsi, a volume disk using "mode='host'",
and a volume disk using "mode='direct'"

11 years agoReport secret usage error message similarly
John Ferlan [Tue, 6 Aug 2013 11:52:46 +0000 (07:52 -0400)]
Report secret usage error message similarly

Each of the modules handled reporting error messages from the secret fetching
slightly differently with respect to the error. Provide a similar message
for each error case and provide as much data as possible.

11 years agoqemu_conf: Fix broken logic for adding passthrough iscsi lun
Osier Yang [Mon, 19 Aug 2013 12:55:24 +0000 (08:55 -0400)]
qemu_conf: Fix broken logic for adding passthrough iscsi lun

Following XML would fail :

    <disk type='network' device='lun'>
      <driver name='qemu' type='raw'/>
      <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi/1'>
        <host name='example.com' port='3260'/>
      </source>
      <target dev='sda' bus='scsi'/>
    </disk>

With the message:

error: Failed to start domain iscsilun
error: Unable to get device ID 'iqn.2013-07.com.example:iscsi/1': No such fi

Cause was commit id '1f49b05a' which added 'virDomainDiskSourceIsBlockType'

11 years agovirsh: Print cephx and iscsi usage
John Ferlan [Tue, 6 Aug 2013 11:49:23 +0000 (07:49 -0400)]
virsh: Print cephx and iscsi usage

When using virsh secret-list - if the secret types are cephx or iscsi,
then allow fetch/print of the usage information. Prior to the change
the following would print:

UUID                                 Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 Unused
a5ba3efe-6adf-4a6a-b243-f010a043e314 Unused

Afterwards:

UUID                                 Usage
-----------------------------------------------------------
1b40a534-8301-45d5-b1aa-11894ebb1735 ceph ceph_example
a5ba3efe-6adf-4a6a-b243-f010a043e314 iscsi libvirtiscsi

11 years agolibxl: Resolve possible NULL dereference
John Ferlan [Tue, 20 Aug 2013 17:20:56 +0000 (13:20 -0400)]
libxl: Resolve possible NULL dereference

If we reached cleanup: prior to allocating cpus, it was possible that
'nr_nodes' had a value, but cpus was NULL leading to a possible NULL
deref. Add a 'cpus' as an end condition to for loop

11 years agoselinux: enhance test to cover nfs label failure
Eric Blake [Tue, 13 Aug 2013 20:19:14 +0000 (14:19 -0600)]
selinux: enhance test to cover nfs label failure

Daniel Berrange (correctly) pointed out that we should do a better
job of testing selinux labeling fallbacks on NFS disks that lack
labeling support.

* tests/securityselinuxhelper.c (includes): Makefile already
guaranteed xattr support.  Add additional headers.
(init_syms): New function, borrowing from vircgroupmock.c.
(setfilecon_raw, getfilecon_raw): Fake NFS failure.
(statfs): Fake an NFS mount point.
(security_getenforce, security_get_boolean_active): Don't let host
environment affect test.
* tests/securityselinuxlabeldata/nfs.data: New file.
* tests/securityselinuxlabeldata/nfs.xml: New file.
* tests/securityselinuxlabeltest.c (testSELinuxCreateDisks)
(testSELinuxDeleteDisks): Setup and cleanup for fake NFS mount.
(testSELinuxCheckLabels): Test handling of SELinux NFS denial.
Fix memory leak.
(testSELinuxLabeling): Avoid infinite loop on dirty tree.
(mymain): Add new test.

11 years agoselinux: distinguish failure to label from request to avoid label
Eric Blake [Mon, 12 Aug 2013 15:15:42 +0000 (09:15 -0600)]
selinux: distinguish failure to label from request to avoid label

https://bugzilla.redhat.com/show_bug.cgi?id=924153

Commit 904e05a2 (v0.9.9) added a per-<disk> seclabel element with
an attribute relabel='no' in order to try and minimize the
impact of shutdown delays when an NFS server disappears.  The idea
was that if a disk is on NFS and can't be labeled in the first
place, there is no need to attempt the (no-op) relabel on domain
shutdown.  Unfortunately, the way this was implemented was by
modifying the domain XML so that the optimization would survive
libvirtd restart, but in a way that is indistinguishable from an
explicit user setting.  Furthermore, once the setting is turned
on, libvirt avoids attempts at labeling, even for operations like
snapshot or blockcopy where the chain is being extended or pivoted
onto non-NFS, where SELinux labeling is once again possible.  As
a result, it was impossible to do a blockcopy to pivot from an
NFS image file onto a local file.

The solution is to separate the semantics of a chain that must
not be labeled (which the user can set even on persistent domains)
vs. the optimization of not attempting a relabel on cleanup (a
live-only annotation), and using only the user's explicit notation
rather than the optimization as the decision on whether to skip
a label attempt in the first place.  When upgrading an older
libvirtd to a newer, an NFS volume will still attempt the relabel;
but as the avoidance of a relabel was only an optimization, this
shouldn't cause any problems.

In the ideal future, libvirt will eventually have XML describing
EVERY file in the backing chain, with each file having a separate
<seclabel> element.  At that point, libvirt will be able to track
more closely which files need a relabel attempt at shutdown.  But
until we reach that point, the single <seclabel> for the entire
<disk> chain is treated as a hint - when a chain has only one
file, then we know it is accurate; but if the chain has more than
one file, we have to attempt relabel in spite of the attribute,
in case part of the chain is local and SELinux mattered for that
portion of the chain.

* src/conf/domain_conf.h (_virSecurityDeviceLabelDef): Add new
member.
* src/conf/domain_conf.c (virSecurityDeviceLabelDefParseXML):
Parse it, for live images only.
(virSecurityDeviceLabelDefFormat): Output it.
(virDomainDiskDefParseXML, virDomainChrSourceDefParseXML)
(virDomainDiskSourceDefFormat, virDomainChrDefFormat)
(virDomainDiskDefFormat): Pass flags on through.
* src/security/security_selinux.c
(virSecuritySELinuxRestoreSecurityImageLabelInt): Honor labelskip
when possible.
(virSecuritySELinuxSetSecurityFileLabel): Set labelskip, not
norelabel, if labeling fails.
(virSecuritySELinuxSetFileconHelper): Fix indentation.
* docs/formatdomain.html.in (seclabel): Document new xml.
* docs/schemas/domaincommon.rng (devSeclabel): Allow it in RNG.
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-labelskip.xml:
* tests/qemuxml2argvdata/qemuxml2argv-seclabel-*-labelskip.args:
* tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-*-labelskip.xml:
New test files.
* tests/qemuxml2argvtest.c (mymain): Run the new tests.
* tests/qemuxml2xmltest.c (mymain): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
11 years agovirsh: Don't leak list of volumes when undefining domain with storage
Peter Krempa [Thu, 15 Aug 2013 14:48:20 +0000 (16:48 +0200)]
virsh: Don't leak list of volumes when undefining domain with storage

Use the new semantics of vshStringToArray to avoid leaking the array of
volumes to be deleted. The array would be leaked in case the first
volume was found in the domain definition. Also refactor the code a bit
to sanitize naming of variables hoding arrays and dimensions of the
arrays.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=996050