Eric Blake [Sat, 23 Feb 2013 00:10:48 +0000 (17:10 -0700)]
run: license as LGPL
It makes no sense to prohibit reuse of the wrapper in other LGPL
projects, since most of libvirt is designed to be LGPL. Of
course, when using the wrapper to wrap a GPL program, the combined
result is still effectively GPL, but that shouldn't force us to
license the wrapper as GPL in isolation.
This function does the source part of NBD magic. It
invokes drive-mirror on each non shared and RW disk with
a source and wait till the mirroring process completes.
When it does we can proceed with migration.
Currently, an active waiting is done: every 500ms libvirt
asks qemu if block-job is finished or not. However, once
the job finishes, qemu doesn't report its progress so we
can only assume if the job finished successfully or not.
The better solution would be to listen to the event which
is sent as soon as the job finishes. The event does
contain the result of job.
We need to start NBD server and feed it with all non-<shared/>,
RW and source-full disks. Moreover, with new virPortAllocator we
must ensure the borrowed port for NBD server will be returned if
either migration completes or qemu process is torn down.
Michal Privoznik [Thu, 22 Nov 2012 15:17:13 +0000 (16:17 +0100)]
qemu: Introduce nbd-server-add command
This will be used with new migration scheme.
This patch creates basically just monitor stub
functions. Wiring them into something useful
is done in later patches.
Michal Privoznik [Thu, 22 Nov 2012 15:08:52 +0000 (16:08 +0100)]
qemu: Introduce nbd-server-start command
This will be used with new migration scheme.
This patch creates basically just monitor stub
functions. Wiring them into something useful
is done in later patches.
Michal Privoznik [Tue, 29 Jan 2013 12:38:50 +0000 (13:38 +0100)]
Introduce NBD migration cookie
This migration cookie is meant for two purposes. The first is to be sent
in begin phase from source to destination to let it know we support new
implementation of VIR_MIGRATE_NON_SHARED_{DISK,INC} so destination can
start NBD server. Then, the second purpose is, destination can let us
know, on which port the NBD server is running.
Pieter Hollants [Thu, 21 Feb 2013 22:40:17 +0000 (23:40 +0100)]
Add support for <option> tag in network config
This patch adds support for a new <option>-Tag in the <dhcp> block of
network configs, based on a subset of the fifth proposal by Laine
Stump in the mailing list discussion at
https://www.redhat.com/archives/libvir-list/2012-November/msg01054.html.
Any such defined option will result in a dhcp-option=<number>,"<value>"
statement in the generated dnsmasq configuration file.
Currently, DHCP options can be specified by number only and there is
no whitelisting or blacklisting of option numbers, which should
probably be added.
Signed-off-by: Pieter Hollants <pieter@hollants.com> Signed-off-by: Laine Stump <laine@laine.org>
Eric Blake [Sat, 23 Feb 2013 00:10:48 +0000 (17:10 -0700)]
run: use portable shell
Nothing in run required bash, except for the shebang. On systems
where /bin/bash doesn't exist (I hit it on FreeBSD), using /bin/sh
instead fixes a 'make check' failure:
gmake[3]: Entering directory `/usr/home/dummy/libvirt/python'
GEN check-local
/usr/local/bin/bash: ../run: /bin/bash: bad interpreter: No such file or directory
Sage Weil [Fri, 22 Feb 2013 23:43:00 +0000 (15:43 -0800)]
storage: use f_frsize, not f_bsize, for calculating available space
The bfree and blocks fields are supposed to be in units of frsize. We were
calculating capacity correctly using those units, but the available
calculation was using bsize instead. Most file systems report these as the
same value specifically because many programs are buggy, but that is no
reason to rely on that behavior, or to behave inconsistently.
This bug has been present since e266ded (2008) and aa296e6c, when the code
was originally introduced (the latter via cut and paste).
Jiri Denemark [Fri, 8 Feb 2013 08:58:03 +0000 (09:58 +0100)]
qemu: Parse more fields from query-migrate QMP command
As a side effect, this also fixes reporting disk migration process.
It was added to memory migration progress, which was wrong. Disk
progress has dedicated fields in virDomainJobInfo structure.
Jiri Denemark [Wed, 30 Jan 2013 09:28:54 +0000 (10:28 +0100)]
remote: Auto-allocate params in remoteDeserializeTypedParameters
remoteDeserializeTypedParameters can now be called with either
preallocated params array (size of which is announced by nparams) or it
can allocate params array according to the number of parameters received
from the server.
Doug Goldstein [Fri, 15 Feb 2013 22:48:07 +0000 (16:48 -0600)]
interface: Improve udev backend device type id
Refactored the interface device type identification to make it more
clear about the operations. Add support for udev devtype to detect
VLANs on Linux 3.7 and newer. Move VLAN detection based on device
name to fallback case.
Doug Goldstein [Fri, 15 Feb 2013 22:32:34 +0000 (16:32 -0600)]
interface: Refactor interface vlan to helper func
Mechanical move to break up udevIfaceGetIfaceDef() into different
helpers for each of the interface types to hopefully make the code
easier to follow. This moves the vlan code to
udevIfaceGetIfaceDefVlan().
Based on feedback from Laine Stump, improve a number of the error
handling cases to report the issue to the user instead of not generating
data or giving vague errors. Added the bridge device name to every error
message as well to make it clear which bridge failed.
Doug Goldstein [Fri, 15 Feb 2013 16:49:08 +0000 (10:49 -0600)]
interface: Refactor udev bridge to helper func
Mechanical move to break up udevIfaceGetIfaceDef() into different
helpers for each of the interface types to hopefully make the code
easier to follow. This moves the bridge code to
udevIfaceGetIfaceDefBridge().
Eric Blake [Wed, 20 Feb 2013 22:34:48 +0000 (15:34 -0700)]
qemu: check backing chains even when cgroup is omitted
https://bugzilla.redhat.com/show_bug.cgi?id=896685 points out
a regression caused by commit 38c4a9c - libvirt only labels
the backing chain if the backing chain cache is populated, but
the code to populate the cache was only conditionally performed
if cgroup labeling was necessary.
* src/qemu/qemu_cgroup.c (qemuSetupCgroup): Hoist cache setup...
* src/qemu/qemu_process.c (qemuProcessStart): ...earlier into
caller, where it is now unconditional.
Jiri Denemark [Fri, 15 Feb 2013 12:05:12 +0000 (13:05 +0100)]
qemu: Turn closeCallbacks into virObjectLockable
To avoid having to hold the qemu driver lock while iterating through
close callbacks and calling them. This fixes a real deadlock when a
domain which is being migrated from another host gets autodestoyed as a
result of broken connection to the other host.
Guannan Ren [Wed, 20 Feb 2013 11:28:13 +0000 (19:28 +0800)]
qemu: fix an off-by-one error in qemuDomainGetPercpuStats
The max value of number of cpus to compute(id) should not
be equal or greater than max cpu number.
The bug ocurrs when id value is equal to max cpu number which
leads to the off-by-one error in the following for loop.
# virsh cpu-stats guest --start 1
error: Failed to virDomainGetCPUStats()
Jim Fehlig [Wed, 20 Feb 2013 20:02:28 +0000 (13:02 -0700)]
libxl: Fix setting of disk backend
The libxl driver was setting the backend field of libxl_device_disk
structure to LIBXL_DISK_BACKEND_TAP when the driver element of disk
configuration was not specified. This needlessly forces the use of
blktap driver, which may not be loaded in dom0
Michal Privoznik [Tue, 19 Feb 2013 17:07:58 +0000 (18:07 +0100)]
qemu: Run lzop with '--ignore-warn'
Currently, if lzop decompression binary produces a warning, it
doesn't exit with zero status but 2 instead. Terrifying, but
true. However, warnings may be ignored using '--ignore-warn'
command line argument. Moreover, in which case, the exit status
will be zero.
Osier Yang [Tue, 19 Feb 2013 12:27:45 +0000 (20:27 +0800)]
qemu: Remove the shared disk entry if the operation is ejecting or updating
For both AttachDevice and UpdateDevice APIs, if the disk device
is 'cdrom' or 'floppy', the operations could be ejecting, updating,
and inserting. For either ejecting or updating, the shared disk
entry of the original disk src has to be removed, because it's
not useful anymore.
And since the original disk def will be changed, new disk def passed
as argument will be free'ed in qemuDomainChangeEjectableMedia, so
we need to copy the orignal disk def before
qemuDomainChangeEjectableMedia, to use it for qemuRemoveSharedDisk.
Osier Yang [Tue, 19 Feb 2013 12:27:44 +0000 (20:27 +0800)]
qemu: Move the shared disk adding and sgio setting prior to attaching
The disk def could be free'ed by qemuDomainChangeEjectableMedia,
which can thus cause crash if we reference the disk pointer. On
the other hand, we have to remove the added shared disk entry from
the table on error codepath.
Osier Yang [Wed, 20 Feb 2013 07:43:55 +0000 (15:43 +0800)]
qemu: Record names of domain which uses the shared disk in hash table
The hash entry is changed from "ref" to {ref, @domains}. With this, the
caller can simply call qemuRemoveSharedDisk, without afraid of removing
the entry belongs to other domains. qemuProcessStart will obviously
benifit from it on error codepath (which calls qemuProcessStop to do
the cleanup).
Osier Yang [Tue, 19 Feb 2013 12:27:41 +0000 (20:27 +0800)]
qemu: Merge qemuCheckSharedDisk into qemuAddSharedDisk
Based on moving various checking into qemuAddSharedDisk, this
avoids the caller using it in wrong ways. Also this adds two
new checking for qemuCheckSharedDisk (disk device not 'lun'
and kernel doesn't support unpriv_sgio simply returns 0).
Jiri Denemark [Wed, 20 Feb 2013 09:30:38 +0000 (10:30 +0100)]
qemu: Do not ignore mandatory features in migration cookie
Due to "feature"/"features" nasty typo, any features marked as mandatory
by one side of a migration are silently considered optional by the other
side. The following is the code that formats mandatory features in
migration cookie:
for (i = 0 ; i < QEMU_MIGRATION_COOKIE_FLAG_LAST ; i++) {
if (mig->flagsMandatory & (1 << i))
virBufferAsprintf(buf, " <feature name='%s'/>\n",
qemuMigrationCookieFlagTypeToString(i));
}
Ján Tomko [Fri, 15 Feb 2013 06:48:49 +0000 (07:48 +0100)]
qemu: switch PCI address alocation to use virDevicePCIAddress
Some functions were using virDomainDeviceInfo where virDevicePCIAddress
would suffice. Some were only using integers for slots and functions,
assuming the bus numbers are always 0.
Switch from virDomainDeviceInfoPtr to virDevicePCIAddressPtr:
qemuPCIAddressAsString
qemuDomainPCIAddressCheckSlot
qemuDomainPCIAddressReserveAddr
qemuDomainPCIAddressReleaseAddr
Switch from int slot to virDevicePCIAddressPtr:
qemuDomainPCIAddressReserveSlot
qemuDomainPCIAddressReleaseSlot
qemuDomainPCIAddressGetNextSlot
Deleted functions (they would take the same parameters
as ReserveAddr/ReleaseAddr do now.)
qemuDomainPCIAddressReserveFunction
qemuDomainPCIAddressReleaseFunction
Eric Blake [Sat, 16 Feb 2013 03:25:09 +0000 (20:25 -0700)]
maint: fix header file owners of private symbols
Recent renames were not reflected into the comments of
libvirt_private.syms; furthermore, since we mix private headers from
several directories into this file, knowing where the file lives
can be helpful.
Jiri Denemark [Mon, 18 Feb 2013 14:10:04 +0000 (15:10 +0100)]
rpc: Avoid deadlock when closing client connection
We need to drop the server lock before calling virObjectUnlock(client)
since in case we had the last reference to the client, its dispose
callback would be called and that could possibly try to lock the server
and cause a deadlock. This is exactly what happens when there is only
one QEMU domain running and it is marked to be autodestroyed when the
connection dies. This results in qemuProcessAutoDestroy ->
qemuProcessStop -> virNetServerRemoveShutdownInhibition call sequence,
where the last function locks the server.
Jiri Denemark [Mon, 18 Feb 2013 14:07:48 +0000 (15:07 +0100)]
Avoid resetting errors in virTypedParamsFree
The function does not report any errors so there should be no need too
reset an existing error first. Moreover, virTypedParamsFree is mostly
called in cleanup phase where it has the potential to reset any useful
reported earlier.
Eric Blake [Mon, 18 Feb 2013 20:34:58 +0000 (13:34 -0700)]
build: force correct gcc syntax for attribute_nonnull
Gcc lets you do:
int ATTRIBUTE_NONNULL(1) foo(void *param);
int foo(void *param) ATTRIBUTE_NONNULL(1);
int ATTRIBUTE_NONNULL(1) foo(void *param) { ... }
but chokes on:
int foo(void *param) ATTRIBUTE_NONNULL(1) { ... }
However, since commit eefb881, we have intentionally been disabling
ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that
is, gcc doesn't do decent warning reporting, then compiles code that
mysteriously fails if you break the contract of the attribute, which
is surprisingly easy to do), leaving it on only for Coverity (which
does a much better job of improved static analysis when the attribute
is present).
But completely eliding the macro makes it too easy to write code that
uses the fourth syntax option, if you aren't using Coverity. So this
patch forces us to avoid syntax errors, even when not using the
attribute under gcc. It also documents WHY we disable the warning
under gcc, rather than forcing you to find the commit log.
* src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute,
rather than nothing, when on gcc.
Doug Goldstein [Mon, 18 Feb 2013 05:46:29 +0000 (23:46 -0600)]
Add capabilities bit for -no-kvm-pit-reinjection
The conversion to qemuCaps dropped the ability with qemu{,-kvm} 1.2 and
newer to set the lost tick policy for the PIT. While the
-no-kvm-pit-reinjection option is depreacated, it is still supported at
least through 1.4, it is better to not lose the functionality.
John Ferlan [Wed, 13 Feb 2013 13:11:39 +0000 (08:11 -0500)]
security: Remove unnecessary checks for mgr == NULL
Coverity found the DACGenLabel was checking for mgr == NULL after a
possible dereference; however, in order to get into the function the
virSecurityManagerGenLabel would have already dereferenced sec_managers[i]
so the check was unnecessary. Same check is made in SELinuxGenSecurityLabel.
Stefan Berger [Sat, 16 Feb 2013 02:33:37 +0000 (21:33 -0500)]
Fix libvirt upgrade path when nwfilter is used
Between revision 65fb9d49 and before this patch, an upgrade of libvirt while
VMs are running and instantiating iptables filtering rules due to nwfilter
rules, may leave stray iptables rules behind when shutting VMs down.
Left-over iptables rules may look like this:
Chain libvirt-out (1 references)
target prot opt source destination
FO-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-out vnet0
The reason is that the recent nwfilter code only removed filtering rules in
the libvirt-out chain that contain the --physdev-is-bridged parameter.
Older rules didn't match and were not removed.
Note that the user-defined chain FO-vnet0 could not be removed due to the
reference from the rule in libvirt-out.
Often the work around may be done through
service iptables restart
kill -SIGHUP $(pidof libvirtd)
This patch now also removes older libvirt versions' iptables rules.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Eric Blake [Wed, 13 Feb 2013 18:04:05 +0000 (11:04 -0700)]
storage: test backing chain traversal
Testing our backing chain handling will make it much easier to
ensure that we avoid issues in the future. If only I had written
this test before I first caused several regressions...
* tests/virstoragetest.c: New test.
* tests/Makefile.am (test_programs): Build it.
* .gitignore: Ignore new files.
Eric Blake [Wed, 6 Feb 2013 22:48:15 +0000 (15:48 -0700)]
storage: don't follow backing chain symlinks too eagerly
If you have a qcow2 file /path1/to/file pointed to by symlink
/path2/symlink, and pass qemu /path2/symlink, then qemu treats
a relative backing file in the qcow2 metadata as being relative
to /path2, not /path1/to. Yes, this means that it is possible
to create a qcow2 file where the choice of WHICH directory and
symlink you access its contents from will then determine WHICH
backing file (if any) you actually find; the results can be
rather screwy, but we have to match what qemu does.
Libvirt and qemu default to creating absolute backing file
names, so most users don't hit this. But at least VDSM uses
symlinks and relative backing names alongside the
--reuse-external flags to libvirt snapshot operations, with the
result that libvirt was failing to follow the intended chain of
backing files, and then backing files were not granted the
necessary sVirt permissions to be opened by qemu.
See https://bugzilla.redhat.com/show_bug.cgi?id=903248 for
more gory details. This fixes a regression introduced in
commit 8250783.
I tested this patch by creating the following chain:
ls /home/eblake/Downloads/Fedora.iso # raw file for base
cd /var/lib/libvirt/images
qemu-img create -f qcow2 \
-obacking_file=/home/eblake/Downloads/Fedora.iso,backing_fmt=raw one
mkdir sub
cd sub
ln -s ../one onelink
qemu-img create -f qcow2 \
-obacking_file=../sub/onelink,backing_fmt=qcow2 two
mv two ..
ln -s ../two twolink
qemu-img create -f qcow2 \
-obacking_file=../sub/twolink,backing_fmt=qcow2 three
mv three ..
ln -s ../three threelink
then pointing my domain at /var/lib/libvirt/images/sub/threelink.
Prior to this patch, I got complaints about missing backing
files; afterwards, I was able to verify that the backing chain
(and hence DAC and SELinux relabels) of the entire chain worked.
Eric Blake [Wed, 6 Feb 2013 23:33:45 +0000 (16:33 -0700)]
storage: refactor metadata lookup
Prior to this patch, we had the callchains:
external users
\-> virStorageFileGetMetadataFromFD
\-> virStorageFileGetMetadataFromBuf
virStorageFileGetMetadataRecurse
\-> virStorageFileGetMetadataFromFD
\-> virStorageFileGetMetadataFromBuf
However, a future patch wants to add an additional parameter to
the bottom of the chain, for use by virStorageFileGetMetadataRecurse,
without affecting existing external callers. Since there is only a
single caller of the internal function, we can repurpose it to fit
our needs, with this patch giving us:
* src/util/virstoragefile.c (virStorageFileGetMetadataFromFD):
Move most of the guts...
(virStorageFileGetMetadataFromBuf): ...here, and rename...
(virStorageFileGetMetadataInternal): ...to this.
(virStorageFileGetMetadataRecurse): Use internal helper.
Eric Blake [Sat, 9 Feb 2013 13:41:01 +0000 (06:41 -0700)]
storage: prepare for refactoring
virStorageFileGetMetadataFromFD is the only caller of
virStorageFileGetMetadataFromBuf; and it doesn't care about the
difference between a return of 0 (total success) or 1
(metadata was inconsistent, but pointer was populated as best
as possible); only about a return of -1 (could not read metadata
or out of memory). Changing the return type, and normalizing
the variable names used, will make merging the functions easier
in the next commit.
* src/util/virstoragefile.c (virStorageFileGetMetadataFromBuf):
Change return value, and rename some variables.
(virStorageFileGetMetadataFromFD): Rename some variables.
Eric Blake [Fri, 15 Feb 2013 22:39:33 +0000 (15:39 -0700)]
build: more mingw fixes
More mingw build failures:
CCLD libvirt-lxc.la
/usr/lib64/gcc/i686-w64-mingw32/4.7.2/../../../../i686-w64-mingw32/bin/ld: cannot find libvirt_lxc.def: No such file or directory
CC virportallocatortest-virportallocatortest.o
../../tests/virportallocatortest.c: In function 'main':
../../tests/virportallocatortest.c:195:1: error: implicit declaration of function 'setenv' [-Werror=implicit-function-declaration]
Eric Blake [Mon, 11 Feb 2013 22:12:16 +0000 (15:12 -0700)]
regex: gnulib guarantees that we have regex support
No need to use HAVE_REGEX_H - our use of gnulib guarantees that
the header exists and works, regardless of platform. Similarly,
we can unconditionally assume a compiling <sys/wait.h> (although
the mingw version of this header is not full-featured).
* src/storage/storage_backend.c: Drop useless conditional.
* tests/testutils.c: Likewise.
Laine Stump [Thu, 7 Feb 2013 16:45:27 +0000 (11:45 -0500)]
util: maintain caps when running command with uid != 0
virCommand was previously calling virSetUIDGID() to change the uid and
gid of the child process, then separately calling
virSetCapabilities(). This did not work if the desired uid was != 0,
since a setuid to anything other than 0 normally clears all
capabilities bits.
The solution is to use the new virSetUIDGIDWithCaps(), sending it the
uid, gid, and capabilities bits. This will get the new process setup
properly.
Since the static functions virSetCapabilities() and
virClearCapabilities are no longer called, they have been removed.
NOTE: When combined with "filecap $path-to-qemu sys_rawio", this patch
will make CAP_SYS_RAWIO (which is required for passthrough of generic
scsi commands to a guest - see commits e8daeeb, 177db08, 397e6a7, and 74e0349) be retained by qemu when necessary. Apparently that
capability has been broken for non-root qemu ever since it was
originally added.