Eric Blake [Fri, 11 Nov 2011 18:10:18 +0000 (11:10 -0700)]
xenapi: remove unused variable
CC libvirt_driver_xenapi_la-xenapi_driver.lo
xenapi/xenapi_driver.c: In function 'xenapiDomainGetVcpus':
xenapi/xenapi_driver.c:1209:21: error: variable 'cpus' set but not used [-Werror=unused-but-set-variable]
Eric Blake [Tue, 11 Oct 2011 21:10:17 +0000 (15:10 -0600)]
build: fix 'make distcheck'
I got these distcheck failures with sanlock enabled:
ERROR: files left in build directory after distclean:
./tools/virt-sanlock-cleanup
./src/locking/qemu-sanlock.conf
* src/Makefile.am (DISTCLEANFILES) [HAVE_SANLOCK]: Clean built
file.
* tools/Makefile.am (DISTCLEANFILES): Likewise.
(cherry picked from commit c654ba889314c5ab50840584e55e3dfc3eac5860)
plus tweak to DISTCLEANFILES from commit ddf3bd32ce12, although that
full commit is too invasive to backport
since it attempts a sub-run of a VPATH 'make check' where $(srcdir)
is intentionally read-only. I'm not sure which commit introduced
the problem, although I suspect it was around 62dee6f when I
refactored protocol struct checking to be more powerful.
$(@F) is required by POSIX, and although it is not yet portable
to all make implementations, we already require GNU make.
* src/Makefile.am (PDWTAGS): Generate temp file into current
directory, since $(srcdir) is read-only during distcheck.
(cherry picked from commit 2d45ae5a013a0353688d92c3e93b7e53a2939a96)
Every now & then, with parallel builds, we get a failure to
validate hvsupport.html.in. I eventually noticed that this
is because we get 2 instances of the generator running at
once.
We already list hvsupport.html.in in BUILT_SOURCES but this
was not working. It turns out the flaw is that we were
adding deps to the 'all:' target instead of the 'all-am:'
target. BUILT_SOURCES is a dep of 'all', so any custom
targets written in Makefile.am must use 'all-am:' so that
they don't get run until BUILT_SOURCES are completely
generated
Eric Blake [Fri, 6 Jan 2012 21:07:23 +0000 (14:07 -0700)]
tests: avoid test failure on rawhide gnutls
I hit a VERY weird testsuite failure on rawhide, which included
_binary_ output to stderr, followed by a hang waiting for me
to type something! (Here, using ^@ for NUL):
$ ./commandtest
TEST: commandtest
WARNING: gnome-keyring:: couldn't send data: Bad file descriptor
.WARNING: gnome-keyring:: couldn't send data: Bad file descriptor
.WARNING: gnome-keyring:: couldn't send data: Bad file descriptor
WARNING: gnome-keyring:: couldn't send data: Bad file descriptor
.8^@^@^@8^@^@^@^A^@^@^@^Bay^A^@^@^@)PRIVATE-GNOME-KEYRING-PKCS11-PROTOCOL-V-1
I finally traced it to the fact that gnome-keyring, called via
gnutls_global_init which is turn called by virNetTLSInit, opens
an internal fd that it expects to communicate to via a
pthread_atfork handler (never mind that it violates POSIX by
using non-async-signal-safe functions in that handler:
https://bugzilla.redhat.com/show_bug.cgi?id=772320).
Our problem stems from the fact that we pulled the rug out from
under the library's expectations by closing an fd that it had
just opened. While we aren't responsible for fixing the bugs
in that pthread_atfork handler, we can at least avoid the bugs
by not closing the fd in the first place.
Cole Robinson [Wed, 25 Jan 2012 17:07:14 +0000 (12:07 -0500)]
storage: Fix any VolLookupByPath if we have an empty logical pool
On F16 at least, empty volume groups don't have a directory under /dev.
The directory only appears once a logical volume is created.
This tickles some behavior in BackendStablePath which ends with
libvirt sleeping for 5 seconds while waiting for the directory to appear.
This causes all sorts of problems for the virStorageVolLookupByPath API
which virtinst uses, even if trying to resolve a path that is independent
of the logical pool.
In reality we don't even need to do that checking since logical pools
always have a stable target path. Short circuit the polling in that
case.
Peter Krempa [Fri, 2 Mar 2012 14:12:32 +0000 (15:12 +0100)]
daemon: Remove deprecated HAL from init script dependencies
The init script for the daemon requests to start HAL although it has
been deprecated long time ago. This patch removes the dependency.
(cherry picked from commit 2dcca3ec0a28c6562ebfbe7a5eae4729fb5de904)
It is a good practise to set revents to zero before doing any poll().
Moreover, we should check if event we waited for really occurred or
if any of fds we were polling on didn't encountered hangup.
(cherry picked from commit 06b9c5b9231ef4dbd4b5ff69564305cd4f814879)
Michal Privoznik [Tue, 11 Oct 2011 08:40:36 +0000 (10:40 +0200)]
qemu: Check for domain being active on successful job acquire
As this is needed. Although some functions check for domain
being active before obtaining job, we need to check it after,
because obtaining job unlocks domain object, during which
a state of domain can be changed.
(cherry picked from commit 9bc9999b6eb815268798120d7fe8834d822f098d)
For unknown reasons, the shunloadtest will crash on Fedora 16
inside dlopen()
(gdb) bt
#0 0x00000000000050e6 in ?? ()
#1 0x00007ff61a77b9d5 in floor () from /lib64/libm.so.6
#2 0x00007ff61e522963 in _dl_relocate_object () from /lib64/ld-linux-x86-64.so.2
#3 0x00007ff61e5297e6 in dl_open_worker () from /lib64/ld-linux-x86-64.so.2
#4 0x00007ff61e525006 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#5 0x00007ff61e52917a in _dl_open () from /lib64/ld-linux-x86-64.so.2
#6 0x00007ff61e0f6f26 in dlopen_doit () from /lib64/libdl.so.2
#7 0x00007ff61e525006 in _dl_catch_error () from /lib64/ld-linux-x86-64.so.2
#8 0x00007ff61e0f752f in _dlerror_run () from /lib64/libdl.so.2
#9 0x00007ff61e0f6fc1 in dlopen@@GLIBC_2.2.5 () from /lib64/libdl.so.2
#10 0x0000000000400a15 in main (argc=<optimized out>, argv=<optimized out>) at shunloadtest.c:105
Changing from RTLD_NOW to RTLD_LAZY avoids this problem,
but quite possibly does not fix the root cause.
Eric Blake [Mon, 5 Dec 2011 17:22:10 +0000 (10:22 -0700)]
spec: make it easier to autoreconf when building rpm
Over time, Fedora and RHEL RPMs have often backported upstream
patches that touched configure.ac and/or Makefile.am; this
necessitates rerunning the autotools for the patch to be effective.
Making this a one-liner spec tweak will make it easier for future
backports to pull patches without having to find all the places
to touch to properly use the autotools. Meanwhile, there have been
historical instances where an update in the autotools caused FTBFS
situations, so this is not on by default.
* libvirt.spec.in (enable_autotools): New variable, default off.
(BuildRequires): Conditionally add autotools.
(%build): Conditionally use them before configure.
* mingw32-libvirt.spec.in: Likewise.
(cherry picked from commit 9c417636c4045735712c066595ff31539ac88bff)
Conflicts:
mingw32-libvirt.spec.in - caused by change in context
Laine Stump [Fri, 9 Dec 2011 20:55:14 +0000 (15:55 -0500)]
test: replace deprecated "fedora-13" machine with "pc-0.13"
One of the xml tests in the test suite was created using a
now-deprecated qemu machine type ("fedora-13", which was only ever
valid for Fedora builds of qemu). Although strictly speaking it's not
necessary to replace it with an actual supported qemu machine type
(since the xml in question is never actually sent to qemu), this patch
changes it to the actually-supported "pc-0.13" just for general
tidiness. (Also, on some Fedora builds which contain a special patch
to rid the world of "fedora-13", having it mentioned in the test suite
will cause make check to fail.)
(cherry picked from commit 7204a9fd31b06a6ce8fe5b08483c39a75faa0392)
Laine Stump [Tue, 6 Dec 2011 20:13:50 +0000 (15:13 -0500)]
network: don't add iptables rules for externally managed networks
This patch addresses https://bugzilla.redhat.com/show_bug.cgi?id=760442
When a network has any forward type other than route, nat or none, the
network configuration should be done completely external to libvirt -
libvirt only uses these types to allow configuring guests in a manner
that isn't tied to a specific host (all the host-specific information,
in particular interface names, port profile data, and bandwidth
configuration is in the network definition, and the guest
configuration only references it).
Due to a bug in the bridge network driver, libvirt was adding iptables
rules for networks with forward type='bridge' etc. any time libvirtd
was restarted while one of these networks was active.
This patch eliminates that error by only "reloading" iptables rules if
forward type is route, nat, or none.
(cherry picked from commit ae1232b298323dd7bef909426e2ebafa6bca9157)
Commit ecd8725 tried to silence a spurious warning on the initial
libvirt install, and commit ba6cbb1 tried to fix up the logic to the
correct Fedora version, but the warning was still present due to a
logic bug: since %{fedora} and %{rhel} are never simulatanously
set, then 0%{rhel} <= 6 made the %if always true. Checking for
minimum versions (via >=) is okay, but checking for maximum versions
(via <=) requires a prerequisite test that the platform being tested
is non-zero.
Also fix a bogus setting of with_libxl (although we previously
hard-code with_libxl to 0 for rhel earlier in the file, so this
was not as severe a bug).
Eric Blake [Tue, 6 Dec 2011 23:16:34 +0000 (16:16 -0700)]
spec: don't use chkconfig --list
https://bugzilla.redhat.com/show_bug.cgi?id=694403 reports that
the specfile is incorrectly checking for a running libvirt-guests
service. For example,
will fail to find 5:on, even though it is active. But chkconfig
already has a mode where you can silently use the exit status to
check for an active service.
* libvirt.spec.in (%post): Use simpler chkconfig options, to avoid
issues with localization.
(cherry picked from commit fea83dde7b7e0725e4353a2cc5b6e5c7c61cb294)
Eric Blake [Mon, 5 Dec 2011 17:32:41 +0000 (10:32 -0700)]
spec: add dmidecode as prereq
https://bugzilla.redhat.com/show_bug.cgi?id=754909 complains that
because libvirt didn't require dmidecode, that the logs are noisy
and virConnectGetSysinfo needlessly fails. Even 'virt-what' requires
dmidecode, so it's not that onerous of a dependency. We may be
able to drop this in the future when we move to parsing sysfs data,
but for now, listing the dependency will help matters.
* libvirt.spec.in (Requires): Sort Requires before BuildRequires.
Add dmidecode.
(cherry picked from commit e7dfa468f9de796430a4cc9594eb256109a517bf)
extra requires for with_systemd removed, since the patch that adds
that hasn't been backported
Eric Blake [Wed, 30 Nov 2011 22:34:42 +0000 (15:34 -0700)]
spec: mark directories in /var/run as ghosts
We have several directories that are created on the fly, and which
only contain state relevant to a running libvirtd process (all
located in /var/run). Since the directories are created as needed,
and make no sense without a running libvirtd, we want them deleted
if libvirt is uninstalled. And in F15 and newer, /var/run is on
tmpfs (forcing us to recreate on the fly); which means that someone
trying to verify a complete rpm will fail if the directory does not
currently exist because libvirtd has not been started since boot.
The solution, then, is to mark the directories as %ghost, so that
rpm knows that we own them and will clean it up if libvirt is
uninstalled, but will no longer create the directory for us at
install, nor complain at verify time if the directory does not exist.
See https://bugzilla.redhat.com/show_bug.cgi?id=656611.
* libvirt.spec.in (%files): Add %ghost to temporary directories
that we don't install, but want cleaned up on libvirt removal.
(cherry picked from commit 764574f7c73d72e42f69f225b6bbe61fae5ab28b)
The virTimestamp and virTimeMs functions in src/util/util.h
duplicate functionality from virtime.h, in a non-async signal
safe manner. Remove them, and convert all code over to the new
APIs.
* src/util/util.c, src/util/util.h: Delete virTimeMs and virTimestamp
* src/lxc/lxc_driver.c, src/qemu/qemu_domain.c,
src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
src/qemu/qemu_process.c, src/util/event_poll.c: Convert to use
virtime APIs
(cherry picked from commit a8bb75a3e65f0ae866f3b3fd60c57b2aa2050017)
Conflicts:
src/lxc/lxc_driver.c
src/qemu/qemu_domain.c
src/qemu/qemu_driver.c
* the patches contained context with extra include files not
yet introduced on the branch.
src/util/event_poll.c
* the branch had context with a call to EVENT_DEBUG that
was no longer existing in the original patch.
The logging APIs need to be able to generate formatted timestamps
using only async signal safe functions. This rules out using
gmtime/localtime/malloc/gettimeday(!) and much more.
Introduce a new internal API which is async signal safe.
virTimeMillisNowRaw replacement for gettimeofday. Uses clock_gettime
where available, otherwise falls back to the unsafe
gettimeofday
virTimeFieldsNowRaw replacements for gmtime(), convert a timestamp
virTimeFieldsThenRaw into a broken out set of fields. No localtime()
replacement is provided, because converting to
local time is not practical with only async signal
safe APIs.
virTimeStringNowRaw replacements for strftime() which print a timestamp
virTimeStringThenRaw into a string, using a pre-determined format, with
a fixed size buffer (VIR_TIME_STRING_BUFLEN)
For each of these there is also a version without the Raw postfix
which raises a full libvirt error. These versions are not async
signal safe
* src/Makefile.am, src/util/virtime.c, src/util/virtime.h: New files
* src/libvirt_private.syms: New APis
* configure.ac: Check for clock_gettime in -lrt
* tests/virtimetest.c, tests/Makefile.am: Test new APIs
(cherry picked from commit 3ec128989606278635a7c5dfbeee959692d12e15)
Laine Stump [Thu, 29 Sep 2011 17:00:32 +0000 (13:00 -0400)]
qemu: make PCI multifunction support more manual
When support for was added for PCI multifunction cards (in commit
9f8baf, first included in libvirt 0.9.3), it was done by always
turning on the multifunction bit for all PCI devices. Since that time
it has been realized that this is not an ideal solution, and that the
multifunction bit must be selectively turned on. For example, see
This patch modifies multifunction support so that the multifunction=on
option is only added to the qemu commandline for a device if its PCI
<address> definition has the attribute "multifunction='on'", e.g.:
In practice, the multifunction bit should only be turned on if
function='0' AND other functions will be used in the same slot - it
usually isn't needed for functions 1-7 (although there are apparently
some exceptions, e.g. the Intel X53 according to the QEMU source
code), and should never be set if only function 0 will be used in the
slot. The test cases have been changed accordingly to illustrate.
With this patch in place, if a user attempts to assign multiple
functions in a slot without setting the multifunction bit for function
0, libvirt will issue an error when the domain is defined, and the
define operation will fail. In the future, we may decide to detect
this situation and automatically add multifunction=on to avoid the
error; even then it will still be useful to have a manual method of
turning on multifunction since, as stated above, there are some
devices that excpect it to be turned on for all functions in a slot.
A side effect of this patch is that attempts to use the same PCI
address for two different devices will now log an error (previously
this would cause the domain define operation to fail, but there would
be no log message generated). Because the function doing this log was
almost completely rewritten, I didn't think it worthwhile to make a
separate patch for that fix (the entire patch would immediately be
obsoleted).
(cherry picked from commit c329db7180d77c8077b9f9cd167a71d7f347227a)
Laine Stump [Wed, 28 Sep 2011 18:19:59 +0000 (14:19 -0400)]
conf: remove unused VIR_ENUM_DECL
While adding a new enum, I noticed a VIR_ENUM_DECL for a type that
doesn't exist. There is also of course no matching VIR_ENUM_IMPL for
it.
(cherry picked from commit be7bc4d5ccb502c2da85d3b3db804fd53b70449e)
Eric Blake [Mon, 26 Sep 2011 20:28:47 +0000 (14:28 -0600)]
spec: F15 still uses cgconfig, RHEL lacks hyperv
Commit ecd8725c dropped attempts to probe the cgconfig service on
new enough Fedora where systemd took over that aspect of the system,
but mistakenly used F14 instead of F15 as the cutoff point.
* libvirt.spec.in (with_cgconfig): Check cgconfig service in F15.
(%{?rhel}): Provide default for with_hyperv.
(cherry picked from commit ba6cbb182b49ca1f94a40285f40f342f84c0bd2a)
Eric Blake [Thu, 22 Sep 2011 06:02:03 +0000 (14:02 +0800)]
snapshot: remove snapshot metadata on transient exit
Commit 282fe1f0 documented that transient domains will auto-delete
any snapshot metadata when the last reference to the domain is
removed, and that management apps are in charge of grabbing any
snapshot metadata prior to that point. However, this was not
actually implemented for qemu until now.
Eric Blake [Wed, 21 Sep 2011 19:08:51 +0000 (13:08 -0600)]
snapshot: prepare to remove transient snapshot metadata
This patch is mostly code motion - moving some functions out
of qemu_driver and into qemu_domain so they can be reused by
multiple qemu_* files (since qemu_driver.h must not grow).
It also adds a new helper function, qemuDomainRemoveInactive,
which will be used in the next patch.
Eric Blake [Wed, 21 Sep 2011 19:08:50 +0000 (13:08 -0600)]
snapshot: fix logic bug in qemu undefine
Commit 19f8c98 introduced VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA,
with the intent that omitting the flag makes undefine fail, and
including the flag deletes metadata. But it used the wrong logic.
Also, hoist the transient domain sooner, so that we don't
accidentally remove metadata of a transient domain.
* src/qemu/qemu_driver.c (qemuDomainUndefineFlags): Check correct
flag value.
Eric Blake [Wed, 21 Sep 2011 20:22:57 +0000 (14:22 -0600)]
sanlock: fix memory leak
Detected by Coverity. The only way to get to error_unlink is if
path was successfully assigned, so the if was useless. Meanwhile,
there was a return statement that did not free path.
* src/locking/lock_driver_sanlock.c
(virLockManagerSanlockSetupLockspace): Fix mem-leak, and drop
useless if.
snapshot-create-as dom name --diskspec spec --diskspec spec
failed to parse the second spec, because the first spec had marked
that option as no longer requiring an argument.
In commit 85d2810, I fixed it by making argv options no longer mark
the option as seen. But this in turn breaks mandatory argv options,
which now complain that the argv option is missing.
This patch reverts that part of 85d2810, and instead replaces it with
fixes to no longer clear opts_need_arg of an argv argument.
* tools/virsh.c (vshCmddefGetOption, vshCmddefGetData)
(vshCommandParse): Fix option parsing for required argv option.
(vshCmddefOptParse): Check that argv option is last.
* tests/virsh-optparse: Enhance test.
Osier Yang [Wed, 21 Sep 2011 06:29:49 +0000 (14:29 +0800)]
virsh: More friendly err if no pool is specified for looking up a vol
There are 3 ways to lookup a volume, only virStorageVolLookupByName
needs pool object. So if no --pool is specified, it will tries to
get the volume via virStorageVolLookupByPath/virStorageVolLookupByKey.
But if all 3 ways fails, and no --pool is specified, a friendly
error might help the user get right way quickly.
Osier Yang [Wed, 21 Sep 2011 06:02:30 +0000 (14:02 +0800)]
storage: Wait udev events are handled before removing lvm vol
Related #BZ: https://bugzilla.redhat.com/show_bug.cgi?id=702260.
There are two problems described in the BZ:
1) "Can't remove open logical volume".
2) "Unable to deactivate logical volume "foo""
This patch just intends to fix 2), as 1) is expected if the vol
is still used by something, and you never known if "lvchange -an"
will fail or not either (sometime, it will succeed, sometimes not).
We'd better not look for trouble, :-)
For 2), that's caused by race between lvremove and udev event handling,
the only workable way now is to wait the events handling are finished,
though it might introduce latencies, as "udevadmin settle" exits
after *all* events are handled, it's the only way we can fix
the racing in libvirt layer.
See https://bugzilla.redhat.com/show_bug.cgi?id=570359 for more
details.
Alex Jia [Wed, 21 Sep 2011 19:02:44 +0000 (03:02 +0800)]
qemu: avoid dereferencing a NULL pointer
* src/qemu/qemu_process.c: Taking if (qemuDomainObjEndJob(driver, obj) == 0)
true branch then 'obj' is NULL, virDomainObjIsActive(obj) and
virDomainObjUnref(obj) will dereference NULL pointer.
Once virDomainReboot is called for a domain, guest OS initiated shutdown
would always result in reboot instead of shutdown. Only
virDomainShutdown would actually shutd such domain down. That's because
we forgot to reset fakeReboot flag once we asked the domain to reboot.
The commit that prevents disk corruption on domain shutdown
(96fc4784177ecb70357518fa863442455e45ad0e) causes regression with QEMU
0.14.* and 0.15.* because of a regression bug in QEMU that was fixed
only recently in QEMU git. The affected versions of QEMU do not quit on
SIGTERM if started with -no-shutdown, which we use to implement fake
reboot. Since -no-shutdown tells QEMU not to quit automatically on guest
shutdown, domains started using the affected QEMU cannot be shutdown
properly and stay in a paused state.
This patch disables fake reboot feature on such QEMU by not using
-no-shutdown, which makes shutdown work as expected. However,
virDomainReboot will not work in this case and it will report "Requested
operation is not valid: Reboot is not supported with this QEMU binary".
Eric Blake [Tue, 20 Sep 2011 18:11:31 +0000 (12:11 -0600)]
xen: use typical allocations
The next patch will add a syntax check that flags this usage in xen
as awkward - while it was valid memory management, it was very hard
to maintain. Swapping to a more traditional allocation may be a bit
slower, but easier to understand.
* src/xen/xend_internal.c (xenDaemonListDomainsOld): Use two-level
allocation, rather than abusing allocation function.
(xenDaemonLookupByUUID): Update caller.
Eric Blake [Tue, 20 Sep 2011 22:53:58 +0000 (16:53 -0600)]
build: silence warning on 32-bit build
gcc warns when building libvirt 0.9.5 on a 32-bit machine:
qemu/qemu_migration.c: In function 'qemuMigrationToFile':
qemu/qemu_migration.c:2727:38: error: large integer implicitly truncated to unsigned type [-Woverflow]
* src/qemu/qemu_domain.h (QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX): Cap
to long when building for 32-bit platform.
Daniel Veillard [Tue, 20 Sep 2011 03:51:50 +0000 (11:51 +0800)]
Fix crash on events due to allocation errors
remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
were using const string directly in rpc structure, before calling
remoteDispatchDomainEventSend(). But that routine now frees up all
the pointed allocated memory from the rpc structure and we end up
with a double free.
This now strdup() all the strings passed and provide mem_error goto
labels to be used when an allocation error occurs.
Note that the cleanup isn't completely finished because all relaying
function also call make_nonnull_domain() which also allocate a string
and never handle the error case. This patches doesn't try to address
this as this is only error correctness a priori and touches far more
functions in this module:
* daemon/remote.c: fix string allocations and memory error handling
for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
Update to require sanlock 1.8 for license compliance
Inexplicably the sanlock code all got placed under the GPLv2-only,
so libvirt's use of sanlock introduces a license incompatibility.
The sanlock developers have now rearranged the code such that there
is a 'sanlock_client.so' which is LGPLv2+ while their daemon remains
GPLv2-only. To use the new client library we need to call the new
sanlock_init and sanlock_align APIs instead of sanlock_direct_init
and sanlock_direct_align. These APIs calls are now routed via the
sanlock daemon, instead of doing direct I/O calls to disk.
For all this we require sanlock >= 1.8
* configure.ac: Check for sanlock_client.so instead of sanlock.so
and fix various comments
* libvirt.spec.in: Mandate sanlock >= 1.8
* src/Makefile.am: Link to -lsanlock_client
* src/locking/lock_driver_sanlock.c: Use sanlock_init and
sanlock_align
Osier Yang [Thu, 1 Sep 2011 13:33:29 +0000 (21:33 +0800)]
conf: Assign newDef of active domain as persistent conf if it is NULL
Libvirt loads the domain conf from status XML if it's running when
starting up. The problem is there is no record of the original conf.
(dom->newDef is NULL here).
So libvirt won't be able to restore the domain conf to original one
when destroying/shutdown. E.g.
1) attach a device without "--persistent"
2) restart libvirtd
3) destroy domain
4) start domain
One will see the the disk still exists.
This patch is to fix the peoblem by assigning persistent domain conf
to dom->newDef if it's NULL and the domain is running.
Eric Blake [Mon, 19 Sep 2011 20:49:08 +0000 (14:49 -0600)]
virsh: tweak previous domblkstat patch
Translators are likely to botch trailing spacing; by doing the
formatting outside of the translation, we can generally get
better alignment. Also, for consistency, use 'bytes read' to
match 'bytes written'.
Peter Krempa [Mon, 19 Sep 2011 12:57:07 +0000 (14:57 +0200)]
virsh: doc: Fix supported driver types for attach-disk command
Virsh man page lists driver types to be used with attach-device
command, but does not specify that those are usable only with the XEN
Hypervisor.
This patch adds statement, that those options specified are applicable
only on the Xen hypervisor and adds option usable with qemu emulator.
This patch also changes type of error returned by QEMU driver if the
user specifies incompatible driver type from VIR_ERR_INTERNAL_ERROR to
VIR_ERR_CONFIG_UNSUPPORTED.
Peter Krempa [Mon, 19 Sep 2011 12:23:12 +0000 (14:23 +0200)]
virsh: Add more human-friendly output of domblkstat command
Users of virsh complain that output of the domblkstat command
is not intuitive enough. This patch adds explanation of fields
returned by this command to the help section for domblkstat and
the man page of virsh. Also a switch --human is added for
domblkstat that prints the fields with more descriptive
texts.
This patch also changes sequence of the output fields and their
names back to the order and spelling established by previous
versions of virsh to maintain compatibility with scripts.
virsh # domblkstat 1 vda --human
Device: vda
number of read operations: 3726
number of read bytes: 82815488
number of write operations: 478
number of bytes written: 4965376
* src/vmx/vmx.c: fix memory leak, 'def' has a initial value 'NULL', so
'goto cleanup' is perfected instead of adding a virConfFree before
'return NULL'.
Eric Blake [Fri, 16 Sep 2011 22:40:25 +0000 (16:40 -0600)]
spec: silence warnings when installing in F16
https://bugzilla.redhat.com/show_bug.cgi?id=738725 documents that
'yum install libvirt' in Fedora 16 is rather noisy. This fixes
the problems.
* libvirt.spec.in (%post client): Silence chkconfig warning about
SysV services.
(%post) [with_cgconfig]: Drop for Fedora 15 and newer, where
systemd does this automatically.
Eric Blake [Sat, 17 Sep 2011 12:16:08 +0000 (06:16 -0600)]
snapshot: allow disk snapshots of qcow2 disks
For all types of disks other than qcow2, we were requesting that
SELinux labeling visit the new file as if it were qcow2, which
means labeling would try to find the backing files of an empty file.
And for a pre-existing qcow2 disk, we were passing NULL, which meant
that labelling tried to probe the file type (and if probing is
disabled, per the default qemu.conf, this made snapshots fail).
What we really want is to make SELinux labeling visit the new
file as raw; it will later be converted to qcow2 if qemu successfully
made the snapshot.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Force SELinux labeling
to avoid probe of new file.
Eric Blake [Sat, 17 Sep 2011 03:46:21 +0000 (21:46 -0600)]
snapshot: affect persistent xml after disk snapshot
For external snapshots to be useful on persistent domains, we must
alter the persistent definition alongside the running definition.
Thanks to the possibility of disk hotplug as well as of edits that
only affect the persistent xml, we can't assume that vm->def and
vm->newDef have the same disk at the same index, so we can only
update the persistent copy if the device destination matches up.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive)
(qemuDomainSnapshotCreateSingleDiskActive): Also affect newDef, if
present.
Eric Blake [Fri, 16 Sep 2011 15:12:07 +0000 (09:12 -0600)]
build: work around lack of MacOS fdatasync
Mingw lacks fsync, but gnulib provides that. Meanwhile, gnulib does
not (yet) provide fdatasync, so this is a quick hack to fake that
function on MacOS X; we can revert this configure change once gnulib
gives us a real module.
We have been implicitly relying on gnulib's largefile module being
pulled in by other modules, but it's better to make that explicit.
* bootstrap.conf (gnulib_modules): Add fsync. Make largefile use
explicit.
* configure.ac (AC_CHECK_FUNCS_ONCE): Check for fdatasync, and
fake it with fsync when not present.
When libvirt calls virInitialize it creates a thread local
for the virErrorPtr storage, and registers a callback to
cleanup memory when a thread exits. When libvirt is dlclose()d
or otherwise made non-resident, the callback function is
removed from memory, but the thread local may still exist
and if a thread later exists, it will invoke the callback
and SEGV. There may also be other thread locals with callbacks
pointing to libvirt code, so it is in general never safe to
unload libvirt.so from memory once initialized.
To allow dlclose() to succeed, but keep libvirt.so resident
in memory, link with '-z nodelete'. This issue was first
found with the libvirt CIM provider, but can potentially
hit many of the dynamic language bindings which all ultimately
involve dlopen() in some way, either on libvirt.so itself,
or on the glue code for the binding which in turns links
to libvirt
* configure.ac, src/Makefile.am: Ensure libvirt.so is linked
with -z nodelete
* cfg.mk, .gitignore, tests/Makefile.am, tests/shunloadhelper.c,
tests/shunloadtest.c: A test case to unload libvirt while
a thread is still running.
Qemu sends STOP event as part of the shutdown process. Detect such STOP
event and consider shutdown to be reason of emitting such event. That's
the best we can do until qemu provides us the reason directly in STOP
event. This allows us to report shutdown reason for paused state so that
apps can detect domains that failed to finish the shutdown process
(e.g., because qemu is buggy and doesn't exit on SIGTERM or it is
blocked in flushing disk buffers).
Ever since we introduced fake reboot, we call qemuProcessKill as a
reaction to SHUTDOWN event. Unfortunately, qemu doesn't guarantee it
flushed all internal buffers before sending SHUTDOWN, in which case
killing the process forcibly may result in (virtual) disk corruption.
By sending just SIGTERM without SIGKILL we give qemu time to to flush
all buffers and exit. Once qemu exits, we will see an EOF on monitor
connection and tear down the domain. In case qemu ignores SIGTERM or
just hangs there, the process stays running but that's not any different
from a possible hang anytime during the shutdown process so I think it's
just fine.
Also qemu (since 0.14 until it's fixed) has a bug in SIGTERM processing
which causes it not to exit but instead send new SHUTDOWN event and keep
waiting. I think the best we can do is to ignore duplicate SHUTDOWN
events to avoid a SHUTDOWN-SIGTERM loop and leave the domain in paused
state.
When a domain is rebooted using libvirt API, we use fake reboot
consisting of shutting down and resetting the domain. Thus we see a
SHUTDOWN event and set gotShutdown flag. But we never reset it back and
if the domain crashes after it was rebooted this way, we consider it was
a normal shutdown and not a crash.
Commit 4454a9efc728b91e791b1f14c26ea23a19d57f48 changed shutoff reason
from VIR_DOMAIN_SHUTOFF_CRASHED to VIR_DOMAIN_SHUTOFF_FAILED in case we
see an unexpected EOF on monitor connection. But FAILED reason is
dedicated for domains that fail to start. CRASHED reason is the right
one to use in this situation.
Eric Blake [Wed, 14 Sep 2011 18:41:17 +0000 (12:41 -0600)]
rpc: convert unknown procedures to VIR_ERR_NO_SUPPORT
Libvirt special-cases a specific VIR_ERR_RPC from the remote driver
back into VIR_ERR_NO_SUPPORT on the client, so that clients can
handle missing rpc functions the same whether the hypervisor driver
is local or remote. However, commit c1b22644 introduced a regression:
VIR_FROM_THIS changed from VIR_FROM_REMOTE to VIR_FROM_RPC, so the
special casing no longer works if the server uses the newer error
domain.
* src/rpc/virnetclientprogram.c
(virNetClientProgramDispatchError): Also cater to 0.9.3 and newer.
Alex Jia [Fri, 16 Sep 2011 05:54:47 +0000 (13:54 +0800)]
conf: avoid memory leak on virDomainDefParseXML
* conf/domain_conf.c: allocate memory to def->redirdevs in
virDomainDefParseXML such as VIR_ALLOC_N(def->redirdevs, n),
however, virDomainDefFree(def) hasn't released these memory.
* Detected in valgrind run:
==19820== 209 (16 direct, 193 indirect) bytes in 1 blocks are definitely lost in loss record 25 of 26
==19820== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==19820== by 0x4A13AF: virAllocN (memory.c:129)
==19820== by 0x4D4A0E: virDomainDefParseXML (domain_conf.c:7258)
==19820== by 0x4D4C93: virDomainDefParseNode (domain_conf.c:7512)
==19820== by 0x4D562F: virDomainDefParse (domain_conf.c:7465)
==19820== by 0x415863: testCompareXMLToXMLFiles (qemuxml2xmltest.c:35)
==19820== by 0x415982: testCompareXMLToXMLHelper (qemuxml2xmltest.c:80)
==19820== by 0x416D31: virtTestRun (testutils.c:140)
==19820== by 0x415604: mymain (qemuxml2xmltest.c:192)
==19820== by 0x416437: virtTestMain (testutils.c:689)
==19820== by 0x3CA7A1ECDC: (below main) (in /lib64/libc-2.12.so)
==19820==
==19820== LEAK SUMMARY:
==19820== definitely lost: 16 bytes in 1 blocks
==19820== indirectly lost: 193 bytes in 5 blocks
==19820== possibly lost: 0 bytes in 0 blocks
==19820== still reachable: 1,054 bytes in 21 blocks
* How to reproduce?
% valgrind -v --leak-check=full ./tests/qemuxml2xmltest
Peter Krempa [Fri, 16 Sep 2011 12:14:16 +0000 (14:14 +0200)]
build: storage: Macro 'MKFS' is undefined on some platforms.
Mac OS X 10.6. Snow Leopard and probably other do not provide a mkfs
command to create filesystems. Macro MKFS then remained undefined and
did not provide any substitute, so that build failed on a missing
argument.
Peter Krempa [Fri, 16 Sep 2011 12:14:15 +0000 (14:14 +0200)]
build: storage: Conditionaly compiled structure caused build fail on OSX
Struct virStoragePoolProbeResult was compiled in conditionaly, but
virStorageBackendFileSystemProbe used it unconditionaly. This patch
exempts the struct from conditional include.
Eric Blake [Wed, 14 Sep 2011 21:20:08 +0000 (15:20 -0600)]
snapshot: tweak snapshot-create-as diskspec docs
With this patch, it is hopefully a bit more obvious that for
snapshot-create-as, a literal '--diskspec' is mandatory if name
or description was omitted, but optional if all earlier options
were provided.
These all denote two diskspecs and a description:
virsh snapshot-create-as dom name desc vda vdb
virsh snapshot-create-as dom name desc --diskspec vda --diskspec vdb
virsh snapshot-create-as dom name desc --diskspec vda vdb
virsh snapshot-create-as dom name desc vda --diskspec vdb
virsh snapshot-create-as dom --diskspec vda --diskspec vdb name desc
This gives two diskspecs but no description:
virsh snapshot-create-as dom name --diskspec vda --diskspec vdb
And this treats 'vda' as the description, with only one diskspec:
virsh snapshot-create-as dom name vda vdb
The help output now shows:
snapshot-create-as <domain> [<name>] [<description>] [--print-xml] [--no-metadata] [--halt] [--disk-only] [[--diskspec] <string>]...
I also checked the help output for echo and send-key, which are two
other variants of argv commands.
* tools/virsh.pod (snapshot-create-as): Document when a literal
--diskspec must preceed a diskspec argument.
* tools/virsh.c (vshCmddefHelp): Update help output for argv when
naming the option is useful.
(vshCmddefGetData): Fix logic on when argv was seen.
* tests/virsh-optparse: Add tests to avoid regressions.
Jim Fehlig [Fri, 26 Aug 2011 18:10:27 +0000 (12:10 -0600)]
Use max bandwidth from qemuDomainObjPrivate struct when migrating
Adjust qemuMigrationRun() to use migMaxBandwidth in qemuDomainObjPrivate
structure when setting qemu migration speed. Caller-specified 'resource'
parameter overrides migMaxBandwidth.
There's no need to throttle migration when targeting a file, so set migration
speed to unlimited prior to migration, and restore to libvirt default value
after migration.
Default units is MB for migrate_set_speed monitor command, so
(INT64_MAX / (1024 * 1024)) is used for unlimited migration speed.
Jim Fehlig [Fri, 26 Aug 2011 18:10:22 +0000 (12:10 -0600)]
Store max migration bandwidth in qemuDomainObjPrivate struct
The maximum bandwidth that can be consumed when migrating a domain
is better classified as an operational vs configuration parameter of
the dommain. As such, store this parameter in qemuDomainObjPrivate
structure.
Eric Blake [Tue, 13 Sep 2011 20:21:10 +0000 (14:21 -0600)]
snapshot: ABI stability must include memory sizing
Commit 973fcd8f introduced the ability for qemu to reject snapshot
reversion on an ABI incompatibility; but the very example that was
first proposed on-list[1] as a demonstration of an ABI incompatibility,
namely that of changing the max memory allocation, was not being
checked for, resulting in a cryptic failure when running with larger
max mem than what the snapshot was created with:
error: operation failed: Error -22 while loading VM state
This commit merely protects the three variables within mem that are
referenced by qemu_command.c, rather than all 7 (the other 4 variables
affect cgroup handling, but as far as I can tell, have no visible effect
to the qemu guest). This also affects migration and save file handling,
which are other places where we perform ABI compatibility checks.
Peter Krempa [Wed, 14 Sep 2011 14:17:57 +0000 (16:17 +0200)]
xml: Clean up rest of virtual XML document names for XML strings
Commit 498d783 cleans up some of virtual file names for parsing strings
in memory. This patch cleans up (hopefuly) the rest forgotten by the
first patch.
This patch also changes all of the previously modified "filenames" to
valid URI's replacing spaces for underscores.
Changes to v1:
- Replace all spaces for underscores, so that the strings form valid
URI's
- Replace spaces in places changed by commit 498d783