Until now, it was possible to crash libvirtd when defining domain with
channel device with missing source element.
When creating new virDomainChrDef, target.port is set to -1, but
unfortunately it is an union with addresses that virDomainChrDefFree
tries to free in case the deviceType is channel. Having the port set
to -1 is intended, however the cleanest way to get around the problems
with the crash seems to be renumbering the VIR_DOMAIN_CHR_CHANNEL_
target types to cover new NONE type (with value 0) being the default
(no target type yet).
(cherry picked from commit 830d035ff5a88c3896a6e508364e34697de941a1)
virsh: make domiftune interface help string consistent
Append '(MAC Address)' after the help string of domiftune virsh
command as it takes the same type of argument as domif-{get,set}link
which have it specified.
(cherry picked from commit 4b2273074c2459136c67e657d2a2ac2e9db6f5a4)
Michal Privoznik [Fri, 15 Jun 2012 16:00:13 +0000 (18:00 +0200)]
qemu_agent: Wait for events instead of agent response
With latest changes to qemu-ga success on some commands is not reported
anymore, e.g. guest-shutdown or guest-suspend-*. However, errors are
still being reported. Therefore, we need to find different source of
indication if operation was successful. Events.
(cherry picked from commit d97a234c62d272eb140c836cb93d750a4dec3354)
Eric Blake [Wed, 13 Jun 2012 16:09:39 +0000 (10:09 -0600)]
build: hoist qemu dependence on yajl to configure
Commit 6e769eba made it a runtime error if libvirt was compiled
without yajl support but targets a new enough qemu. But enough
users are hitting this on self-compiled libvirt that it is worth
erroring out at compilation time, rather than an obscure failure
when trying to use the built executable.
* configure.ac: If qemu is requested and -version works, require
yajl when qemu version is new enough.
* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add
comment.
(cherry picked from commit 350583c859deaaddc98d7319e1c2ad649e4d3e83)
Cole Robinson [Fri, 15 Jun 2012 17:46:51 +0000 (13:46 -0400)]
autogen: Always abide --system
If we do ./autogen.sh && ./configure, then later try ./autogen.sh --system,
configure isn't invoked with the requested params. Instead
config.status --recheck is run.
(cherry picked from commit 2cd29c3107ff1f135e05fa55f3b389f37a6cb867)
Jim Fehlig [Thu, 14 Jun 2012 17:20:17 +0000 (11:20 -0600)]
Check for errors when parsing bridge interface XML
The return status of virInterfaceDefParseBridge() was not being
checked, potentially resulting in the creation of a broken
interface.
(cherry picked from commit cc004a664750e84a234d01ae4a5d774f7f980307)
Osier Yang [Fri, 15 Jun 2012 02:34:04 +0000 (10:34 +0800)]
schema: Update domain XML schema
'boot' tag shouldn't be exclusive with 'kernel', 'initrd', and 'cmdline',
though the boot sequence doesn't make sense when the guest boots from
kernel directly. But it's useful if booting from kernel is to install
a newguest, even if it's not to install a guest, there is no hurt. And
on the other hand, we allow 'boot' and the kernel tags when parsing.
(cherry picked from commit b34324fbf09c82fbd4ad2c0dcc140172ee045c8a)
Wen Congyang [Fri, 25 May 2012 10:12:00 +0000 (18:12 +0800)]
qemu: fix potential dead lock
If we lock the qemu_driver, we should call qemuDomainObjBeginJobWithDriver()
not qemuDomainObjBeginJob().
(cherry picked from commit 5136c5799fabe411eb0fadc59377572e21386d83)
Osier Yang [Thu, 14 Jun 2012 11:09:53 +0000 (19:09 +0800)]
virsh: Null terminated the string memcpy from buffer explicitly
Detected by valgrind:
==16217== 1 errors in context 1 of 12:
==16217== Invalid read of size 1
==16217== at 0x4A07804: __GI_strlen (mc_replace_strmem.c:284)
==16217== by 0x3019F167F6: xdr_string (in /lib64/libc-2.12.so)
==16217== by 0x3033709E8D: xdr_remote_nonnull_string (remote_protocol.c:31)
==16217== by 0x303370E5CB: xdr_remote_domain_update_device_flags_args (remote_protocol.c:2028)
==16217== by 0x30337197D1: virNetMessageEncodePayload (virnetmessage.c:341)
==16217== by 0x30337135E1: virNetClientProgramCall (virnetclientprogram.c:327)
==16217== by 0x30336F1EFD: callWithFD (remote_driver.c:4586)
==16217== by 0x30336F1F7B: call (remote_driver.c:4607)
==16217== by 0x30336F42F2: remoteDomainUpdateDeviceFlags (remote_client_bodies.h:2865)
==16217== by 0x30336D46E5: virDomainUpdateDeviceFlags (libvirt.c:9457)
==16217== by 0x41AEE8: cmdChangeMedia (virsh.c:15249)
==16217== by 0x413CB4: vshCommandRun (virsh.c:18669)
==16217== Address 0x4ec5e25 is 0 bytes after a block of size 293 alloc'd
==16217== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==16217== by 0x303364F1DB: virAllocN (memory.c:129)
==16217== by 0x41A844: vshPrepareDiskXML (virsh.c:15043)
==16217== by 0x41AECC: cmdChangeMedia (virsh.c:15246)
==16217== by 0x413CB4: vshCommandRun (virsh.c:18669)
==16217== by 0x423973: main (virsh.c:20261)
(cherry picked from commit e3843d7f046da93d5dfb8f1a8a8f4fcee8f6a5c1)
The public libvirt API does not have any application visible
dependency on Xen libraries. The xen-devel dependency is thus
bogus
(cherry picked from commit 899bf6680a8c7c715e79ed6e80b00528eacb73cc)
In case nparams was originally set to 0, virDomain* API would fill it
with the number of typed parameters it can provide and we would use this
number (rather than zero) to clear params. Because VIR_ALLOC* returns
non-NULL pointer even if size is 0, the code would end up walking
through random memory. If we were lucky enough and the memory contained
7 (VIR_TYPED_PARAM_STRING) at the right place, we would try to free a
random pointer and crash.
libvirt-guests: systemd host shutdown does not work
I originally postet this into the Fedora bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=843836
Currently gracefully shutting down guest vms on host shutdown does not work on
Fedora 17, the guests are killed hard on system shutdown.
The reason is systemd considers libvirt-guests.service to be stopped when the
system is running:
$ systemctl status libvirt-guests.service
libvirt-guests.service - Suspend Active Libvirt Guests
Loaded: loaded (/usr/lib/systemd/system/libvirt-guests.service;
enabled)
Active: deactivating (stop) since Fri, 27 Jul 2012 15:47:31 +0200;
2min 48s ago
Process: 1085 ExecStart=/etc/init.d/libvirt-guests start
(code=exited, status=0/SUCCESS)
Control: 1150 (libvirt-guests)
CGroup: name=systemd:/system/libvirt-guests.service
└ control
├ 1150 /bin/sh /etc/init.d/libvirt-guests stop
└ 2257 sleep 1
libvirt-guests.service is defined as type "simple" in systemd (the default).
That means systemd will shut down the service when the start executable is
terminated after starting is done. Systemd will not call stop again on system
shutdown because it thinks it is already stopped.
The solution is to define it as type "oneshot" and set the flag
"RemainAfterExit". Then systemd will consider the service as active after
startup and will call the stop function on host shutdown.
(cherry picked from commit 79ca7e4e5763795b041e95738c73d4e64bf1024b)
Eric Blake [Tue, 10 Jul 2012 15:37:07 +0000 (09:37 -0600)]
build: update to latest gnulib, for secure tarball
Pick up some build fixes in the latest gnulib. In particular,
we want to ensure that official tarballs are secure, but don't
want to penalize people who don't run 'make dist', since fixed
automake still hasn't hit common platforms like Fedora 17.
* .gnulib: Update to latest, for Automake CVE-2012-3386 detection.
* bootstrap: Resync from gnulib.
* bootstrap.conf (gnulib_extra_files): Drop missing, since gnulib
has dropped it in favor of Automake's version.
* cfg.mk (local-checks-to-skip): Conditionally skip the security
check in cases where it doesn't matter.
(cherry picked from commit f12e1396212b9127306245c61566f171a3df7aa3)
Update to latest GNULIB to fix compat with Mingw64 toolchain
On both x86_64-w64-mingw32 and i686-w64-mingw32 there were
the following warnings/errors:
CC fstat.lo
../../../gnulib/lib/fstat.c:27:0: warning: "stat" redefined [enabled by default]
In file included from ./sys/stat.h:32:0,
from ../../../gnulib/lib/fstat.c:25:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/sys/stat.h:258:0: note: this is the location of the previous definition
../../../gnulib/lib/fstat.c:28:0: warning: "fstat" redefined [enabled by default]
In file included from ./sys/stat.h:32:0,
from ../../../gnulib/lib/fstat.c:25:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/sys/stat.h:259:0: note: this is the location of the previous definition
CC stat.lo
../../../gnulib/lib/stat.c:32:0: warning: "stat" redefined [enabled by default]
In file included from ./sys/stat.h:32:0,
from ../../../gnulib/lib/stat.c:27:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/sys/stat.h:258:0: note: this is the location of the previous definition
CC stdio-read.lo
../../../gnulib/lib/stdio-read.c:102:1: error: redefinition of 'vscanf'
In file included from ./stdio.h:43:0,
from ../../../gnulib/lib/stdio-read.c:21:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/stdio.h:397:7: note: previous definition of 'vscanf' was here
../../../gnulib/lib/stdio-read.c:108:1: error: redefinition of 'vfscanf'
In file included from ./stdio.h:43:0,
from ../../../gnulib/lib/stdio-read.c:21:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/stdio.h:384:7: note: previous definition of 'vfscanf' was here
make[3]: *** [stdio-read.lo] Error 1
make[3]: Leaving directory `/home/berrange/src/virt/libvirt/build/gnulib/lib'
While on x86_64-w64-mingw32 only there was:
In file included from ../../../gnulib/lib/regex.c:69:0:
../../../gnulib/lib/regcomp.c: In function 'parse_dup_op':
../../../gnulib/lib/regcomp.c:2624:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
../../../gnulib/lib/regcomp.c: In function 'mark_opt_subexp':
../../../gnulib/lib/regcomp.c:3859:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
Eric Blake [Mon, 28 May 2012 12:48:26 +0000 (06:48 -0600)]
build: update to latest gnulib
Gnulib finally relaxed the isatty license, needed as first mentioned here:
https://www.redhat.com/archives/libvir-list/2012-February/msg01022.html
Other improvements include better syntax-check rules (we can delete one
of ours now that it is a duplicate) and better compiler warning usage.
* .gnulib: Update to latest, for isatty.
* cfg.mk (sc_prohibit_strncpy): Drop a now-redundant rule.
* bootstrap.conf (gnulib_modules): Add isatty.
* bootstrap: Resync from gnulib.
(cherry picked from commit e925ea3156f2cc8d57a29eeeb839202adc989391)
Eric Blake [Wed, 6 Jun 2012 15:45:04 +0000 (09:45 -0600)]
build: fix build of fresh checkout
Commit 7bff56a worked in an incremental build, but fails for a
fresh clone; apparently, if make sees both an actual file
spelling and an inference rule, only the exact spelling is used.
CCLD libvirt_driver_test.la
CC libvirt_driver_remote_la-remote_driver.lo
remote/remote_driver.c:4707:34: fatal error: remote_client_bodies.h: No such file or directory
compilation terminated.
BUILT_SOURCES to the rescue, instead of trying to mess with .lo
dependencies directly.
* src/Makefile.am (REMOTE_DRIVER_PREREQS, %remote_driver.lo): Drop...
(BUILT_SOURCES): ...and add here instead.
(cherry picked from commit ca02b101d74510df424c5ea659d7b7d481874f98)
Eric Blake [Wed, 30 May 2012 17:30:18 +0000 (11:30 -0600)]
build: fix 'make distcheck' issues
We had a distributed file (remote_protocol.h, which in turn was
a prereq to remote_driver.c) depending on a generated file
(libvirt_probes.h), which is a no-no for a VPATH build from a
read-only source tree (no wonder 'make distcheck' tests precisely
that situation):
File `libvirt_driver_remote.la' does not exist.
File `libvirt_driver_remote_la-remote_driver.lo' does not exist.
Prerequisite `libvirt_probes.h' is newer than target `../../src/remote/remote_protocol.h'.
Must remake target `../../src/remote/remote_protocol.h'.
Invoking recipe from Makefile:7464 to update target `../../src/remote/remote_protocol.h'.
make[3]: Entering directory `/home/remote/eblake/libvirt-tmp2/build/libvirt-0.9.12/_build/src'
GEN ../../src/remote/remote_protocol.h
cannot create ../../src/remote/remote_protocol.h: Permission denied at ../../src/rpc/genprotocol.pl line 31.
make[3]: *** [../../src/remote/remote_protocol.h] Error 13
Rather than making distributed .c files depend on generated files, we
really want to ensure that compilation into .lo files is not attempted
until the generated files are present, done by this patch. Since there
were two different sets of conditionally generated files that both
feed the .lo file, I had to introduce a new variable REMOTE_DRIVER_PREREQS
to keep automake happy.
After that fix, the next issue was that make treats './foo' and 'foo'
differently in determining whether an implicit %foo rule is applicable,
with the result that locking/qemu-sanlock.conf wasn't properly being
built at the right times. Also, the output for using the .aug test
files was a bit verbose.
After fixing the src directory, the next error is related to the docs
directory, where the tarball is missing a stamp file and thus tries to
regenerate files that are already present:
GEN ../../docs/apibuild.py.stamp
Traceback (most recent call last):
File "../../docs/apibuild.py", line 2511, in <module>
rebuild("libvirt")
File "../../docs/apibuild.py", line 2495, in rebuild
builder.serialize()
File "../../docs/apibuild.py", line 2424, in serialize
output = open(filename, "w")
IOError: [Errno 13] Permission denied: '../../docs/libvirt-api.xml'
make[5]: *** [../../docs/apibuild.py.stamp] Error 1
and fixing that exposed another case of a distributed file (generated
html) depending on a built file (libvirt.h), but only when doing an
in-tree build, because of a file glob.
* src/Makefile.am ($(srcdir)/remote/remote_driver.c): Change...
(libvirt_driver_remote_la-remote_driver.lo): ...to the real
dependency.
($(builddir)/locking/%-sanlock.conf): Drop $(builddir), so that
rule gets run in time for test_libvirt_sanlock.aug.
(test_libvir*.aug): Cater to silent build.
(conf_DATA): Don't ship qemu-sanlock.conf in the tarball, since it
is trivial to regenerate.
* docs/Makefile.am (EXTRA_DIST): Ship our stamp file.
($(APIBUILD_STAMP)): Don't depend on generated file.
(cherry picked from commit 7bff56a0d1514cb955eb14adc14281626e80e96c)
Eric Blake [Wed, 13 Jun 2012 03:49:22 +0000 (21:49 -0600)]
build: fix 'make dist' on virgin checkout
'make dist' was depending on *protocol-structs files, which are
stored in git but in turn depended on generated files. We still
want to ship the protocol-structs files, but by renaming the
tests to something not matching a file name, we separate 'make
check' (which depends on the generated file) from 'make dist'
(which only depends on the git files). After all, the tarball
should never depend on a generated file not stored in git.
I found one more case of a git file depending on a generated
file, in a bogus virkeycode.c listing; but at least this one
had no associated rules so it never broke 'make dist'.
Reported by Wen Congyang. Latent bug has been present since
commit 62dee6f, but only recently exposed by commit 7bff56a.
* src/Makefile.am ($(srcdir)/util/virkeycode.c): Drop useless
dependency.
(BUILT_SOURCES): ...and build virkeymaps.h sooner.
(PROTOCOL_STRUCTS): Rather than depend on the struct file...
(check-local): ...convert things into a phony target of...
(check-protocol): ...a new check.
($(srcdir)/remote_protocol-struct): Rename to isolate the distributed
file from the conditional test.
(PDWTAGS): Deal with rename. Swap to compare 'expected actual'.
(cherry picked from commit 9b5970c6c4ef267f2ba7d43d8b4d4996659c9fe1)
Peter Krempa [Thu, 14 Jun 2012 12:48:42 +0000 (14:48 +0200)]
qemu: Enable disconnecting SPICE clients without changing password
Libvirt updates the configuration of SPICE server only when something
changes. This is unfortunate when the user wants to disconnect a
existing spice session when the connected attribute is already
"disconnect".
This patch modifies the conditions for calling the password updater to
be called when nothing changes, but the connected attribute is already
"disconnect".
(cherry picked from commit e0f0131d33e283f1b76c05eb7bef4bbbb3f2fd72)
Peter Krempa [Thu, 14 Jun 2012 08:29:36 +0000 (10:29 +0200)]
qemu: Fix off-by-one error while unescaping monitor strings
While unescaping the commands the commands passed through to the monitor
function qemuMonitorUnescapeArg() initialized lenght of the input string
to strlen()+1 which is fine for alloc but not for iteration of the
string.
This patch fixes the off-by-one error and drops the pointless check for
a single trailing slash that is automaticaly handled by the default
branch of switch.
(cherry picked from commit 0f4660c8787cc41fe67f869984c0ae11d680037e)
Peter Krempa [Wed, 13 Jun 2012 14:55:51 +0000 (16:55 +0200)]
virsh: Don't generate invalid XML in attach-disk command
The attach-disk command used with parameter --cache created an invalid
XML snippet as the beginning of the <driver> element was not printed
when used solely with --cache and no other attribute to driver.
(cherry picked from commit 5b4740265cf1e9cd00165e54e1f5e81f9008bc63)
Eric Blake [Mon, 11 Jun 2012 20:49:03 +0000 (14:49 -0600)]
python: fix snapshot listing bugs
Python exceptions are different than libvirt errors, and we had
some corner case bugs on OOM situations.
* python/libvirt-override.c (libvirt_virDomainSnapshotListNames)
(libvirt_virDomainSnapshotListChildrenNames): Use correct error
returns, avoid segv on OOM, and avoid memory leaks on error.
(cherry picked from commit a0de5d78ef593958bcb83e563fe54137fe43c4ff)
Eric Blake [Mon, 11 Jun 2012 20:32:47 +0000 (14:32 -0600)]
python: use simpler methods
* python/libvirt-override.c (libvirt_virDomainGetVcpus)
(libvirt_virDomainGetVcpuPinInfo): Use Py_XDECREF instead of
open-coding it.
(cherry picked from commit 8566618f65d983b31efb8efec13c25c6410b1718)
Michal Privoznik [Mon, 11 Jun 2012 13:57:19 +0000 (15:57 +0200)]
qemu: Don't overwrite security labels
Currently, if qemuProcessStart fail at some point, e.g. because
domain being started wants a PCI/USB device already assigned to
a different domain, we jump to cleanup label where qemuProcessStop
is performed. This unconditionally calls virSecurityManagerRestoreAllLabel
which is wrong because the other domain is still using those devices.
However, once we successfully label all devices/paths in
qemuProcessStart() from that point on, we have to perform a rollback
on failure - that is - we have to virSecurityManagerRestoreAllLabel.
(cherry picked from commit 86032b2276ace5a7977aad2bbae73b4c33e31914)
Michal Privoznik [Mon, 11 Jun 2012 13:20:44 +0000 (15:20 +0200)]
qemuProcessStop: Switch to flags
Currently, we are passing only one boolean (migrated) so there is
no real profit in this. But it creates starting position for
next patch.
(cherry picked from commit 69dd77149cacc6b12740bd70b729b6cb7506f4e0)
Laine Stump [Mon, 11 Jun 2012 21:16:17 +0000 (17:16 -0400)]
docs: minor fixes to domain interface documentation
A few examples for <interface> had a type='direct' interface with no
sub-elements. This is not allowed - a type='direct' interface must
have at least a source element. (Most likely the example was copied
from the type='user' or type='ethernet' examples - they *do* allow an
instance with no sub-elements).
There was also one place that mistakenly used %lt; ... %gt; instead of
< ... > (for some reason, I make that typo all the time).
(cherry picked from commit 549741ee44c9d3dcaf0231d2f09838f46ac1d74d)
Eric Blake [Thu, 7 Jun 2012 19:23:49 +0000 (13:23 -0600)]
docs: link to FLOSS Weekly podcast, virt blogs
Eric Blake and Guido Günther were guests during this week's
FLOSS Weekly podcast, giving insights into libvirt as a Free
Software project. Also, there are several useful blogs on
virt-related topics.
Eric Blake [Fri, 8 Jun 2012 03:46:47 +0000 (21:46 -0600)]
snapshot: avoid virsh crash with older servers
Commits 51082301, 16d7b39, and 521cc447 introduced support for
'virsh snapshot-list --from' when talking to a server older than
0.9.5, but broke support for plain 'virsh snapshot-list' for the
same old server in the process. Because the code is not properly
gated, we end up with a SIGSEGV during a strcmp with a NULL argument.
* tools/virsh.c (cmdSnapshotList): Don't waste time on fallbacks
when --from is not present.
(cherry picked from commit d9f606659829e666803d95d05aee9ca1ecced955)
Li Zhang [Tue, 29 May 2012 08:35:17 +0000 (16:35 +0800)]
Assign correct address type to spapr-vlan and spapr-vty.
For pseries guest, spapr-vlan and spapr-vty is based
on spapr-vio address. According to model of network
device, the address type should be assigned automatically.
For serial device, serial pty device is recognized as
spapr-vty device, which is also on spapr-vio.
So this patch is to correct the address type of
spapr-vlan and spapr-vty, and build correct
command line of spapr-vty.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> Reviewed-by: Michael Ellerman<michaele@au1.ibm.com>
(cherry picked from commit 04a319ba4e5f82e56af47e18dc055e5735ddd74d)
Eric Blake [Thu, 7 Jun 2012 16:48:05 +0000 (10:48 -0600)]
maint: make it easier to copy FORTIFY_SOURCE snippet
While libvirt intentionally avoids -Wundef (after all, C99
guarantees sane semantics of treating undefined macros as 0),
the glibc insanity of #warning on _FORTIFY_SOURCE coupled with
what some people feel is the black magic of autoconf means
that other projects are likely to copy our snippet verbatim.
We can be nicer to other projects by making it easier to
integrate into projects that use -Wundef.
Suggested by Christophe Fergeau.
* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Be nice
to other projects using -Wundef.
(cherry picked from commit 1c2edf0fbd4d20b3955cd5a3d8986413e94fba51)
Eric Blake [Sat, 26 May 2012 02:17:01 +0000 (20:17 -0600)]
command: avoid potential deadlock on handshake
There is a theoretical problem of an extreme bug where we can get
into deadlock due to command handshaking. Thanks to a pair of pipes,
we have a situation where the parent thinks the child reported an
error and is waiting for a message from the child to explain the
error; but at the same time the child thinks it reported success
and is waiting for the parent to acknowledge the success; so both
processes are now blocked.
Thankfully, I don't think this deadlock is possible without at
least one other bug in the code, but I did see exactly that sort
of situation prior to commit da831af - I saw a backtrace where a
double close bug in the parent caused the parent to read from the
wrong fd and assume the child failed, even though the child really
sent success.
This potential deadlock is not quite like commit 858c247 (a deadlock
due to multiple readers on one pipe preventing a write from completing),
although the solution is similar - always close unused pipe fds before
blocking, rather than after.
Eric Blake [Thu, 31 May 2012 21:50:07 +0000 (15:50 -0600)]
command: avoid deadlock on EPIPE situation
It is possible to deadlock libvirt by having a domain with XML
longer than PIPE_BUF, and by writing a hook script that closes
stdin early. This is because libvirt was keeping a copy of the
child's stdin read fd open, which means the write fd in the
parent will never see EPIPE (remember, libvirt should always be
run with SIGPIPE ignored, so we should never get a SIGPIPE signal).
Since there is no error, libvirt blocks waiting for a write to
complete, even though the only reader is also libvirt. The
solution is to ensure that only the child can act as a reader
before the parent does any writes; and then dealing with the
fallout of dealing with EPIPE.
Thankfully, this is not a security hole - since the only way to
trigger the deadlock is to install a custom hook script, anyone
that already has privileges to install a hook script already has
privileges to do any number of other equally disruptive things
to libvirt; it would only be a security hole if an unprivileged
user could install a hook script to DoS a privileged user.
* src/util/command.c (virCommandRun): Close parent's copy of child
read fd earlier.
(virCommandProcessIO): Don't let EPIPE be fatal; the child may
be done parsing input.
* tests/commandhelper.c (main): Set up a SIGPIPE situation.
* tests/commandtest.c (test20): Trigger it.
* tests/commanddata/test20.log: New file.
(cherry picked from commit 858c2476d9dccf630ad5b7f4d8e0e61e9295c2b6)
Eric Blake [Mon, 4 Jun 2012 17:33:20 +0000 (11:33 -0600)]
build: allow building with newer glibc-headers and -O0
glibc 2.15 (on Fedora 17) coupled with explicit disabling of
optimization during development dies a painful death:
In file included from /usr/include/limits.h:27:0,
from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/limits.h:169,
from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/syslimits.h:7,
from /usr/lib/gcc/x86_64-redhat-linux/4.7.0/include/limits.h:34,
from util/bitmap.c:26:
/usr/include/features.h:314:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
cc1: all warnings being treated as errors
Work around this by only conditionally defining _FORTIFY_SOURCE,
in the case where glibc can actually use it. The trick is using
AH_VERBATIM instead of AC_DEFINE.
* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Squelch
_FORTIFY_SOURCE when needed to avoid glibc #warnings.
(cherry picked from commit 2af63b1c349114df98c163a8401fd9cf2facdabe)
Radu Caragea [Mon, 4 Jun 2012 07:32:37 +0000 (09:32 +0200)]
Fix sync issue in virNetClientStreamEventRemoveCallback
The stream lock is unlocked twice instead of being locked and then
unlocked. Probably a typo.
(cherry picked from commit 107f51b69c44b9902ed76ab0e33e89dde5d21cb1)
The problem is that an interface with type='hostdev' will have an
alias of the form "hostdev%d", while the function that looks through
existing netdevs to determine the name to use for a new addition will
fail if there's an existing entry that does not match the form
"net%d".
This is another of the handful of places that need an exception due to
the hybrid nature of <interface type='hostdev'> (which is not exactly
an <interface> or a <hostdev>, but is both at the same time).
(cherry picked from commit 6734ce7bc8ec4da7900080aa17fef20c68d401ef)
tools: make virt-pki-validate work with acls and xattrs
This patch makes virt-pki-validate work with certificates that have
acl or xattr set. Otherwise it failing due to wrong permissions.
(cherry picked from commit d4fb6694a4b7d5c613136e3bfc1bbf8d6b98ec74)
Wen Congyang [Wed, 30 May 2012 09:20:44 +0000 (17:20 +0800)]
qemu: avoid closing fd more than once
If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT,
we will close spec->dest.fd.local in qemuMigrationRun(). So we
should set spec->dest.fd.local to -1 in qemuMigrationRun().
Eric Blake [Wed, 30 May 2012 15:20:37 +0000 (09:20 -0600)]
fdstream: avoid double close bug
Wen Congyang reported that we have a double-close bug if we fail
virFDStreamOpenInternal, since childfd duplicated one of the fds[]
array contents. In truth, since we always transfer both members
of fds to other variables, we should close the fds through those
other names, and just use fds[] for pipe().
* src/fdstream.c (virFDStreamOpenFileInternal): Swap scope of
childfd and fds[], to avoid a double close.
(cherry picked from commit f3cfc7c8848006b613e09bb1acd081716854527e)
Eric Blake [Tue, 29 May 2012 23:47:58 +0000 (17:47 -0600)]
command: avoid double close bugs
KAMEZAWA Hiroyuki reported a nasty double-free bug when virCommand
is used to convert a string into input to a child command. The
problem is that the poll() loop of virCommandProcessIO would close()
the write end of the pipe in order to let the child see EOF, then
the caller virCommandRun() would also close the same fd number, with
the second close possibly nuking an fd opened by some other thread
in the meantime. This in turn can have all sorts of bad effects.
The bug has been present since the introduction of virCommand in
commit f16ad06f.
This is based on his first attempt at a patch, at
https://bugzilla.redhat.com/show_bug.cgi?id=823716
* src/util/command.c (_virCommand): Drop inpipe member.
(virCommandProcessIO): Add argument, to avoid closing caller's fd
without informing caller.
(virCommandRun, virCommandNewArgs): Adjust clients.
(cherry picked from commit da831afcf2f6c0d3ed1ea3128a6208f548a05d8f)
Wen Congyang [Wed, 30 May 2012 09:20:46 +0000 (17:20 +0800)]
avoid fd leak
virCommandRunAsync() will set errfd if it succeed. We should
close it if virFDStreamOpenInternal() fails.
(cherry picked from commit 655cffa0f25be40c68624036af80bde102177612)
Eric Blake [Tue, 29 May 2012 14:10:44 +0000 (08:10 -0600)]
build: silence warning from autoconf
Autoconf 2.60 and later insist on using ${datarootdir}, rather than
the derived ${datadir} (although the latter defaults to the former,
it is possible to set configure arguments so that they differ):
config.status: creating libvirt.pc
config.status: WARNING: 'libvirt.pc.in' seems to ignore the --datarootdir setting
This patch follows the autoconf manual's suggestions for how to
support 2.59 (RHEL 5) and newer simultaneously.
* libvirt.pc.in (datarootdir): Define, so ${datadir} will not ignore
datarootdir when using newer autoconf.
(cherry picked from commit aabf77aa41e9589faa7ce44add87b97f5a6dde2e)
Michal Privoznik [Wed, 30 May 2012 12:17:26 +0000 (14:17 +0200)]
virCommand: Extend debug message for handshake
Currently, we are logging only one side of pipes we
create in virCommandRequireHandshake(); This is enough
in cases where pipe2() returns two consecutive FDs. However,
it is not guaranteed and it may return any FDs.
Therefore, it's wise to log the other ends as well.
(cherry picked from commit 7454849ec5489c7b52d3b41a8022f2bc6fda9eee)
When getting number of CPUs the host has assigned, there was always
number "1" returned. Even though all lxc domains with no pinning
launched by libvirt run on all pCPUs (by default, no matter what's the
number), we should at least return the same number as the user
specified when creating the domain.
(cherry picked from commit 87dfdb0b92f234a08408332095454260f1c86917)
Dave Allan [Tue, 29 May 2012 22:49:13 +0000 (16:49 -0600)]
examples: add consolecallback example python script
A while back I wrote the attached code to demonstrate how to use
events and serial console to create a serial console that stays up
even when the VM is down. It might need some work, as I am not
terribly strong with Python.
* examples/python/consolecallback.py: New file.
* examples/python/Makefile.am (EXTRA_DIST): Ship it.
(cherry picked from commit ffc9f7ab4f203ce6825dbdc22b75f0894ae833b2)
Stefan Berger [Tue, 29 May 2012 10:25:59 +0000 (06:25 -0400)]
leak_fix.diff
==3240== 23 bytes in 1 blocks are definitely lost in loss record 242 of 744
==3240== at 0x4C2A4CD: malloc (vg_replace_malloc.c:236)
==3240== by 0x8077537: __vasprintf_chk (vasprintf_chk.c:82)
==3240== by 0x509C677: virVasprintf (stdio2.h:199)
==3240== by 0x509C733: virAsprintf (util.c:1912)
==3240== by 0x1906583A: qemudStartup (qemu_driver.c:679)
==3240== by 0x511991D: virStateInitialize (libvirt.c:809)
==3240== by 0x40CD84: daemonRunStateInit (libvirtd.c:751)
==3240== by 0x5098745: virThreadHelper (threads-pthread.c:161)
==3240== by 0x7953D8F: start_thread (pthread_create.c:309)
==3240== by 0x805FF5C: clone (clone.S:115)
"accel3d" was specified twice, the second one is obviously "accel2d"
(also from the text down below.
(cherry picked from commit 94618eb66ac79cae49bd564f397ae3a4f1e49f98)
When you try to connect to a socket in the abstract namespace,
the error will be ECONNREFUSED for a non-listening daemon. With
the non-abstract namespace though, you instead get ENOENT. Add
a check for this extra errno when auto-spawning the daemon
Jim Meyering [Sat, 26 May 2012 09:21:47 +0000 (11:21 +0200)]
maint: avoid new automake warning about AM_PROG_CC_STDC
* configure.ac (AM_PROG_CC_STDC): Stop using this macro.
It provokes warnings from newer automake and is superseded by
autoconf's AC_PROG_CC, which we're already using.
(cherry picked from commit 0d07aa512c775d9b4e8460f203898364ba5413dd)
Add a note about setting the LIBVIRT_DRIVER_DIR env variable,
explain --system and fix example to use --disable-werror
(cherry picked from commit 4878a33125a0793b8af3a1f9837bb1c396a80420)
Eric Blake [Wed, 24 Nov 2010 20:41:50 +0000 (13:41 -0700)]
tests: run valgrind on real executables, not libtool wrappers
* tests/Makefile.am (valgrind): Use libtool's ability to bypass
libtool wrappers when running valgrind.
(cherry picked from commit 428fc2bf3138136de2f340e06fae287a37d3f5f2)
Add parsing for VIR_ENUM_IMPL & VIR_ENUM_DECL in apibuild.py
The apibuild.py parser needs to be able to parse & ignore
any VIR_ENUM_IMPL/VIR_ENUM_DECL macros in the source. Add
some special case code to deal with this rather than trying
to figure out a generic syntax for parsing macros.
The libvirt-lock-sanlock RPM requires libvirtd, so its RPM dep
should be on libvirt-daemon, not libvirt
(cherry picked from commit aad1625da693f765af60ab0a184b23069cb5cb4e)
Adding syslog.target is obsolete, avahi.target does not
exist and dbus.target is also obsolete
Reported-by: Lennart Poettering <lpoetter@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 4c7973e184a4ce52f78e63f1a92f9597709279ab)
Jiri Denemark [Mon, 21 May 2012 14:02:05 +0000 (16:02 +0200)]
Revert "rpc: Discard non-blocking calls only when necessary"
This reverts commit b1e374a7ac56927cfe62435179bf0bba1e08b372, which was
rather bad since I failed to consider all sides of the issue. The main
things I didn't consider properly are:
- a thread which sends a non-blocking call waits for the thread with
the buck to process the call
- the code doesn't expect non-blocking calls to remain in the queue
unless they were already partially sent
Thus, the reverted patch actually breaks more than what it fixes and
clients (which may even be libvirtd during p2p migrations) will likely
end up in a deadlock.
(cherry picked from commit 63643f67abcdeaa33a0f85ea8e54da75ea9908e4)
Peter Krempa [Mon, 21 May 2012 14:31:53 +0000 (16:31 +0200)]
qemu_hotplug: Don't free the PCI device structure after hot-unplug
The pciDevice structure corresponding to the device being hot-unplugged
was freed after it was "stolen" from activeList. The pointer was still
used for eg-inactive list. This patch removes the free of the structure
and frees it only if reset fails on the device.
(cherry picked from commit db19417fc012416639c2230e5f19717b84245ce5)
Fix potential events deadlock when unref'ing virConnectPtr
When the last reference to a virConnectPtr is released by
libvirtd, it was possible for a deadlock to occur in the
virDomainEventState functions. The virDomainEventStatePtr
holds a reference on virConnectPtr for each registered
callback. When removing a callback, the virUnrefConnect
function is run. If this causes the last reference on the
virConnectPtr to be released, then virReleaseConnect can
be run, which in turns calls qemudClose. This function has
a call to virDomainEventStateDeregisterConn which is intended
to remove all callbacks associated with the virConnectPtr
instance. This will try to grab a lock on virDomainEventState
but this lock is already held. Deadlock ensues
Thread 1 (Thread 0x7fcbb526a840 (LWP 23185)):
Since each callback associated with a virConnectPtr holds a
reference on virConnectPtr, it is impossible for the qemudClose
method to be invoked while any callbacks are still registered.
Thus the call to virDomainEventStateDeregisterConn must in fact
be a no-op. Thus it is possible to just remove all trace of
virDomainEventStateDeregisterConn and avoid the deadlock.
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Delete virDomainEventStateDeregisterConn
* src/libxl/libxl_driver.c, src/lxc/lxc_driver.c,
src/qemu/qemu_driver.c, src/uml/uml_driver.c: Remove
calls to virDomainEventStateDeregisterConn
(cherry picked from commit 2cb0899eec72376629a0583647dcad39b00c5715)
Jim Fehlig [Mon, 21 May 2012 15:23:41 +0000 (09:23 -0600)]
Fix build when configuring with polkit0
Commit 2223ea98 removed the only use of 'server' param in
remoteDispatchAuthPolkit(). Mark the parameter with ATTRIBUTE_UNUSED
to fix the build when configuring with polkit0.
(cherry picked from commit 651d7124529b997031d525ea30bbee5d112c2e88)
Eric Blake [Fri, 18 May 2012 15:42:25 +0000 (09:42 -0600)]
build: fix virnetlink on glibc 2.11
We were being lazy - virnetlink.c was getting uint32_t as a
side-effect from glibc 2.14's <unistd.h>, but older glibc 2.11
does not provide uint32_t from <unistd.h>. In fact, POSIX states
that <unistd.h> need only provide intptr_t, not all of <stdint.h>,
so the bug really is ours. Reported by Jonathan Alescio.
Michal Privoznik [Thu, 17 May 2012 11:40:52 +0000 (13:40 +0200)]
qemu: Don't delete USB device on failed qemuPrepareHostdevUSBDevices
If qemuPrepareHostdevUSBDevices fail it will roll back devices added
to the driver list of used devices. However, if it may fail because
the device is being used already. But then again - with roll back.
Therefore don't try to remove a usb device manually if the function
fail. Although, we want to remove the device if any operation
performed afterwards fail.
(cherry picked from commit 9c484e3dc5464dfbb538744360b401a0bc59c1c6)
Michal Privoznik [Wed, 16 May 2012 14:42:02 +0000 (16:42 +0200)]
qemu: Rollback on used USB devices
One of our latest USB device handling patches 05abd1507d66aabb6cad12eeafeb4c4d1911c585 introduced a regression.
That is, we first create a temporary list of all USB devices that
are to be used by domain just starting up. Then we iterate over and
check if a device from the list is in the global list of currently
assigned devices (activeUsbHostdevs). If not, we add it there and
continue with next iteration then. But if a device from temporary
list is either taken already or adding to the activeUsbHostdevs fails,
we remove all devices in temp list from the activeUsbHostdevs list.
Therefore, if a device is already taken we remove it from
activeUsbHostdevs even if we should not. Thus, next time we allow
the device to be assigned to another domain.
(cherry picked from commit 2f5fdc886ec7ed8b871ebd0576271f8ee5be1f71)
There is no 'udev.target' unit in systemd (only 'udev.service')
yet libvirtd's unit file had a dep on one. There's no compelling
reason for a dep on udev, so remove it altogether.
Set a sensible default master start port for ehci companion controllers
The uhci1, uhci2, uhci3 companion controllers for ehci1 must
have a master start port set. Since this value is predictable
we should set it automatically if the app does not supply it
(cherry picked from commit 03b804a200e214e3faf264b16f41a640c1344962)
(crobinso: Drop an element that wasn't backported to -maint branch.)
Fix logic for assigning PCI addresses to USB2 companion controllers
Currently each USB2 companion controller gets put on a separate
PCI slot. Not only is this wasteful of PCI slots, but it is not
in compliance with the spec for USB2 controllers. The master
echi1 and all companion controllers should be in the same slot,
with echi1 in function 7, and uhci1-3 in functions 0-2 respectively.
* src/qemu/qemu_command.c: Special case handling of USB2 controllers
to apply correct pci slot assignment
* tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args,
tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml: Expand
test to cover automatic slot assignment
(cherry picked from commit 1ebd52cb871f87b7868503b28448e96d59e41d63)
Fix virDomainDeviceInfoIsSet() to check all struct fields
The virDomainDeviceInfoIsSet API was only checking if an
address or alias was set in the struct. Thus if only a
rom bar setting / filename, boot index, or USB master
value was set, they could be accidentally dropped when
formatting XML
(cherry picked from commit 2c195fdbf3293792e39bc4f06755ae751025b0ea)
Allow stack traces to be included with log messages
Sometimes it is useful to see the callpath for log messages.
This change enhances the log filter syntax so that stack traces
can be show by setting '1:+NAME' instead of '1:NAME'.
* docs/logging.html.in: Document new syntax
* configure.ac: Check for execinfo.h
* src/util/logging.c, src/util/logging.h: Add support for
stack traces
* tests/testutils.c: Adapt to API change
This patch is to fix the problem by using the CPU index in
caps->host.numaCell[i]->cpus[i] to set the bitmask instead of
assuming the CPU index of the NUMA nodes are always sequential.
(cherry picked from commit d1bdeca875d92c6fe9c9eb854fa1f3d891fef84a)
projects / thirdparty / libvirt.git / log
