Commit Graph

924 Commits

Author SHA1 Message Date
Daniel P. Berrange
517368a377 Remove uid param from directory lookup APIs
Remove the uid param from virGetUserConfigDirectory,
virGetUserCacheDirectory, virGetUserRuntimeDirectory,
and virGetUserDirectory

These functions were universally called with the
results of getuid() or geteuid(). To make it practical
to port to Win32, remove the uid parameter and hardcode
geteuid()

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-05-28 10:55:06 +01:00
Daniel P. Berrange
2cb0899eec 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
2012-05-21 18:50:47 +01:00
Hu Tao
fe0aac0503 Adds support to param 'vcpu_time' in qemu_driver.
This involves setting the cpuacct cgroup to a per-vcpu granularity,
as well as summing the each vcpu accounting into a common array.
Now that we are reading more than one cgroup file, we double-check
that cpus weren't hot-plugged between reads to invalidate our
summing.

Signed-off-by: Eric Blake <eblake@redhat.com>
2012-05-18 08:53:49 -06:00
Daniel Walsh
73580c60d1 Pass the virt driver name into security drivers
To allow the security drivers to apply different configuration
information per hypervisor, pass the virtualization driver name
into the security manager constructor.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-05-16 10:05:46 +01:00
Jiri Denemark
63b4243624 qemu: Add support for -no-user-config
Thanks to this new option we are now able to use modern CPU models (such
as Westmere) defined in external configuration file.

The qemu-1.1{,-device} data files for qemuhelptest are filled in with
qemu-1.1-rc2 output for now. I will update those files with real
qemu-1.1 output once it is released.
2012-05-15 20:29:12 +02:00
William Jon McCann
32a9aac2e0 Use XDG Base Directories instead of storing in home directory
As defined in:
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

This offers a number of advantages:
 * Allows sharing a home directory between different machines, or
sessions (eg. using NFS)
 * Cleanly separates cache, runtime (eg. sockets), or app data from
user settings
 * Supports performing smart or selective migration of settings
between different OS versions
 * Supports reseting settings without breaking things
 * Makes it possible to clear cache data to make room when the disk
is filling up
 * Allows us to write a robust and efficient backup solution
 * Allows an admin flexibility to change where data and settings are stored
 * Dramatically reduces the complexity and incoherence of the
system for administrators
2012-05-14 15:15:58 +01:00
Peter Krempa
3163682b58 qemu: Re-detect virtual cpu threads after cpu hot (un)plug.
After a cpu hotplug the qemu driver did not refresh information about
virtual processors used by qemu and their corresponding threads. This
patch forces a re-detection as is done on start of QEMU.

This ensures that correct information is reported by the
virDomainGetVcpus API and "virsh vcpuinfo".

A failure to obtain the thread<->vcpu mapping is treated non-fatal and
the mapping is not updated in a case of failure as not all versions of
QEMU report this in the info cpus command.
2012-05-11 16:40:05 +02:00
Peter Krempa
e99ad93d02 qemu: Refactor qemuDomainSetVcpusFlags
This patch changes a switch statement into ifs when handling live vs.
configuration modifications getting rid of redundant code in case when
both live and persistent configuration gets changed.
2012-05-11 16:40:05 +02:00
Eric Blake
8be304ecb9 snapshot: allow block devices past cgroup
It turns out that when cgroups are enabled, the use of a block device
for a snapshot target was failing with EPERM due to libvirt failing
to add the block device to the cgroup whitelist.  See also
https://bugzilla.redhat.com/show_bug.cgi?id=810200

* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive): Account for cgroup.
(qemuDomainSnapshotCreateDiskActive): Update caller.
2012-05-08 15:59:58 -06:00
Jiri Denemark
409b5f5495 qemu: Emit compatible XML when migrating a domain
When we added the default USB controller into domain XML, we efficiently
broke migration to older versions of libvirt that didn't support USB
controllers at all (0.9.4 and earlier) even for domains that don't use
anything that the older libvirt can't provide. We still want to present
the default USB controller in any XML seen by a user/app but we can
safely remove it from the domain XML used during migration. If we are
migrating to a new enough libvirt, it will add the controller XML back,
while older libvirt won't be confused with it although it will still
tell qemu to create the controller.

Similar approach can be used in the future whenever we find out we
always enabled some kind of device without properly advertising it in
domain XML.
2012-05-07 14:26:02 +02:00
Jiri Denemark
cd603008b1 qemu: Don't use virDomainDefFormat* directly
Always use appropriate qemuDomain{,Def}Format wrapper since it may do
some additional magic based on the flags.
2012-05-05 00:37:30 +02:00
Eric Blake
13f9a19326 qemu: reject blockiotune if qemu too old
Commit 4c82f09e added a capability check for qemu per-device io
throttling, but only applied it to domain startup.  As mentioned
in the previous commit (98cec05), the user can still get an 'internal
error' message during a hotplug attempt, when the monitor command
doesn't exist.  It is confusing to allow tuning on inactive domains
only to then be rejected when starting the domain.

* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Reject
offline tuning if online can't match it.
2012-05-04 16:13:56 -06:00
Eric Blake
98cec05288 qemu: don't modify domain on failed blockiotune
If you have a qemu build that lacks the blockio tune monitor command,
then this command:

$ virsh blkdeviotune rhel6u2 hda --total_bytes_sec 1000
error: Unable to change block I/O throttle
error: internal error Unexpected error

fails as expected (well, the error message is lousy), but the next
dumpxml shows that the domain was modified anyway.  Worse, that means
if you save the domain then restore it, the restore will likely fail
due to throttling being unsupported, even though no throttling should
even be active because the monitor command failed in the first place.

* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Check for
error before making modification permanent.
2012-05-04 16:13:53 -06:00
Stefan Berger
c0774482ff qemu: fix resource leak
Error: RESOURCE_LEAK:
/libvirt/src/qemu/qemu_driver.c:6968:
alloc_fn: Calling allocation function "calloc".
/libvirt/src/qemu/qemu_driver.c:6968:
var_assign: Assigning: "nodeset" =  storage returned from "calloc(1UL, 1UL)".
/libvirt/src/qemu/qemu_driver.c:6977:
noescape: Variable "nodeset" is not freed or pointed-to in function "virTypedParameterAssign".
/libvirt/src/qemu/qemu_driver.c:6997:
leaked_storage: Variable "nodeset" going out of scope leaks the storage it points to.
2012-05-04 10:42:09 -04:00
Li Zhang
0d631e9182 Correct indent errors in the function qemuDomainNetsRestart
qemuDomainNetsRestart indents with 3 spaces.

This patch is to correct it.

Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
2012-05-03 17:25:40 +08:00
Josh Durgin
b57e01532a qemu: allow snapshotting of sheepdog and rbd disks
Signed-off-by: Josh Durgin <josh.durgin@dreamhost.com>
2012-05-01 08:54:18 -06:00
Jiri Denemark
9d2ac5453e qemu: Make sure qemu can access its directory in hugetlbfs
When libvirtd is started, we create "libvirt/qemu" directories under
hugetlbfs mount point. Only the "qemu" subdirectory is chowned to qemu
user and "libvirt" remains owned by root. If umask was too restrictive
when libvirtd started, qemu user may lose access to "qemu"
subdirectory. Let's explicitly grant search permissions to "libvirt"
directory for all users.
2012-04-30 08:17:40 +02:00
Eric Blake
2eabac008e blockjob: fix block-stream bandwidth race
With RHEL 6.2, virDomainBlockPull(dom, dev, bandwidth, 0) has a race
with non-zero bandwidth: there is a window between the block_stream
and block_job_set_speed monitor commands where an unlimited amount
of data was let through, defeating the point of a throttle.

This race was first identified in commit a9d3495e, and libvirt was
able to reduce the size of the window for that race.  In the meantime,
the qemu developers decided to fix things properly; per this message:
https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03793.html
the fix will be in qemu 1.1, and changes block-job-set-speed to use
a different parameter name, as well as adding a new optional parameter
to block-stream, which eliminates the race altogether.

Since our documentation already mentioned that we can refuse a non-zero
bandwidth for some hypervisors, I think the best solution is to do
just that for RHEL 6.2 qemu, so that the race is obvious to the user
(anyone using stock RHEL 6.2 binaries won't have this patch, and anyone
building their own libvirt with this patch for RHEL can also rebuild
qemu to get the modern semantics, so it is no real loss in behavior).

Meanwhile the code must be fixed to honor actual qemu 1.1 naming.
Rename the parameter to 'modern', since the naming difference now
covers more than just 'async' block-job-cancel.  And while at it,
fix an unchecked integer overflow.

* src/qemu/qemu_monitor.h (enum BLOCK_JOB_CMD): Drop unused value,
rename enum to match conventions.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Reflect enum rename.
* src/qemu_qemu_monitor_json.h (qemuMonitorJSONBlockJob): Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Likewise,
and support difference between RHEL 6.2 and qemu 1.1 block pull.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Reject
bandwidth during pull with too-old qemu.
* src/libvirt.c (virDomainBlockPull, virDomainBlockRebase):
Document this.
2012-04-27 13:00:56 -06:00
Jiri Denemark
2d76fea134 qemu: Use common helper when probing qemu capabilities
QEMU binary is called several times when we probe different kinds of
capabilities the binary supports. This patch introduces new common
helper so that all probes use a consistent way of invoking qemu.
2012-04-27 12:09:32 +02:00
Eric Blake
8e532d3403 qemu: improve errors related to offline domains
https://bugzilla.redhat.com/show_bug.cgi?id=816662 pointed out
that attempting 'virsh blockpull' on an offline domain gave a
misleading error message about qemu lacking support for the
operation, even when qemu was specifically updated to support it.
The real problem is that we have several capabilities that are
only determined when starting a domain, and therefore are still
clear when first working with an inactive domain (namely, any
capability set by qemuMonitorJSONCheckCommands).

While this patch was able to hoist an existing check in one of the
three culprits, it had to add redundant checks in the other two
places (because you always have to check for an active domain after
obtaining a VM job lock, but the capability bits were being checked
prior to obtaining the job lock).

Someday it would be nice to patch libvirt to cache the set of
capabilities per qemu binary (as determined by inode and timestamp),
rather than re-probing the binary every time a domain is started,
and to teach the cache how to query the monitor during the one
time the probe is made rather than having to wait until a guest
is started; then, a capability probe would succeed even for offline
guests because it just refers to the cache, and the single check for
an active domain after grabbing the job lock would be sufficient.
But since that will involve a lot more coding, I'm happy to go
with this simpler solution for an immediate solution.

* src/qemu/qemu_driver.c (qemuDomainPMSuspendForDuration)
(qemuDomainSnapshotCreateXML, qemuDomainBlockJobImpl): Check for
offline state before checking an online-only cap.
2012-04-26 16:43:05 -06:00
Eric Blake
6fb8a64d93 qemu: use consistent error when qemu binary is too old
Most of our errors complaining about an inability to support a
particular action due to qemu limitations used CONFIG_UNSUPPORTED,
but we had a few outliers.  Reported by Jiri Denemark.

* src/qemu/qemu_command.c (qemuBuildDriveDevStr): Prefer
CONFIG_UNSUPPORTED.
* src/qemu/qemu_driver.c (qemuDomainReboot)
(qemuDomainBlockJobImpl): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainAttachPciControllerDevice):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorTransaction)
(qemuMonitorBlockJob, qemuMonitorSystemWakeup): Likewise.
2012-04-17 11:09:44 -06:00
Michal Privoznik
63ddc65d63 qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass
to qemuProcessStart(). As libvirt grows it is harder and harder
to remember them and their position. Therefore we should
switch to flags instead.
2012-04-16 17:20:04 +02:00
Michal Privoznik
b1256816ff qemuOpenFile: Don't force chown on NFS
If dynamic_ownership is off and we are creating a file on NFS
we force chown. This will fail as chown/chmod are not supported
on NFS. However, with no dynamic_ownership we are not required
to do any chown.
2012-04-12 13:53:38 +02:00
Eric Blake
a9d3495e67 blockjob: allow for fast-finishing job
In my testing, I was able to provoke an odd block pull failure:

$ virsh blockpull dom vda --bandwidth 10000
error: Requested operation is not valid: No active operation on device: drive-virtio-disk0

merely by using gdb to artifically wait to do the block job set speed
until after the pull had already finished.  But in reality, that should
be a success, since the pull finished before we had a chance to set
speed.  Furthermore, using a double job lock is not only annoying, but
a bug in itself - if you do parallel virDomainBlockRebase, and hit
the race window just right, the first call grabs the VM job to start
a fast block job, then the second call grabs the VM job to start
a long-running job with unspecified speed, then the first call finally
regrabs the VM job and sets the speed, which ends up running the
second job under the speed from the first call.  By consolidating
things into a single job, we avoid opening that race, as well as reduce
the time between starting the job and changing the speed, for less
likelihood of the speed change happening after block job completion
in the first place.

* src/qemu/qemu_monitor.h (BLOCK_JOB_CMD): Add new mode.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Move secondary
job call...
(qemuDomainBlockJobImpl): ...here, for fewer locks.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Change
return value on new internal mode.
2012-04-11 21:45:43 -06:00
Eric Blake
a91ce852b5 blockjob: wire up qemu async virDomainBlockJobAbort
Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will internally
poll using qemu's "query-block-jobs" API and will not return until the
operation has been completed.  API users are advised that this operation
is unbounded and further interaction with the domain during this period
may block.  Future patches may refactor things to allow other queries in
parallel with this polling.  For older qemu, we synthesize the cancellation
event, since qemu won't generate it.

The choice of polling duration copies from the code in qemu_migration.c.

Signed-off-by: Adam Litke <agl@us.ibm.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2012-04-11 21:22:06 -06:00
Eric Blake
2b085f5bc5 blockjob: add qemu capabilities related to block pull jobs
RHEL 6.2 was released with an early version of block jobs, which only
worked on the qed file format, where the commands were spelled with
underscore (contrary to QMP style), and where 'block_job_cancel' was
synchronous and did not trigger an event.

The upcoming qemu 1.1 release has fixed these short-comings [1][2]:
the commands now work on multiple file types, are spelled with dash,
and 'block-job-cancel' is asynchronous and emits an event upon conclusion.

[1]qemu commit 370521a1d6f5537ea7271c119f3fbb7b0fa57063
[2]https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01248.html

This patch recognizes the new spellings, and fixes virDomainBlockRebase
to give a graceful error when talking to a too-old qemu on a partial
rebase attempt.  Fixes for the new semantics will come later.  This
patch also removes a bogus ATTRIBUTE_NONNULL mistakenly added in
commit 10ec36e2.

* src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCKJOB_SYNC)
(QEMU_CAPS_BLOCKJOB_ASYNC): New bits.
* src/qemu/qemu_capabilities.c (qemuCaps): Name them.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set
them.
(qemuMonitorJSONBlockJob): Manage both command names.
(qemuMonitorJSONDiskSnapshot): Minor formatting fix.
* src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Alter signature.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob): Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Pass through
capability bit.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Update callers.
2012-04-11 20:43:53 -06:00
Peter Krempa
3d3de46a67 qemu: Fix deadlock when qemuDomainOpenConsole cleans up a connection
The new safe console handling introduced a possibility to deadlock the
qemu driver when a new console connection forcibly disconnects a
previous console stream that belongs to an already closed connection.

The virStreamFree function calls subsequently a the virReleaseConnect
function that tries to lock the driver while discarding the connection,
but the driver was already locked in qemuDomainOpenConsole.

Backtrace of the deadlocked thread:
0  0x00007f66e5aa7f14 in __lll_lock_wait () from /lib64/libpthread.so.0
1  0x00007f66e5aa3411 in _L_lock_500 () from /lib64/libpthread.so.0
2  0x00007f66e5aa322a in pthread_mutex_lock () from/lib64/libpthread.so.0
3  0x0000000000462bbd in qemudClose ()
4  0x00007f66e6e178eb in virReleaseConnect () from/usr/lib64/libvirt.so.0
5  0x00007f66e6e19c8c in virUnrefStream () from /usr/lib64/libvirt.so.0
6  0x00007f66e6e3d1de in virStreamFree () from /usr/lib64/libvirt.so.0
7  0x00007f66e6e09a5d in virConsoleHashEntryFree () from/usr/lib64/libvirt.so.0
8  0x00007f66e6db7282 in virHashRemoveEntry () from/usr/lib64/libvirt.so.0
9  0x00007f66e6e09c4e in virConsoleOpen () from /usr/lib64/libvirt.so.0
10 0x00000000004526e9 in qemuDomainOpenConsole ()
11 0x00007f66e6e421f1 in virDomainOpenConsole () from/usr/lib64/libvirt.so.0
12 0x00000000004361e4 in remoteDispatchDomainOpenConsoleHelper ()
13 0x00007f66e6e80375 in virNetServerProgramDispatch () from/usr/lib64/libvirt.so.0
14 0x00007f66e6e7ae11 in virNetServerHandleJob () from/usr/lib64/libvirt.so.0
15 0x00007f66e6da897d in virThreadPoolWorker () from/usr/lib64/libvirt.so.0
16 0x00007f66e6da7ff6 in virThreadHelper () from/usr/lib64/libvirt.so.0
17 0x00007f66e5aa0c5c in start_thread () from /lib64/libpthread.so.0
18 0x00007f66e57e7fcd in clone () from /lib64/libc.so.6

* src/qemu/qemu_driver.c: qemuDomainOpenConsole()
        -- unlock the qemu driver right after acquiring the domain
        object
2012-04-11 10:45:53 +02:00
Eric Blake
1413560966 snapshot: fix memory leak on error
Leak introduced in commit 0436d32.  If we allocate an actions array,
but fail early enough to never consume it with the qemu monitor
transaction call, we leaked memory.

But our semantics of making the transaction command free the caller's
memory is awkward; avoiding the memory leak requires making every
intermediate function in the call chain check for error.  It is much
easier to fix things so that the function that allocates also frees,
while the call chain leaves the caller's data intact.  To do that,
I had to hack our JSON data structure to make it easy to protect a
portion of an arbitrary JSON tree from being freed.

* src/util/json.h (virJSONType): Name the enum.
(_virJSONValue): New field.
* src/util/json.c (virJSONValueFree): Use it to protect a portion
of an array.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONTransaction): Avoid
freeing caller's data.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive):
Free actions array on failure.
2012-04-06 08:39:34 -06:00
Michal Privoznik
650da0e99c qemu_ga: Don't overwrite errors on FSThaw
We can tell qemuDomainSnapshotFSThaw if we want it to report errors or
not. However, if we don't want to and an error has been already set by
previous qemuReportError() we must keep copy of that error not just a
pointer to it. Otherwise, it get overwritten if FSThaw reports an error.
2012-04-06 13:42:04 +02:00
Jiri Denemark
66cab01ae1 qemu: Start nested job in qemuDomainCheckEjectableMedia
Originally, qemuDomainCheckEjectableMedia was entering monitor with qemu
driver lock. Commit 2067e31bf9, which I
made to fix that, revealed another issue we had (but didn't notice it
since the driver was locked): we didn't set nested job when
qemuDomainCheckEjectableMedia is called during migration. Thus the
original fix I made was wrong.
2012-04-02 21:44:27 +02:00
Daniel P. Berrange
ec8cae93db Consistent style for usage of sizeof operator
The code is splattered with a mix of

  sizeof foo
  sizeof (foo)
  sizeof(foo)

Standardize on sizeof(foo) and add a syntax check rule to
enforce it

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-30 11:47:24 +01:00
Eric Blake
a14eda311e snapshot: don't pass NULL to QMP command creation
Commit d42a2ff caused a regression in creating a disk-only snapshot
of a qcow2 disk; by passing the wrong variable to the monitor call,
libvirt ended up creating JSON that looked like "format":null instead
of the intended "format":"qcow2".

To make it easier to diagnose this in the future, make JSON creation
error out if "s:arg" is paired with NULL (it is still possible to
use "n:arg" in the rare cases where qemu will accept a null).

* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Pass correct value.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommandRaw):
Improve error message.
2012-03-27 09:34:07 -06:00
D. Herrendoerfer
bd6b0a052e qemu,util: on restart of libvirt restart vepa callbacks
When libvirtd is restarted, also restart the netlink event
message callbacks for existing VEPA connections and send
a message to lldpad for these existing links, so it learns
the new libvirtd pid.

Signed-off-by: D. Herrendoerfer <d.herrendoerfer@herrendoerfer.name>
2012-03-27 10:48:39 -04:00
Laine Stump
cf57d345b5 build: avoid frame size error when building without -O2
libvirt always adds -Werror-frame-larger-than=4096 to the flags when
it builds. When building on Fedora 17, two functions with multiple
1024 buffers declared inside if {} blocks would generate frame size
errors; apparently the version of gcc on Fedora 16 will merge these
multiple buffers into a single buffer even when optimization is off,
but Fedora 17 won't.

The fix is to declare a single 1024 buffer at the top of the two
offending functions, and reuse the single buffer throughout the
functions.
2012-03-26 17:08:30 -04:00
Martin Kletzander
9943276fd2 Cleanup for a return statement in source files
Return statements with parameter enclosed in parentheses were modified
and parentheses were removed. The whole change was scripted, here is how:

List of files was obtained using this command:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' |             \
grep -e '\.[ch]$' -e '\.py$'

Found files were modified with this command:
sed -i -e                                                                 \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'

Then checked for nonsense.

The whole command looks like this:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' |             \
grep -e '\.[ch]$' -e '\.py$' | xargs sed -i -e                            \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'
2012-03-26 14:45:22 -06:00
Eric Blake
d42a2ffc07 snapshot: improve qemu handling of reused snapshot targets
The oVirt developers have stated that the real reasons they want
to have qemu reuse existing volumes when creating a snapshot are:
1. the management framework is set up so that creation has to be
done from a central node for proper resource tracking, and having
libvirt and/or qemu create things violates the framework, and
2. qemu defaults to creating snapshots with an absolute path to
the backing file, but oVirt wants to manage a backing chain that
uses just relative names, to allow for easier migration of a chain
across storage locations.

When 0.9.10 added VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT (commit
4e9953a4), it only addressed point 1, but libvirt was still using
O_TRUNC which violates point 2.  Meanwhile, the new qemu
'transaction' monitor command includes a new optional mode argument
that will force qemu to reuse the metadata of the file it just
opened (with the burden on the caller to have valid metadata there
in the first place).  So, this tweaks the meaning of the flag to
cover both points as intended for use by oVirt.  It is not strictly
backward-compatible to 0.9.10 behavior, but it can be argued that
the O_TRUNC of 0.9.10 was a bug.

Note that this flag is all-or-nothing, and only selects between
'existing' and the default 'absolute-paths'.  A more flexible
approach that would allow per-disk selections, as well as adding
support for the 'no-backing-file' mode, would be possible by
extending the <domainsnapshot> xml to have a per-disk mode, but
until we have a management application expressing a need for that
additional complexity, it is not worth doing.

* src/libvirt.c (virDomainSnapshotCreateXML): Tweak documentation.
* src/qemu/qemu_monitor.h (qemuMonitorDiskSnapshot): Add
parameters.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskSnapshot):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorDiskSnapshot): Pass them
through.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskSnapshot): Use
new monitor command arguments.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive)
(qemuDomainSnapshotCreateSingleDiskActive): Adjust callers.
(qemuDomainSnapshotDiskPrepare): Allow qed, modify rules on reuse.
2012-03-23 16:38:20 -06:00
Eric Blake
0436d328f5 snapshot: wire up qemu transaction command
The hardest part about adding transactions is not using the new
monitor command, but undoing the partial changes we made prior
to a failed transaction.

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive): Use
transaction when available.
(qemuDomainSnapshotUndoSingleDiskActive): New function.
(qemuDomainSnapshotCreateSingleDiskActive): Pass through actions.
(qemuDomainSnapshotCreateXML): Adjust caller.
2012-03-23 16:38:20 -06:00
Eric Blake
64d5e815b7 snapshot: add support for qemu transaction command
QEmu 1.1 is adding a 'transaction' command to the JSON monitor.
Each element of a transaction corresponds to a top-level command,
with the additional guarantee that the transaction flushes all
pending I/O, then guarantees that all actions will be successful
as a group or that failure will roll back the state to what it
was before the monitor command.  The difference between a
top-level command:

{ "execute": "blockdev-snapshot-sync", "arguments":
  { "device": "virtio0", ... } }

and a transaction:

{ "execute": "transaction", "arguments":
  { "actions": [
    { "type": "blockdev-snapshot-sync", "data":
      { "device": "virtio0", ... } } ] } }

is just a couple of changed key names and nesting the shorter
command inside a JSON array to the longer command.  This patch
just adds the framework; the next patch will actually use a
transaction.

* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommand): Move
guts...
(qemuMonitorJSONMakeCommandRaw): ...into new helper.  Add support
for array element.
(qemuMonitorJSONTransaction): New command.
(qemuMonitorJSONDiskSnapshot): Support use in a transaction.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskSnapshot): Add
argument.
(qemuMonitorJSONTransaction): New declaration.
* src/qemu/qemu_monitor.h (qemuMonitorTransaction): Likewise.
(qemuMonitorDiskSnapshot): Add argument.
* src/qemu/qemu_monitor.c (qemuMonitorTransaction): New wrapper.
(qemuMonitorDiskSnapshot): Pass argument on.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Update caller.
2012-03-23 16:38:20 -06:00
Eric Blake
4c4cc1b96d snapshot: rudimentary qemu support for atomic disk snapshot
Taking an external snapshot of just one disk is atomic, without having
to pause and resume the VM.  This also paves the way for later patches
to interact with the new qemu 'transaction' monitor command.

The various scenarios when requesting atomic are:
online, 1 disk, old qemu - safe, allowed by this patch
online, more than 1 disk, old qemu - failure, this patch
offline snapshot - safe, once a future patch implements offline disk snapshot
online, 1 or more disks, new qemu - safe, once future patch uses transaction

Taking an online system checkpoint snapshot is atomic, since it is
done via a single 'savevm' monitor command.  Taking an offline system
checkpoint snapshot is atomic, thanks to the previous patch.

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support
new flag for single-disk setups.
(qemuDomainSnapshotDiskPrepare): Check for atomic here.
(qemuDomainSnapshotCreateDiskActive): Skip pausing the VM when
atomic supported.
(qemuDomainSnapshotIsAllowed): Use bool instead of int.
2012-03-23 16:38:20 -06:00
Daniel P. Berrange
1f66c18f79 Centralize error reporting for URI parsing/formatting problems
Move error reporting out of the callers, into virURIParse
and virURIFormat, to get consistency.

* include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_URI
* src/util/viruri.c, src/util/viruri.h: Add error reporting
* src/esx/esx_driver.c, src/libvirt.c, src/libxl/libxl_driver.c,
  src/lxc/lxc_driver.c, src/openvz/openvz_driver.c,
  src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
  src/remote/remote_driver.c, src/uml/uml_driver.c,
  src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xen/xen_driver.c,
  src/xen/xend_internal.c, tests/viruritest.c: Remove error
  reporting

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-23 12:59:21 +00:00
Jiri Denemark
1fdc53c385 qemu: Avoid dangling migration-out job when client dies
When a client which started non-p2p migration dies in a bad time, the
source libvirtd never clears the migration job and almost nothing can be
done with the domain without restarting the daemon. This patch makes use
of connection close callbacks and ensures that migration job is properly
discarded when the client disconnects.
2012-03-21 17:31:09 +01:00
Jiri Denemark
527d867a94 qemu: Make autodestroy utilize connection close callbacks 2012-03-21 17:31:09 +01:00
Jiri Denemark
791273603e qemu: Add connection close callbacks
Add support for registering arbitrary callback to be called for a domain
when a connection gets closed.
2012-03-21 17:31:09 +01:00
Eric Blake
7c736bab06 snapshot: make quiesce a bit safer
If a guest is paused, we were silently ignoring the quiesce flag,
which results in unclean snapshots, contrary to the intent of the
flag.  Since we can't quiesce without guest agent support, we should
instead fail if the guest is not running.

Meanwhile, if we attempt a quiesce command, but the guest agent
doesn't respond, and we time out, we may have left the command
pending on the guest's queue, and when the guest resumes parsing
commands, it will freeze even though our command is no longer
around to issue a thaw.  To be safe, we must _always_ pair every
quiesce call with a counterpart thaw, even if the quiesce call
failed due to a timeout, so that if a guest wakes up and starts
processing a command backlog, it will not get stuck in a frozen
state.

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive):
Always issue thaw after a quiesce, even if quiesce failed.
(qemuDomainSnapshotFSThaw): Add a parameter.
2012-03-19 10:58:18 -06:00
Daniel P. Berrange
f987d17511 Fix handling of blkio deviceWeight empty string
A common coding pattern for changing blkio parameters is

  1. virDomainGetBlkioParameters

  2. change one or more params

  3. virDomainSetBlkioParameters

For this to work, it must be possible to roundtrip through
the methods without error. Unfortunately virDomainGetBlkioParameters
will return "" for the deviceWeight parameter for guests by default,
which virDomainSetBlkioParameters will then reject as invalid.

This fixes the handling of "" to be a no-op, and also improves the
error message to tell you what was invalid
2012-03-16 15:05:05 +00:00
Osier Yang
3165602a55 qemu: Use scsi-block for lun passthrough instead of scsi-disk
And don't allow to hotplug a usb disk with "device == lun". This
is the missed pieces in previous virtio-scsi patchset:

http://www.redhat.com/archives/libvir-list/2012-February/msg01052.html
2012-03-14 23:32:53 +08:00
Jiri Denemark
041109afef qemu: Fix (managed)save and snapshots with host mode CPU
When host-model and host-passthrouh CPU modes were introduced, qemu
driver was properly modify to update guest CPU definition during
migration so that we use the right CPU at the destination. However,
similar treatment is needed for (managed)save and snapshots since they
need to save the exact CPU so that a domain can be properly restored.
To avoid repetition of such situation, all places that need live XML
share the code which generates it.

As a side effect, this patch fixes error reporting from
qemuDomainSnapshotWriteMetadata().
2012-03-13 07:59:36 +01:00
Eric Blake
759095f636 cpustats: report user and sys times
Thanks to cgroups, providing user vs. system time of the overall
guest is easy to add to our existing API.

* include/libvirt/libvirt.h.in (VIR_DOMAIN_CPU_STATS_USERTIME)
(VIR_DOMAIN_CPU_STATS_SYSTEMTIME): New constants.
* src/util/virtypedparam.h (virTypedParameterArrayValidate)
(virTypedParameterAssign): Enforce checking the result.
* src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Fix offender.
(qemuDomainGetTotalcpuStats): Implement new parameters.
* tools/virsh.c (cmdCPUStats): Tweak output accordingly.
2012-03-12 08:46:56 -06:00
Laine Stump
7a23ba090d qemu: eliminate memory leak in qemuDomainUpdateDeviceConfig
This function was freeing a virDomainNetDef with
VIR_FREE(). virDomainNetDef is a complex structure with many pointers
to other dynamically allocated data; to properly free it
virDomainNetDefFree() must be called instead, otherwise several
strings (and potentially other things) will be leaked.
2012-03-08 16:58:53 -05:00
Laine Stump
edb6fc3a7f qemu: support persistent hotplug of <hostdev> devices
For some reason, although live hotplug of <hostdev> devices is
supported, persistent hotplug is not. This patch adds the proper
VIR_DOMAIN_DEVICE_HOSTDEV cases to the switches in
qemuDomainAttachDeviceConfig and qemuDomainDetachDeviceConfig.
2012-03-08 16:58:40 -05:00