Add support for using 3-arg pkcheck syntax for process (CVE-2013-4311)
With the existing pkcheck (pid, start time) tuple for identifying
the process, there is a race condition, where a process can make
a libvirt RPC call and in another thread exec a setuid application,
causing it to change to effective UID 0. This in turn causes polkit
to do its permission check based on the wrong UID.
To address this, libvirt must get the UID the caller had at time
of connect() (from SO_PEERCRED) and pass a (pid, start time, uid)
triple to the pkcheck program.
This fix requires that libvirt is re-built against a version of
polkit that has the fix for its CVE-2013-4288, so that libvirt
can see 'pkg-config --variable pkcheck_supports_uid polkit-gobject-1'
Signed-off-by: Colin Walters <walters@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
qemu: Avoid dangling job in qemuDomainSetBlockIoTune
virDomainSetBlockIoTuneEnsureACL was incorrectly called after we already
started a job. As a result of this, the job was not cleaned up when an
access driver had forbidden the action.
Tomas Meszaros [Tue, 10 Sep 2013 15:54:26 +0000 (17:54 +0200)]
virsh: Add vshCompleter to each option
completer and completer_flags added to the _vshCmdOptDef
structure so it will be possible for completion generators to
conveniently call option completer functions with desired flags.
Diego Woitasen [Fri, 13 Sep 2013 16:31:07 +0000 (13:31 -0300)]
Add forwarder attribute to <dns/> element
Useful to set custom forwarders instead of using the contents of
/etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to
resolve VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net> Signed-off-by: Eric Blake <eblake@redhat.com>
Doug Goldstein [Tue, 13 Aug 2013 15:56:56 +0000 (10:56 -0500)]
VMX: Add a VMWare Fusion 5 configuration for tests
A user was having an issue with this specific VMWare Fusion config and
he gave me permission to add it as part of our test suite to further
expand our VMX test coverage. Unfortunately our VMX parser and
generator does not support many features contained within and just
silently ignores fields it does not understand so they had to
be removed out in the xml2vmx test. The original unmodified version
exists in the vmx2xml test.
Doug Goldstein [Tue, 13 Aug 2013 15:56:01 +0000 (10:56 -0500)]
VMX: Add support for 'auto detect' fileNames
VMWare Fusion 5 can set the CD-ROM's device name to be 'auto detect' when
using the physical drive via 'cdrom-raw' device type. VMWare will then
connect to first available host CD-ROM to the virtual machine upon start
up according to VMWare documentation. If no device is available, it
appears that the device will remain disconnected.
To better model this a CD-ROM that is marked as "auto detect" when in
the off state would be modeled as the following with this patch:
<disk type='block' device='lun'>
<source startupPolicy='optional'/>
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
Once the domain transitions to the powered on state, libvirt can
populate the remaining source data with what is connected, if anything.
However future power cycles, the domain may not always start with that
device attached.
Currently the XML parser already allows the following syntax:
<disk type='block' device='cdrom'>
<source startupPolicy='optional'/>
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
But it if the dev value is NULL then it would not have the leading
"<source ", resulting in invalid XML.
Eric Blake [Tue, 17 Sep 2013 17:11:25 +0000 (11:11 -0600)]
build: skip ld_preload tests on non-Linux systems
A cross build to mingw fails with:
CC virsystemdtest-virsystemdtest.o
../../tests/virsystemdtest.c: In function 'testCreateNoSystemd':
../../tests/virsystemdtest.c:97:9: error: implicit declaration of function 'unsetenv' [-Werror=implicit-function-declaration]
unsetenv("FAIL_NO_SERVICE");
^
../../tests/virsystemdtest.c:97:9: error: nested extern declaration of 'unsetenv' [-Werror=nested-externs]
We could cop out and pull in the gnulib unsetenv module. But when
you stop and think about it, this test requires LD_PRELOAD to work,
and systemd is a Linux-only concept anyways, both of which mean
the test could never work on mingw in the first place. Simpler is
to just fix the test to behave like our other LD_PRELOAD tests.
* tests/virsystemdtest.c: Provide non-Linux implementation.
Eric Blake [Tue, 17 Sep 2013 16:25:42 +0000 (10:25 -0600)]
build: hoist system-specific checks before library checks
Commit f92c7e3 fixed a regression for native builds, but introduced
a regression for cross-compilation builds; in particular,
./autobuild.sh on a Fedora system with mingw cross-compiler fails
with:
checking for qemu-kvm... /usr/bin/qemu-kvm
checking for yajl_parse_complete in -lyajl... no
checking for yajl_tree_parse in -lyajl... no
configure: error: You must install the libyajl library & headers to compile libvirt
Since we default $with_qemu to 'yes' rather than 'check', and then
flip that default based on platform-specific checks, those platform
specifics need to come prior to any library checks that depend on
the value of $with_qemu.
* configure.ac: Ensure system defaults are sane before checking
for things that make decisions based on system default.
Peter Krempa [Tue, 17 Sep 2013 13:20:24 +0000 (15:20 +0200)]
test: Refactor setting of dummy error handlers
Multiple tests need to register a function to quiesce errors from
libvirt when using a connection and doing negative tests. Each of those
tests had a static function to do so. This can be replaced by a utility
function that enables the errors when debug is enabled.
This patch adds virtTestQuiesceLibvirtErrors() and refactors test that
use private handlers.
Peter Krempa [Tue, 17 Sep 2013 11:27:58 +0000 (13:27 +0200)]
conf: Avoid false positive of uninitialized variable use
GCC 4.8.0+ whines about variable "new" being uninitialized since
commit 73bfac0e7182a3abd. This is a false positive as the
xmlFreeNode(new) statement can be only reached if new was actually
allocated successfully.
CC conf/libvirt_conf_la-domain_conf.lo
conf/domain_conf.c: In function 'virDomainDefSetMetadata':
conf/domain_conf.c:18650:24: error: 'new' may be used uninitialized in this function [-Werror=maybe-uninitialized]
xmlFreeNode(new);
Reported independently by John Ferlan and Michal Privoznik.
Eric Blake [Tue, 17 Sep 2013 11:29:23 +0000 (05:29 -0600)]
build: fix VPATH build of remote driver
Commit 073e1575 tried to set things up so that 1) generated files
to be shipped in the tarball always live in srcdir, and 2) we have
no files in SOURCES that depend on any other files with a literal
$(srcdir) in the name, because that situation can cause confusing
results for the make expansion of $@ depending on whether the file
is found locally or via VPATH. But all my testing for that patch
was done incrementally, where all the protocol.[ch] files had
already been generated prior to the patch and were up-to-date in
the srcdir, and thus I missed one case where $@ causes grief in a
VPATH build from a fresh checkout:
We have a pattern rule for generating remote_protocol.[ch], and
what's more, the rule for protocol.c depends on protocol.h AND
on the protocol.x file. The pattern for protocol.c is only
satisfied via the VPATH lookup for protocol.x, and if protocol.h
doesn't yet exist, the VPATH rule kicks in and we end up with a
dependency on a file with $(srcdir) in the name. Based on make's
rules for $@, this resulted in make building remote_protocol.h
into srcdir (where we want it), then remote_protocol.c into
builddir (oops, not so good for the tarball), and also causes
the build to fail (the compiler can't find the .h if it lives
in a different directory than the .c):
CC remote/libvirt_driver_remote_la-remote_protocol.lo
remote/remote_protocol.c:7:29: fatal error: remote_protocol.h: No such file or directory
#include "remote_protocol.h"
^
compilation terminated.
As before, the fix is to hard-code the output file to go into
srcdir in spite of $@; but since this is in a pattern rule, we
are forced to use $@ in the recipe, so the patch is a bit
trickier than what was done in commit 073e1575.
* src/Makefile.am (%protocol.c, %protocol.h): Force output to srcdir.
Peter Krempa [Tue, 17 Sep 2013 09:16:30 +0000 (11:16 +0200)]
conf: Don't corrupt metadata on OOM
Eric Blake suggested that we could do a little better in case copying of
the metadata to be set fails. With this patch, the old metadata is
discarded after the new string is copied successfuly.
If the ABI compatibility check with the "migratable" user XML is
successful, we would leak the originally parsed XML from the user that
would not be used in this case.
Peter Krempa [Tue, 10 Sep 2013 09:25:55 +0000 (11:25 +0200)]
lib: Don't force the key argument when deleting metadata
virDomainSetMetadata when operating on the metadata element was
requesting the @key argument to be passed even if @metadata was NULL
used to delete the corresponding metadata element. This is not needed as
the key is only used when adding the element and matching is done via
the XML namespace.
Peter Krempa [Fri, 6 Sep 2013 15:34:43 +0000 (17:34 +0200)]
conf: Add support for requesting of XML metadata via the API
The virDomainGetMetadata function was designed to support also retrieval
of app specific metadata from the <metadata> element. This functionality
was never implemented originally.
Peter Krempa [Tue, 11 Jun 2013 13:45:05 +0000 (15:45 +0200)]
qemu: Factor out body of qemuDomainSetMetadata for universal use
The function implemented common behavior that can be reused for other
hypervisor drivers that use the virDomainObj data structures. Factor out
the core into a separate helper func.
Peter Krempa [Tue, 11 Jun 2013 13:45:05 +0000 (15:45 +0200)]
qemu: Factor out body of qemuDomainGetMetadata for universal use
The function implemented common behavior that can be reused for other
hypervisor drivers that use the virDomainObj data structures. Factor out
the core into a separate helper func.
Peter Krempa [Mon, 16 Sep 2013 11:40:42 +0000 (13:40 +0200)]
qemu: Use "migratable" XML definition when doing external checkpoints
In the original implementation of external checkpoints I've mistakenly
used the live definition to be stored in the save image. The normal
approach is to use the "migratable" definition. This was discovered when
commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67 changed the behavior to
use a converted XML from the user to do the compatibility check to fix
problem when using the regular machine saving.
As the previous patch added a compatibility layer, we can now change the
type of the XML in the image.
Peter Krempa [Mon, 16 Sep 2013 11:37:34 +0000 (13:37 +0200)]
qemu: Fix checking of ABI stability when restoring external checkpoints
External checkpoints have a bug in the implementation where they use the
normal definition instead of the "migratable" one. This causes errors
when the snapshot is being reverted using the workaround method via
qemuDomainRestoreFlags() with a custom XML. This issue was introduced
when commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67 changed the code to
compare "migratable" XMLs from the user as we should have used
migratable in the image too.
This patch adds a compatibility layer, so that fixing the snapshot code
won't make existing snapshots fail to load.
CD-ROMs and Floppies are allowed to have no source to imply they are
empty or disconnected. Since the LUN type is used for raw CD-ROM access
with QEMU (and VMWare in the future), it also needs to allow an empty
source when the raw CD-ROM device is disconnected from the domain.
Ján Tomko [Mon, 16 Sep 2013 11:26:46 +0000 (13:26 +0200)]
Always free network and graphics cookies
qemuMigrationEatCookie has flags to control if these should
be parsed, but it does not fill mig->flags. These cookies might
get leaked if these flags are not set by qemuMigrationBakeCookie.
42 (32 direct, 10 indirect) bytes in 1 blocks are definitely lost in
loss record 361 of 662
==123== by 0x1BA33FCA: qemuMigrationEatCookie (qemu_migration.c:678)
==123== by 0x1BA34A1E: qemuMigrationRun (qemu_migration.c:3108)
==123== by 0x1BA3622B: doNativeMigrate (qemu_migration.c:3343)
==123== by 0x1BA3B408: qemuMigrationPerform (qemu_migration.c:4138)
1,003 bytes in 1 blocks are definitely lost in loss record 599 of 635
==404== by 0x50728A7: virBufferAddChar (virbuffer.c:185)
==404== by 0x50BC466: virSystemdEscapeName (virsystemd.c:67)
==404== by 0x50BC6B2: virSystemdMakeSliceName (virsystemd.c:108)
==404== by 0x50BC870: virSystemdCreateMachine (virsystemd.c:169)
==404== by 0x5078267: virCgroupNewMachine (vircgroup.c:1498)
Eric Blake [Sat, 14 Sep 2013 11:29:42 +0000 (05:29 -0600)]
build: make autobuild require rpm build deps
I spent far too long on a new machine trying to figure out why
./autobuild.sh failed due to an rpm build failure (complaining
that libvirt_parthelper was supposed to be packaged but was not
built), and finally traced it to a missing parted-devel
installation. I learned that --nodeps is in place for
autobuilder setups, but for developers, removing it would make
rpmbuild error out much sooner for a less cryptic failure.
* autobuild.sh: Conditionally drop --nodeps from rpmbuild lines.
Eric Blake [Fri, 13 Sep 2013 16:11:26 +0000 (10:11 -0600)]
build: fix build with latest rawhide kernel headers
Bother those kernel developers. In the latest rawhide, kernel
and glibc have now been unified so that <netinet/in.h> and
<linux/in6.h> no longer clash; but <linux/if_bridge.h> is still
not self-contained. Because of the latest header change, the
build is failing with:
checking for linux/param.h... no
configure: error: You must install kernel-headers in order to compile libvirt with QEMU or LXC support
with details:
In file included from conftest.c:561:0:
/usr/include/linux/in6.h:71:18: error: field 'flr_dst' has incomplete type
struct in6_addr flr_dst;
We need a workaround to avoid our workaround :)
* configure.ac (NETINET_LINUX_WORKAROUND): New test.
* src/util/virnetdevbridge.c (includes): Use it.
virsh: move command maxvcpus from domain group to host group.
Since the maxvcpus command query the maximum number of virtual
CPUs supported for a guest VM on this connection, it should be
in virsh-host.c but not virsh-domain.c.
Explicitly link libvirt_net_rpc against SELINUX_LIBS
Since virnetsocket conditionally uses selinux we need to link against it
otherwise the build fails with:
CCLD libvirtd
/usr/bin/ld: ../src/.libs/libvirt-lxc.so: undefined reference to symbol 'freecon'
/lib/i386-linux-gnu/libselinux.so.1: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[3]: *** [libvirtd] Error 1
Eric Blake [Thu, 12 Sep 2013 19:19:23 +0000 (13:19 -0600)]
docs: mention hostname subtlety
An off-list bug report mentioned some confusion where the public
documentation of libvirt.c:virConnectGetHostname did not match
the private documentation of util/virutil.c:virGetHostname.
Fix naming of permission for detecting storage pools
The VIR_ACCESS_PERM_CONNECT_DETECT_STORAGE_POOLS enum
constant had its string format be 'detect_storage_pool',
note the missing trailing 's'. This prevent the ACL
check from ever succeeding. Fix this and add a simple
test script to validate this problem of matching names.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
LXC: don't try to mount selinux filesystem when user namespace enabled
Right now we mount selinuxfs even user namespace is enabled and
ignore the error. But we shouldn't ignore these errors when user
namespace is not enabled.
This patch skips mounting selinuxfs when user namespace enabled.
Peter Krempa [Thu, 12 Sep 2013 09:37:57 +0000 (11:37 +0200)]
qemu: Fix checking of guest ABI compatibility when reverting snapshots
When reverting a live internal snapshot with a live guest the ABI
compatiblity check was comparing a "migratable" definition with a normal
one. This resulted in the check failing with:
revert requires force: Target device address type none does not match source pci
This patch generates a "migratable" definition from the actual one to
check against the definition from the snapshot to avoid this problem.
Laine Stump [Wed, 28 Aug 2013 16:56:21 +0000 (12:56 -0400)]
netcf driver: use a single netcf handle for all connections
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=983026
The netcf interface driver previously had no state driver associated
with it - as a connection was opened, it would create a new netcf
instance just for that connection, and close it when it was
finished. the problem with this is that each connection to libvirt
used up a netlink socket, and there is a per process maximum of ~1000
netlink sockets.
The solution is to create a state driver to go along with the netcf
driver. The state driver will opens a netcf instance, then all
connections share that same netcf instance, thus only a single
netlink socket will be used no matter how many connections are mde to
libvirtd.
This was rather simple to do - a new virObjectLockable class is
created for the single driverState object, which is created in
netcfStateInitialize and contains the single netcf handle; instead of
creating a new object for each client connection, netcfInterfaceOpen
now just increments the driverState object's reference count and puts
a pointer to it into the connection's privateData. Similarly,
netcfInterfaceClose() just un-refs the driverState object (as does
netcfStateCleanup()), and virNetcfInterfaceDriverStateDispose()
handles closing the netcf instance. Since all the functions already
have locking around them, the static lock functions used by all
functions just needed to be changed to call virObjectLock() and
virObjectUnlock() instead of directly calling the virMutex* functions.
Laine Stump [Tue, 27 Aug 2013 16:26:48 +0000 (12:26 -0400)]
rename "struct interface_driver" to virNetcfDriverState
This better fits the modern naming scheme in libvirt, and anticipates
an upcoming change where a single instance of this state will be
maintained by a separate state driver, and every instance of the netcf
driver will share the same state.
Then any submounts under / should also end up readonly, except
for those setup as basic mounts. eg if the user has /home on a
separate volume, they'd expect /home to be readonly, but we
should not touch the /sys, /proc, etc dirs we setup ourselves.
Users can selectively make sub-mounts read-write again by
simply listing them as new mounts without the <readonly>
flag set
Move array of mounts out of lxcContainerMountBasicFS
Move the array of basic mounts out of the lxcContainerMountBasicFS
function, to a global variable. This is to allow it to be referenced
by other methods wanting to know what the basic mount paths are.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix launching of VMs on when only logind part of systemd is present
Debian systems may run the 'systemd-logind' daemon, which causes the
/sys/fs/cgroup/systemd mount to be setup, but no other cgroup
controllers are created. While the LXC driver considers cgroups to
be mandatory, the QEMU driver is supposed to accept them as optional.
We detect whether they are present by looking in /proc/mounts for
any mounts of type 'cgroups', but this is not sufficient. We need to
skip any named mounts (as seen by a name=XXX string in the mount
options), so that we only detect actual resource controllers.
The polkit access driver used the wrong permission names for checks
on storage pools, volumes and node devices. This led to them always
being denied access.
The 'dettach' permission was also mis-spelt and should have been
'detach'. While permission names are ABI sensitive, the fact that
the code used the wrong object name for checking node device
permissions, means that no one could have used the mis-spelt
'dettach' permission.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 38ab1225 changed the default value of ret from true to false but
forgot to set ret = true when job is NONE. Thus, virsh domjobinfo
returned 1 when there was no job running for a domain but it used to
(and should) return 0 in this case.
Daniel Veillard [Thu, 12 Sep 2013 09:18:32 +0000 (17:18 +0800)]
Repair the search on libvirt.org
First make sure that the input is xhtml as the stylesheets expect
namespaced element, then use a span element instead of a as a
is treated specially, finally adjust the makefile to check for
the new span element and replace it with the PHP code
Michal Novotny [Wed, 11 Sep 2013 12:19:19 +0000 (14:19 +0200)]
api-docs: Fix description of virConnectGetType() API function
This fixes the description of virConnectGetType() API function in
API documentation to match the real functionality that it can be
used to get driver name, and provide a hint on how to learn about
full capabilities.
Signed-off-by: Michal Novotny <minovotn@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake [Wed, 4 Sep 2013 21:12:48 +0000 (15:12 -0600)]
build: require libnl-3 if netcf uses it
Commits 9298bfb and f6c2951 both tried to make it possible to
select the correct libnl (1 vs. 3) according to what netcf
used, when both libraries are installed. This works to avoid
libnl-3 when netcf used libnl-1. But on the converse side, if
only libnl-1 development code is installed, while netcf uses
libnl-3, then configure happily uses libnl-1 anyways, leading
to a test failure:
$ VIR_TEST_DEBUG=1 ./virdrivermoduletest
TEST: virdrivermoduletest
1) Test driver "network" ... OK
2) Test driver "storage" ... OK
3) Test driver "nodedev" ... OK
4) Test driver "secret" ... OK
5) Test driver "nwfilter" ... OK
6) Test driver "interface"
... lt-virdrivermoduletest: route/tc.c:973: rtnl_tc_register: Assertion
`0' failed.
Aborted
It's much nicer to prevent this at configure time, by requiring that
if we know what netcf used, then we want the same libnl version. As
before, this can be bypassed by someone who knows what they are doing
by setting LIBNL_CFLAGS (perhaps useful to the rare person where the
build box has a different version of netcf than the installation box).
* configure.ac (LIBNL): If we can prove netcf used libnl-3, then
don't let configure succeed with libnl-1.
The change to query org.freedesktop.DBus.ListActivatableNames
to detect systemd broke the test suite, since we did not have
stubs to respond to this dbus call.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
LXC: umount the temporary filesystem created by libvirt
The devpts, dev and fuse filesystems are mounted temporarily.
there is no need to export them to container if container shares
the root directory with host.
Ján Tomko [Wed, 11 Sep 2013 08:49:39 +0000 (10:49 +0200)]
Add test for the nodemask double free crash
Commit ef5d51d fixed a crash for numatune with auto placement and
nodeset specified:
<numatune>
<memory mode='preferred' placement='auto' nodeset='0'/>
</numatune>
Fix cgroups when all are mounted on /sys/fs/cgroup
Some users in Ubuntu/Debian seem to have a setup where all the
cgroup controllers are mounted on /sys/fs/cgroup rather than
any /sys/fs/cgroup/<controller> name. In the loop which detects
which controllers are present for a mount point we were modifying
'mnt_dir' field in the 'struct mntent' var, but not always restoring
the original value. This caused detection to break in the all-in-one
mount setup.
Fix that logic bug and add test case coverage for this mount
setup.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Eric Blake [Thu, 5 Sep 2013 20:32:25 +0000 (14:32 -0600)]
build: fix regression in requiring yajl for new enough qemu
Jonathan Lebon reported an issue to me off-list about his build
failing to use qemu because he failed to install yajl-devel. But
I recalled specifically tweaking configure.ac to die in that
situation (commits 350583c, ba9c38b). After a bit more
head-scratching, we found the cause of the regression: commit 654c709 rearranged things so that the qemu version check now
occurs before AC_ARG_WITH has had a chance to set either
$with_qemu or $with_yajl.
Coincidentally, this fix aligns with a documentation patch that
was just posted to the autoconf mailing list :)
http://thread.gmane.org/gmane.comp.sysutils.autoconf.patches/8324
Eric Blake [Thu, 5 Sep 2013 21:24:55 +0000 (15:24 -0600)]
build: avoid obsolete AC_HELP_STRING
Autoconf states that AC_HELP_STRING is obsolete, and that new
programs should use AS_HELP_STRING. We also had instances of
not properly quoting the macro usage, and not relying on autoconf's
word-wrapping abilities to avoid long lines. I validated that this
commit has no impact to the generated configure file.
* configure.ac (AC_ARG_WITH, AC_ARG_ENABLE): Autoconf recommends
the use of AS_HELP_STRING. Also, use proper quoting and wrap long
lines.
* m4/virt-apparmor.m4 (LIBVIRT_CHECK_APPARMOR): Likewise.
* m4/virt-selinux.m4 (LIBVIRT_CHECK_SELINUX): Likewise.
Eric Blake [Mon, 9 Sep 2013 19:07:29 +0000 (13:07 -0600)]
qemu: endjob returns a bool
Osier Yang pointed out that ever since commit 31cb030, the
signature of qemuDomainObjEndJob was changed to return a bool.
While comparison against 0 or > 0 still gives the right results,
it looks fishy; we also had one place that was comparing < 0
which is effectively dead code.
Eric Blake [Sat, 7 Sep 2013 22:18:06 +0000 (16:18 -0600)]
build: use automake subdir-objects
Automake 2.0 will enable subdir-objects by default; in preparation
for that change, automake 1.14 outputs LOADS of warnings:
daemon/Makefile.am:38: warning: source file '../src/remote/remote_protocol.c' is in a subdirectory,
daemon/Makefile.am:38: but option 'subdir-objects' is disabled
automake-1.14: warning: possible forward-incompatibility.
automake-1.14: At least a source file is in a subdirectory, but the 'subdir-objects'
automake-1.14: automake option hasn't been enabled. For now, the corresponding output
automake-1.14: object file(s) will be placed in the top-level directory. However,
automake-1.14: this behaviour will change in future Automake versions: they will
automake-1.14: unconditionally cause object files to be placed in the same subdirectory
automake-1.14: of the corresponding sources.
automake-1.14: You are advised to start using 'subdir-objects' option throughout your
automake-1.14: project, to avoid future incompatibilities.
daemon/Makefile.am:38: warning: source file '../src/remote/lxc_protocol.c' is in a subdirectory,
daemon/Makefile.am:38: but option 'subdir-objects' is disabled
...
As automake 1.9 also supported this option, and the previous patches
fixed up the code base to work with it, it is safe to now turn it on
unconditionally.
* configure.ac (AM_INIT_AUTOMAKE): Enable subdir-objects.
* .gitignore: Ignore .dirstamp directories.
* src/Makefile.am (PDWTAGS, *-protocol-struct): Adjust to
new subdir-object location of .lo files.
Eric Blake [Sat, 7 Sep 2013 21:22:18 +0000 (15:22 -0600)]
tests: check remaining .x files
We have been adding new .x files without keeping the list of
*-structs files up-to-date. This adds the support for the
recent additions.
In the process of testing this, I also noticed that Fedora 19's
use of dwarves-1.10 (providing pdwtags version 1.9) was producing
a single line on stderr but still giving enough useful info on
stdout that we could check structs; the real goal of checking
stderr separately from stdout was to avoid the bug in dwarves-1.9
where stdout was empty (see bug http://bugzilla.redhat.com/772358).
* src/Makefile.am (struct_prefix, PROTOCOL_STRUCTS): Add missing
struct tests.
(PDWTAGS): Work with Fedora 19 pdwtags.
(lxc_monitor_protocol-struct, lock_protocol-struct): New rules.
* src/lxc_monitor_protocol-structs: New file.
* src/lock_protocol-structs): Likewise.
* cfg.mk (generated_files): Enlarge list.
Eric Blake [Sun, 8 Sep 2013 02:49:17 +0000 (20:49 -0600)]
build: use library rather than cross-directory compilation
If we use subdir-objects with automake, any reference to a
cross-directory .c file will result in automake creating
rules that track dependency in the cross directory. But this
presents a problem during 'make distclean' - if the cross
directory is cleaned up first, then the daemon directory will
be left with dangling references to .Po dependency files that
no longer exist.
Meanwhile, referring to the cross-directory .c file means
that we are compiling the file twice - once in src, and once
in daemon. Better is to compile just once in src into a
convenience library, and then use that library from daemon.
The tests directory had a similar situation of a cross-directory
.c file; to solve that, we actually need a convenience library.
* daemon/Makefile.am (DAEMON_SOURCES): Drop .c files...
(libvirtd_LDADD): ...and instead use library.
(libvirtd_conf_la_SOURCES): Declare a new convenience library.
(libvirtd_LDFLAGS): Drop duplicate flag.
* tests/Makefile.am (libvirtdconftest_SOURCES): Drop .c file...
(libvirtdconftest_LDADD): ..and instead use library.
Eric Blake [Fri, 6 Sep 2013 22:54:39 +0000 (16:54 -0600)]
build: avoid $(srcdir) in *_SOURCES
Trying to enable automake's subdir-objects option resulted in
the creation of literal directories such as src/$(srcdir)/remote/.
I traced this to the fact that we had used a literal $(srcdir)
in a location that later fed an automake *_SOURCES variable.
This has also been reported as an automake bug:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13928
but it's better to fix our code than to wait for an automake fix.
Some things to remember that affect VPATH builds, and where an
in-tree build is blissfully unaware of the issues: if a VPATH
build fails to find a file that was used as a prereq of any
other target, then the rule for that file will expand $@ to
prefer the current build dir (bad because a VPATH build on a
fresh checkout will then stick $@ in the current directory
instead of the desired srcdir); conversely, if a VPATH build
finds the file in srcdir but decides it needs to be rebuilt,
then the rule for that file will expand $@ to include the
directory where it was found out-of-date (bad for an explicit
listing of $(srcdir)/$@ because an incremental VPATH build will
then expand srcdir twice). As we want these files to go into
srcdir unconditionally, we have to massage or avoid $@ for any
recipe that involves one of these files.
Therefore, this patch removes all uses of $(srcdir) from any
generated file name that later feeds a *_SOURCES variable, and
then rewrites all the recipes to generate those files to
hard-code their creation into srcdir without the use of $@.
* src/Makefile.am (REMOTE_DRIVER_GENERATED): Drop $(srcdir); VPATH
builds know how to find the files, and automake subdir-objects
fails with it in place.
(LXC_MONITOR_PROTOCOL_GENERATED, (LXC_MONITOR_GENERATED)
(ACCESS_DRIVER_GENERATED, LOCK_PROTOCOL_GENERATED): Likewise.
(*_client_bodies.h): Hard-code rules to write into srcdir, as
VPATH tries to build $@ locally if missing.
(util/virkeymaps.h): Likewise.
(lxc/lxc_monitor_dispatch.h): Likewise.
(access/viraccessapi*): Likewise.
(locking/lock_daemon_dispatch_stubs.h): Likewise.
* daemon/Makeflie.am (DAEMON_GENERATED, remote_dispatch.h):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
fixup DAEMON_GENERATED
Eric Blake [Wed, 28 Aug 2013 20:27:44 +0000 (14:27 -0600)]
qemu: don't leak vm on failure
Failure to attach to a domain during 'virsh qemu-attach' left
the list of domains in an odd state:
$ virsh qemu-attach 4176
error: An error occurred, but the cause is unknown
$ virsh list --all
Id Name State
----------------------------------------------------
2 foo shut off
$ virsh qemu-attach 4176
error: Requested operation is not valid: domain is already active as 'foo'
$ virsh undefine foo
error: Failed to undefine domain foo
error: Requested operation is not valid: cannot undefine transient domain
$ virsh shutdown foo
error: Failed to shutdown domain foo
error: invalid argument: monitor must not be NULL
It all stems from leaving the list of domains unmodified on
the initial failure; we should follow the lead of createXML
which removes vm on failure (the actual initial failure still
needs to be fixed in a later patch, but at least this patch
gets us to the point where we aren't getting stuck with an
unremovable "shut off" transient domain).
While investigating, I also found a leak in qemuDomainCreateXML;
the two functions should behave similarly. Note that there are
still two unusual paths: if dom is not allocated, the user will
see an OOM error even though the vm remains registered (but oom
errors already indicate tricky cleanup); and if the vm starts
and then quits again all before the job ends, it is possible
to return a non-NULL dom even though the dom will no longer be
useful for anything (but this at least lets the user know their
short-lived vm ran).
* src/qemu/qemu_driver.c (qemuDomainCreateXML): Don't leak vm on
failure to obtain job.
(qemuDomainQemuAttach): Match cleanup of qemuDomainCreateXML.
Li Zhang [Fri, 6 Sep 2013 11:15:25 +0000 (12:15 +0100)]
qemu: avoid users specifying CPU features for non-x86 plaftorm.
Currently, only X86 provides users CPU features with CPUID instruction.
If users specify the features for non-x86, it should tell users to
remove them.
This patch is to report one error if features are specified by
users for non-x86 platform.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Eric Blake [Wed, 28 Aug 2013 22:22:46 +0000 (16:22 -0600)]
qemu: don't leave shutdown inhibited on attach failure
While debugging a failure of 'virsh qemu-attach', I noticed that
we were leaking the count of active domains on failure. This
means that a libvirtd session that is supposed to quit after
active domains disappear will hang around forever.
* src/qemu/qemu_process.c (qemuProcessAttach): Undo count of
active domains on failure.
Eric Blake [Wed, 28 Aug 2013 03:45:46 +0000 (21:45 -0600)]
qemu: recognize -machine accel=kvm when parsing native
In Fedora 19, 'qemu-kvm' is a simple wrapper that calls
'qemu-system-x86_64 -machine accel=kvm'. Attempting
to use 'virsh qemu-attach $pid' to a machine started as:
was failing with:
error: XML error: No PCI buses available
because we did not see 'kvm' in the executable name read from
/proc/$pid/cmdline, and tried to assign os.machine as
"accel=kvm" instead of "pc"; this in turn led to refusal to
recognize the pci bus.
Noticed while investigating https://bugzilla.redhat.com/995312
although there are still other issues to fix before that bug
will be completely solved.
I've concluded that the existing parser code for native-to-xml
is a horrendous hodge-podge of ad-hoc approaches; I basically
rewrote the -machine section to be a bit saner.
* src/qemu/qemu_command.c (qemuParseCommandLine): Don't assume
-machine argument is always appropriate for os.machine; set
virtType if accel is present.
Eric Blake [Mon, 26 Aug 2013 22:04:19 +0000 (16:04 -0600)]
qemu: only parse basename when determining emulator properties
'virsh domxml-from-native' and 'virsh qemu-attach' could misbehave
for an emulator installed in (a somewhat unlikely) location
such as /usr/local/qemu-1.6/qemu-system-x86_64 or (an even less
likely) /opt/notxen/qemu-system-x86_64. Limit the strstr seach
to just the basename of the file where we are assuming details
about the binary based on its name.
While testing, I accidentally triggered a core dump during strcmp
when I forgot to set os.type on one of my code paths; this patch
changes such a coding error to raise a nicer internal error instead.
Jim Fehlig [Wed, 4 Sep 2013 22:14:30 +0000 (16:14 -0600)]
libxl: Compile regular expression where it is used
The regular expression used to determine guest capabilities
was compiled in libxlCapsInitHost() but used in libxlCapsInitGuests().
Move compilation to libxlCapsInitGuests() where it is used, and free
the compiled regex after use. Ensure not to free the regex if
compilation fails.
Eric Blake [Wed, 4 Sep 2013 20:59:49 +0000 (14:59 -0600)]
spec: make client-only testing easier
Several recent patches cleaned up 'make rpm' for the situation
when client_only is true; these were done by manual spec file
editing (since it's relatively hard to come by a RHEL 5 s390
box). Make it easier to do in the future via a simpler command
line override.
* libvirt.spec.in (client_only): Allow for override.
Eric Blake [Wed, 4 Sep 2013 16:26:30 +0000 (10:26 -0600)]
build: shut up automake warnings
I'm tired of seeing screenfuls of messages like these when using
automake 1.13 (Fedora 19):
configure.ac:2121: warning: The 'AM_PROG_MKDIR_P' macro is deprecated, and its use is discouraged.
configure.ac:2121: You should use the Autoconf-provided 'AC_PROG_MKDIR_P' macro instead,
configure.ac:2121: and use '$(MKDIR_P)' instead of '$(mkdir_p)'in your Makefile.am files.
daemon/Makefile.am:19: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
seeing as how we MUST use those constructs for the benefit of
automake 1.9 (RHEL 5). Conversely, RHEL 5 automake complained:
aclocal:configure.ac:36: warning: macro `AM_SILENT_RULES' not found in library
Obviously, I tested this patch on both Fedora 19 and RHEL 5.