Commit Graph

8800 Commits

Author SHA1 Message Date
John Ferlan
903f43ce6d keepalive: Guard against integer overflow
Don't allow interval to be > MAX_INT/1000 in virKeepAliveStart()

Guard against possible overflow in virKeepAliveTimeout() by setting the
timeout to be MAX_INT/1000 since the math following will multiply it by 1000.
2013-02-20 16:56:59 -05:00
Jim Fehlig
567779e51a 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

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

Ian Campbell suggested that LIBXL_DISK_BACKEND_UNKNOWN is a better
default in this case

https://www.redhat.com/archives/libvir-list/2013-February/msg01126.html
2013-02-20 14:52:10 -07:00
John Ferlan
2bff35d5bb Remove a couple of misplaced VIR_FREE 2013-02-20 12:43:00 -05:00
Michal Privoznik
0eeedf52e7 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.
2013-02-20 18:10:01 +01:00
Sergey Fionov
1d8193ee8a Fix memory leak in virNetClientIOWriteMessage
Commit 18937c3ae0 introduced the
memory leak when client->msg.fds is copied to thecall->msg
and then never freed.
2013-02-20 17:56:35 +01:00
Osier Yang
d0172d2b1b 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.
2013-02-21 00:31:24 +08:00
Osier Yang
0db7ff59cc 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.
2013-02-21 00:31:24 +08:00
Osier Yang
d0e4b76204 qemu: Update shared disk table when reconnecting qemu process 2013-02-21 00:31:24 +08:00
Osier Yang
a4504ac184 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).
2013-02-21 00:31:24 +08:00
Osier Yang
371df778eb 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).
2013-02-21 00:31:24 +08:00
Osier Yang
dab878a861 qemu: Add checking in helpers for sgio setting
This moves the various checking into the helpers, to avoid the
callers missing the checking.
2013-02-21 00:31:24 +08:00
Eric Blake
6ea7b3e8d1 maint: enforce private symbol section sorting
Automating a sorting check is the only way to ensure we don't
regress.  Suggested by Dan Berrange.

* src/check-symsorting.pl (check_sorting): Add a parameter,
validate that groups are in order, and that files exist.
* src/Makefile.am (check-symsorting): Adjust caller.
* src/libvirt_private.syms: Fix typo.
* src/libvirt_linux.syms: Fix file name.
* src/libvirt_vmx.syms: Likewise.
* src/libvirt_xenxs.syms: Likewise.
* src/libvirt_sasl.syms: Likewise.
* src/libvirt_libssh2.syms: Likewise.
* src/libvirt_esx.syms: Mention file name.
* src/libvirt_openvz.syms: Likewise.
2013-02-20 08:27:03 -07:00
Jiri Denemark
69660042fb 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));
    }
2013-02-20 15:24:01 +01:00
Ján Tomko
bc28e56b35 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
2013-02-20 13:57:59 +01:00
Eric Blake
f190a636e5 maint: sort private syms to reflect recent header renames
Purely mechanical (roughly, s/\n/~/; s/~~/\n/; sort by line;
s/~/\n/)

* src/libvirt_private.syms: Sort sections by header file name.
2013-02-19 14:59:05 -07:00
Eric Blake
8a256f3cb4 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.

* src/libvirt_private.sym: Reflect recent names.
2013-02-19 14:59:05 -07:00
Natanael Copa
bac8b2ca09 net: use structs for address and port ranges
We pass over the address/port start/end values many times so we put
them in structs.

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
2013-02-19 14:42:18 -05:00
Natanael Copa
1716e7a6c5 net: add support for specifying port range for forward mode nat
Let users set the port range to be used for forward mode NAT:

...
  <forward mode='nat'>
    <nat>
      <port start='1024' end='65535'/>
    </nat>
  </forward>
...

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
2013-02-19 14:42:18 -05:00
Natanael Copa
905629f47e net: support set public ip range for forward mode nat
Support setting which public ip to use for NAT via attribute
address in subelement <nat> in <forward>:

...
  <forward mode='nat'>
      <address start='1.2.3.4' end='1.2.3.10'/>
  </forward>
...

This will construct an iptables line using:

  '-j SNAT --to-source <start>-<end>'

instead of:

  '-j MASQUERADE'

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
2013-02-19 14:42:18 -05:00
Jiri Denemark
5d6f636764 qemu: Use atomic ops for driver->nactive 2013-02-19 19:11:23 +01:00
Jiri Denemark
921af42980 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.
2013-02-19 10:52:37 +01:00
Jiri Denemark
ee1d6d91d1 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.
2013-02-19 10:52:37 +01:00
Eric Blake
e086deda3c 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.
2013-02-18 14:17:40 -07:00
Guido Günther
272be1a840 qemu: pass "-1" as uid/gid for unprivileged qemu
so we don't try to change uid/git to 0 when probing capabilities.
2013-02-18 12:08:38 -06:00
Doug Goldstein
41046256fe 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.
2013-02-18 12:03:52 -06:00
Doug Goldstein
5eb621fcb3 interface: fix udev backend use after free
udevIfaceListAllInterface() used the udev_device after it had its ref
count decremented which results in a use after free issue.
2013-02-18 08:59:23 -06:00
John Ferlan
676688b69b 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.
2013-02-16 07:44:35 -05:00
John Ferlan
277aaeeebf vircommand: Remove unnecessary sa_assert
Changes from commit '3178df9a' removed the need for the sa_assert(infd).
2013-02-16 07:44:35 -05:00
Stefan Berger
b7d00de2bd 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 FP-vnet0 (1 references)
target     prot opt source               destination         
DROP       tcp  --  0.0.0.0/0            0.0.0.0/0            tcp spt:122
ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0           

[...]

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>
2013-02-15 21:33:37 -05:00
Eric Blake
d1333dd0fb 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.

* src/util/virstoragefile.h (_virStorageFileMetadata): Add
directory member.
* src/util/virstoragefile.c (absolutePathFromBaseFile): Drop,
replaced by...
(virFindBackingFile): ...better function.
(virStorageFileGetMetadataInternal): Add an argument.
(virStorageFileGetMetadataFromFD, virStorageFileChainLookup)
(virStorageFileGetMetadata): Update callers.
2013-02-15 16:07:01 -07:00
Eric Blake
2485f92153 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:

external users
  \-> virStorageFileGetMetadataFromFD
      \-> virStorageFileGetMetadataInternal
virStorageFileGetMetadataRecurse /
  \-> virStorageFileGetMetadataInternal

* src/util/virstoragefile.c (virStorageFileGetMetadataFromFD):
Move most of the guts...
(virStorageFileGetMetadataFromBuf): ...here, and rename...
(virStorageFileGetMetadataInternal): ...to this.
(virStorageFileGetMetadataRecurse): Use internal helper.
2013-02-15 16:07:00 -07:00
Eric Blake
b7df4f92d6 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.
2013-02-15 16:07:00 -07:00
Eric Blake
5e4946d4d9 storage: rearrange functions
No semantic change; done so the next patch doesn't need a forward
declaration of a static function.

* src/util/virstoragefile.c (virStorageFileProbeFormatFromBuf):
Hoist earlier.
2013-02-15 16:07:00 -07:00
Eric Blake
c51c3e45d5 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]

* src/Makefile.am (GENERATED_SYM_FILES): Also generate
libvirt_lxc.def.
* bootstrap.conf (gnulib_modules): Import setenv.
2013-02-15 15:45:52 -07:00
Eric Blake
660db5bf72 build: fix mingw build
Commits 2025356 and ba72cb12 introduced typos.

* src/util/virpci.c (virPCIIsVirtualFunction) [!__linux__]: Fix
function name.
* src/util/virutil.c (virGetDeviceID): Fix attribute spelling.
2013-02-15 15:05:25 -07:00
Eric Blake
ec2cc0f860 build: fix vircommand build on mingw
CC       libvirt_util_la-vircommand.lo
../../src/util/vircommand.c:2358:1: error: 'virCommandHandshakeChild' defined but not used [-Werror=unused-function]

The function is only implemented inside #ifndef WIN32.

* src/util/vircommand.c (virCommandHandshakeChild): Hoist earlier,
so that win32 build doesn't hit an unused forward declaration.
2013-02-15 13:16:46 -07:00
Eric Blake
ec8a2d0327 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.
2013-02-15 13:01:23 -07:00
Jiri Denemark
2e5d7798df vbox: Don't overwrite errors returned by virDomainDeviceDefParse 2013-02-14 09:05:36 +01:00
Laine Stump
7a2e845a86 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.
2013-02-13 16:11:16 -05:00
Laine Stump
e11451f42e util: virSetUIDGIDWithCaps - change uid while keeping caps
Normally when a process' uid is changed to non-0, all the capabilities
bits are cleared, even those explicitly set with calls to
capng_update()/capng_apply() made immediately before setuid. And
*after* the process' uid has been changed, it no longer has the
necessary privileges to add capabilities back to the process.

In order to set a non-0 uid while still maintaining any capabilities
bits, it is necessary to either call capng_change_id() (which
unfortunately doesn't currently call initgroups to setup auxiliary
group membership), or to perform the small amount of calisthenics
contained in the new utility function virSetUIDGIDWithCaps().

Another very important difference between the capabilities
setting/clearing in virSetUIDGIDWithCaps() and virCommand's
virSetCapabilities() (which it will replace in the next patch) is that
the new function properly clears the capabilities bounding set, so it
will not be possible for a child process to set any new
capabilities.

A short description of what is done by virSetUIDGIDWithCaps():

1) clear all capabilities then set all those desired by the caller (in
capBits) plus CAP_SETGID, CAP_SETUID, and CAP_SETPCAP (which is needed
to change the capabilities bounding set).

2) call prctl(), telling it that we want to maintain current
capabilities across an upcoming setuid().

3) switch to the new uid/gid

4) again call prctl(), telling it we will no longer want capabilities
maintained if this process does another setuid().

5) clear the capabilities that we added to allow us to
setuid/setgid/change the bounding set (unless they were also requested
by the caller via the virCommand API).

Because the modification/maintaining of capabilities is intermingled
with setting the uid, this is necessarily done in a single function,
rather than having two independent functions.

Note that, due to the way that effective capabilities are computed (at
time of execve) for a process that has uid != 0, the *file*
capabilities of the binary being executed must also have the desired
capabilities bit(s) set (see "man 7 capabilities"). This can be done
with the "filecap" command. (e.g. "filecap /usr/bin/qemu-kvm sys_rawio").
2013-02-13 16:11:16 -05:00
Laine Stump
c0e3e685cd util: drop capabilities immediately after changing uid/gid of child
This is an interim measure to make sure everything still works in this
order. The next step will be to perform capabilities drop and
setuid/gid as a single operation (which is the only way to keep any
capabilities when switching to a non-root uid).
2013-02-13 16:11:16 -05:00
Laine Stump
0345c7281b qemu: let virCommand set child process security labels/uid/gid
The qemu driver had been calling virSecurityManagerSetProcessLabel()
from a "pre-exec hook" function that is run after the child is forked,
but before exec'ing qemu. This is problematic because the uid and gid
of the child are set by the security driver, but capabilities are
dropped by virCommand - such separation doesn't work; the two
operations must be done together or the capabilities do not transfer
properly to the child process.

This patch switches to using virSecurityManagerSetChildProcessLabel(),
which is called prior to virCommandRun() (rather than being called
*during* virCommandrun() by the hook function), and doesn't set the
UID/GID/security label directly, but instead merely informs virCommand
what it should set them all to when the time is appropriate.

This lets virCommand choose to do the uid/gid and caps dropping all at
the same time if it wants (it does *want* to, but isn't doing so yet;
that's for an upcoming patch).
2013-02-13 16:11:16 -05:00
Laine Stump
7bf1aa0b9b security: add new virSecurityManagerSetChildProcessLabel API
The existing virSecurityManagerSetProcessLabel() API is designed so
that it must be called after forking the child process, but before
exec'ing the child. Due to the way the virCommand API works, that
means it needs to be put in a "hook" function that virCommand is told
to call out to at that time.

Setting the child process label is a basic enough need when executing
any process that virCommand should have a method of doing that. But
virCommand must be told what label to set, and only the security
driver knows the answer to that question.

The new virSecurityManagerSet*Child*ProcessLabel() API is the way to
transfer the knowledge about what label to set from the security
driver to the virCommand object. It is given a virCommandPtr, and each
security driver calls the appropriate virCommand* API to tell
virCommand what to do between fork and exec.

1) in the case of the DAC security driver, it calls
virCommandSetUID/GID() to set a uid and gid that must be set for the
child process.

2) for the SELinux security driver, it calls
virCommandSetSELinuxLabel() to save a copy of the char* that will be
sent to setexeccon_raw() *after forking the child process*.

3) for the AppArmor security drivers, it calls
virCommandSetAppArmorProfile() to save a copy of the char* that will
be sent to aa_change_profile() *after forking the child process*.

With this new API in place, we will be able to remove
virSecurityManagerSetProcessLabel() from any virCommand pre-exec
hooks.

(Unfortunately, the LXC driver uses clone() rather than virCommand, so
it can't take advantage of this new security driver API, meaning that
we need to keep around the older virSecurityManagerSetProcessLabel(),
at least for now.)
2013-02-13 16:11:16 -05:00
Laine Stump
6c3f3d0d89 util: add security label setting to virCommand
virCommand gets two new APIs: virCommandSetSELinuxLabel() and
virCommandSetAppArmorProfile(), which both save a copy of a
null-terminated string in the virCommand. During virCommandRun, if the
string is non-NULL and we've been compiled with AppArmor and/or
SELinux security driver support, the appropriate security library
function is called for the child process, using the string that was
previously set. In the case of SELinux, setexeccon_raw() is called,
and for AppArmor, aa_change_profile() is called.

This functionality has been added so that users of virCommand can use
the upcoming virSecurityManagerSetChildProcessLabel() prior to running
a child process, rather than needing to setup a hook function to be
called (and in turn call virSecurityManagerSetProcessLabel()) *during*
the setup of the child process.
2013-02-13 16:11:15 -05:00
Laine Stump
4a56e80fa5 build: define SECDRIVER_LIBS in Makefile.am
This makes it simpler to include the necessary system security driver
libraries for a particular system. For this patch, several existing
conditional sections from the Makfile were replaced; I'll later be
adding SECDRIVER_LIBS to libvirt_util_la_LIBADD, because vircommand.c
will be calling a function from $securitylib.
2013-02-13 16:11:15 -05:00
Laine Stump
dd5bb43962 qemu: replace exec hook with virCommandSetUID/GID in storage_backend 2013-02-13 16:11:15 -05:00
Laine Stump
6a8ecc373e qemu: replace exec hook with virCommandSetUID/GID in qemuCaps*
Setting the uid/gid of the child process was the only thing done by
the hook function in this case, and that can now be done more simply
with virCommandSetUID/GID.
2013-02-13 16:11:15 -05:00
Laine Stump
f506a4c115 util: make virSetUIDGID a NOP only when uid or gid is -1
Rather than treating uid:gid of 0:0 as a NOP, we blindly pass that
through to the lower layers. However, we *do* check for a requested
value of "-1" to mean "don't change this setting". setregid() and
setreuid() already interpret -1 as a NOP, so this is just an
optimization, but we are also calling getpwuid_r and initgroups, and
it's unclear what the former would do with a uid of -1.
2013-02-13 16:11:15 -05:00
Laine Stump
417182b072 util: add virCommandSetUID and virCommandSetGID
If a uid and/or gid is specified for a command, it will be set just
after the user-supplied post-fork "hook" function is called.

The intent is that this can replace user hook functions that set
uid/gid. This moves the setting of uid/gid and dropping of
capabilities closer to each other, which is important since the two
should really be done at the same time (libcapng provides a single
function that does both, which we will be unable to use, but want to
mimic as closely as possible).
2013-02-13 16:11:15 -05:00
Laine Stump
ad5cb11be6 util: refactor virCommandHook into virExec and virCommandHandshakeChild 2013-02-13 16:11:15 -05:00
Laine Stump
5f2ce53984 util: eliminate extra args from virExec
All args except "cmd" in the call to virExec are now redundant, since
they can all be found in cmd, so remove the args and reference the
data directly in cmd. One exception to this is that "infd" was being
modified within virExec, and modifying the original in cmd caused make
check failures, so cmd->infd is copied to a local, and the local is
used during virExec().
2013-02-13 16:11:15 -05:00
Laine Stump
b6decc57b1 util: eliminate generic hook from virExecWithHook
virExecWithHook is only called from one place, so it always has the
same "hook" function (virHookCommand), and the data sent to that
function is always a virCommandPtr, so eliminate the function and
generic data from the arglist, and replace it with "virCommandPtr
cmd". The call to (hook)(data) is replaced with
"virHookCommand(cmd)". Finally, virExecWithHook is renamed to virExec.

Indentation has been updated only for code that will remain after the
next patch, which will remove all other args to virExec (since they
are now redundant, as they're all members of virCommandPtr).
2013-02-13 16:11:15 -05:00
Daniel P. Berrange
a9e97e0c30 Remove qemuDriverLock from almost everywhere
With the majority of fields in the virQEMUDriverPtr struct
now immutable or self-locking, there is no need for practically
any methods to be using the QEMU driver lock. Only a handful
of helper APIs in qemu_conf.c now need it
2013-02-13 11:10:30 +00:00
Michal Privoznik
3178df9afa virCommand: Don't misuse the eventloop for async IO
Currently, if a command wants to do asynchronous IO, a callback
is registered in the libvirtd eventloop to handle writes and
reads. However, there's a race in virCommandWait. The eventloop
may already be executing the callback, while virCommandWait is
mangling internal state of virCommand. To deal with it, we need
to either introduce locking or spawn a separate thread where we
poll() on stdio from child. The former, however, requires to
unlock all mutexes held, as the event loop may execute other
callbacks which tries to lock one of the mutexes, deadlock and
thus never wake us up. So it's safer to spawn a separate thread.
2013-02-13 09:54:19 +01:00
Eric Blake
069b5c5a58 xen: clean up the mess with cpumap
Commit 8b55992f added some Coverity comments to silence what was
a real bug in the code.  Since then, we've had a miserable run
of trying to fix the underlying problem (commits c059cde and
ba5193c), and still have a problem on 32-bit machines.

This fixes the problem for once and for all, by realizing that
on older xen, cpumap_t is identical to uint64_t, and using the
new virendian.h to do the transformation from the API (documented
to be little-endian) to the host structure.

* src/xen/xen_hypervisor.c (virXen_setvcpumap): Do the conversion
correctly.  Finally.
2013-02-12 09:00:17 -07:00
Eric Blake
731ad69240 util: use new virendian.h macros
This makes code easier to read, by avoiding lines longer than
80 columns and removing the repetition from the callers.

* src/util/virstoragefile.c (qedGetHeaderUL, qedGetHeaderULL):
Delete in favor of more generic macros.
(qcow2GetBackingStoreFormat, qcowXGetBackingStore)
(qedGetBackingStore, virStorageFileMatchesVersion)
(virStorageFileGetMetadataInternal): Use new macros.
* src/cpu/cpu_x86.c (x86VendorLoad): Likewise.
2013-02-12 09:00:17 -07:00
Eric Blake
c6f1060ca7 util: add virendian.h macros
We have several cases where we need to read endian-dependent
data regardless of host endianness; rather than open-coding
these call sites, it will be nicer to funnel things through
a macro.

The virendian.h file can be expanded to add writer functions,
and/or 16-bit access patterns, if needed.  Also, if we need
to turn things into a function to avoid multiple evaluations
of buf, that can be done later.  But for now, a macro worked.

* src/util/virendian.h: New file.
* src/Makefile.am (UTIL_SOURCES): Ship it.
* tests/virendiantest.c: New test.
* tests/Makefile.am (test_programs, virendiantest_SOURCES): Run
the test.
* .gitignore: Ignore built file.
2013-02-12 09:00:15 -07:00
Daniel P. Berrange
d1c7b00b60 Release VM lock before acquiring virDomainObjListPtr lock
When removing a VM from the virDomainObjListPtr, we must not
be holding the VM lock while acquiring the list lock. Re-order
code to ensure that we can release the VM lock early.
2013-02-12 11:06:03 +00:00
Daniel P. Berrange
61b52d2e38 Fix potential deadlock across fork() in QEMU driver
The hook scripts used by virCommand must be careful wrt
accessing any mutexes that may have been held by other
threads in the parent process. With the recent refactoring
there are 2 potential flaws lurking, which will become real
deadlock bugs once the global QEMU driver lock is removed.

Remove use of the QEMU driver lock from the hook function
by passing in the 'virQEMUDriverConfigPtr' instance directly.

Add functions to the virSecurityManager to be invoked before
and after fork, to ensure the mutex is held by the current
thread. This allows it to be safely used in the hook script
in the child process.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-12 11:05:31 +00:00
Eric Blake
736a87b916 build: fix compilation of selinux on RHEL 5
On RHEL 5, I got:

security/security_selinux.c: In function 'getContext':
security/security_selinux.c:971: warning: unused parameter 'mgr' [-Wunused-parameter]

* src/security/security_selinux.c (getContext): Mark potentially
unused parameter.
2013-02-11 17:12:25 -07:00
Viktor Mihajlovski
0bbbd42c30 S390: domain_conf support for CCW
Add necessary handling code for the new s390 CCW address type to
virDomainDeviceInfo. Further, introduce  memory management, XML
parsing, output formatting and range validation for the new
virDomainDeviceCCWAddress type.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
2013-02-11 16:38:21 -07:00
Laine Stump
6627ea3cbf build: fix make check of remote_protocol-structs
Broken by incorrect formatting / spelling of remote_nonnull in commit
39758e7567
2013-02-11 15:34:49 -05:00
John Ferlan
ba5193c8d8 hypervisor: Restore pm initialization
Adjustment for 'c059cdeaf' due to older compiler complaint about pm
not being initialized even though the j&7 == 0 does the trick.
2013-02-12 00:32:57 +08:00
Osier Yang
fb2e465362 nodedev: Implement virNodeDeviceLookupSCSIHostByWWN
This just simply changes nodeDeviceLookupByWWN to be not static,
and its name into nodeDeviceLookupSCSIHostByWWN. And use that for
udev and HAL backends.
2013-02-12 00:23:57 +08:00
Osier Yang
39758e7567 remote: Wire up the remote protocol
Like virNodeDeviceCreateXML, virNodeDeviceLookupSCSIHostByWWN
has to be treated specially when generating the RPC codes. Also
new rules are added in fixup_name to keep the name SCSIHostByWWN.
2013-02-12 00:23:57 +08:00
Osier Yang
efed366eb7 Introduce API virNodeDeviceLookupSCSIHostByWWN
Since the name (like scsi_host10) is not stable for vHBA, (it can
be changed either after recreating or system rebooting), current
API virNodeDeviceLookupByName is not nice to use for management app
in this case. (E.g. one wants to destroy the vHBA whose name has
been changed after system rebooting, he has to find out current
name first).

Later patches will support the persistent vHBA via storage pool,
with which one can identify the vHBA stably by the wwnn && wwpn
pair.

So this new API comes.
2013-02-12 00:23:57 +08:00
Daniel P. Berrange
0ab49601a8 Remove re-entrant API call in SELinux/AppArmor security managers
The security manager drivers are not allowed to call back
out to top level security manager APIs, since that results
in recursive mutex acquisition and thus deadlock. Remove
calls to virSecurityManagerGetModel from SELinux / AppArmor
drivers

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 16:05:05 +00:00
John Ferlan
c059cdeaf3 hypervisor: Revisit Coverity issues regarding cpumap
Turns out the issue regarding ptr_arith and sign_exension weren't false
positives. When shifting an 'unsigned char' as a target, it gets promoted
to an 'int'; however, that 'int' cannot be shifted 32 bits which was how
the algorithm was written. For the ptr_arith rather than index into the
cpumap, change the to address as necessary and assign directly.
2013-02-11 09:50:11 -05:00
John Ferlan
cbdf3b7c97 hypervisor: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:50:01 -05:00
John Ferlan
f9799f1abf xend: Fix a memory leak found by Coverity
Commit id '87b4c10c' moved the VIR_ALLOC_N, but didn't check if 'cpuset'
had been allocated on failure.
2013-02-11 09:50:01 -05:00
John Ferlan
d951c1a0b9 xend: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:49:48 -05:00
John Ferlan
273f34ba36 xm: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.
2013-02-11 09:49:34 -05:00
John Ferlan
d67c2b76c7 inotify: Clean up some function headers 2013-02-11 09:49:17 -05:00
John Ferlan
2ca9c3cefe xs: Remove redundant validity checks, clean up function headers
Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent.
2013-02-11 09:48:51 -05:00
Daniel P. Berrange
8cdd5faf46 Pass virQEMUDriverPtr into APIs managed shared disk list
Currently the APIs for managing the shared disk list take
a virHashTablePtr as the primary argument. This is bad
because it requires the caller to deal with locking of
the QEMU driver. Switch the APIs to take the full
virQEMUDriverPtr instance

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:48:22 +00:00
Daniel P. Berrange
48b49a631a Serialize execution of security manager APIs
Add locking to virSecurityManagerXXX APIs, so that use of the
security drivers is internally serialized. This avoids the need
to rely on the global driver locks to achieve serialization

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:33:44 +00:00
Daniel P. Berrange
11d926659b Turn virSecurityManager into a virObjectLockable
To enable locking to be introduced to the security manager
objects later, turn virSecurityManager into a virObjectLockable
class

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-11 12:33:41 +00:00
Harry Wei
ad11ecd965 sheepdog: skip refresh on creation failure
Don't try to refresh Sheepdog volume if creating volume fails.

Signed-off-by: Harry Wei <harryxiyou@gmail.com>
2013-02-08 14:38:52 -07:00
Natanael Copa
f3531a040c util: refactor iptables command construction into multiple steps
Instead of creating an iptables command in one shot, do it in steps
so we can add conditional options like physdev and protocol.

This removes code duplication while keeping existing behaviour.

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
2013-02-08 14:19:30 -07:00
Laine Stump
66d9bc00ab qemu: support vhost-net for generic ethernet devices
From qemu's point of view these are still just tap devices, so there's
no reason they shouldn't work with vhost-net; as a matter of fact,
Raja Sivaramakrishnan <srajag00@yahoo.com> verified on libvir-list
that at least the qemu_command.c part of this patch works:

  https://www.redhat.com/archives/libvir-list/2012-December/msg01314.html

(the hotplug case is extrapolation on my part).
2013-02-08 13:13:55 -05:00
Michal Privoznik
a6cfed260b network_conf.c: Free xmlDoc after use
The virNetworkObjUpdateParseFile() function was not freeing the xml
variable, leaving us with a memory leak.
2013-02-08 16:01:58 +01:00
Daniel P. Berrange
020a030786 Stop accessing driver->caps directly in QEMU driver
The 'driver->caps' pointer can be changed on the fly. Accessing
it currently requires the global driver lock. Isolate this
access in a single helper, so a future patch can relax the
locking constraints.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:49:16 +00:00
Daniel P. Berrange
32803ba409 Rename 'qemuCapsXXX' to 'virQEMUCapsXXX'
To avoid confusion between 'virCapsPtr' and 'qemuCapsPtr'
do some renaming of various fucntions/variables. All
instances of 'qemuCapsPtr' are renamed to 'qemuCaps'. To
avoid that clashing with the 'qemuCaps' typedef though,
rename the latter to virQEMUCaps.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:49:14 +00:00
Daniel P. Berrange
fed92f08db Turn virCapabilities into a virObject
To enable virCapabilities instances to be reference counted,
turn it into a virObject. All cases of virCapabilitiesFree
turn into virObjectUnref

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:34:26 +00:00
Daniel P. Berrange
5b984370f6 Fix comment about virCgroupPtr locking rules in QEMU driver
The virCgroupPtr instance APIs are safe to use without locking
in the QEMU driver, since all internal state they rely on is
immutable. Update the comment to reflect this.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-02-08 11:34:25 +00:00
Michal Privoznik
0d36f228a4 virCondDestroy: Lose attribute RETURN_CHECK
We are wrapping it in ignore_value() anyway.
2013-02-08 09:12:11 +01:00
Michal Privoznik
4ca6f5089f Drop useless virFileWrapperFdCatchError
We are requesting for stderr catching for all cases in
virFileWrapperFdNew(). There is no need to have a separate
function just to report an error, esp. when we can do it in
virFileWrapperFdClose().
2013-02-08 09:11:51 +01:00
John Ferlan
2792ad143e virnetttlcontext: Resolve issues found by Valgrind
Need to initialize 'usage' and 'critical' since the VIR_DEBUG will
attempt to use them.
2013-02-07 14:08:14 -05:00
John Ferlan
890b6b351f qemu_command: Resolve resource leaks found by Valgrind
The qemuParseGlusterString() replaced dst->src without a VIR_FREE() of
what was in there before.

The qemuBuildCommandLine() did not properly free the boot_buf depending
on various usages.

The qemuParseCommandLineDisk() had numerous paths that didn't clean up
the virDomainDiskDefPtr def properly. Adjust the logic to go through an
error: label before cleanup in order to free the resource.
2013-02-07 14:08:14 -05:00
John Ferlan
7af7c42d05 domain_conf: Resolve resource leaks found by Valgrind
Fix various resource leaks discovered while parsing through Valgrind output
2013-02-07 14:08:14 -05:00
John Ferlan
cbd5dc09e8 vport_profile_conf: Resolve memory leak found by Valgrind
The 'virtPortInterfaceID' was not VIR_FREE()'d
2013-02-07 14:08:14 -05:00
John Ferlan
ba9c7a3026 netdev_vlan_conf: Resolve memory leak found by Valgrind.
The 'trunk' is filled in with virXPathString() value, but was
never VIR_FREE()'d.
2013-02-07 14:08:14 -05:00
Eric Blake
542fbfb076 build: fix build of HAL node backend
Commit 2025356 missed uses of PCI functions in the older HAL-related
code, probably because hal-devel is no longer available in latest Fedora.

* src/node_device/node_device_hal.c (gather_pci_cap): Reflect
function rename.
2013-02-05 16:32:02 -07:00
Eric Blake
98fc0137f1 bitmap: add way to find next clear bit
We had an easy way to iterate set bits, but not for iterating
cleared bits.

* src/util/virbitmap.h (virBitmapNextClearBit): New prototype.
* src/util/virbitmap.c (virBitmapNextClearBit): Implement it.
* src/libvirt_private.syms (bitmap.h): Export it.
* tests/virbitmaptest.c (test4): Test it.
2013-02-05 16:23:14 -07:00
John Ferlan
75fabbdf3f qemu_hotplug: Need to call virUSBDeviceFree() 2013-02-05 17:11:06 -05:00
John Ferlan
0ea43d177c rpc: Need to virCommandFree on error path 2013-02-05 16:51:07 -05:00
John Ferlan
ce6de782e9 uml: Resolve leak if need to requery in umlIdentifyOneChrPTY()
Coverity noted that in the retry logic loop if res had been set, then
it could be leaked so add a VIR_FREE(res) prior to retry.
2013-02-05 16:51:07 -05:00
John Ferlan
40ef77cd86 openvz: Need to error check openvzDomainSetVcpusFlagsInternal() 2013-02-05 16:51:07 -05:00
John Ferlan
7f94521703 parallels: Need to free memory on error path 2013-02-05 16:51:07 -05:00
John Ferlan
3fa23653ee network: Remove conditional settings to resolve resource leak
The conditional setting of cmdout in networkBuildDhcpDaemonCommandLine()
caused Coverity to complain that 'cmd' could be leaked if !cmdout.  Since
the function is local and only called with cmdout being passed those checks
have been removed.
2013-02-05 16:51:07 -05:00