Commit Graph

9959 Commits

Author SHA1 Message Date
Eric Blake
4a86c2bb4b python: improve conversion validation
Laszlo Ersek pointed out that in trying to convert a long to an
unsigned int, we used:

long long_val = ...;
if ((unsigned int)long_val == long_val)

According to C99 integer promotion rules, the if statement is
equivalent to:

(unsigned long)(unsigned int)long_val == (unsigned long)long_val

since you get an unsigned comparison if at least one side is
unsigned, using the largest rank of the two sides; but on 32-bit
platforms, where unsigned long and unsigned int are the same size,
this comparison is always true and ends up converting negative
long_val into posigive unsigned int values, rather than rejecting
the negative value as we had originally intended (python longs
are unbounded size, and we don't want to do silent modulo
arithmetic when converting to C code).

Fix this by using direct comparisons, rather than casting.

* python/typewrappers.c (libvirt_intUnwrap, libvirt_uintUnwrap)
(libvirt_ulongUnwrap, libvirt_ulonglongUnwrap): Fix conversion
checks.
2012-03-31 09:16:00 -06:00
Daniel P. Berrange
8bf0442e83 Fix client only RPM build & other misc RPM problems
* libvirt.spec.in: Remove obsolete --with-remote-pid-file arg.
  Add missing %{without_libxl} statement. Fix handling of docs
  in client only build. Put systemtap files in -client RPM
  instead of -daemon RPM
* examples/xml/nwfilter/Makefile.am: Don't install examples if
  nwfilter is disabled.
2012-03-31 13:22:40 +01:00
Daniel P. Berrange
06a0d57f5a Refactor the libvirt RPM daemon pieces
There are a number of flaws with our packaging of the libvirtd
daemon:

 - Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
   etc which are required to actually run the hypervisor in
   question
 - Installing 'libvirt' pulls in the default configuration
   files which may not be wanted & cause problems if installed
   inside a guest
 - It is not possible to explicitly required all the peices
   required to manage a specific hypervisor

This change takes the 'libvirt' RPM and and changes it thus

 - libvirt: just a virtual package with dep on libvirt-daemon,
   libvirt-daemon-config-network & libvirt-daemon-config-nwfilter
 - libvirt-daemon: the libvirt daemon and related pieces
 - libvirt-daemon-config-network: the default network config
 - libvirt-daemon-config-nwfilter: the network filter configs
 - libvirt-docs: the website HTML

We then introduce some more virtual (empty) packages

 - libvirt-daemon-qemu: Deps on libvirt-daemon & 'qemu'
 - libvirt-daemon-kvm: Deps on libvirt-daemon & 'qemu-kvm'
 - libvirt-daemon-lxc: Deps on libvirt-daemon
 - libvirt-daemon-uml: Deps on libvirt-daemon
 - libvirt-daemon-xen: Deps on libvirt-daemon & 'xen'

 - libvirt-qemu: Deps on libvirt-daemon-qemu & libvirt-daemon-config-{network,nwfilter}
 - libvirt-kvm: Deps on libvirt-daemon-kvm & libvirt-daemon-config-{network,nwfilter}
 - libvirt-lxc: Deps on libvirt-daemon-lxc & libvirt-daemon-config-{network,nwfilter}
 - libvirt-uml: Deps on libvirt-daemon-uml & libvirt-daemon-config-{network,nwfilter}
 - libvirt-xen: Deps on libvirt-daemon-xen & libvirt-daemon-config-network

My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-XXXX
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.

It is recommended that applications required a locally installed
libvirtd daemon, use either 'Requires: libvirt-daemon-XXXX' or
'Requires: libvirt-XXX' and *not* "Requires: libvirt-daemon"
or 'Requires: libvirt'

* libvirt.spec.in: Refactor RPMs
* docs/packaging.html.in, docs/sitemap.html.in: Document
  new RPM split rationale
2012-03-31 12:20:38 +01:00
Hendrik Schwartke
2711ac8716 qemu: support live change of the bridge used by a guest network device
This patch was created to resolve this upstream bug:

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

and is at least a partial solution to this RHEL RFE:

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

Previously the only attribute of a network device that could be
modified by virUpdateDeviceFlags() ("virsh update-device") was the
link state; attempts to change any other attribute would log an error
and fail.

This patch adds recognition of a change in bridge device name, and
supports reconnecting the guest's interface to the new device.
Standard audit logs for detaching and attaching a network device are
also generated. Although the current auditing function doesn't log the
bridge being attached to, this will later be changed in a separate
patch.
2012-03-30 20:14:36 -04:00
Eric Blake
8768149545 build: fix build on cygwin
Regression introduced when we changed types in commit 3e2c3d8f6.

We've done this sort of cleanup before (see commit c685993d7).

* src/conf/storage_conf.c (virStoragePoolDefFormat)
(virStorageVolTargetDefFormat): Cast gid_t and uid_t.
2012-03-30 16:51:41 -06:00
Eric Blake
1012dc2933 build: fix mingw ssize_t, syntax check
We are so close to a release that we don't want to pull in a
gnulib submodule update and risk regressions, since there has
been a lot of other gnulib churn upstream.  However, there are
a couple of gnulib issues that are worth fixing in isolation,
by applying local patches to gnulib.

There was an upstream gnulib bug in maint.mk that rendered most
of our syntax checks ineffective (and fixing it flushed out a
minor bug in our code):
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html

There is still an upstream bug where gnulib uses the wrong type
for ssize_t on mingw; we need the fix now even though it has not
yet been accepted into gnulib:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00188.html

* gnulib/local/top/maint.mk.diff: Pick up upstream gnulib
maint.mk.
* gnulib/local/m4/ssize_t.m4.diff: Work around gnulib bug.
* src/libvirt.c: Remove unused header.
* cfg.mk
(exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Exempt
gnulib local files.
2012-03-30 11:10:54 -06:00
Laine Stump
ecde15910a qemu: eliminate nested switch, simplify code
qemuBuildHostNetStr had a switch-within-a-switch where both were
looking at the same variable. This was apparently to take advantage of
code common to three different cases (while also taking care of some
code that was different). However, there were only 2 lines common to
all, one of those can be eliminated by merging it into the
virAsprintfs that are in each case. On top of that, all the extra
empty cases cause Coverity complaints (because they are unreachable),
but absence of the empty cases causes a compile error due to
"enumeration value not handled in switch".

The solution is to just make each toplevel case independent, folding
in the common code to each.
2012-03-30 12:41:18 -04:00
Laine Stump
1133ee2b38 Revert "Set default name for SPICE agent channel"
This patch reverts commit b0e2bb3. Its functionality has been replaced
by commit 3269ee6.
2012-03-30 12:38:52 -04:00
Laine Stump
3269ee657c qemu: set default name for SPICE agent channel when generating command
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).

This patch does not revert commit b0e2bb33 (it will be reverted in a
separate patch) but adds the alternate implementation of simply
inserting the default value in the appropriate place on the qemu
commandline when no value is provided.
2012-03-30 12:37:52 -04:00
Michal Privoznik
075c8518c6 qemu_agent: Issue guest-sync prior to every command
If we issue guest command and GA is not running, the issuing thread
will block endlessly. We can check for GA presence by issuing
guest-sync with unique ID (timestamp). We don't want to issue real
command as even if GA is not running, once it is started, it process
all commands written to GA socket.
2012-03-30 18:16:17 +02:00
Michal Privoznik
cde3c054fb virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
With latest gnulib we are checking even the lowest level functions
whether they check flags. Moreover, we are shadowing the real error
on system without TUNSETIFF support.
2012-03-30 15:28:59 +02:00
Daniel P. Berrange
454b927630 Fix typo in previous patch
* src/remote/remote_driver.c: s/sizeof(ret)2/sizeof(ret2)/
2012-03-30 12:44:28 +01: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
Daniel P. Berrange
57351139a7 Disable build of commandhelper & ssh on Win32
The commandhelper.c & ssh.c programs rely on various APIs not present
on Win32. Disable them, since the tests that uses these helpers are
already disabled

* tests/commandhelper.c, tests/ssh.c: Disable on WIN32

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-30 11:47:14 +01:00
Daniel P. Berrange
12f0d2ece9 Don't redefine the CPU comparison constants in CPU test
Defining an enum with names like "ERROR" causes  a world of
hurt on Win32 whose headers have such symbol names already

* tests/cputest.c: Remove redefinition of CPU constants

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-30 11:47:04 +01:00
Daniel P. Berrange
5d7929af46 Fix some format specifiers for size_t vs ssize_t
A handful of places used %zd for format specifiers even
though the args was size_t, not ssize_t.

* src/remote/remote_driver.c, src/util/xml.c: s/%zd/%zu/

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-30 11:46:56 +01:00
Alex Jia
814fcb9e08 conf: Prevent crash of libvirtd without channel target name
* src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): avoid
  crashing libvirtd due to derefing a NULL pointer.

For details, please see bug:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=808371

Signed-off-by: Alex Jia <ajia@redhat.com>
2012-03-30 18:10:56 +08:00
Wen Congyang
ff68d6eeb5 fix a deadlock when qemu cannot start
When qemu cannot start, we may call qemuProcessStop() twice.
We have check whether the vm is running at the beginning of
qemuProcessStop() to avoid libvirt deadlock. We call
qemuProcessStop() with driver and vm locked. It seems that
we can avoid libvirt deadlock. But unfortunately we may
unlock driver and vm in the function qemuProcessKill() while
vm->def->id is not -1. So qemuProcessStop() will be run twice,
and monitor will be freed unexpectedly. So we should set
vm->def->id to -1 at the beginning of qemuProcessStop().
2012-03-30 14:21:49 +08:00
Eric Blake
e8aa5a16c9 build: silence recent syntax check violations
An upstream gnulib bug[1] meant that some of our syntax checks
weren't being run.  Fix up our offenders before we upgrade to
a newer gnulib.

[1] https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html

* src/util/virnetdevtap.c (virNetDevTapCreate): Use flags.
* tests/lxcxml2xmltest.c (mymain): Strip useless ().
2012-03-29 21:23:59 -06:00
Eric Blake
63e086310f build: avoid 'devname' for BSD
Commit 21b5daa1 was the last time we cleaned this up.

* tools/virt-host-validate-common.c (virHostValidateDevice):
Rename local variable.
2012-03-29 17:08:27 -06:00
Zhou Peng
a1e50e820b private.syms: Add virNetDevMacVLanRestartWithVPortProfile
virNetDevMacVLanRestartWithVPortProfile is omitted in src/libvirt_private.syms,
which causes link err.
2012-03-29 17:03:37 +02:00
Christian Benvenuti
a02500d010 qemu: Make migration fail when port profile association fails on the dst host
In the current V3 migration protocol, Libvirt does not
check the result of the function

  qemuMigrationVPAssociatePortProfiles

This means that it is possible for a migration to complete
successfully even when the VM loses network connectivity on
the destination host.

With this change libvirt aborts the migration
(during the "finish" step) when the above function fails, that
is to say when at least one of the port profile associations fails.

Signed-off by: Christian Benvenuti <benve@cisco.com>
2012-03-28 10:45:22 -06:00
Christophe Fergeau
b0e2bb33d8 Set default name for SPICE agent channel
libvirt documentation for channels with type 'spicevmc' says that the
'target' child node has:
"an optional attribute name controls how the guest will have access
 to the channel, and defaults to name='com.redhat.spice.0'."

However, this default value is never set in libvirt code base,
there's only a check in qemu_command.c to error out if the name
attribute doesn't have the expected value (if it's set).

This commit sets a default target name for spicevmc channels during
the domain configuration parsing so that the code agrees with the
documentation.
2012-03-28 18:33:32 +02:00
Alex Jia
03ca3317a6 virsh: plug memory leaks on failure path
Leaks are introduced in commit 1cf0e3d and fe383bb.

Fixing memory leaks, in addition, the patch also fixes a potential missing
return value issue in 'if (from)' statement, without the fixing, although
the programming met a error, the subsequent codes will be executed
continually.

* tools/virsh.c (cmdSnapshotList): fix memory leaks and missing return value.

* How to reproduce?

% virsh snapshot-list <domain> --parent --roots
% virsh snapshot-list <domain> --parent --tree
% virsh snapshot-list <domain> --roots --tree

actual result:
error: --parent and --roots are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

error: --parent and --tree are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

error: --roots and --tree are mutually exclusive
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

% virsh snapshot-create-as <domain> --name "hello"
% virsh snapshot-create-as <domain> --name "libvirt"
% virsh snapshot-list <domain> --roots --from "hello"

actual result:
error: --roots and --from are mutually exclusive
 Name                 Creation Time             State
------------------------------------------------------------
 libvirt              2012-03-28 13:46:51 +0800 running

Signed-off-by: Alex Jia <ajia@redhat.com>
2012-03-28 23:05:08 +08:00
Guannan Ren
1aeb3d9e7f python: make python APIs use these helper functions
*setPyVirTypedParameter
    *libvirt_virDomainGetCPUStats
2012-03-28 08:54:06 -06:00
Guannan Ren
384ebd3fc5 python: Add new helper functions for python to C integral conversion
int libvirt_intUnwrap(PyObject *obj, int *val);
    int libvirt_uintUnwrap(PyObject *obj, unsigned int *val);
    int libvirt_longUnwrap(PyObject *obj, long *val);
    int libvirt_ulongUnwrap(PyObject *obj, unsigned long *val);
    int libvirt_longlongUnwrap(PyObject *obj, long long *val);
    int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val);
    int libvirt_doubleUnwrap(PyObject *obj, double *val);
    int libvirt_boolUnwrap(PyObject *obj, bool *val);
2012-03-28 08:42:40 -06:00
Eric Blake
3ff4b06968 snapshot: fix virsh docs
Commit d42a2ff forgot to touch up virsh documentation, and commit
4e9953a mis-spelled the option name.

* tools/virsh.pod (snapshot-create, snapshot-create-as): Fix typo
and match recent change in flag meaning.
2012-03-27 22:03:20 -06:00
Daniel P. Berrange
851117bd7a Enable all warnings permanently & default to -Werror for GIT builds
Given that we auto-detect whether each -Wxxxx flag is supported by
GCC, and we are warning-free and use automake silent rules, there
is no compelling reason to allow compile warnings to be disabled.

Replace the --enable-compile-warnings flag with a simpler
--enable-werror flag, which defaults to 'yes' if building
from GIT, or 'no' if building from tar.gz

This helps ensure that everyone writing patches for libvirt will
take care to fix their warning problems before submitting for
review

* autobuild.sh: Force -Werror
* configure.ac: Update for LIBVIRT_COMPILE_WARNINGS macro change
* m4/virt-compile-warnings.m4: Permanently enable all warnings,
  auto-enable Werror for GIT builds
2012-03-27 17:08:06 +01:00
Daniel P. Berrange
3a2fc2770b Enable build of test suite programs by default for GIT checkouts
Add a new flag '--with-test-suite' to configure to control whether
the test suite binaries are built by default. ie built with a
plain 'make', as opposed to delayed until 'make check'

For builds from tar.gz tests will not be built by default. For
builds from GIT, tests with be on by default, to try and ensure
that patch developers don't accidentally break the test suites
without noticing.

* configure.ac: Add --with-test-suite
* tests/Makefile.am: Use noinst_PROGRAMS instead of check_PROGRAMS
  if building tests by default. Consolidate setting of TESTS and
  {noinst,check}_PROGRAMS to avoid duplication
2012-03-27 17:08:06 +01:00
Stef Walter
53e1d56dd4 Change the default of mdns_adv to false
* Don't advertise information on the network without consent of
   the user, either through manual configuration, or a user
   interface that drives this option.
 * Since libvirtd must be configured for network access anyway
   (for all but ssh), this setting was not useful "out of the box",
   so changing this default setting does not remove "out of the box"
   functionality.
2012-03-27 09:54:49 -06: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
Daniel P. Berrange
72c507317a Add LXC XML files to schema test & fix problems this uncovers
* docs/schemas/domaincommon.rng: Add missing lxc & openvz
  console target types. Allow arch on LXC <os> type element
* tests/domainschematest: Include tests/lxcxml2xmldata/
2012-03-27 15:52:25 +01:00
Daniel P. Berrange
c91cff255f Add support for setting init argv for LXC
Pass argv to the init binary of LXC, using a new <initarg> element.

* docs/formatdomain.html.in: Document <os> usage for containers
* docs/schemas/domaincommon.rng: Add <initarg> element
* src/conf/domain_conf.c, src/conf/domain_conf.h: parsing and
  formatting of <initarg>
* src/lxc/lxc_container.c: Setup LXC argv
* tests/Makefile.am, tests/lxcxml2xmldata/lxc-systemd.xml,
  tests/lxcxml2xmltest.c, tests/testutilslxc.c,
  tests/testutilslxc.h: Test parsing/formatting of LXC related
  XML parts
2012-03-27 15:52:25 +01:00
Daniel P. Berrange
eb8f31c16b Detect location fo selinux mount point
The SELinux mount point moved from /selinux to /sys/fs/selinux
when systemd came along.

* configure.ac: Probe for SELinux mount point
* src/lxc/lxc_container.c: Use SELinux mount point determined
  by configure.ac
2012-03-27 15:52:25 +01:00
Daniel P. Berrange
05e981df2d Expand docs for timer tick policy 2012-03-27 15:52:25 +01: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
Jiri Denemark
2067e31bf9 qemu: Avoid entering monitor with locked driver
This avoids possible deadlock of the qemu driver in case a domain is
begin migrated (in Begin phase) and unrelated connection to qemu driver
is closed at the right time.

I checked all callers of qemuDomainCheckEjectableMedia() and they are
calling this function with qemu driver locked.
2012-03-27 14:18:12 +02:00
Laine Stump
ecb4d92d57 build: fix "missing initializer" error in qemu_process.c
Found when attempting to build on Fedora 17 alpha with:

   ./autogen.sh --system --enable-compile-warnings=error

(this same build command works without problem on Fedora 16). Since
the consumer of the qemuProcessReconnectData doesn't assume that the
other fields of the struct are initialized (although it uses them
internally), the simpler solution is to just switch to C99-style
struct initialization (which doesn't require specification of all
fields).
2012-03-26 17:08:30 -04:00
Laine Stump
05c25e6071 build: fix "missing initializer" errors in virsh.c
Found when attempting to build on Fedora 17 alpha with:

   ./autogen.sh --system --enable-compile-warnings=error

(this same build command works without problem on Fedora 16). All
other struct initializers for this struct have the extra field filled
in (almost always to 0), so the two errant ones were fixed by adding
in the extra 0 field.
2012-03-26 17:08:30 -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
f094452526 Added syntax-check rule for return with parentheses
After cleanup introduced with previous commit, there is a need for
syntax-check rule taking care of return(). Regexp used in 'prohibit'
parameter is taken from the cleanup commit and modified so it fits
'grep -E' format. Semicolon at the end is needed, otherwise the regexp
could match return with cast.

Exception is created for python source files because we don't have any
documentation restricting the use of return that matches this case.
2012-03-26 14:50:48 -06: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
70c07e01de Fix and test round-trip of query parameters
When qparams support was dropped in commit bc1ff160, we forgot
to add tests to ensure that viruri can do the same round trip
handling of a URI. This round trip was broken, due to use
of the old 'query' field of xmlUriPtr, instead of the new
'query_raw'

Also, we forgot to report an OOM error.

* tests/viruritest.c (mymain): Add tests based on just-deleted
qparamtest.
(testURIParse): Allow difference in input and expected output.
* src/util/viruri.c (virURIFormat): Add missing error. Use
  query_raw, instead of query for xmlUriPtr object.
2012-03-26 11:23:45 +01:00
Osier Yang
beb76e3742 spec: Add missed dependancy for numad
numad is available since Fedora 17 and RHEL6.X. And it's not supported
on s390[x] and ARM.
2012-03-24 09:35:20 +08: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
Eric Blake
922d498e1c snapshot: make offline qemu snapshots atomic
Offline internal snapshots can be rolled back with just a little
bit of refactoring, meaning that we are now automatically atomic.

* src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Move
guts...
(qemuDomainSnapshotForEachQcow2Raw): ...to new helper, to allow
rollbacks.
2012-03-23 16:38:20 -06:00
Eric Blake
4d5533ca87 snapshot: add atomic create flag
Right now, it is appallingly easy to cause qemu disk snapshots
to alter a domain then fail; for example, by requesting a two-disk
snapshot where the second disk name resides on read-only storage.
In this failure scenario, libvirt reports failure, but modifies
the live domain XML in-place to record that the first disk snapshot
was taken; and places a difficult burden on the management app
to grab the XML and reparse it to see which disks, if any, were
altered by the partial snapshot.

This patch adds a new flag where implementations can request that
the hypervisor make snapshots atomically; either no changes to
XML occur, or all disks were altered as a group.  If you request
the flag, you either get outright failure up front, or you take
advantage of hypervisor abilities to make an atomic snapshot. Of
course, drivers should prefer the atomic means even without the
flag explicitly requested.

There's no way to make snapshots 100% bulletproof - even if the
hypervisor does it perfectly atomic, we could run out of memory
during the followup tasks of updating our in-memory XML, and report
a failure.  However, these sorts of catastrophic failures are rare
and unlikely, and it is still nicer to know that either all
snapshots happened or none of them, as that is an easier state to
recover from.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC): New flag.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Expose it.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.
2012-03-23 16:38:20 -06:00