In the auth config file, it is currently required to have
an entry for each hostname to connect to, eg
[auth-libvirt-prod1.example.com]
credentials=prod
This is inconvenient when there are large numbers of machines
all with the same credentials. Add support for a default
entry:
[auth-default]
credentials=prod
This function is plenty of ifdefs providing implementations for
Linux, *BSD and OS-X. However, if we are being build for any
other architecture, all that's left behind by preprocessor is
just a error reporting call and return of -1. In that case,
passed arguments are unused:
../../src/util/virhostcpu.c: In function 'virHostCPUGetInfo':
../../src/util/virhostcpu.c:966:33: error: unused parameter 'cpus' [-Werror=unused-parameter]
unsigned int *cpus,
^~~~
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The virQEMUDriverConfig object contains lists of
loader:nvram pairs to advertise firmwares supported by
by the driver, and qemu_conf.c contains code to populate
the lists, all of which is useful for other drivers too.
To avoid code duplication, introduce a virFirmware object
to encapsulate firmware details and switch the qemu driver
to use it.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
* Fix misspelt function name:
s/virHostCPUGetStatsFreebsd/virHostCPUGetStatsFreeBSD/
* Mark the first argument to virHostCPUGetInfo with ATTRIBUTE_UNUSED
as it's not actually used on non-Linux
Move all APIs with a virHostMEM name prefix out into new
util/virhostmem.h & util/virhostmem.c files
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Move all APIs with a virHostCPU name prefix out into new
util/virhostcpu.h & util/virhostcpu.c files
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In preparation for moving all the CPU related APIs out of
the nodeinfo file, give them a virHostCPU name prefix.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In preparation for moving all the memory related APIs out of
the nodeinfo file, give them a virHostMem name prefix.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Nearly all the methods in the nodeinfo file are given a
'const char *sysfs_prefix' parameter to override the
default sysfs path (/sys/devices/system). Every single
caller passes in NULL for this, except one use in the
unit tests. Furthermore this parameter is totally
Linux-specific, when the APIs are intended to be cross
platform portable.
This removes the sysfs_prefix parameter and instead gives
a new method linuxNodeInfoSetSysFSSystemPath for use by
the test suite.
For two of the methods this hardcodes use of the constant
SYSFS_SYSTEM_PATH, since the test suite does not need to
override the path for thos methods.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The sd_notify method is used to tell systemd when libvirtd
has finished starting up. All it does is send a datagram
containing the string parameter to systemd on a UNIX socket
named in the NOTIFY_SOCKET environment variable. Rather than
pulling in the systemd libraries for this, just code the
notification directly in libvirt as this is a stable ABI
from systemd's POV which explicitly allows independant
implementations:
See "Reimplementable Independently" column in the
"$NOTIFY_SOCKET Daemon Notifications" row:
https://www.freedesktop.org/wiki/Software/systemd/InterfacePortabilityAndStabilityChart/
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1314881
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Move the module from qemu_command.c to a new module virqemu.c and
rename the API to virQEMUBuildObjectCommandline.
This API will then be shareable with qemu-img and the need to build
a security object for luks support.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When building using -Og, gcc sees that some variables can be used
uninitialized It can be debatable whether it is possible with our
codeflow, but functions should be self-contained and initializations are
always good. The return instead of goto is due to actualType being used
in the cleanup.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So imagine the following. You connect read only to a daemon and
try to fetch stats for a shut off domain, e.g.:
virsh -r domstats $dom
but all of a sudden, virsh instead of printing the stats throws
the following error at you:
error: Disconnected from qemu:///system due to I/O error
error: End of file while reading data: Input/output error
The daemon crashed. This is its backtrace:
#0 0x00007fa43e3751a8 in virPerfEventIsEnabled (perf=0x0, type=VIR_PERF_EVENT_MBMT) at util/virperf.c:241
#1 0x00007fa424a9f042 in qemuDomainGetStatsPerf (driver=0x7fa3f4022a30, dom=0x7fa3f40e24c0, record=0x7fa41c000e20, maxparams=0x7fa4360b38d0, privflags=1) at qemu/qemu_driver.c:19110
#2 0x00007fa424a9f2e7 in qemuDomainGetStats (conn=0x7fa41c001b20, dom=0x7fa3f40e24c0, stats=127, record=0x7fa4360b3970, flags=1) at qemu/qemu_driver.c:19213
#3 0x00007fa424a9f672 in qemuConnectGetAllDomainStats (conn=0x7fa41c001b20, doms=0x7fa41c0017f0, ndoms=1, stats=127, retStats=0x7fa4360b3a50, flags=0) at qemu/qemu_driver.c:19303
#4 0x00007fa43e4e15f6 in virDomainListGetStats (doms=0x7fa41c0017f0, stats=0, retStats=0x7fa4360b3a50, flags=0) at libvirt-domain.c:11615
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f28d1a38700 (LWP 16154)]
0x00007f28da4fa1a8 in virPerfEventIsEnabled (perf=0x0, type=VIR_PERF_EVENT_MBMT) at util/virperf.c:241
241 return event->enabled;
Problem is, shut off domains don't have priv->perf allocated.
Therefore if in frame #1 qemuDomainGetStatsPerf() tries to check
if perf events are enabled, NULL is passed to
virPerfEventIsEnabled() which due to some incredible
implementation dereference it. Fix this by checking whether
passed object is not NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function is not used anywhere. Moreover, the code that would
use lives in virperf.c and therefore has access to the FD anyway.
Well, for instance virPerfReadEvent is doing just that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So far, this function has just three callers. Two of them call
virNetDevSetupControl to create a socket that we can then
optionally use for ioctl() to fetch data. However, querying sysfs
is preferred. Therefore it doesn't make much sense to require
users to set up the socket if they don't even know it will be
used in favour of sysfs. We can set up the socket iff we need to.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Yet another one of those where signed int (or long int) is not
enough. And useless to as we're aiming at unsigned anyway.
../../src/util/virsocketaddr.c: In function 'virSocketAddrIsPrivate':
../../src/util/virsocketaddr.c:289:45: error: result of '192l << 24' requires 33 bits to represent, but 'long int' only has 32 bits [-Werror=shift-overflow=]
return ((val & 0xFFFF0000) == ((192L << 24) + (168 << 16)) ||
^~
../../src/util/virsocketaddr.c:290:45: error: result of '172l << 24' requires 33 bits to represent, but 'long int' only has 32 bits [-Werror=shift-overflow=]
(val & 0xFFF00000) == ((172L << 24) + (16 << 16)) ||
^~
cc1: all warnings being treated as errors
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Apparently, 1 << 31 is signed which in turn does not fit into
a signed integer variable:
../../include/libvirt/libvirt-domain.h:1881:57: error: result of '1 << 31' requires 33 bits to represent, but 'int' only has 32 bits [-Werror=shift-overflow=]
VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS = 1 << 31, /* enforce requested stats */
^~
cc1: all warnings being treated as errors
The solution is to make it an unsigned value. I've found only two
such occurrences in our code base.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit c8b1a83605 changed the function, making it
impossible for callers to be able to tell whether a
non-negative return value means "physical function
address found and parsed correctly" or "couldn't find
corresponding physical function".
The important difference between the two being that,
in the latter case, the returned pointer is NULL and
should never, ever be dereferenced.
In order to cope with these changes, the callers
have to be updated.
Move the logic from qemuDomainGenerateRandomKey into this new
function, altering the comments, variable names, and error messages
to keep things more generic.
NB: Although perhaps more reasonable to add soemthing to virrandom.c.
The virrandom.c was included in the setuid_rpc_client, so I chose
placement in vircrypto.
Introduce virCryptoHaveCipher and virCryptoEncryptData to handle
performing encryption.
virCryptoHaveCipher:
Boolean function to determine whether the requested cipher algorithm
is available. It's expected this API will be called prior to
virCryptoEncryptdata. It will return true/false.
virCryptoEncryptData:
Based on the requested cipher type, call the specific encryption
API to encrypt the data.
Currently the only algorithm support is the AES 256 CBC encryption.
Adjust tests for the API's
Seems recent versions of Coverity have (mostly) resolved the issue using
ternary operations in VIR_FREE (and now VIR_DISPOSE*) macros. So let's
just remove it and if necessary handle one off issues as the arise.
Rather than return 0/-1 and/or a pointer to some memory, adjust the
helper to just return the allocated structure or NULL on failure.
Adjust the callers in order to handle that
Signed-off-by: John Ferlan <jferlan@redhat.com>
If we get to the error: label and clear out the *virtual_functions[]
pointers and then return w/ error to the caller - the caller has it's
own cleanup of the same array in the out: label which is keyed off the
value of num_virt_fns, which wasn't reset to 0 in the called function
leading to a possible problem.
Just clear the value (found by Coverity)
Signed-off-by: John Ferlan <jferlan@redhat.com>
Some Intel processor families (e.g. the Intel Xeon processor E5 v3
family) introduced some RDT (Resource Director Technology) features
to monitor or control shared resource. Among these features, MBM
(Memory Bandwidth Monitoring), which is build on the CMT (Cache
Monitoring Technology) infrastructure, provides OS/VMM a way to
monitor bandwidth from one level of cache to another.
With current perf framework, this patch adds support to perf event
for MBM.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1331552
Instead of disabling auto-login of all scsi targets (even those
that do not "belong" to libvirt), use iscsiadm's "--op nonpersistent"
during discovery of iSCSI targets (e.g. "iscsiadm --mode discovery
--type sendtargets") in order to avoid the node database being altered
which led to the need for the "large hammer" approach taken by
commit id '3c12b654'.
This commit removes the virISCSITargetAutologin adjustment (eg. the setting
of node.startup to "manual"). The iscsiadm command has supported this mode
of operation as of commit id 'ad873767' to open-iscsi.
Utilize the exit status parameter for virCommandRunRegex in order to
check the return error from the 'iscsiadm --mode session' command.
Without this enabled, if there are no sessions running then virCommandRun
would have displayed an error such as:
2016-05-13 15:17:15.165+0000: 10920: error : virCommandWait:2553 :
internal error: Child process (iscsiadm --mode session)
unexpected exit status 21: iscsiadm: No active sessions.
It is possible that for certain paths (when probe is true) we only care
whether it's running or not to make certain decisions. Spitting out
the error for those paths is unnecessary.
If we do have a situation where probe = false and there's an error,
then display the error from iscsiadm if it's there.
Rather than have virCommandRun just spit out the error, allow callers
to decide to pass the exitstatus so the caller can make intelligent
decisions based on the error.
For a few cases where we handle secret information it's good to clear
the buffers containing sensitive data before freeing them.
Introduce VIR_DISPOSE, VIR_DISPOSE_N and VIR_DISPOSE_STRING that allow
simple clearing fo the buffers holding sensitive information on cleanup
paths.
Move some parts of virStorageFileRemoveLastPathComponent
into a separate function so they can be reused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Both virGetLastError and virGetLastErrorMessage call virLastErrorObject method
that returns a thread-local error object. However, if a direct call to malloc
or pthread_setspecific (probably also due to malloc, since it sets ENOMEM)
fail, virLastErrorObject returns NULL which, although incorrectly interpreted
by virGetLastError as no error, still requires the caller to check for NULL
pointer. This isn't the case with virGetLastErrorMessage that also treated it
incorrectly as no error, but returned the literal "no error".
This patch tweaks the checks in the virGetLastErrorMessage function, so that
if virLastErrorObject failed, it returned "unknown error" which is equivalent
to the current approach with virGetLastError and if it returned NULL,
"unknown error" was set.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1318993
Commit id 'dd519a294' caused a regression cloning a volume into a
logical pool by removing just the 'allocation' adjustment during
storageVolCreateXMLFrom. Combined with the change to not require the
new volume input XML to have a capacity listed (commit id 'e3f1d2a8')
left the possibility that a zero allocation value (e.g., not provided)
would create a thin/sparse logical volume. When a thin lv becomes fully
populated, then LVM sets the partition 'inactive' and the subsequent
fdatasync() fails.
Add a new 'has_allocation' flag to be set at XML parse time to indicate
that allocation was provided. This is done so that if it's not provided
the create-from code uses the capacity value since we document that if
omitted, the volume will be fully allocated at time of creation.
For a logical backend, that creation time is 'createVol', while for a
file backend, creation doesn't set the size, but the 'createRaw' called
during buildVolFrom will decide whether the file is sparse or not based
on the provided capacity and allocation value.
For volume clones that provide different allocation and capacity values
to allow for sparse files, there is no change.
Usage of this keyword in front of function declaration that is exported via a
header file is unnecessary, since internally, this has been the default for most
compilers for quite some time.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
SRIOV VFs used in macvtap passthrough mode can take advantage of the
SRIOV card's transparent vlan tagging. All the code was there to set
the vlan tag, and it has been used for SRIOV VFs used for hostdev
interfaces for several years, but for some reason, the vlan tag for
macvtap passthrough devices was stubbed out with a -1.
This patch moves a bit of common validation down to a lower level
(virNetDevReplaceNetConfig()) so it is shared by hostdev and macvtap
modes, and updates the macvtap caller to actually send the vlan config
instead of -1.
Commit 0b36b0e9 broke polkit agent startup when attempting to fix a
coverity warning. Refactor it properly so that we don't need the 'cmd'
intermediate variable.
For disks sources described by a libvirt volume we don't need to do a
complicated check since virStorageTranslateDiskSourcePool already
correctly determines the actual disk type.
Replace the checks using a new accessor that does not open-code the
whole logic.
Fron c3bd0019c0 on instead of creating the following path for
cgroups:
/sys/fs/cgroupX/$name.libvirt-$driver
we generate rather more verbose one:
/sys/fs/cgroupX/$driver-$id-$name.libvirt-$driver
where $name is optional and included iff contains allowed chars.
See original commit for more reasoning. Now, problem with the
original commit is that we are unable to start any LXC domain
after it. Because when starting LXC container, the CGroup layout
is created by our lxc_controller process and then detected and
validated by libvirtd. The validation is done by trying to match
detected layout against all the possible patterns for cgroup
paths that we've ever had. And the commit in question forgot to
update this part of the code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
json_reformat uses two spaces for when indenting nested objects, let's
do the same. The result of virJSONValueToString will be exactly the same
as json_reformat would produce.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Our socket address format is in a rather non-standard format and that is
because sasl library requires the IP address and service to be delimited by a
semicolon. The string form is a completely internal matter, however once the
admin interfaces to retrieve client identity information are merged, we should
return the socket address string in a common format, e.g. format defined by
URI rfc-3986, i.e. the IP address and service are delimited by a colon and
in case of an IPv6 address, square brackets are added:
Examples:
127.0.0.1:1234
[::1]:1234
This patch changes our default format to the one described above, while adding
separate methods to request the non-standard SASL format using semicolon as a
delimiter.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Just like with server-related APIs, before any of client-based APIs can be
called, a reference to a client-side client object needs to be obtained. For
this purpose, a lookup method should exist. Apart from the client retrieval
logic, a new error code for non-existent client had to be added as well.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
We had both and the only difference was that the latter also included
information about multifunction setting. The problem with that was that
we couldn't use functions made for only one of the structs (e.g.
parsing). To consolidate those two structs, use the one in virpci.h,
include that in domain_conf.h and add the multifunction member in it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
$ echo -n 'log_level=1' > ~/.config/libvirt/libvirtd.conf
$ libvirtd --timeout=10
2014-10-10 10:30:56.394+0000: 6626: info : libvirt version: 1.1.3.6, package: 1.fc20 (Fedora Project, 2014-09-08-17:50:42, buildvm-05.phx2.fedoraproject.org)
2014-10-10 10:30:56.394+0000: 6626: error : main:1261 : Can't load config file: configuration file syntax error: /home/rjones/.config/libvirt/libvirtd.conf:1: expecting a value: /home/rjones/.config/libvirt/libvirtd.conf
Rather than try to fix this in the depths of the parser, just catch
the case when a config file doesn't end in a newline, and manually
append a newline to the content before parsing
https://bugzilla.redhat.com/show_bug.cgi?id=1151409
This an ubuntu/debian packaging convention. At one point it may have
been an actually different binary, but at least as of ubuntu precise
(the oldest supported ubuntu distro, released april 2012) kvm-img is
just a symlink to qemu-img for back compat.
I think it's safe to drop support for it
QEMU introduced the query-gic-capabilities QMP command
with commit 4468d4e0f383: use the command, if available,
to probe available GIC capabilities.
The information obtained is stored in a virQEMUCaps
instance, and will be later used to fill in a
virDomainCaps instance.
So in glibc-2.23 sys/sysmacros.h is no longer included from sys/types.h
and we don't build because of the usage of major/minor/makedev macros.
Autoconf already has AC_HEADER_MAJOR macro that check where exactly
these functions/macros are defined, so let's use that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Since threadpool increments the current number of threads according to current
load, i.e. how many jobs are waiting in the queue. The count however, is
constrained by max and min limits of workers. The logic of this new API works
like this:
1) setting the minimum
a) When the limit is increased, depending on the current number of
threads, new threads are possibly spawned if the current number of
threads is less than the new minimum limit
b) Decreasing the minimum limit has no possible effect on the current
number of threads
2) setting the maximum
a) Icreasing the maximum limit has no immediate effect on the current
number of threads, it only allows the threadpool to spawn more
threads when new jobs, that would otherwise end up queued, arrive.
b) Decreasing the maximum limit may affect the current number of
threads, if the current number of threads is less than the new
maximum limit. Since there may be some ongoing time-consuming jobs
that would effectively block this API from killing any threads.
Therefore, this API is asynchronous with best-effort execution,
i.e. the necessary number of workers will be terminated once they
finish their previous job, unless other workers had already
terminated, decreasing the limit to the requested value.
3) setting priority workers
- both increase and decrease in count of these workers have an
immediate impact on the current number of workers, new ones will be
spawned or some of them get terminated respectively.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In order for the client to see all thread counts and limits, current total
and free worker count getters need to be introduced. Client might also be
interested in the job queue length, so provide a getter for that too. As with
the other getters, preparing for the admin interface, mutual exclusion is used
within all getters.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
So far, the values the affected getters retrieve are static, i.e. there's no
way of changing them during runtime. But admin interface will later enable
not only getting but changing them as well. So to prevent phenomenons like
torn reads or concurrent reads and writes of unaligned values, use mutual
exclusion when getting these values (writes do, understandably, use them
already).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When either creating a threadpool, or creating a new thread to accomplish a job
that had been placed into the jobqueue, every time thread-specific data need to
be allocated, threadpool needs to be (re)-allocated and thread count indicators
updated. Make the code clearer to read by compressing these operations into a
more complex one.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
virTypedParamsValidate currently uses an index based check to find
duplicate parameters. This check does not work. Consider the following
simple example:
We have only 2 keys
A (multiples allowed)
B (multiples NOT allowed)
We are given the following list of parameters to check:
A
A
B
If you work through the validation loop you will see that our last iteration
through the loop has i=2 and j=1. In this case, i > j and keys[j].value.i will
indicate that multiples are not allowed. Both conditionals are satisfied so
an incorrect error will be given: "parameter '%s' occurs multiple times"
This patch replaces the index based check with code that remembers
the name of the last parameter seen and only triggers the error case if
the current parameter name equals the last one. This works because the
list is sorted and duplicate parameters will be grouped together.
In reality, we hit this bug while using selective block migration to migrate
a guest with 5 disks. 5 was apparently just the right number to push i > j
and hit this bug.
virsh migrate --live guestname --copy-storage-all
--migrate-disks vdb,vdc,vdd,vde,vdf
qemu+ssh://dsthost/system
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
In a few places in libvirt we busy-wait for events, for example qemu
creating a monitor socket. This is problematic because:
- We need to choose a sufficiently small polling period so that
libvirt doesn't add unnecessary delays.
- We need to choose a sufficiently large polling period so that
the effect of busy-waiting doesn't affect the system.
The solution to this conflict is to use an exponential backoff.
This patch adds two functions to hide the details, and modifies a few
places where we currently busy-wait.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Refreshes meta-information such as allocation, capacity, format, etc.
Ploop volumes differ from other volume types. Path to volume is the path
to directory with image file root.hds and DiskDescriptor.xml.
https://openvz.org/Ploop/format
Due to this fact, operations of opening the volume have to be done once
again. get the information.
To decide whether the given volume is ploops one, it is necessary to check
the presence of root.hds and DiskDescriptor.xml files in volumes' directory.
Only in this case the volume can be manipulated as the ploops one.
Such strategy helps us to resolve problems that might occure, when we
upload some other volume type from ploop source.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
virSocketAddrFormat() wants a single pointer, not a double pointer.
Fixes the following compilation error on FreeBSD:
util/virnetdev.c:1448:72: error: incompatible pointer types passing
'virSocketAddr **' to parameter of type 'const virSocketAddr *';
remove & [-Werror,-Wincompatible-pointer-types]
if (VIR_SOCKET_ADDR_VALID(peer) && !(peerstr = virSocketAddrFormat(&peer)))
^~~~~
./util/virsocketaddr.h:92:48: note: passing argument to parameter 'addr' here
char *virSocketAddrFormat(const virSocketAddr *addr);
^
FreeBSD lacks ENODATA, and viruuid.c redefines it to EIO, but it's not
actually using it. On the other hand, we have virrandom.c that's using
ENODATA. So make this re-definition common by moving it to internal.h,
so all the current and possible future users don't need to care about
that.
Using the existing virUUIDGenerateRandomBytes, move API to virrandom.c
rename it to virRandomBytes and add it to libvirt_private.syms.
This will be used as a fallback for generating a domain master key.
This reverts commit ee4cfb5643.
Since we're still not persisting our bookkeeping lists across
daemon restarts, we might have lost some information
virPCIDeviceReattach() relies on, for example whether the
device needs to be unbound from the stub driver.
As a result, if the daemon has been restarted in the meantime,
the device might end up remaining bound to the stub driver even
after 'virsh nodedev-reattach' or similar has been called, with
no way of giving it back to the host short of messing with
sysfs behind libvirt's back.
Revert back to the previous behavior of always trying to bind
the device to the host driver, regardless of its status when it
was detached, until persistent bookkeeping lists have been
implemented.
Do I really need to explain why?
Well, if read() is interrupted int the middle of reading, we will
never read the rest (even though it's highly unlikely as we are
reading just 8 bytes).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have @flags we can support changing perf events just
in active or inactive configuration regardless of the other.
Previously, calling virDomainSetPerfEvents set events in both
active and inactive configuration at once. Even though we allow
users to set perf events that are to be enabled once domain is
started up. The virDomainGetPerfEvents API was flawed too. It
returned just runtime info.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In some cases it's impractical to use the regular APIs as the bitmap
size needs to be pre-declared. These new APIs allow to use bitmaps that
self expand.
The new code adds a property to the bitmap to track the allocation of
memory so that VIR_RESIZE_N can be used.
This patch implement a set of interfaces for perf event. Based on
these interfaces, we can implement internal driver API for perf,
and get the results of perf conuter you care about.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Message-id: 1459171833-26416-4-git-send-email-qiaowei.ren@intel.com
After the patches that added tracking of in-use macvtap names (commit
370608, first appearing in libvirt-1.3.2), if the function to allocate
a new macvtap device came to a device name created outside libvirt, it
would retry the same device name MACVLAN_MAX_ID (8191) times before
finally giving up in failure.
The problem was that virBitmapNextClearBit was always being called
with "0" rather than the value most recently checked (which would
increment each time through the loop), so it would always return the
same id (since we dutifully release that id after failing to create a
new device using it).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1321546
Signed-off-by: Laine Stump <laine@laine.org>
Instead of forcing the values for the unbind_from_stub, remove_slot
and reprobe properties, look up the actual device and use that when
calling virPCIDeviceReattach().
This ensures the device is restored to its original state after
reattach: for example, if it was not bound to any driver before
detach, it will not be bound forcefully during reattach.
We would be just fine looking up the information in pcidevs most
of the time; however, some corner cases would not be handled
properly, so look up the actual device instead.
After this patch, ownership of virPCIDevice instances is very easy
to keep track of: for each host PCI device, the only instance that
actually matters is the one inside one of the bookkeeping list.
Whenever some operation needs to be performed on a PCI device, the
actual device is looked up first; when this is not the case, a
comment explains the reason.
Unmanaged devices, as the name suggests, are not detached
automatically from the host by libvirt before being attached to a
guest: it's the user's responsability to detach them manually
beforehand. If that preliminary step has not been performed, the
attach operation can't complete successfully.
Instead of relying on the lower layers to error out with cryptic
messages such as
error: Failed to attach device from /tmp/hostdev.xml
error: Path '/dev/vfio/12' is not accessible: No such file or directory
prevent the situation altogether and provide the user with a more
useful error message.
Unmanaged devices are attached to guests in two steps: first,
the device is detached from the host and marked as inactive;
subsequently, it is marked as active and attached to the guest.
If the daemon is restarted between these two operations, we lose
track of the inactive device.
Steps 5 and 6 of virHostdevPreparePCIDevices() already subtly
take care of this situation, but some planned changes will make
it so that's no longer the case. Plus, explicit is always better
than implicit.
This allows setting the address in host and/or network order and makes
the naming consistent. Now you don't need to call [hn]to[nh]l()
functions as that is taken care of by these functions. Also, now
the *NetOrder take the address in network order, the other functions in
host order so the naming and usage is consistent. Some places were
having the address in network order and calling ntohl() just so the
original function can call htonl() again. This makes it nicer to read.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
If we expose this information, which is one byte in every PCI config
file, we let all mgmt apps know whether the device itself is an endpoint
or not so it's easier for them to decide whether such device can be
passed through into a VM (endpoint) or not (*-bridge).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1317531
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The implementation is pretty straightforward. Moreover, because
of the nature of things, gethostbyname_r and gethostbyname2_r can
be implemented at the same time too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This is a missing counterpart for virSocketAddrSetIPv4Addr()
and is going to be needed later in the tests.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function is going to be used later in such context where the
argument makes no sense. Teach this function to cope with that
instead of the caller having to deal with passing some dummy
argument.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
These functions are going to be reused very shortly. So instead
of duplicating the code, lets move them into utils module.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We include the file in plenty of places. This is mostly due to
historical reasons. The only place that needs something from the
header file is storage_backend_fs which opens _PATH_MOUNTED. But
it gets the file included indirectly via mntent.h. At no other
place in our code we need _PATH_.*. Drop the include and
configure check then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Refactor series 0b231195 worked with virLogDestination type which, depending
on the compiler, might be (and probably will be) an unsigned data type.
However, virEnumFromString may return -1 in case of an error. So, when enum
happens to be unsigned, some compilers will naturally complain about foo:
'if (foo < 0)'
The problem with the original virLogParseOutputs method was that the way it
parsed the input, walking the string char by char and using absolute jumps
depending on the virLogDestination type, was rather complicated to read.
This patch utilizes virStringSplit method twice, first time to filter out any
spaces and split the input to individual log outputs and then for each
individual output to tokenize it by to the parts according to our
PRIORITY:DESTINATION?(:DATA) format. Also, to STREQLEN for matching destination
was replaced with virDestinationTypeFromString call.
In order to refactor the ugly virLogParseOutputs method, this is a neat way of
finding out whether the destination type (in the form of a string) user
provided is a valid one. As a bonus, if it turns out it is valid, we get the
actual enum which will later be passed to any of virLogAddOutput methods right
away.
These comments explain the difference between a virPCIDevice
instance used for lookups and an actual device instance; some
information is also provided for specific uses.
virHostdevGetPCIHostDeviceList() is similar but does not filter out
devices that are not in the active list; that said, we are looking
up the device in the active list just a few lines after anyway, so
we might as well just keep a single function around.
This also helps stress the fact the objects contained in pcidevs are
only for looking up the actual devices, which is something later
commits will make even more explicit.
We're in the hostdev module, so mgr is not an ambiguous name, and
in fact it's already used in some cases. Switch all the code over.
Take the chance to shorten declaration of
virHostdevIsPCINodeDeviceUsedData structures.
When we want to look up a device in a device list and we already
have the IDs from another source, we can simply use
virPCIDeviceListFindByIDs() instead of creating a temporary device
object.
If 'last_processed_hostdev_vf != -1' is false then, since the
loop counter 'i' starts at 0, 'i <= last_processed_hostdev_vf'
can't possibly be true and the loop body will never be executed.
However, since 'i' is unsigned and 'last_processed_hostdev_vf'
is signed, we can't just get rid of the check completely; what
we can do is move it outside of the loop to avoid checking its
value on every iteration and cluttering the actual loop
condition.
This serves the same purpose as VIR_ERR_NO_xxx where xxx is any object
that API can be called upon. Only this particular one is used for
daemon's servers.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The comment claimed that virPCIDeviceReattach() does not reattach
a device to the host driver; except it actually does, so the
comment is just confusing and we're better off removing it.
Replace the term "loop" with the more generic "step". This allows us
to be more flexible and eg. have a step that consists in a single
function call.
Don't include the number of steps in the first comment of the
function, so that we can add or remove steps without having to worry
about keeping that comment in sync.
For the same reason, remove the summary contained in that comment.
Clean up some weird vertical spacing while we're at it.
While trying to build with -Os I've encountered some build
failures.
util/vircommand.c: In function 'virCommandAddEnvFormat':
util/vircommand.c:1257:1: error: inlining failed in call to 'virCommandAddEnv': call is unlikely and code size would grow [-Werror=inline]
virCommandAddEnv(virCommandPtr cmd, char *env)
^
util/vircommand.c:1308:5: error: called from here [-Werror=inline]
virCommandAddEnv(cmd, env);
^
This function is big enough for the compiler to be not inlined.
This is the error message I'm seeing:
Then virDomainNumatuneNodeSpecified is exported and called from
other places. It shouldn't be inlined then.
In file included from network/bridge_driver_platform.h:30:0,
from network/bridge_driver_platform.c:26:
network/bridge_driver_linux.c: In function 'networkRemoveRoutingFirewallRules':
./conf/network_conf.h:350:1: error: inlining failed in call to 'virNetworkDefForwardIf.constprop': call is unlikely and code size would grow [-Werror=inline]
virNetworkDefForwardIf(const virNetworkDef *def, size_t n)
^
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
More fallout from changing to using virPolkitAgent and handling error
paths. Needed to clear the 'cmd' once stored and of course add the
virCommandFree(cmd) in the error: label.
In virPolkitAgentCreate neglected to initialize agent to NULL. If
there was an error in the pipe, then we jump to error and would have
an issue. Found by coverity.
qemuProcessSetupEmulator runs at a point in time where there is only
the qemu main thread. Use virCgroupAddTask to put just that one task
into the emulator cgroup. That patch makes virCgroupMoveTask and
virCgroupAddTaskStrController obsolete.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
Introduce virPolkitAgentCreate and virPolkitAgentDestroy
virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous
command in order to handle the local agent authentication via stdin/stdout.
The code makes use of the pkttyagent --notify-fd mechanism to let it know
when the agent is successfully registered.
virPolkitAgentDestroy will close the command effectively reaping our
child process
When there isn't a ssh -X type session running and a user has not
been added to the libvirt group, attempts to run 'virsh -c qemu:///system'
commands from an otherwise unprivileged user will fail with rather
generic or opaque error message:
"error: authentication failed: no agent is available to authenticate"
This patch will adjust the error code and message to help reflect the
situation that the problem is the requested mechanism is UNAVAILABLE and
a slightly more descriptive error. The result on a failure then becomes:
"error: authentication unavailable: no polkit agent available to
authenticate action 'org.libvirt.unix.manage'"
A bit more history on this - at one time a failure generated the
following type message when running the 'pkcheck' as a subprocess:
"error: authentication failed: polkit\56retains_authorization_after_challenge=1
Authorization requires authentication but no agent is available."
but, a patch was generated to adjust the error message to help provide
more details about what failed. This was pushed as commit id '96a108c99'.
That patch prepended a "polkit: " to the output. It really didn't solve
the problem, but gave a hint.
After some time it was deemed using DBus API calls directly was a
better way to go (since pkcheck calls them anyway). So, commit id
'1b854c76' (more or less) copied the code from remoteDispatchAuthPolkit
and adjusted it. Then commit id 'c7542573' adjusted the remote.c
code to call the new API (virPolkitCheckAuth). Finally, commit id
'308c0c5a' altered the code to call DBus APIs directly. In doing
so, it reverted the failing error message to the generic message
that would have been received from DBus anyway.
Use virCgroupAddTaskController in virCgroupAddTask so we have one
single point where we add tasks to cgroups.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
The virHostdevIsVirtualFunction() was called exactly twice, and in
both cases the return value was saved to a temporary variable before
being checked. This would be okay if it improved readability, but in
this case is pretty pointless.
Get rid of the temporary variable and check the return value
directly; while at it, change the check from '<= 0' to '!= 1' to
align it with the way other similar *IsVirtualFunction() functions
are used thorough the code.
virNetDevIsVirtualFunction() returns 1 if the interface is a
virtual function, 0 if it isn't and -1 on error. This means that,
despite the name suggesting otherwise, using it as a predicate is
not correct.
Fix two callers that were doing so adding an explicit check on
the return value.
It may be useful in some cases to call TristateSwitch helper with TristateBool.
Document that enum values equivalency in the code.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
GIC v2 is the default, but checking against that specific version when
we want to know whether the default has been selected is potentially
error prone; using an alias instead makes it safer.
In cf113e8d we changed the declaration of
virCgroupAllowDevicePath() and virCgroupDenyDevicePath().
However, while updating the stub for non-cgroup platforms for the
former we forgot to update the latter too causing a build
failure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The method will now return 0 on success and -1 on error, rather than number of
items which it iterated over before it returned back to the caller. Since the
only place where we actually check the number of elements iterated is in
virhashtest, return value of 0 and -1 can be a pretty accurate hint that it
iterated over all the items. However, if we really want to know the number of
items iterated over (like virhashtest does), a counter has to be provided
through opaque data to each iterator call. This patch adjusts return value of
virHashForEach, refactors the body, so it returns as soon as one of the
iterators fail and adjusts virhashtest to reflect these changes.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When adding disk images to ACL we may call those functions on NFS
shares. In that case we might get an EACCES, which isn't really relevant
since NFS would not hold a block device. This patch adds a flag that
allows to stop reporting an error on EACCES to avoid spaming logs.
Currently there's no functional change.
Since commit 47e5b5ae virCgroupAllowDevice allows to pass -1 as either
the minor or major device number and it automatically uses '*' in place
of that. Reuse the new approach through the code and drop the duplicated
functions.
