All APIs that take typed parameters are only using params address in
their entry point debug messages. With the new VIR_TYPED_PARAMS_DEBUG
macro, all functions can easily log all individual typed parameters
passed to them.
When unsupported parameter is passed to virTypedParamsValidate,
VIR_ERR_ARGUMENT_UNSUPPORTED should be returned rather than
VIR_ERR_INVALID_ARG, which is more appropriate for supported parameters
used incorrectly.
virPCIDeviceDetach would previously sometimes consume the input device
object (to put it on the inactive list) and sometimes not. Avoiding
memory leaks required checking beforehand to see if the device was
already on the list, and freeing the device object in the caller only
if there wasn't already an identical object on the inactive list.
This patch makes it consistent - virPCIDeviceDetach will *never*
consume the input virPCIDevice object; if it needs to put one on the
inactive list, it will create a copy and put *that* on the list. This
way the caller knows that it is always their responsibility to free
the device object they created.
virPCIDeviceReattach was making the assumption that the dev object
given to it was one and the same with the dev object on the
inactiveDevs list. If that had been the case, it would not need to
free the dev object it removed from the inactive list, because the
caller of virPCIDeviceReattach always frees the dev object that it
passes in. Since the dev object passed in is *never* the same object
that's on the list (it is a different object with the same name and
attributes, created just for the purpose of searching for the actual
object), simply doing a "ListSteal" to remove the object from the list
results in one leaked object; we need to actually free the object
after removing it from the list.
* virPCIDeviceFindByIDs - find a device on a list w/o creating an object
This makes searching for an existing device on a list lighter weight.
* virPCIDeviceCopy - make a copy of an existing virPCIDevice object.
* virPCIDeviceGetDriverPathAndName - construct new strings containing
1) the name of the driver bound to this device.
2) the full path to the sysfs config for that driver.
(This code was lifted from virPCIDeviceUnbindFromStub, and replaced
there with a call to this new function).
Previously stubDriver was always set from a string literal, so it was
okay to use a const char * that wasn't freed when the virPCIDevice was
freed. This will not be the case in the near future, so it is now a
char* that is allocated in virPCIDeviceSetStubDriver() and freed
during virPCIDeviceFree().
This patch introduces the virAccessManagerPtr class as the
interface between virtualization drivers and the access
control drivers. The viraccessperm.h file defines the
various permissions that will be used for each type of object
libvirt manages
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It's not used anywhere except for the switch in
virStorageBackendCreateQemuImgOpts, where leaving it in causes
a dead code coverity warning and omitting it breaks compilation
because of unhandled enum value.
Introduced by 6298f74.
Detect qcow2 images with version 3 in the image header as
VIR_STORAGE_FILE_QCOW2.
These images have a feature bitfield, with just one feature supported
so far: lazy_refcounts.
The header length changed too, moving the location of the backing
format name.
Call virLogVMessage instead of virLogMessage, since libudev
called us with a va_list object, not a list of arguments.
Honor message priority and strip the trailing newline.
https://bugzilla.redhat.com/show_bug.cgi?id=969152
virProcessGetNamespaces() opens files in /proc/XXX/ns/ which will
later be passed to setns(). We have to make sure that the file
descriptors in the array are in the correct order. In particular
the 'user' namespace must be first otherwise setns() may fail
for other namespaces.
The order has been taken from util-linux's sys-utils/nsenter.c
Also we must ignore EINVAL in setns() which occurs if the
namespace associated with the fd, matches the calling process'
current namespace.
Signed-off-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Such as on FreeBSD. Broken in commit aa2a4cff7.
* src/util/virstoragefile.c (virStorageFileResize): Add missing ';',
mark conditionally unused variables.
Signed-off-by: Eric Blake <eblake@redhat.com>
The document for "vol-resize" says the new capacity will be sparse
unless "--allocate" is specified, however, the "--allocate" flag
is never implemented. This implements the "--allocate" flag for
fs backend's raw type volume, based on posix_fallocate and the
syscall SYS_fallocate.
We can't use GNULIB's fprintf-posix due to licensing
incompatibilities. We do already have a portable
formatting via virAsprintf() which we got from GNULIB
though. We can use to create a virFilePrintf() function.
But really gnulib could just provide a 'fprintf'
module, that depended on just its 'asprintf' module.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
I noticed several unusual spacings in for loops, and decided to
fix them up. See the next commit for the syntax check that found
all of these.
* examples/domsuspend/suspend.c (main): Fix spacing.
* python/libvirt-override.c: Likewise.
* src/conf/interface_conf.c: Likewise.
* src/security/virt-aa-helper.c: Likewise.
* src/util/virconf.c: Likewise.
* src/util/virhook.c: Likewise.
* src/util/virlog.c: Likewise.
* src/util/virsocketaddr.c: Likewise.
* src/util/virsysinfo.c: Likewise.
* src/util/viruuid.c: Likewise.
* src/vbox/vbox_tmpl.c: Likewise.
* src/xen/xen_hypervisor.c: Likewise.
* tools/virsh-domain-monitor.c (vshDomainStateToString): Drop
default case, to let compiler check us.
* tools/virsh-domain.c (vshDomainVcpuStateToString): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
When src is NULL, VIR_STRDUP will return 0 directly.
This patch will set dest to NULL before VIR_STRDUP return.
Example:
[root@yds-pc libvirt]# virsh
Welcome to virsh, the virtualization interactive terminal.
Type: 'help' for help with commands
'quit' to quit
virsh # connect
error: Failed to connect to the hypervisor
error: internal error Unable to parse URI �N�*
Signed-off-by: yangdongsheng <yangds.fnst@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
With previous patch, we accept negative value as length of string to
duplicate. So there is no need to pass strlen(src) in case we want to do
duplicate the whole string.
It may shorten the code a bit as the following pattern:
VIR_STRNDUP(dst, src, cond ? n : strlen(src))
is used on several places among our code. However, we can
move the strlen into virStrndup and thus write just:
VIR_STRNDUP(dst, src, cond ? n : -1)
Currently, the controllers argument to virCgroupDetect acts both as
a result filter and a required controller specification, which is
a bit overloaded. If both functionalities are needed, it would be
better to have them seperated into a filter and a requirement mask.
The only situation where it is used today is to ensure that only
CPU related controllers are used for the VCPU directories. But here
we clearly do not want to enforce the existence of cpu, cpuacct and
specifically not cpuset at the same time.
This commit changes the semantics of controllers to "filter only".
Should a required mask ever be needed, more work will have to be done.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Within whole vircgroup.c we 'return -errno', e.g. 'return -ENOMEM'.
However, in this specific function virCgroupAddTaskStrController
we weren't returning -ENOMEM but -1 despite fact that later in
the function we are returning one of errno values indeed.
In my previous patches I enabled the IFF_MULTI_QUEUE flag every
time the user requested multiqueue TAP device. However, this
works only at runtime. During build time the flag may be
undeclared.
In order to learn libvirt multiqueue several things must be done:
1) The '/dev/net/tun' device needs to be opened multiple times with
IFF_MULTI_QUEUE flag passed to ioctl(fd, TUNSETIFF, &ifr);
2) Similarly, '/dev/vhost-net' must be opened as many times as in 1)
in order to keep 1:1 ratio recommended by qemu and kernel folks.
3) The command line construction code needs to switch from 'fd=X' to
'fds=X:Y:...:Z' and from 'vhostfd=X' to 'vhostfds=X:Y:...:Z'.
4) The monitor handling code needs to learn to pass multiple FDs.
https://bugzilla.redhat.com/show_bug.cgi?id=965169 documents a
problem starting domains when cgroups are enabled; I was able
to reliably reproduce the race about 5% of the time when I added
hooks to domain startup by 3 seconds (as that seemed to be about
the length of time that qemu created and then closed a temporary
thread, probably related to aio handling of initially opening
a disk image). The problem has existed since we introduced
virCgroupMoveTask in commit 9102829 (v0.10.0).
There are some inherent TOCTTOU races when moving tasks between
kernel cgroups, precisely because threads can be created or
completed in the window between when we read a thread id from the
source and when we write to the destination. As the goal of
virCgroupMoveTask is merely to move ALL tasks into the new
cgroup, it is sufficient to iterate until no more threads are
being created in the old group, and ignoring any threads that
die before we can move them.
It would be nicer to start the threads in the right cgroup to
begin with, but by default, all child threads are created in
the same cgroup as their parent, and we don't want vcpu child
threads in the emulator cgroup, so I don't see any good way
of avoiding the move. It would also be nice if the kernel were
to implement something like rename() as a way to atomically move
a group of threads from one cgroup to another, instead of forcing
a window where we have to read and parse the source, then format
and write back into the destination.
* src/util/vircgroup.c (virCgroupAddTaskStrController): Ignore
ESRCH, because a thread ended between read and write attempts.
(virCgroupMoveTask): Loop until all threads have moved.
Signed-off-by: Eric Blake <eblake@redhat.com>
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=927620
#kill -STOP `pidof qemu-kvm`
#virsh destroy $guest --graceful
error: Failed to destroy domain testVM
error: An error occurred, but the cause is unknown
With --graceful, SIGTERM always is emitted to kill driver
process, but it won't success till burning out waiting time
in case of process being stopped.
But domain destroy without --graceful can work, SIGKILL will
be emitted to the stopped process after 10 secs which always
kills a process even one that is currently stopped.
So report an error after burning out waiting time in this case.
Change bbe97ae968 caused the
QEMU driver to ignore ENOENT errors from cgroups, in order
to cope with missing /proc/cgroups. This is not good though
because many other things can cause ENOENT and should not
be ignored. The callers expect to see ENXIO when cgroups
are not present, so adjust the code to report that errno
when /proc/cgroups is missing
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In an upcoming patch, I need the way to safely transfer a nested
virJSON object out of its parent container for independent use,
even after the parent is freed.
* src/util/virjson.h (virJSONValueObjectRemoveKey): New function.
(_virJSONObject, _virJSONArray): Use correct type.
* src/util/virjson.c (virJSONValueObjectRemoveKey): Implement it.
* src/libvirt_private.syms (virjson.h): Export it.
* tests/jsontest.c (mymain): Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
network: static route support for <network>
This patch adds the <route> subelement of <network> to define a static
route. the address and prefix (or netmask) attribute identify the
destination network, and the gateway attribute specifies the next hop
address (which must be directly reachable from the containing
<network>) which is to receive the packets destined for
"address/(prefix|netmask)".
These attributes are translated into an "ip route add" command that is
executed when the network is started. The command used is of the
following form:
ip route add <address>/<prefix> via <gateway> \
dev <virbr-bridge> proto static metric <metric>
Tests are done to validate that the input data are correct. For
example, for a static route ip definition, the address must be a
network address and not a host address. Additional checks are added
to ensure that the specified gateway is directly reachable via this
network (i.e. that the gateway IP address is in the same subnet as one
of the IP's defined for the network).
prefix='0' is supported for both family='ipv4' address='0.0.0.0'
netmask='0.0.0.0' or prefix='0', and for family='ipv6' address='::',
prefix=0', although care should be taken to not override a desired
system default route.
Anytime an attempt is made to define a static route which *exactly*
duplicates an existing static route (for example, address=::,
prefix=0, metric=1), the following error message will be sent to
syslog:
RTNETLINK answers: File exists
This can be overridden by decreasing the metric value for the route
that should be preferred, or increasing the metric for the route that
shouldn't be preferred (and is thus in place only in anticipation that
the preferred route may be removed in the future). Caution should be
used when manipulating route metrics, especially for a default route.
Note: The use of the command-line interface should be replaced by
direct use of libnl so that error conditions can be handled better. But,
that is being left as an exercise for another day.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Signed-off-by: Laine Stump <laine@laine.org>
Currently we report a bogus error message when macvlan
creation fails:
error: Failed to start domain migtest
error: operation failed: Unable to create macvlan device
With this removed, we see the real error:
error: Failed to start domain migtest
error: Unable to get index for interface p31p1: No such device
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Use of the select() system call is inherantly dangerous since
applications will hit a buffer overrun if any FD number exceeds
the size of the select set size (typically 1024). Replace the
two uses of select() with poll() and use cfg.mk to ban any
future use of select().
NB: This changes the phyp driver so that it uses an infinite
timeout, instead of busy-waiting for 1ms at a time.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Found that I was unable to start existing domains after updating
to a kernel with no cgroups support
# zgrep CGROUP /proc/config.gz
# CONFIG_CGROUPS is not set
# virsh start test
error: Failed to start domain test
error: Unable to initialize /machine cgroup: Cannot allocate memory
virCgroupPartitionNeedsEscaping() correctly returns errno (ENOENT) when
attempting to open /proc/cgroups on such a system, but it was being
dropped in virCgroupSetPartitionSuffix().
Change virCgroupSetPartitionSuffix() to propagate errors returned by
its callees. Also check for ENOENT in qemuInitCgroup() when determining
if cgroups support is available.
Add a virFileNBDDeviceAssociate method, which given a filename
will setup a NBD device, using qemu-nbd as the server.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To correctly handle errors from readdir() you must set 'errno'
to zero before invoking it & check its value afterwards to
distinguish error from EOF.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The helper works for default sysfs_prefix, but for user specified
prefix, it doesn't work. (Detected when writing test cases. A later
patch will add the test cases for fc_host).
In case of the caller can pass a "prefix" (or "sysfs_prefix")
without the trailing slash, and Unix-Like system always eats
up the redundant "slash" in the filepath, let's add it explicitly.
Introduced by commit 244ce462e2, which refactored the helper for wwn
reading, however, it forgot to change the old "strndup" and "sizeof(buf)",
"sizeof(buf)" operates on the fixed length array ("buf") in the old code,
but now "buf" is a pointer.
Before the fix:
% virsh nodedev-dumpxml scsi_host5
<device>
<name>scsi_host5</name>
<parent>pci_0000_04_00_1</parent>
<capability type='scsi_host'>
<host>5</host>
<capability type='fc_host'>
<wwnn>2001001b</wwnn>
<wwpn>2101001b</wwpn>
<fabric_wwn>2001000d</fabric_wwn>
</capability>
</capability>
</device>
With the fix:
% virsh nodedev-dumpxml scsi_host5
<device>
<name>scsi_host5</name>
<parent>pci_0000_04_00_1</parent>
<capability type='scsi_host'>
<host>5</host>
<capability type='fc_host'>
<wwnn>0x2001001b32a9da4e</wwnn>
<wwpn>0x2101001b32a9da4e</wwpn>
<fabric_wwn>0x2001000dec9877c1</fabric_wwn>
</capability>
</capability>
</device>
Commit bfe7721d introduced a regression, but only on platforms
like FreeBSD that lack posix_fallocate and where mmap serves as
a nice fallback for safezero.
util/virfile.c: In function 'safezero':
util/virfile.c:837: error: 'PROT_READ' undeclared (first use in this function)
* src/util/virutil.c (includes): Move use of <sys/mman.h>...
* src/util/virfile.c (includes): ...to the file that uses mmap.
Signed-off-by: Eric Blake <eblake@redhat.com>
Apps using libvirt will often have code like
if (virXXXX() < 0) {
virErrorPtr err = virGetLastError();
fprintf(stderr, "Something failed: %s\n",
err && err->message ? err->message :
"unknown error");
return -1;
}
Checking for a NULL error object or message leads to very
verbose code. A virGetLastErrorMessage() helper from libvirt
can simplify this to
if (virXXXX() < 0) {
fprintf(stderr, "Something failed: %s\n",
virGetLastErrorMessage());
return -1;
}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
- provide virNetDevSetMAC() implementation based on SIOCSIFLLADDR
ioctl.
- adjust virNetDevExists() to check for ENXIO error because
FreeBSD throws it when device doesn't exist
Signed-off-by: Eric Blake <eblake@redhat.com>
These all existed before virfile.c was created, and for some reason
weren't moved.
This is mostly straightfoward, although the syntax rule prohibiting
write() had to be changed to have an exception for virfile.c instead
of virutil.c.
This movement pointed out that there is a function called
virBuildPath(), and another almost identical function called
virFileBuildPath(). They really should be a single function, which
I'll take care of as soon as I figure out what the arglist should look
like.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=851411https://bugzilla.redhat.com/show_bug.cgi?id=955500
The first problem was that virFileOpenAs was returning fd (-1) in one
of the error cases rather than ret (-errno), so the caller thought
that the error was EPERM rather than ENOENT.
The second problem was that some log messages in the general purpose
qemuOpenFile() function would always say "Failed to create" even if
the caller hadn't included O_CREAT (i.e. they were trying to open an
existing file).
This fixes virFileOpenAs to jump down to the error return (which
returns ret instead of fd) in the previously mentioned incorrect
failure case of virFileOpenAs(), removes all error logging from
virFileOpenAs() (since the callers report it), and modifies
qemuOpenFile to appropriately use "open" or "create" in its log
messages.
NB: I seriously considered removing logging from all callers of
virFileOpenAs(), but there is at least one case where the caller
doesn't want virFileOpenAs() to log any errors, because it's just
going to try again (qemuOpenFile()). We can't simply make a silent
variation of virFileOpenAs() though, because qemuOpenFile() can't make
the decision about whether or not it wants to retry until after
virFileOpenAs() has already returned an error code.
Likewise, I also considered changing virFileOpenAs() to return -1 with
errno set on return, and may still do that, but only as a separate
patch, as it obscures the intent of this patch too much.
The individual hypervisor drivers were directly referencing
APIs in virnodesuspend.c in their virDriverPtr struct. Separate
these methods, so there is always a wrapper in the hypervisor
driver. This allows the unused virConnectPtr args to be removed
from the virnodesuspend.c file. Again this will ensure that
ACL checks will only be performed on invocations that are
directly associated with public API usage.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virGetHostname() API has a bogus virConnectPtr
parameter. This is because virtualization drivers directly
reference this API in their virDriverPtr tables, tieing its
API design to the public virConnectGetHostname API design.
This also causes problems for access control checks since
these must only be done for invocations from the public
API, not internal invocation.
Remove the bogus virConnectPtr parameter, and make each
hypervisor driver provide a dedicated function for the
driver API impl. This will allow access control checks
to be easily inserted later.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are various methods named "virXXXXSecurityContext",
which are specific to SELinux. Rename them all to
"virXXXXSELinuxContext". They will still raise errors at
runtime if SELinux is not compiled in
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
While reviewing proposed VIR_STRDUP conversions, I've already noticed
several places that do:
if (str && VIR_STRDUP(dest, str) < 0)
which can be simplified by allowing str to be NULL (something that
strdup() doesn't allow). Meanwhile, code that wants to ensure a
non-NULL dest regardless of the source can check for <= 0.
Also, make it part of the VIR_STRDUP contract that macro arguments
are evaluated exactly once.
* src/util/virstring.h (VIR_STRDUP, VIR_STRDUP_QUIET, VIR_STRNDUP)
(VIR_STRNDUP_QUIET): Improve contract.
* src/util/virstring.c (virStrdup, virStrndup): Change return
conventions.
* docs/hacking.html.in: Document this.
* HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
VIR_APPEND_ELEMENT(array, size, elem) was not safe if the expression
for 'size' had side effects. While no one in the current code base
was trying to pass side effects, we might as well be robust and
explicitly document our intentions.
* src/util/viralloc.c (virInsertElementsN): Add special case.
* src/util/viralloc.h (VIR_APPEND_ELEMENT): Use it.
(VIR_ALLOC, VIR_ALLOC_N, VIR_REALLOC_N, VIR_EXPAND_N)
(VIR_RESIZE_N, VIR_SHRINK_N, VIR_INSERT_ELEMENT)
(VIR_DELETE_ELEMENT, VIR_ALLOC_VAR, VIR_FREE): Document
which macros are safe in the presence of side effects.
* docs/hacking.html.in: Document this.
* HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
The code adaptation is not done right now, but in subsequent patches.
Hence I am not implementing syntax-check rule as it would break
compilation. Developers are strongly advised to use these new macros.
They are similar to VIR_ALLOC() logic: VIR_STRDUP(dst, src) returns zero
on success, -1 otherwise. In case you don't want to report OOM error,
use the _QUIET variant of a macro.
POSIX says pthread_t is opaque. We can't guarantee if it is scaler
or a pointer, nor what size it is; and BSD differs from Linux.
We've also had reports of gcc complaining on attempts to cast it,
if we use a cast to the wrong type (for example, pointers have to be
cast to void* or intptr_t before being narrowed; while casting a
function return of scalar pthread_t to void* triggers a different
warning).
Give up on casts, and use unions to get at decent bits instead. And
rather than futz around with figuring which 32 bits of a potentially
64-bit pointer are most likely to be unique, convert the rest of
the code base to use 64-bit values when using a debug id.
Based on a report by Guido Günther against kFreeBSD, but with a
fix that doesn't regress commit 4d970fd29 for FreeBSD.
* src/util/virthreadpthread.c (virThreadSelfID, virThreadID): Use
union to get at a decent bit representation of thread_t bits.
* src/util/virthread.h (virThreadSelfID, virThreadID): Alter
signature.
* src/util/virthreadwin32.c (virThreadSelfID, virThreadID):
Likewise.
* src/qemu/qemu_domain.h (qemuDomainJobObj): Alter type of owner.
* src/qemu/qemu_domain.c (qemuDomainObjTransferJob)
(qemuDomainObjSetJobPhase, qemuDomainObjReleaseAsyncJob)
(qemuDomainObjBeginNestedJob, qemuDomainObjBeginJobInternal): Fix
clients.
* src/util/virlog.c (virLogFormatString): Likewise.
* src/util/vireventpoll.c (virEventPollInterruptLocked):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 776d49f4 added a static function that is only called
conditionally; leading to this compile error on mingw:
CC libvirt_util_la-virprocess.lo
../../src/util/virprocess.c:624:26: error: 'struct rlimit' declared inside parameter list [-Werror]
../../src/util/virprocess.c:624:26: error: its scope is only this definition or declaration, which is probably not what you want [-Werror]
../../src/util/virprocess.c:622:1: error: 'virProcessPrLimit' defined but not used [-Werror=unused-function]
* src/util/virprocess.c (virProcessPrLimit): Only declare
virProcessPrLimit when used.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 7c9a2d88 cleaned up too many headers; FreeBSD builds
failed due to:
util/virutil.c:556: warning: implicit declaration of function 'canonicalize_file_name'
(Not sure which Linux header leaked this declaration, but gnulib
only guarantees it in stdlib.h)
libvirt.c:956: warning: implicit declaration of function 'virGetUserConfigDirectory'
(Here, a build on Linux was picking up virutil.h indirectly via
one of the conditional driver headers, where that driver was not
being built on my FreeBSD setup)
* src/util/virutil.c (includes): Need <stdlib.h> for
canonicalize_file_name.
* src/libvirt.c (includes): Use "virutil.h" unconditionally,
rather than relying on conditional indirect inclusion.
Signed-off-by: Eric Blake <eblake@redhat.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
virPCIDeviceReattach and virPCIDeviceUnbindFromStub (called by
virPCIDeviceReattach) had previously required the name of the stub
driver as input. This is unnecessary, because the name of the driver
the device is currently bound to can be found by looking at the link:
/sys/bus/pci/dddd:bb:ss.ff/driver
Instead of requiring that the name of the expected stub driver name
and only unbinding if that one name is matched, we no longer take a
driver name in the arglist for either of these
functions. virPCIDeviceUnbindFromStub just compares the name of the
currently bound driver to a list of "well known" stubs (right now
contains "pci-stub" and "vfio-pci" for qemu, and "pciback" for xen),
and only performs the unbind if it's one of those devices.
This allows virsh nodedevice-reattach to work properly across a
libvirtd restart, and fixes a couple of cases where we were
erroneously still hard-coding "pci-stub" as the drive name.
For some unknown reason, virPCIDeviceReattach had been calling
modprobe on the stub driver prior to unbinding the device. This was
problematic because we no longer know the name of the stub driver in
that function. However, it is pointless to probe for the stub driver
at that time anyway - because the device is bound to the stub driver,
we are guaranteed that it is already loaded, and so that call to
modprobe has been removed.
On cygwin, compilation failed because SIOCSIFHWADDR is undefined.
* src/util/virnetdev.c (virNetDevSetMAC): Cygwin can query but not
set mac address.
Signed-off-by: Eric Blake <eblake@redhat.com>
FreeBSD (and maybe other BSDs) have different member
names in struct ifreq when compared to Linux, such as:
- uses ifr_data instead of ifr_newname for setting
interface names
- uses ifr_index instead of ifr_ifindex for interface
index
Also, add a check for SIOCGIFHWADDR for virNetDevValidateConfig().
Use AF_LOCAL if AF_PACKET is not available.
Signed-off-by: Eric Blake <eblake@redhat.com>
These fixes solve a compilation failure on FreeBSD:
util/virnetdevtap.c: In function 'virNetDevTapGetName':
util/virnetdevtap.c:56: warning: unused parameter 'tapfd' [-Wunused-parameter]
util/virnetdevtap.c:56: warning: unused parameter 'ifname' [-Wunused-parameter]
* src/util/virnetdevtap.c (virNetDevTapGetName): Add attributes
when TUNGETIFF is not present.
Signed-off-by: Eric Blake <eblake@redhat.com>
This will be used on a tap file descriptor returned by the bridge helper
to populate the <target> element, because the helper does not provide
the interface name.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This patch adds two sets of functions:
1) lower level virProcessSet*() functions that will immediately set
the RLIMIT_MEMLOCK. RLIMIT_NPROC, or RLIMIT_NOFILE of either the
current process (using setrlimit()) or any other process (using
prlimit()). "current process" is indicated by passing a 0 for pid.
2) functions for virCommand* that will setup a virCommand object to
set those limits at a later time just after it has forked a new
process, but before it execs the new program.
configure.ac has prlimit and setrlimit added to the list of functions
to check for, and the low level functions log an "unsupported" error)
on platforms that don't support those functions.
If a user cgroup name begins with "cgroup.", "_" or with any of
the controllers from /proc/cgroups followed by a dot, then they
need to be prefixed with a single underscore. eg if there is
an object "cpu.service", then this would end up as "_cpu.service"
in the cgroup filesystem tree, however, "waldo.service" would
stay "waldo.service", at least as long as nobody comes up with
a cgroup controller called "waldo".
Since we require a '.XXXX' suffix on all partitions, there is
no scope for clashing with the kernel 'tasks' and 'release_agent'
files.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If the partition named passed in the XML does not already have
a suffix, ensure it gets a '.partition' added to each component.
The exceptions are /machine, /user and /system which do not need
to have a suffix, since they are fixed partitions at the top
level.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Recently we changed to create VM cgroups with the naming pattern
$VMNAME.$DRIVER.libvirt. Following discussions with the systemd
community it was decided that only having a single '.' in the
names is preferrable. So this changes the naming scheme to be
$VMNAME.libvirt-$DRIVER. eg for LXC 'mycontainer.libvirt-lxc' or
for KVM 'myvm.libvirt-qemu'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This can be set when the virPCIDevice is created and placed on a list,
then used later when traversing the list to determine which stub
driver to bind/unbind for managed devices.
The existing Detach and Attach functions' signatures haven't been
changed (they still accept a stub driver name in the arg list), but if
the arg list has NULL for stub driver and one is available in the
device's object, that will be used. (we may later deprecate and remove
the arg from those functions).
POSIX says that both basename() and dirname() may return static
storage (aka they need not be thread-safe); and that they may but
not must modify their input argument. Furthermore, <libgen.h>
is not available on all platforms. For these reasons, you should
never use these functions in a multi-threaded library.
Gnulib instead recommends a way to avoid the portability nightmare:
gnulib's "dirname.h" provides useful thread-safe counterparts. The
obvious dir_name() and base_name() are GPL (because they malloc(),
but call exit() on failure) so we can't use them; but the LGPL
variants mdir_name() (malloc's or returns NULL) and last_component
(always points into the incoming string without modifying it,
differing from basename semantics only on corner cases like the
empty string that we shouldn't be hitting in the first place) are
already in use in libvirt. This finishes the swap over to the safe
functions.
* cfg.mk (sc_prohibit_libgen): New rule.
* src/util/vircgroup.c: Fix offenders.
* src/parallels/parallels_storage.c (parallelsPoolAddByDomain):
Likewise.
* src/parallels/parallels_network.c (parallelsGetBridgedNetInfo):
Likewise.
* src/node_device/node_device_udev.c (udevProcessSCSIHost)
(udevProcessSCSIDevice): Likewise.
* src/storage/storage_backend_disk.c
(virStorageBackendDiskDeleteVol): Likewise.
* src/util/virpci.c (virPCIGetDeviceAddressFromSysfsLink):
Likewise.
* src/util/virstoragefile.h (_virStorageFileMetadata): Avoid false
positive.
Signed-off-by: Eric Blake <eblake@redhat.com>
Refactoring done in 19c6ad9ac7 didn't
correctly take into account the order cgroup limit modification needs to
be done in. This resulted into errors when decreasing the limits.
The operations need to take place in this order:
decrease hard limit
change swap hard limit
or
change swap hard limit
increase hard limit
This patch also fixes the check if the hard_limit is less than
swap_hard_limit to print better error messages. For this purpose I
introduced a helper function virCompareLimitUlong to compare limit
values where value of 0 is equal to unlimited. Additionally the check is
now applied also when the user does not provide all of the tunables
through the API and in that case the currently set values are used.
This patch resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=950478
Create the utility function virSocketAddrGetIpPrefix() to
determine the prefix for this network. The code in this
function was adapted from virNetworkIpDefPrefix().
Update virNetworkIpDefPrefix() in src/conf/network_conf.c
to use the new utility function.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
http://www.uhv.edu/ac/newsletters/writing/grammartip2009.07.01.htm
(and several other sites) give hints that 'onto' is best used if
you can also add 'up' just before it and still make sense. In many
cases in the code base, we really want the two-word form, or even
a simplification to just 'on' or 'to'.
* docs/hacking.html.in: Use correct 'on to'.
* python/libvirt-override.c: Likewise.
* src/lxc/lxc_controller.c: Likewise.
* src/util/virpci.c: Likewise.
* daemon/THREADS.txt: Use simpler 'on'.
* docs/formatdomain.html.in: Better usage.
* docs/internals/rpc.html.in: Likewise.
* src/conf/domain_event.c: Likewise.
* src/rpc/virnetclient.c: Likewise.
* tests/qemumonitortestutils.c: Likewise.
* HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
When running unprivileged, virSetUIDGIDWithCaps will fail because it
tries to add the requested capabilities to the permitted and effective
sets.
Detect this case, and invoke the child with cleared permitted and
effective sets. If it is a setuid program, it will get them.
Some care is needed also because you cannot drop capabilities from the
bounding set without CAP_SETPCAP. Because of that, ignore errors from
setting the bounding set.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The need_prctl variable is not really needed. If it is false,
capng_apply will be called twice with the same set, causing
a little extra work but no problem. This keeps the code a bit
simpler.
It is also clearer to invoke capng_apply(CAPNG_SELECT_BOUNDS)
separately, to make sure it is done while we have CAP_SETPCAP.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The recent qemu requires "0x" prefix for the disk wwn, this patch
changes virValidateWWN to allow the prefix, and prepend "0x" if
it's not specified. E.g.
qemu-kvm: -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,wwn=6000c60016ea71ad:
Property 'scsi-hd.wwn' doesn't take value '6000c60016ea71ad'
Though it's a qemu regression, but it's nice to allow the prefix,
and doesn't hurt for us to always output "0x".
Detected by a simple Shell script:
for i in $(git ls-files -- '*.[ch]'); do
awk 'BEGIN {
fail=0
}
/# *include.*\.h/{
match($0, /["<][^">]*[">]/)
arr[substr($0, RSTART+1, RLENGTH-2)]++
}
END {
for (key in arr) {
if (arr[key] > 1) {
fail=1
printf("%d %s\n", arr[key], key)
}
}
if (fail == 1)
exit 1
}' $i
if test $? != 0; then
echo "Duplicate header(s) in $i"
fi
done;
A later patch will add the syntax-check to avoid duplicate
headers.
Fix the error
util/vircgroup.c: In function 'virCgroupNewDomainPartition':
util/vircgroup.c:1299:11: error: declaration of 'dirname' shadows a global declaration [-Werror=shadow]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Add a virCgroupIsolateMount method which looks at where the
current process is place in the cgroups (eg /system/demo.lxc.libvirt)
and then remounts the cgroups such that this sub-directory
becomes the root directory from the current process' POV.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a cgroup controller is co-mounted with another, eg
/sys/fs/cgroup/cpu,cpuacct
Then it is a requirement that there exist symlinks at
/sys/fs/cgroup/cpu
/sys/fs/cgroup/cpuacct
pointing to the real mount point. Add support to virCgroupPtr
to detect and track these symlinks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virCgroupNewDriver method had a 'bool privileged' param.
If a false value was ever passed in, it would simply not
work, since non-root users don't have any privileges to create
new cgroups. Just delete this broken code entirely and make
the QEMU driver skip cgroup setup in non-privileged mode
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A resource partition is an absolute cgroup path, ignoring the
current process placement. Expose a virCgroupNewPartition API
for constructing such cgroups
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently if virCgroupMakeGroup fails, we can get in a situation
where some controllers have been setup, but others not. Ensure
we call virCgroupRemove to remove what we've done upon failure
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virCgroupPtr struct contains 3 pieces of
information
- path - path of the cgroup, relative to current process'
cgroup placement
- placement - current process' placement in each controller
- mounts - mount point of each controller
When reading/writing cgroup settings, the path & placement
strings are combined to form the file path. This approach
only works if we assume all cgroups will be relative to
the current process' cgroup placement.
To allow support for managing cgroups at any place in the
heirarchy a change is needed. The 'placement' data should
reflect the absolute path to the cgroup, and the 'path'
value should no longer be used to form the paths to the
cgroup attribute files.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Rename all the virCgroupForXXX methods to use the form
virCgroupNewXXX since they are all constructors. Also
make sure the output parameter is the last one in the
list, and annotate all pointers as non-null. Fix up
all callers, and make sure they use true/false not 0/1
for the boolean parameters
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The definition of structs for cgroups are kept in vircgroup.c since
they are intended to be private from users of the API. To enable
effective testing, however, they need to be accessible. To address
the latter issue, without compronmising the former, this introduces
a new vircgrouppriv.h file to hold the struct definitions.
To prevent other files including this private header, it requires
that __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ be defined before inclusion
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virCgroupForDriver method recently gained an 'int controllers'
parameter, but the stub impl did not
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Though they are the same thing, mixed use of them is uncomfortable.
"unsigned" is used a lot in old codes, this just tries to change the
ones in utils.
Commit 9a3ff01d7f (which was ACKed at
the end of January, but for some reason didn't get pushed until during
the 1.0.4 freeze) fixed the logic in virPCIGetVirtualFunctions().
Unfortunately, a typo in the fix (replacing VIR_REALLOC_N with
VIR_ALLOC_N during code movement) caused not only a memory leak, but
also resulted in most of the elements of the result array being
replaced with NULL. virNetDevGetVirtualFunctions() assumed (and I think
rightly so) that virPCIGetVirtualFunctions() wouldn't return any NULL
elements in the array, so it ended up segfaulting.
This was found when attempting to use a virtual network with an
auto-created pool of SRIOV VFs, e.g.:
<forward mode='hostdev' managed='yes'>
<pf dev='eth4'/>
</forward>
(the pool of PCI addresses is discovered by calling
virNetDevGetVirtualFunctions() on the PF dev).
Even though http://libvirt.org/formatdomain.html#elementsMetadata
states that it requires RFC4122 compliance UUIDs that are generated
by virUUIDGenerate() are not. Following patch modifies generated
UUIDs to conform to rules described in RFC.
Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz>
The virCgroupMounted method is badly named, since a controller can be
mounted, but disabled in the current object. Rename the method to be
virCgroupHasController. Also make it tolerant to a NULL virCgroupPtr
and out-of-range controller index, to avoid duplication of these
checks in all callers
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This finds the parent for vHBA by iterating over all the HBA
which supports vport_ops capability on the host, and return
the first one which is online, not saturated (vports in use
is less than max_vports).
The helper iterates over sysfs, to find out the matched scsi host
name by comparing the wwnn,wwpn pair. It will be used by checkPool
and refreshPool of storage scsi backend. New helper getAdapterName
is introduced in storage_backend_scsi.c, which uses the new util
helper virGetFCHostNameByWWN to get the fc_host adapter name.
The current way virObject instances are allocated using
VIR_ALLOC_N causes alignment warnings
util/virobject.c: In function 'virObjectNew':
util/virobject.c:195:11: error: cast increases required alignment of target type [-Werror=cast-align]
Changing to use VIR_ALLOC_VAR will avoid the need todo
the casts entirely.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virNetlinkCommand() method takes an 'unsigned char **'
parameter to be filled with the received netlink message.
The callers then immediately cast this to 'struct nlmsghdr',
triggering (bogus) warnings about increasing alignment
requirements
util/virnetdev.c: In function 'virNetDevLinkDump':
util/virnetdev.c:1300:12: warning: cast increases required alignment of target type [-Wcast-align]
resp = (struct nlmsghdr *)*recvbuf;
^
util/virnetdev.c: In function 'virNetDevSetVfConfig':
util/virnetdev.c:1429:12: warning: cast increases required alignment of target type [-Wcast-align]
resp = (struct nlmsghdr *)recvbuf;
Since all callers cast to 'struct nlmsghdr' we can avoid
the warning problem entirely by simply changing the
signature of virNetlinkCommand to return a 'struct nlmsghdr **'
instead of 'unsigned char **'. The way we do the cast inside
virNetlinkCommand does not have any alignment issues.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Playing games with field offsets in a struct causes all sorts
of alignment warnings on ARM platforms
util/virkeycode.c: In function '__virKeycodeValueFromString':
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
(*(typeof(field_type) *)((char *)(object) + field_offset))
^
util/virkeycode.c:91:28: note: in expansion of macro 'getfield'
const char *name = getfield(virKeycodes + i, const char *, name_offset);
^
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
(*(typeof(field_type) *)((char *)(object) + field_offset))
^
util/virkeycode.c:94:20: note: in expansion of macro 'getfield'
return getfield(virKeycodes + i, unsigned short, code_offset);
^
util/virkeycode.c: In function '__virKeycodeValueTranslate':
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
(*(typeof(field_type) *)((char *)(object) + field_offset))
^
util/virkeycode.c:127:13: note: in expansion of macro 'getfield'
if (getfield(virKeycodes + i, unsigned short, from_offset) == key_value)
^
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
(*(typeof(field_type) *)((char *)(object) + field_offset))
^
util/virkeycode.c:128:20: note: in expansion of macro 'getfield'
return getfield(virKeycodes + i, unsigned short, to_offset);
There is no compelling reason to use a struct for the keycode
tables. It can easily just use an array of arrays instead,
avoiding all alignment problems
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently when getting an instance of virCgroupPtr we will
create the path in all cgroup controllers. Only at the virt
driver layer are we attempting to filter controllers. This
is bad because the mere act of creating the dirs in the
controllers can have a functional impact on the kernel,
particularly for performance.
Update the virCgroupForDriver() method to accept a bitmask
of controllers to use. Only create dirs in the controllers
that are requested. When creating cgroups for domains,
respect the active controller list from the parent cgroup
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virCgroupGetAppRoot is not clear in its meaning. Change
to virCgroupForSelf to highlight that this returns the
cgroup config for the caller's process
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The Raspberry Pi runs the armv6l architecture and apparently
people are trying to run libvirt LXC on it. So we should allow
that as a valid arch
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Implement the bare minimal sysinfo for ARM platforms by
reading the CPU models from /proc/cpuinfo
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Format the address using the helper instead of having similar code in
multiple places.
This patch also fixes leak of the MAC address string in
ebtablesRemoveForwardAllowIn() and ebtablesAddForwardAllowIn() in
src/util/virebtables.c
The domain XML generator creates the mac addres strings with lowercase
strings with a separate piece of code. This patch changes the formating
helper to do the same stuff to allow using it to normalize a string
provided by the user. After this change some of the tests that are
outputing the mac address will need to be changed.
iptables-1.4.18 removed the long deprecated "state" match.
Use "conntrack" instead in forwarding rules.
Fixes openSUSE bug https://bugzilla.novell.com/811251#811251.
When we write a log message into a log, we separate thread ID from
timestamp using ": ". However, when storing the message into the ring
buffer, we omitted the separator, e.g.:
2013-02-27 11:49:11.852+00003745: ...
virPCIGetVirtualFunctions returns 0 even if there is no "virtfn"
entry under the device sysfs path.
And virPCIGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!virPCIGetVirtualFunctions(syspath,
&data->pci_dev.virtual_functions,
&data->pci_dev.num_virtual_functions) ||
data->pci_dev.num_virtual_functions > 0)
data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with "virtual_function" cap.
However, this results in a VF will aslo get "virtual_function" cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so virPCIGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int ret = virPCIGetVirtualFunctions(syspath,
&data->pci_dev.virtual_functions,
&data->pci_dev.num_virtual_functions);
if (ret < 0 )
goto out;
if (data->pci_dev.num_virtual_functions > 0)
data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
This abstracts nodeDeviceVportCreateDelete as an util function
virManageVport, which can be further used by later storage patches
(to support persistent vHBA, I don't want to create the vHBA
using the public API, which is not good).
This adds two util functions (virIsCapableFCHost and virIsCapableVport),
and rename helper check_fc_host_linux as detect_scsi_host_caps,
check_capable_vport_linux is removed, as it's abstracted to the util
function virIsCapableVport. detect_scsi_host_caps nows detect both
the fc_host and vport_ops capabilities. "stat(2)" is replaced with
"access(2)" for saving.
* src/util/virutil.h:
- Declare virIsCapableFCHost and virIsCapableVport
* src/util/virutil.c:
- Implement virIsCapableFCHost and virIsCapableVport
* src/node_device/node_device_linux_sysfs.c:
- Remove check_capable_vport_linux
- Rename check_fc_host_linux as detect_scsi_host_caps, and refactor
it a bit to detect both fc_host and vport_os capabilities
* src/node_device/node_device_driver.h:
- Change/remove the related declarations
* src/node_device/node_device_udev.c: (Use detect_scsi_host_caps)
* src/node_device/node_device_hal.c: (Likewise)
* src/node_device/node_device_driver.c (Likewise)
"open_wwn_file" in node_device_linux_sysfs.c is redundant, on one
hand it duplicates work of virFileReadAll, on the other hand, it's
waste to use a function for it, as there is no other users of it.
So I don't see why the file opening work cannot be done in
"read_wwn_linux".
"read_wwn_linux" can be abstracted as an util function. As what all
it does is to read the sysfs entry.
So this patch removes "open_wwn_file", and abstract "read_wwn_linux"
as an util function "virReadFCHost" (a more general name, because
after changes, it can read each of the fc_host entry now).
* src/util/virutil.h: (Declare virReadFCHost)
* src/util/virutil.c: (Implement virReadFCHost)
* src/node_device/node_device_linux_sysfs.c: (Remove open_wwn_file,
and read_wwn_linux)
src/node_device/node_device_driver.h: (Remove the declaration of
read_wwn_linux, and the related macros)
src/libvirt_private.syms: (Export virReadFCHost)
Some code mistakenly called virIdentityOnceInit directly
instead of virIdentityInitialize(). This meant that one-time
initializer was run many times with predictably bad results.
The virNetSocket & virIdentity classes accidentally got some
conditionals using HAVE_SELINUX instead of WITH_SELINUX.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Intend to reduce the redundant code,use virNumaSetupMemoryPolicy
to replace virLXCControllerSetupNUMAPolicy and
qemuProcessInitNumaMemoryPolicy.
This patch also moves the numa related codes to the
file virnuma.c and virnuma.h
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
qemuGetNumadAdvice will be used by LXC driver, rename
it to virNumaGetAutoPlacementAdvice and move it to virnuma.c
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
If no user identity is available, some operations may wish to
use the system identity. ie the identity of the current process
itself. Add an API to get such an identity.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To allow any internal API to get the current identity, add APIs
to associate a virIdentityPtr with the current thread, via a
thread local
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduce a local object virIdentity for managing security
attributes used to form a client application's identity.
Instances of this object are intended to be used as if they
were immutable, once created & populated with attributes
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We've already scrubbed for comparisons of 'uid_t == -1' (which fail
on platforms where uid_t is a u16), but another one snuck in.
* src/util/virutil.c (virSetUIDGIDWithCaps): Correct uid comparison.
* cfg.mk (sc_prohibit_risky_id_promotion): New rule.
My commit 7a2e845a86 (and its
prerequisites) managed to effectively ignore the
clear_emulator_capabilities setting in qemu.conf (visible in the code
as the VIR_EXEC_CLEAR_CAPS flag when qemu is being exec'ed), with the
result that the capabilities are always cleared regardless of the
qemu.conf setting. This patch fixes it by passing the flag through to
virSetUIDGIDWithCaps(), which uses it to decide whether or not to
clear existing capabilities before adding in those that were
requested.
Note that the existing capabilities are *always* cleared if the new
process is going to run as non-root, since the whole point of running
non-root is to have the capabilities removed (it's still possible to
maintain individual capabilities as needed using the capBits argument
though).
In debug mode, the bug failed to start vm
error: Failed to start domain rhel5u9
error: internal error Out of space while reading console log output:
...
Add a virThreadCancel function. This functional is inherently
dangerous and not something we want to use in general, but
integration with SELinux requires that we provide this stub.
We leave out any Win32 impl to discourage further use and
because obviously SELinux isn't enabled on Win32
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When reading log output from QEMU/LXC we need to skip over any
libvirt log messages. Currently the QEMU driver checks for a
fixed string, but this is better done with a regex. Add a method
virLogProbablyLogMessage to do a regex check
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virNetDevSetupControlFull function was protected by a
conditional on SIOCBRADDBR, which is bogus since it does
not use that symbol. Update the conditionals around all
callers to do stricter checks to ensure we always build
succesfully
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The RHEL4 vintage header files do not define GET_VLAN_VID_CMD.
Conditionally define it in our source, since the kernel can
raise a runtime error if it isn't supported
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The loop.h on RHEL4 is broken and cannot be imported. We already
detect this in configure as a side-effect of looking for whether
LO_FLAGS_AUTOCLEAR is available. We protected the impl with
HAVE_DECL_LO_FLAGS_AUTOCLEAR, but not the header import
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 0df3e89 only touched the header, but the .c file had the
same shadowing potential.
* src/util/viralloc.c (virDeleteElementsN): s/remove/toremove/ to
match the header.
A value which is equal to a integer maximum such as LLONG_MAX is
a valid integer value.
The patch fix the following error:
1, virsh memtune vm --swap-hard-limit -1
2, virsh start vm
In debug mode, it shows error like:
virScaleInteger:1813 : numerical overflow:\
value too large: 9007199254740991KiB
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=912021
Without error handler set, virDefaultErrorFunc will be called, the
error message is prefixed with "libvir:". It become a little better
by using prefix "libvirt:" when working with upper application.
For example:
1, stop libvirtd daemon
2, run virt-top.
libvir: XML-RPC error : Failed to connect \
socket to '/var/run/libvirt/libvirt-sock-ro': \
No such file or directory
libvirt: VIR_ERR_SYSTEM_ERROR: VIR_FROM_RPC: \
Failed to connect socket to '/var/run/libvirt/libvirt-sock-ro': \
No such file or directory
Currently, after we removed the qemu driver lock, it may happen
that two or more threads will start up a machine with macvlan and
race over virNetDevMacVLanCreateWithVPortProfile(). However,
there's a racy section in which we are generating a sequence of
possible device names and detecting if they exits. If we found
one which doesn't we try to create a device with that name.
However, the other thread is doing just the same. Assume it will
succeed and we must therefore fail. If this happens more than 5
times (which in massive parallel startup surely will) we return
-1 without any error reported. This patch is a simple hack to
both of these problems. It introduces a mutex, so only one thread
will enter the section, and if it runs out of possibilities,
error is reported. Moreover, the number of retries is raised to 20.
The code for putting the emulator threads in a separate cgroup
would spam the logs with warnings
2013-02-27 16:08:26.731+0000: 29624: warning : virCgroupMoveTask:887 : no vm cgroup in controller 3
2013-02-27 16:08:26.731+0000: 29624: warning : virCgroupMoveTask:887 : no vm cgroup in controller 4
2013-02-27 16:08:26.732+0000: 29624: warning : virCgroupMoveTask:887 : no vm cgroup in controller 6
This is because it has only created child cgroups for 3 of the
controllers, but was trying to move the processes from all the
controllers. The fix is to only try to move threads in the
controllers we actually created. Also remove the warning and
make it return a hard error to avoid such lazy callers in the
future.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
Explicitly cast the magic -1 to the appropriate type.
Signed-off-by: Philipp Hahn <hahn@univention.de>
The uid_t|gid_t values are explicitly casted to "unsigned long", but the
printf() still used "%d", which is for signed values.
Change the format to "%u".
Signed-off-by: Philipp Hahn <hahn@univention.de>
Originally, only a host name was used to associate a
DHCPv6 request with a specific IPv6 address. Further testing
demonstrates that this is an unreliable method and, instead,
a client-id or DUID needs to be used. According to DHCPv6
standards, this id can be a duid-LLT, duid-LL, or duid-UUID
even though dnsmasq will accept almost any text string.
Although validity checking of a specified string makes sure it is
hexadecimal notation with bytes separated by colons, there is no
rigorous check to make sure it meets the standard.
Documentation and schemas have been updated.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Signed-off-by: Laine Stump <laine@laine.org>
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>
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>
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>
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.
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.
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.
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.
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.
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.
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.
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").
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).
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.
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.
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).
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().
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).
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.
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.
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.
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>
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().
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.
To allow modifications to the lists to be synchronized, convert
virPCIDeviceList and virUSBDeviceList into virObjectLockable
classes. The locking, however, will not be self-contained. The
users of these classes will have to call virObjectLock/Unlock
in the critical regions.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 34e8f63a32 introduced support for catching errors from
libvirt iohelper. However, at those times there wasn't such fancy
API as virCommandDoAsyncIO(), so everything has to be implemented
on our own. But since we do have the API now, we can use it and
drop our implementation then.
Currently, if we want to feed stdin, or catch stdout or stderr of a
virCommand we have to use virCommandRun(). When using virCommandRunAsync()
we have to register FD handles by hand. This may lead to code duplication.
Hence, introduce an internal API, which does this automatically within
virCommandRunAsync(). The intended usage looks like this:
virCommandPtr cmd = virCommandNew*(...);
char *buf = NULL;
...
virCommandSetOutputBuffer(cmd, &buf);
virCommandDoAsyncIO(cmd);
if (virCommandRunAsync(cmd, NULL) < 0)
goto cleanup;
...
if (virCommandWait(cmd, NULL) < 0)
goto cleanup;
/* @buf now contains @cmd's stdout */
VIR_DEBUG("STDOUT: %s", NULLSTR(buf));
...
cleanup:
VIR_FREE(buf);
virCommandFree(cmd);
Note, that both stdout and stderr buffers may change until virCommandWait()
returns.
QEMU is fully capable of handling VDI images and we just refuse to
work with them. As qemu-img knows and supports this, there should be
no problem with this addition.
This is of course, just basic functionality, without searching for any
backing files, etc.
Some files have the magic shifted to some offset other than 0, so we
have to support that. I also cleaned up some lines to be more
readable and added missing magic for iso file format.
Commit 6094ad7b (0.9.3 release) promoted several functions from
internal to public, but forgot to fix the documentation generator
to provide details about those functions.
For an example of what this fixes, look at:
file:///path/to/libvirt/docs/html/libvirt-libvirt.html#virEventAddHandle
before and after the patch.
* docs/apibuild.py (ignored_functions): Don't ignore functions
that were turned into official API.
* src/util/virevent.c: Fix comments to pass through parser.
Way back when I started making changes for Coverity messages my first set
were to a bunch of CHECKED_RETURN errors. In particular virAsprintf() had
a few callers that Coverity noted didn't check their return (although some
did check if the buffer being printed to was NULL or not).
It was suggested at the time as a further patch an ATTRIBUTE_RETURN_CHECK
should be added to virAsprintf(), see:
https://www.redhat.com/archives/libvir-list/2013-January/msg00120.html
This patch does that and fixes a few more instances not found by Coverity
that failed the check.
Setting the log output prefix to 0 is not supported and in fact results
in the following message:
warning : virLogParseOutputs:1021 : Ignoring invalid log output setting.
This patch changes the name of the @sep argument to @terminator and
clarifies it's usage. This patch also explicitly documents that
whitespace can't be used as @terminator as it is skipped multiple times
in the implementation.
When building with static analysis enabled, we turn on attribute
nonnull checking. However, this caused the build to fail with:
../../src/util/virobject.c: In function 'virObjectOnceInit':
../../src/util/virobject.c:55:40: error: null argument where non-null required (argument 1) [-Werror=nonnull]
Creation of the virObject class is the one instance where the
parent class is allowed to be NULL. Making things conditional
will let us keep static analysis checking for all other .c file
callers, without breaking the build on this one exception.
* src/util/virobject.c: Define witness.
* src/util/virobject.h (virClassNew): Use it to force most callers
to pass non-null parameter.
The Coverity static analyzer was generating many false positives for the
unary operation inside the VIR_FREE() definition as it was trying to evaluate
the else portion of the "?:" even though the if portion was (1).
Signed-off-by: Eric Blake <eblake@redhat.com>
Currently, whenever somebody calls saferead() on nonblocking FD
(safewrite() is totally interchangeable for purpose of this message)
he might get wrong return value. For instance, in the first iteration
some data is read. The number of bytes read is stored into local
variable 'nread'. However, in next iterations we can get -1 from
read() with errno == EAGAIN, in which case the -1 is returned despite
fact some data has already been read. So the caller gets confused.
Bare read() should be used for nonblocking FD.
Working with virTypedParameters in clients written in C is ugly and
requires all clients to duplicate the same code. This set of APIs makes
this code for manipulating with virTypedParameters integral part of
libvirt so that all clients may benefit from it.
A build on FreeBSD failed with:
util/virportallocator.c:108: error: storage size of 'addr' isn't known
util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function)
It turns out that while POSIX allows sockaddr_in to leak in through
<arpa/inet.h> (the way Linux does it), it is not mandatory, and
conforming applications are required to get it through <netinet/in.h>.
* src/util/virportallocator.c: Include header for struct
sockaddr_in.
* tests/virportallocatortest.c: Likewise.
The QEMU driver default max port is 65535, but it then increments
this by 1 to 65536. This maps to 0 in an unsigned short :-( This
was apparently done so that for() loops could use "< max" instead
of "<= max". Remove this insanity and just make the loop do the
right thing.
A great many virObject instances require a mutex, so introduce
a convenient class for this which provides a mutex. This avoids
repeating the tedious init/destroy code
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently all classes must directly inherit from virObject.
This allows for arbitrarily deep hierarchy. There's not much
to this aside from chaining up the 'dispose' handlers from
each class & providing APIs to check types.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Make cpuset local to the while loop and free it once done with it each
time through the loop. Add a sa_assert() to virBitmapParse() to keep Coverity
from believing there could be a negative return and possible resource leak.
Commit c308a9ae was incomplete; it resolved the configure failure,
but not a later build failure.
* src/util/virnetdevbridge.c: Include pre-req header.
* configure.ac (AC_CHECK_HEADERS): Prefer standard in.h over
non-standard ip6.h.
There's no need to do lots of readlink() calls to canonicalize
a name if we're only going to use stat() on it, since stat()
already chases symlinks.
* src/util/virutil.c (virGetDeviceID): Let stat() do the symlink
chasing.
Pass stub driver name directly to pciDettachDevice and pciReAttachDevice to fit
for different libvirt drivers. For example, qemu driver prefers pci-stub, but
Xen prefers pciback.
Signed-off-by: Chunyan Liu <cyliu@suse.com>
"virGetDeviceID" could be used across the sources, but it doesn't
relate with this series, and could be done later.
* src/util/virutil.h: (Declare virGetDeviceID, and
vir{Get,Set}DeviceUnprivSGIO)
* src/util/virutil.c: (Implement virGetDeviceID and
vir{Get,Set}DeviceUnprivSGIO)
* src/libvirt_private.syms: Export private symbols of upper helpers
This is an adjustment to the fix for
https://bugzilla.redhat.com/show_bug.cgi?id=889319
to account for two bonehead mistakes I made.
commit ac2797cf2a attempted to fix a
problem with netlink in newer kernels requiring an extra attribute
with a filter flag set in order to receive an IFLA_VFINFO_LIST from
netlink. Unfortunately, the #ifdef that protected against compiling it
in on systems without the new flag went a bit too far, assuring that
the new code would *never* be compiled, and even if it had, the code
was incorrect.
The first problem was that, while some IFLA_* enum values are also
their existence at compile time, IFLA_EXT_MASK *isn't* #defined, so
checking to see if it's #defined is not a valid method of determining
whether or not to add the attribute. Fortunately, the flag that is
being set (RTEXT_FILTER_VF) *is* #defined, and it is never present if
IFLA_EXT_MASK isn't, so it's sufficient to just check for that flag.
And to top it off, due to the code not actually compiling when I
thought it did, I didn't realize that I'd been given the wrong arglist
to nla_put() - you can't just send a const value to nla_put, you have
to send it a pointer to memory containing what you want to add to the
message, along with the length of that memory.
This time I've actually sent the patch over to the other machine
that's experiencing the problem, applied it to the branch being used
(0.10.2) and verified that it works properly, i.e. it does fix the
problem it's supposed to fix. :-/
To bring in line with new naming practice, rename the=
src/util/cgroup.{h,c} files to vircgroup.{h,c}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This patch resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=889319
When assigning an SRIOV virtual function to a guest using "intelligent
PCI passthrough" (<interface type='hostdev'>, which sets the MAC
address and vlan tag of the VF before passing its info to qemu),
libvirt first learns the current MAC address and vlan tag by sending
an NLM_F_REQUEST message for the VF's PF (physical function) to the
kernel via a NETLINK_ROUTE socket (see virNetDevLinkDump()); the
response message's IFLA_VFINFO_LIST section is examined to extract the
info for the particular VF being assigned.
This worked fine with kernels up until kernel commit
115c9b81928360d769a76c632bae62d15206a94a (first appearing in upstream
kernel 3.3) which changed the ABI to not return IFLA_VFINFO_LIST in
the response until a newly introduced IFLA_EXT_MASK field was included
in the request, with the (newly introduced, of course) RTEXT_FILTER_VF
flag set.
The justification for this ABI change was that new fields had been
added to the VFINFO, causing NLM_F_REQUEST messages to fail on systems
with large numbers of VFs if the requesting application didn't have a
large enough buffer for all the info. The idea is that most
applications doing an NLM_F_REQUEST don't care about VFINFO anyway, so
eliminating it from the response would lower the requirements on
buffer size. Apparently, the people who pushed this patch made the
mistaken assumption that iproute2 (the "ip" command) was the only
package that used IFLA_VFINFO_LIST, so it wouldn't break anything else
(and they made sure that iproute2 was fixed.
The logic of this "fix" is debatable at best (one could claim that the
proper fix would be for the applications in question to be fixed so
that they properly sized the buffer, which is what libvirt does
(purely by virtue of using libnl), but it is what it is and we have to
deal with it.
In order for <interface type='hostdev'> to work properly on systems
with a kernel 3.3 or later, libvirt needs to add the afore-mentioned
IFLA_EXT_MASK field with RTEXT_FILTER_VF set.
Of course we also need to continue working on systems with older
kernels, so that one bit of code is compiled conditionally. The one
time this could cause problems is if the libvirt binary was built on a
system without IFLA_EXT_MASK which was subsequently updated to a
kernel that *did* have it. That could be solved by manually providing
the values of IFLA_EXT_MASK and RTEXT_FILTER_VF and adding it to the
message anyway, but I'm uncertain what that might actually do on a
system that didn't support the message, so for the time being we'll
just fail in that case (which will very likely never happen anyway).
This patch fixes the lack of error messages when libvirt fails to find
VFINFO in a returned netlinke response message.
https://bugzilla.redhat.com/show_bug.cgi?id=827519#c10 is an example
of the error message that was previously logged when the
IFLA_VFINFO_LIST object was missing from the netlink response. The
reason for this failure is detailed in
https://bugzilla.redhat.com/show_bug.cgi?id=889319
Even though that root problem has been fixed, the experience of
finding the root cause shows us how important it is to properly log an
error message in these cases. This patch *seems* to replace the entire
function, but really most of the changes are due to moving code that
was previously inside an if() statement out to the top level of the
function (the original if() was reversed and made to log an error and
return).
Revert the complex workaround of commit 39d91e9, now that we have
a nicer framework for shutting up broken gcc.
* src/util/buf.c (virBufferEscape): Simplify.
Historically there was an inconsistency in handling of the
itanium arch. The xen driver & CPU model code treated it
as 'ia64' but the QEMU capabilities code used 'itanium'. On
the grounds that no one has ever seriously used itanium
with QEMU, while RHEL shipped itanium with Xen, we should
favour 'ia64' as the canonical format
Introduce a 'virArch' enum for CPU architectures. Include
data type providing wordsize and endianness, and APIs to
query this info and convert to/from enum and string form.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This is yet another refinement to the fix for CVE-2012-3411:
https://bugzilla.redhat.com/show_bug.cgi?id=833033
It turns out that it would be very intrusive to correctly backport the
entire --bind-dynamic option to older dnsmasq versions
(e.g. dnsmasq-2.48 that is used on RHEL6.x and CentOS 6.x), but very
simple to patch those versions to just use SO_BINDTODEVICE on all
their listening sockets (SO_BINDTODEVICE also has the desired effect
of permitting only traffic that was received on the interface(s) where
dnsmasq was set to listen.)
This patch modifies the dnsmasq capabilities detection to detect the
string:
--bind-interfaces with SO_BINDTODEVICE
in the output of "dnsmasq --version", and in that case realize that
using the old --bind-interfaces option is just as safe as
--bind-dynamic (and therefore *not* forbid creation of networks that
use public IP address ranges).
If -bind-dynamic is available, it is still preferred over
--bind-interfaces.
Note that this patch does no harm in upstream, or in any distro's
downstream if it happens to end up there, but builds for distros that
have a new enough dnsmasq to support --bind-dynamic do *NOT* need to
specifically backport this patch; it's only required for distro
releases that have dnsmasq too old to have --bind-dynamic (and those
distros will need to add the SO_BINDTODEVICE patch to dnsmasq,
*including the extra string in the --version output*, as well.
When LXC labels USB devices during hotplug, it is running in
host context, so it needs to pass in a vroot path to the
container root.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There was a double free issue caused by virSysinfoRead on s390,
as the same manufacturer string instance was assigned to more
than one processor record.
Cleaned up other potential memory issues and restructured the sysinfo
parsing code by moving repeating patterns into a helper function.
The restructuring made it necessary to conditionally disable
-Wlogical-op for some older GCC versions, using pragma GCC diagnostic.
This is a GCC specific pragma, which is acceptable, since we're
using it to work around a GCC specific bug.
Finally, added a function virSysinfoSetup to configure the sysinfo
data source files/script during run time, to facilitate writing test
programs. This function is not published in sysinfo.h and only
there for testing.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
The current virStorageFileGet{LVM,SCSI}Key methods return
the key as the return value. Unfortunately it is desirable
for "NULL" to be a valid return value, as well as an error
indicator. Thus the returned key must instead be provided
as an out-parameter.
When we invoke lvs or scsi_id to extract ID for block devices,
we don't want virCommandWait logging errors messages. Thus we
must explicitly check 'status != 0', rather than letting
virCommandWait do it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QED file format is non-versioned, so although the magic
value matched, libvirt rejected it due to lack of a version
number to compare against. We need to distinguish this case
by allowing a value of '-2' to indicate a non-versioned file
where only the magic is required to match
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
