Not every system out there has syslog, that's why we check for it
in our configure script. However, in 640b58abdf while fixing
another issue, some variables and functions are called that are
defined only when syslog.h is present. But these function
calls/variables were not guarded by #ifdef-s.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In both virLogParseOutput and virLogParseFilter, rather than returning
NULL, goto cleanup since it's possible that for each the first condition
passes, but the || condition doesn't and thus we leak memory.
Handling of outputs and filters has been changed in a way that splits
parsing and defining. Do the same thing for logging priority as well, this
however, doesn't need much of a preparation.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This is mainly virLogAddOutputTo* which were replaced by virLogNewOutputTo* and
the previously poorly named ones virLogParseAndDefine* functions. All of these
are unnecessary now, since all the original callers were transparently switched
to the new model of separate parsing and defining logic.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Similar to outputs, parser should do parsing only, thus the 'define' logic
is going to be stripped from virLogParseAndDefineFilters by replacing calls to
this method to virLogSetFilters instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since virLogParseAndDefineOutputs is going to be stripped from 'output defining'
logic, replace all relevant occurrences with virLogSetOutputs call to make the
change transparent to all original callers (daemons mostly).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This method will eventually replace virLogParseAndDefineFilters which
currently does both parsing and defining.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This API is the entry point to output modification of the logger. Currently,
everything is done by virLogParseAndDefineOutputs. Parsing and defining will be
split into two operations both handled by this method transparently.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Abstraction added over parsing a single filter. The method parses potentially a
set of logging filters, while adding each filter logging object to a
caller-provided array.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Another abstraction added on the top of parsing a single logging output. This
method takes and parses the whole set of outputs, adding each single output
that has already been parsed into a caller-provided array. If the user-supplied
string contained duplicate outputs, only the last occurrence is taken into
account (all the others are removed from the list), so we silently avoid
duplicate logs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Same as for outputs, introduce a new method, that is basically the same as
virLogParseAndDefineFilter with the difference that it does not define the
filter. It rather returns a newly created object that needs to be inserted into
a list and then defined separately.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Introduce a method to parse an individual logging output. The difference
compared to the virLogParseAndDefineOutput is that this method does not define
the output, instead it makes use of the virLogNewOutputTo* methods introduced
in the previous patch and just returns the virLogOutput object that has to be
added to a list of object which then can be defined as a whole via
virLogDefineOutputs. The idea remains still the same - split parsing and
defining of the logging primitives (outputs, filters).
Additionally, since virLogNewOutputTo* methods are now finally used,
ATTRIBUTE_UNUSED can be successfully removed from the methods' definitions,
since that was just to avoid compiler complaints about unused static functions.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Now that we're in the critical section, syslog connection can be re-opened
by issuing openlog, which is something that cannot be done beforehand, since
syslog keeps its file descriptor private and changing the tag earlier might
introduce a log inconsistency if something went wrong with preparing a new set
of logging outputs in order to replace the existing one.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Continuing with the effort to split output parsing and defining, these new
functions return a logging object reference instead of defining the output.
Eventually, these functions will replace the existing ones (virLogAddOutputTo*)
which will then be dropped.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Prepare a method that only defines a set of filters. It takes a list of
filters, preferably created by virLogParseFilters. The original set of filters
is reset and replaced by the new user-provided set of filters.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Prepare a method that only defines a set of outputs. It takes a list of
outputs, preferably created by virLogParseOutputs. The original set of outputs
is reset and replaced by the new user-provided set of outputs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Outputs are a bit trickier than filters, since the user(config)-specified
set of outputs can contain duplicates. That would lead to logging the same
message twice. For compatibility reasons, we cannot just error out and forbid
the daemon to start if we find duplicate outputs which do not make sense.
Instead, we could silently take into account only the last occurrence of the
duplicate output and remove all the previous ones, so that the logger will not
try to use them when it is looping over all of its registered outputs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In order to later split output parsing and output defining, introduce a new
function which will create a new virLogOutput object which the parser will
insert into a list with the list being eventually defined.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
There is really no reason why we could not keep journald's fd within the
journald output object the same way as we do for regular file-based outputs.
By doing this we later won't have to special case the journald-based output
(due to the fd being globally shared) when replacing the existing set of outputs
with a new one. Additionally, by making this change, we don't need the
virLogCloseJournald routine anymore, plain virLogCloseFd will suffice.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Right now virLogParse* functions are doing both parsing and defining of filters
and outputs which should be two separate operations. Since the naming is
apparently a bit poor this patch renames these functions to
virLogParseAndDefine* which eventually will be replaced by virLogSet*.
Additionally, virLogParse{Filter,Output} will be later (after the split) reused,
so that these functions do exactly what the their name suggests.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
During first stage of virlog.c refactor, commit 0b231195 forgot to remove the
macro definition along with its usage.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
If the event is already disabled, then don't bother with setting it
disabled again. Causes unnecessary error on systems that don't support
the feature anyway.
Provide the Steal API for any code paths that will desire to grab the
object array and then free it afterwards rather than relying to freeing
the whole chain from the reply.
Libvirt, on its own, shouldn't decide whether an expired lease should
stay in the custom leases database or not. It should rather rely on
the 'DEL' event from dnsmasq.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is nothing Linux-specific in that function. Also since commit
8c3b5bf481 mingw build is broken due to
the fact that this function is not compiled in the library.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We will need this function shortly when implementing
nodeGetCPUStats in the test driver.
Signed-off-by: Tomáš Ryšavý <tom.rysavy.0@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Name it virNumaGetHostMemoryNodeset and return only NUMA nodes which
have memory installed. This is necessary as the kernel is not very happy
to set the memory cgroup setting for nodes which do not have any memory.
This would break vcpu hotplug with following message on such
configruation:
Invalid value '0,8' for 'cpuset.mems': Invalid argument
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375268
Commit id 'a48c7141' altered how to determine if a volume was encrypted
by adding a peek at an offset into the file at a specific buffer location.
Unfortunately, all that was compared was the first "char" of the buffer
against the expect "int" value.
Restore the virReadBufInt32BE to get the complete field in order to
compare against the expected value from the qcow2EncryptionInfo or
qcow1EncryptionInfo "modeValue" field.
This restores the capability to create a volume with encryption, then
refresh the pool, and still find the encryption for the volume.
When a new filter is being defined, the return code is not handled properly,
thus triggering OOM error reporting routine (bug introduced by 51b2606f).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Commit id 'b00d7f29' shifted the opening of the /sys/devices/intel_cqm/type
file from event enable to perf event initialization. If the file did not
exist, then an error would be written to the domain log:
2016-09-06 20:51:21.677+0000: 7310: error : virFileReadAll:1360 : Failed to open file '/sys/devices/intel_cqm/type': No such file or directory
Since the error is now handled in virPerfEventEnable by checking if the
event_attr->attrType == 0 for CMT, MBML, and MBMT events - we can just
use the Quiet API in order to not log the error we're going to throw away.
Additionally, rather than using virReportSystemError, use virReportError
and VIR_ERR_ARGUMENT_UNSUPPORTED in order to signify that support isn't there
for that type of perf event - adjust the error message as well.
There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.
Backtrace is the following:
Thread #1:
0 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
1 in virCondWait (c=<optimized out>, m=<optimized out>)
at util/virthread.c:154
2 in virThreadPoolFree (pool=0x7f0810110b50)
at util/virthreadpool.c:266
3 in qemuStateCleanup () at qemu/qemu_driver.c:1116
4 in virStateCleanup () at libvirt.c:808
5 in main (argc=<optimized out>, argv=<optimized out>)
at libvirtd.c:1660
Thread #2:
0 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at util/virobject.c:169
1 in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass=<optimized out>) at util/virobject.c:365
2 in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3 in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0 <qemuProcessAutoDestroy>) at util/virclosecallbacks.c:163
4 in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
5 in qemuProcessStop (driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at qemu/qemu_process.c:5854
6 in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at qemu/qemu_driver.c:4585
7 qemuProcessEventHandler (data=<optimized out>, opaque=0x7f081018be50) at qemu/qemu_driver.c:4629
8 in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at util/virthreadpool.c:145
9 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0
Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
In the latest glibc, major() and minor() functions are marked as
deprecated (glibc commit dbab6577):
CC util/libvirt_util_la-vircgroup.lo
util/vircgroup.c: In function 'virCgroupGetBlockDevString':
util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
In the GNU C Library, `major' is defined by <sys/sysmacros.h>.
For historical compatibility, it is currently defined by
<sys/types.h> as well, but we plan to remove this soon.
To use `major', include <sys/sysmacros.h> directly.
If you did not intend to use a system-defined macro `major',
you should #undef it after including <sys/types.h>.
[-Werror=deprecated-declarations]
if (virAsprintf(&ret, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0)
^~
In file included from /usr/include/features.h:397:0,
from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdio.h:28,
from ../gnulib/lib/stdio.h:43,
from util/vircgroup.c:26:
/usr/include/sys/sysmacros.h:87:1: note: declared here
__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
^
Moreover, in the glibc commit, there's suggestion to keep
ordering of including of header files as implemented here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Current implementation uses the dev.cpu.0.freq sysctl that is
provided by the cpufreq(4) framework and returns the actual
CPU frequency. However, there are environments where it's not available,
e.g. when running nested in KVM. In this case fall back to hw.clockrate
that reports CPU frequency at the boot time.
Resolves (hopefully):
https://bugzilla.redhat.com/show_bug.cgi?id=1369964
Currently the QEMU processes inherit their core dump rlimit
from libvirtd, which is really suboptimal. This change allows
their limit to be directly controlled from qemu.conf instead.
With current perf framework, this patch adds support and documentation
for more perf events, including cache misses, cache references, cpu cycles,
and instructions.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Introduce a static attr table and refactor virPerfEventEnable() for
general purpose usage.
This patch creates a static table/matrix that converts the VIR_PERF_EVENT_*
events into their respective "attr.type" and "attr.config" so that
virPerfEventEnable doesn't have the switch the calling function passes
by value the 'type'.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
libvirt uses the new_id PCI sysfs interface to bind a PCI stub driver
to a PCI device. The new_id interface is known to be buggy and racey,
hence a more deterministic interface was introduced in the 3.12 kernel:
driver_override. For more details see
https://www.redhat.com/archives/libvir-list/2016-June/msg02124.html
For more details about the driver_override interface and examples of
its usage, see
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/pci-driver.c?h=v3.12&id=782a985d7af26db39e86070d28f987cad21313c0
This patch adds support for the driver_override interface by
- adding new virPCIDevice{BindTo,UnbindFrom}StubWithOverride functions
that use the driver_override interface
- renames the existing virPCIDevice{BindTo,UnbindFrom}Stub functions
to virPCIDevice{BindTo,UnbindFrom}StubWithNewid to perserve existing
behavior on new_id interface
- changes virPCIDevice{BindTo,UnbindFrom}Stub function to call one of
the above depending on availability of driver_override
The patch includes a bit of duplicate code, but allows for easily
dropping the new_id code once support for older kernels is no
longer desired.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Finding an USB device from the vendor/device values will be needed
by libxl driver to convert from vendor/device to bus/dev addresses.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1367259
Crash occurs because 'secrets' is being dereferenced in call:
if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
&src->encryption->secrets[0]->seclookupdef,
true) < 0)
(gdb) p *src->encryption
$1 = {format = 2, nsecrets = 0, secrets = 0x0, encinfo = {cipher_size = 0,
cipher_name = 0x0, cipher_mode = 0x0, cipher_hash = 0x0, ivgen_name = 0x0,
ivgen_hash = 0x0}}
(gdb) bt
priv=priv@entry=0x7fffc03be160, disk=disk@entry=0x7fffb4002ae0)
at qemu/qemu_domain.c:1087
disk=0x7fffb4002ae0, vm=0x7fffc03a2580, driver=0x7fffc02ca390,
conn=0x7fffb00009a0) at qemu/qemu_hotplug.c:355
Upon entry to qemuDomainAttachVirtioDiskDevice, src->encryption points
at a valid 'secret' buffer w/ nsecrets == 1; however, the call to
qemuDomainDetermineDiskChain will call virStorageFileGetMetadata
and eventually virStorageFileGetMetadataInternal where the src->encryption
was overwritten when probing the volume.
Commit id 'a48c7141' added code to virStorageFileGetMetadataInternal
to determine if the disk/volume would use/need encryption and allocated
a meta->encryption. This overwrote an existing encryption buffer
already provided by the XML
This patch adds a check for meta->encryption already present before
just allocating and overwriting an existing buffer. It then checks the
existing encryption data to ensure the XML provided format for the
disk matches the expected format read from the disk and errors if there
is a mismatch.
The first argument should be const char ** instead of
char **, because this is a search function and as such it
doesn't, and shouldn't, alter the haystack in any way.
This change means we no longer have to cast arrays of
immutable strings to arrays of mutable strings; we still
have to do the opposite, though, but that's reasonable.
Usually, this variable is used to hold the return value for a
function of ours. Well, this is not the case. Its use does not
match our pattern and therefore it is very misleading. Drop it
and define an alternative @rc variable, but only in that single
block where it is needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This variable is very misleading. We use VIR_FORCE_CLOSE to set
it to -1 and returning it even though it does not refer to a FD
at all. It merely holds 0 or -1. Drop it completely. Also, at the
same time some corner cases are fixed too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1240439
In this function we create a macvtap device and open its tap
device. Possibly multiple times. Now the thing is, if opening the
tap device fails, that is virNetDevMacVLanTapOpen() returns a
negative value, we unroll all the changes BUT return 0 fooling
caller into thinking everything went okay.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit b3e4401dc6 introduced a check to ignore an error if the guest
is already terminated. However the check accidentally compared
error.code with VIR_ERR_ERROR, which is an error level, not an error
code. Because of this, almost every error got silently ignored.
Fixes: b3e4401dc6 ("systemd: don't report an error if the guest is
already terminated")
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
The virJSONValueArraySize() function return ssize_t (with
possibly returning -1 if the passed json is not an array).
Storing the return value into size_t is possibly dangerous then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1356436
According to RFC 3721 (https://www.ietf.org/rfc/rfc3721.txt), there are
two ways to "discover" targets in/for the iSCSI environment. Discovery
is the process which allows the initiator to find the targets to which
it has access and at least one address at which each target may be
accessed.
The method currently implemented in libvirt using the virISCSIScanTargets
API is known as "SendTargets" discovery. This method is more useful when
the target IP Address and TCP port information are available, e.g. in
libvirt terms the "portal". It returns a list of targets for the portal.
From that list, the target can be found. This operation can also fill an
iSCSI node table into which iSCSI logins may occur. Commit id '56057900'
altered that filling by adding the "--op nonpersistent" since it was
not necessarily desired to perform that for non libvirt related targets.
The second method is "Static Configuration". This method not only needs
the IP Address and TCP port (e.g. portal), but also the iSCSI target name.
In libvirt terms this would be the device path field from the iSCSI pool
<source> XML. This patch implements the second methodology using that
required device path as the targetname.
The current LUKS support has a "luks" volume type which has
a "luks" encryption format.
This partially makes sense if you consider the QEMU shorthand
syntax only requires you to specify a format=luks, and it'll
automagically uses "raw" as the next level driver. QEMU will
however let you override the "raw" with any other driver it
supports (vmdk, qcow, rbd, iscsi, etc, etc)
IOW the intention though is that the "luks" encryption format
is applied to all disk formats (whether raw, qcow2, rbd, gluster
or whatever). As such it doesn't make much sense for libvirt
to say the volume type is "luks" - we should be saying that it
is a "raw" file, but with "luks" encryption applied.
IOW, when creating a storage volume we should use this XML
<volume>
<name>demo.raw</name>
<capacity>5368709120</capacity>
<target>
<format type='raw'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</target>
</volume>
and when configuring a guest disk we should use
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/home/berrange/VirtualMachines/demo.raw'/>
<target dev='sda' bus='scsi'/>
<encryption format='luks'>
<secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccd2f80d6f'/>
</encryption>
</disk>
This commit thus removes the "luks" storage volume type added
in
commit 318ebb36f1
Author: John Ferlan <jferlan@redhat.com>
Date: Tue Jun 21 12:59:54 2016 -0400
util: Add 'luks' to the FileTypeInfo
The storage file probing code is modified so that it can probe
the actual encryption formats explicitly, rather than merely
probing existance of encryption and letting the storage driver
guess the format.
The rest of the code is then adapted to deal with
VIR_STORAGE_FILE_RAW w/ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS
instead of just VIR_STORAGE_FILE_LUKS.
The commit mentioned above was included in libvirt v2.0.0.
So when querying volume XML this will be a change in behaviour
vs the 2.0.0 release - it'll report 'raw' instead of 'luks'
for the volume format, but still report 'luks' for encryption
format. I think this change is OK because the storage driver
did not include any support for creating volumes, nor starting
guets with luks volumes in v2.0.0 - that only since then.
Clearly if we change this we must do it before v2.1.0 though.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Refactor the virStorageFileMatchesNNN methods so that
they don't take a struct FileFormatInfo parameter, but
instead get the actual raw dat items they needs. This
will facilitate reuse in other contexts.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To allow richer definitions of disk sources add infrastructure that will
allow to register functionst generating a JSON object based definition.
This infrastructure will then convert the definition to the proper
command line syntax and use it in cases where it's necessary. This will
allow to keep legacy definitions for back-compat when possible and use
the new definitions for the configurations requiring them.
Add support for converting objects nested in arrays with a numbering
discriminator on the command line. This syntax is used for the
object-based specification of disk source properties.
Add a modular parser that will allow to parse 'json' backing definitions
that are supported by qemu. The initial implementation adds support for
the 'file' driver.
Due to the approach qemu took to implement the JSON backing strings it's
possible to specify them in two approaches.
The object approach:
json:{ "file" : { "driver":"file",
"filename":"/path/to/file"
}
}
And a partially flattened approach:
json:{"file.driver":"file"
"file.filename":"/path/to/file"
}
Both of the above are supported by qemu and by the code added in this
commit. The current implementation de-flattens the first level ('file.')
if possible and required. Other handling may be added later but
currently only one level was possible anyways.
Since commit c4bdff19, the path to the configuration file has been constructed
in the following manner:
- if no config filename was passed to virConfLoadConfigPath, libvirt.conf was
used as default
- otherwise the filename was concatenated with
"<config_dir>/libvirt/libvirt%s%s.conf" which in admin case resulted in
"libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led to
ignoring all user settings in libvirt-admin.conf. This patch requires the
config filename to be always provided as an argument with the concatenation
being simplified.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357364
Signed-off-by: Erik Skultety <eskultet@redhat.com>
For use with memory hotplug virQEMUBuildCommandLineJSONRecurse attempted
to format JSON arrays as bitmap on the command line. Make the formatter
function configurable so that it can be reused with different syntaxes
of arrays such as numbered arrays for use with disk sources.
This patch extracts the code and adds a parameter for the function that
will allow to plug in different formatters.
Until now the JSON->commandline convertor was used only for objects
created by qemu. To allow reusing it with disk formatter we'll need to
escape ',' as usual in qemu commandlines.
Refactor the command line generator by adding a wrapper (with
documentation) that will handle the outermost object iteration.
This patch also renames the functions and tweaks the error message for
nested arrays to be more universal.
The new function is then reused to simplify qemucommandutiltest.
As we already test that the extraction of the backing store string works
well additional tests for the backing store string parser can be made
simpler.
Export virStorageSourceNewFromBackingAbsolute and use it to parse the
backing store strings, format them using virDomainDiskSourceFormat and
match them against expected XMLs.
The symbol being missing has been reported as causing build
failures on OS X. If it's not already defined, define it to
zero so that it won't have any effect.
Partially resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1301021
If the volume xml was looking to create a luks volume take the necessary
steps in order to make that happen.
The processing will be:
1. create a temporary file (virStorageBackendCreateQemuImgSecretPath)
1a. use the storage driver state dir path that uses the pool and
volume name as a base.
2. create a secret object (virStorageBackendCreateQemuImgSecretObject)
2a. use an alias combinding the volume name and "_luks0"
2b. add the file to the object
3. create/add luks options to the commandline (virQEMUBuildLuksOpts)
3a. at the very least a "key-secret=%s" using the secret object alias
3b. if found in the XML the various "cipher" and "ivgen" options
Signed-off-by: John Ferlan <jferlan@redhat.com>
virConfGetValueLLong() errors out if the value is too big to
fit into a long long integer, but claims the supported range
to be (0,LLONG_MAX) instead of (LLONG_MIN,LLONG_MAX).
When parsing numeric values, we always store them as unsigned
unless they're negative. We can use this fact to simplify the
logic by removing a bunch of unnecessary checks.
Commit 6381c89f8c changed virConfValue to store long long
integers instead of long integers; however, the temporary variable
used in virConfParseLong() was not updated accordingly, causing
trouble for 32-bit machines.
If size_t is the same size as long long, then we can skip
some of the range checks. This avoids triggering some
bogus compiler warning messages.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virConf 'l' field is a 'signed long long', so whenever
the 'type' field is VIR_CONF_ULONG, we should explicitly cast
'l' to a 'unsigned long long' before doing range checks.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This function tries to get a ssize_t value from a config file.
But before returning it, it checks whether the value would fit in
ssize_t and if not an error is printed out among with the range
for the ssize_t type. However, on some platforms SSIZE_MAX may
actually be a signed long type:
util/virconf.c: In function 'virConfGetValueSSizeT':
util/virconf.c:1268:9: error: format '%zd' expects argument of type 'signed size_t', but argument 9 has type 'long int' [-Werror=format=]
virReportError(VIR_ERR_INTERNAL_ERROR,
^
$ grep -r SSIZE_MAX /usr/include/
/usr/include/bits/posix1_lim.h:#ifndef SSIZE_MAX
/usr/include/bits/posix1_lim.h:# define SSIZE_MAX LONG_MAX
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
IPv6 RA always contains an implicit default route via
the link-local address of the source of RA. This forces
the guest to install a route via isolated network, which
may disturb the guest's networking in case of multiple interfaces.
More info in 013427e6e7.
The validity of this route is controlled by "default [route] lifetime"
field of RA. If the lifetime is set to 0 seconds, then no route
is installed by receiver.
dnsmasq 2.67+ supports "ra-param=<interface>,<RA interval>,<default
lifetime>" option. We pass "ra-param=*,0,0"
(here, RA_interval=0 means default) to disable default gateway in RA
for isolated networks.
At least with systemd v210, NOTIFY_SOCKET is abstact, e.g.
@/org/freedesktop/systemd1/notify. sendmsg() fails on such a socket
with "Connection refused". The unix(7) man page contains the following
details wrt abstract socket addresses
abstract: an abstract socket address is distinguished (from a
pathname socket) by the fact that sun_path[0] is a null byte
('\0'). The socket's address in this namespace is given by the
additional bytes in sun_path that are covered by the specified
length of the address structure. (Null bytes in the name have
no special significance.)
So we need to be more precise about the address length, setting it to
the sizeof sa_family_t + length of address copied to sun_path instead
of setting it to the sizeof the entire sockaddr_un struct.
Resolves: https://bugzilla.opensuse.org/show_bug.cgi?id=987668
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
In an unlikely event of execve() failing, the virCommandExec()
function does not report any error, even though checks that are
at the beginning of the function are verbose when failing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently many users of virConf APIs are defining the same
macros for calling virConfValue() and then doing type
checking. To remove this repeated code, add a set of
typesafe accessor methods.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If the config file does not end with a \n, the parser will append
one. When re-allocating the array though, it is mistakenly assuming
that 'len' is the length including the trailing NUL, but it does
not. So we must add 2 to len, when reallocating, not 1.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When storage secret is parsed in virStorageEncryptionSecretParse(),
virSecretLookupParseSecret() which allocates some memory. This is
however never freed.
==21711== 134 bytes in 6 blocks are definitely lost in loss record 70 of 85
==21711== at 0x4C29F80: malloc (vg_replace_malloc.c:296)
==21711== by 0xBCA0356: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4)
==21711== by 0xA9F432E: virXMLPropString (virxml.c:479)
==21711== by 0xA9D25B0: virSecretLookupParseSecret (virsecret.c:70)
==21711== by 0xA9D616E: virStorageEncryptionSecretParse (virstorageencryption.c:172)
==21711== by 0xA9D66B2: virStorageEncryptionParseXML (virstorageencryption.c:281)
==21711== by 0xA9D68DF: virStorageEncryptionParseNode (virstorageencryption.c:338)
==21711== by 0xAA12575: virDomainDiskDefParseXML (domain_conf.c:7606)
==21711== by 0xAA2CAC6: virDomainDefParseXML (domain_conf.c:16658)
==21711== by 0xAA2FC75: virDomainDefParseNode (domain_conf.c:17472)
==21711== by 0xAA2FAE4: virDomainDefParse (domain_conf.c:17419)
==21711== by 0xAA2FB72: virDomainDefParseFile (domain_conf.c:17443)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If we don't HAVE_LINUX_KVM_H, we can't query /dev/kvm to discover
the limits on the number of vCPUs, so we report an error and
return a negative value instead.
As there is an explicit constructor for the special case of empty
bitmaps, we should mention that the generic constructors rejects the
creation of empty bitmaps.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Before the variable 'bits' was initialized with 0 (commit
3470cd860d), the following bug was
possible.
A function call with an empty bitmap leads to undefined
behavior. Because if 'bitmap->map_len == 0' 'unusedBits' will be <= 0
and 'sz == 1'. So the non global and non static variable 'bits' would
have never been set. Consequently the check 'bits == 0' results in
undefined behavior.
This patch clarifies the current version of the function by handling the
empty bitmap explicitly. Also, for an empty bitmap there is obviously no
bit set so we can just return -1 (indicating no bit set) right away. The
explicit check for 'bits == 0' after the loop is unnecessary because we
only get to this point if no set bit was found.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
This is just a convenience method for discarding a list of filters instead of
using a 'for' loop everywhere. It is safe to pass -1 as the number of elements
in the list as well as passing NULL as list reference.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Provide a separate method to free a logging filter object. This will come handy
once a method to create an individual logging filter object is introduced.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This is just a convenience method for discarding a list of outputs instead of
using a 'for' loop everywhere. It is safe to pass -1 as the number of elements
in the list as well as passing NULL as list reference.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Provide a separate method to free a logging output object. This will come handy
once a method to create an individual logging output object is introduced.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Same as with outputs; since the operations will be further divided into smaller
tasks, creating a filter will become a separate operation that will return
a reference to a newly created filter.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Right now, we define outputs one after another. However, the correct flow
should be to define a set of outputs as a whole unit. Therefore each output
should be first created, placed into an array/list and the list will be
defined. Output creation should be a separate operation, so an output will be
returned by a reference. From that perspective, it makes perfect sense to
only store pointers to actual outputs.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
In this particular case, reset is meant as clearing the whole list of
outputs/filters, not resetting it to a predefined default setting. Looking at
it from that perspective, returning the number of records removed doesn't help
the caller in any way (not that any of the callers would actually check for
it). Well, callers could detect an error from the number of successfully
removed records, but the only thing that can fail in virLogReset is force
closing a file descriptor in which case the error isn't propagated back to
virLogReset anyway. Conclusion: there is no practical use for having a return
type of 'int' rather than 'void' in this case.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This will apply to any IP address setting that uses
virNetDevIPInfoAddToDev() (which so far is only the guest-side of LXC
type='ethernet' interfaces).
(This patch had been pushed earlier in
commit cb20f989df, but was reverted in
commit cba06aea8d because it had been
accidentally pushed during the freeze for release 2.0.0)
The peer attribute is used to set the property of the same name in the
interface IP info:
<interface type='ethernet'>
...
<ip family='ipv4' address='192.168.122.5'
prefix='32' peer='192.168.122.6'/>
...
</interface>
Note that this element is used to set the IP information on the
*guest* side interface, not the host side interface - that will be
supported in an upcoming patch.
(This patch now has quite a history: it was originally pushed in
commit 690969af, which was subsequently reverted in commit 1d14b13f,
then reworked and pushed (along with a lot of other related/supporting
patches) in commit 93135abf1; however *that* commit had been
accidentally pushed during dev. freeze for release 2.0.0, so it was
again reverted in commit f6acf039f0).
Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru>
Signed-off-by: Laine Stump <laine@laine.org>
This patch takes the code out of
lxcContainerRenameAndEnableInterfaces() that adds all IP addresses and
IP routes to the interface, and puts it into a utility function
virNetDevIPInfoAddToDev() in virnetdevip.c so that it can be used by
anyone.
One small change in functionality -
lxcContainerRenameAndEnableInterfaces() previously would add all IP
addresses to the interface while it was still offline, then set the
interface online, and then add the routes. Because I don't want the
utility function to set the interface online, I've moved this up so
the interface is first set online, then IP addresses and routes are
added. This is the same order that the network service from
initscripts (in ifup-ether) does it, so it shouldn't pose any problem
(and hasn't, in the tests that I've run).
(This patch had been pushed earlier in commit
f1e0d0da11, but was reverted in commit
05eab47559 because it had been
accidentally pushed during the freeze for release 2.0.0)
In order to use more common code and set up for a future type, modify the
encryption secret to allow the "usage" attribute or the "uuid" attribute
to define the secret. The "usage" in the case of a volume secret would be
the path to the volume as dictated by the backwards compatibility brought
on by virStorageGenerateQcowEncryption where it set up the usage field as
the vol->target.path and didn't allow someone to provide it. This carries
into virSecretObjListFindByUsageLocked which takes the secret usage attribute
value from from the domain disk definition and compares it against the
usage type from the secret definition. Since none of the code dealing
with qcow/qcow2 encryption secrets uses usage for lookup, it's a mostly
cosmetic change. The real usage comes in a future path where the encryption
is expanded to be a luks volume and the secret will allow definition of
the usage field.
This code will make use of the virSecretLookup{Parse|Format}Secret common code.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Commit cf0568b0af moved a bunch of functions from virNetDev
to the more specific virNetDevIP; however, not all of the
existing uses were moved properly, causing build failures on
FreeBSD.
Complete the transition to the new names and drop the
obsolete declarations from the header file while at it.
Not including the header causes
util/virnetdevip.c:520:5: error:
unknown type name 'virCommandPtr'; did you mean 'virCondPtr'?
virCommandPtr cmd = NULL;
^~~~~~~~~~~~~
and plenty more similar failures when compiling on FreeBSD.
The peer attribute is used to set the property of the same name in the
interface IP info:
<interface type='ethernet'>
...
<ip family='ipv4' address='192.168.122.5'
prefix='32' peer='192.168.122.6'/>
...
</interface>
Note that this element is used to set the IP information on the
*guest* side interface, not the host side interface - that will be
supported in an upcoming patch.
(This is an updated *re*-commit of commit 690969af, which was
subsequently reverted in commit 1d14b13f).
Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru>
Signed-off-by: Laine Stump <laine@laine.org>
This patch takes the code out of
lxcContainerRenameAndEnableInterfaces() that adds all IP addresses and
IP routes to the interface, and puts it into a utility function
virNetDevIPInfoAddToDev() in virnetdevip.c so that it can be used by
anyone.
One small change in functionality -
lxcContainerRenameAndEnableInterfaces() previously would add all IP
addresses to the interface while it was still offline, then set the
interface online, and then add the routes. Because I don't want the
utility function to set the interface online, I've moved this up so
the interface is first set online, then IP addresses and routes are
added. This is the same order that the network service from
initscripts (in ifup-ether) does it, so it shouldn't pose any problem
(and hasn't, in the tests that I've run).
It makes more sense to have the logging at the lower level so other
callers can share the goodness.
While removing so much stuff from / touching so many lines in
lxcContainerRenameAndEnableInterfaces() (which used to have this
debug/error logging), label names were changed and it was updated to
use the now-more-common method of initializing ret to -1 (failure),
then setting to 0 right before the cleanup label.
There are currently two places in the domain where this combination is
used, and there is about to be another. This patch puts them together
for brevity and uniformity.
As with the newly-renamed virNetDevIPAddr and virNetDevIPRoute
objects, the new virNetDevIPInfo object will need to be accessed by a
utility function that calls low level Netlink functions (so we don't
want it to be in the conf directory) and will be called from multiple
hypervisor drivers (so it can't be in any hypervisor directory); the
most appropriate place is thus once again the util directory.
The parse and format functions are in conf/domain_conf.c because only
the domain XML (i.e. *not* the network XML) has this exact combination
of IP addresses plus routes. Note that virDomainNetIPInfoFormat() will
end up being the only caller to virDomainNetRoutesFormat() and
virDomainNetIPsFormat(), so it will just subsume those functions in a
later patch, but we can't do that until they are no longer called.
(It would have been nice to include the interface name within the
virNetDevIPInfo object (with a slight name change), but that can't
be done cleanly, because in each case the interface name is provided
in a different place in the XML relative to the routes and IP
addresses, so putting it in this object would actually make the code
more confused rather than simpler).
These functions all need to be called from a utility function that
must be located in the util directory, so we move them all into
util/virnetdevip.[ch] now that it exists.
Function and struct names were appropriately changed for the new
location, but all code is unchanged aside from motion and renaming.
This patch splits virnetdev.[ch] into multiple files, with the new
virnetdevip.[ch] containing all the functions related to setting and
retrieving IP-related info for a device (both addresses and routes).
Commit c9a641 (first appearred in 1.2.12) added support for setting
the guest-side IP address of veth devices in lxc domains.
Unfortunately, it hardcoded the assumption that the proper prefix for
any IP address with no explicit prefix in the config should be "24";
that is only correct for class C IPv4 addresses, but not for any other
IPv4 address, nor for any IPv6 address.
The good news is that there is already a function in libvirt that will
determine the proper default prefix for any IP address. This patch
replaces the use of the ill-fated VIR_SOCKET_ADDR_DEFAULT_PREFIX with
calls to virSocketAddrGetIPPrefix().
There are times when we don't have a netmask pointer to give to
virSocketAddrGetIPPrefix() (e.g. the IP addresses in domain interfaces
only have a prefix, no netmask), but it would have caused a segv if we
called it with NULL instead of a pointer to a netmask. This patch
qualifies the code that would use the netmask or address pointers to
check for NULL first.
I'm tired of mistyping this all the time, so let's do it the same all
the time (similar to how we changed all "Pci" to "PCI" awhile back).
(NB: I've left alone some things in the esx and vbox drivers because
I'm unable to compile them and they weren't obviously *not* a part of
some API. I also didn't change a couple of variables named,
e.g. "somethingIptables", because they were derived from the name of
the "iptables" command)
These had been declared in conf/device_conf.h, but then used in
util/virnetdev.c, meaning that we had to #include conf/device_conf.h
in virnetdev.c (which we have for a long time said shouldn't be done.
This caused a bigger problem when I tried to #include util/virnetdev.h
in a file in src/conf (which is allowed) - for some reason the
"device_conf.h: File not found" error.
The solution is to move the data types and functions used in util
sources from conf to util. Some names were adjusted during the move
("virInterface" --> "virNetDevIf", and "VIR_INTERFACE" -->
"VIR_NETDEV_IF")
virNetDevLinkDump should have been in virnetlink.c, but that file
didn't exist yet when the function was created. It didn't really
matter until now - I found that having virnetlink.h included by
virnetdev.h caused build problems when trying to #include virnetdev.h
in a .c file in src/conf (due to missing directory in -I). Rather than
fix that to further institutionalize the incorrect placement of this
one function, this patch moves the function.
Commit e81de04c switched virNetDevTapGetRealDeviceName() to use
virDirOpen() instead of opendir(), however it mistakenly dropped
DIR *dirp declaration, so restore that to fix build.
The version field historically has been a 4 byte data; however, an upcoming
new type will use a 2 byte version. So let's adjust for that now.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
In order to read 16 bits of data in the native format and convert add
the 16 bit macros to match existing 32 and 64 bit code.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This kvmGetMaxVCPUs() needs to be used at two different places
so move it to utils with appropriate name and mark it as private
global now.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
The directories we iterate over are unlikely to contain any entries
starting with a dot, other than '.' and '..' which is already skipped
by virDirRead.
Move to virsecret.c and rename to virSecretLookupParseSecret. Also convert
to usage xmlNodePtr and virXMLPropString rather than virXPathString.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Move the enum into a new src/util/virsecret.h, rename it to be
virSecretLookupType. Add a src/util/virsecret.h in order to perform
a couple of simple operations on the secret XML and virSecretLookupTypeDef
for clearing and copying.
This includes quite a bit of collateral damage, but the goal is to remove
the "virStorage*" and replace with the virSecretLookupType so that it's
easier to to add new lookups that aren't necessarily storage pool related.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Since introduction of the DAC security driver we've documented that
seclabels with a leading + can be used with numerical uid. This would
not work though with the rest of libvirt if the uid was not actually
used in the system as we'd fail when trying to get a list of
supplementary groups for the given uid. Since a uid without entry in
/etc/passwd (or other user database) will not have any supplementary
groups we can treat the failure to obtain them as such.
This patch modifies virGetGroupList to not report the error for missing
users and makes it return an empty list or just the group specified in
@gid.
All callers will grant less permissions to a user in case of failure of
this function and thus this change is safe.
This will be used for the caller that needs to specify a separator.
Currently identical to virBitmapParse.
Also change one test case to use the new function.
Commit b3d069872c added peer address setting to the low level
virNetDevSetIPAddress() function, but ended up causing a segfault in
cases where the caller passed NULL for peer address.
Commit a3510e33d3 fixed the segfault, but managed to cause us to
skip setting the broadcast address when setting an interface's IP
address. The result is that the broadcast address is 0.0.0.0 for all
libvirt-created bridges (and interfaces in lxc containers with IP
addresses set by libvirt).
This was reported on the mailing list:
https://www.redhat.com/archives/libvir-list/2016-June/msg00027.html
but I was too busy to investigate at the time. I found it by accident
today while refactoring virNetDevSetIPAddress(). Since this regression
is present in the 1.3.5 release, I'm sending the bugfix as a separate
patch from my larger refactoring patchset.
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>
