Clang 3.9 refuses to compile the existing code with the
following error:
util/virfirewall.c:425:20: error: passing an object that undergoes
default argument promotion to 'va_start'
has undefined behavior [-Werror,-Wvarargs]
va_start(args, layer);
^
util/virfirewall.c:420:37: note: parameter of type 'virFirewallLayer'
is declared here
virFirewallLayer layer,
^
This happens because 'layer' is of type virFirewallLayer, which
is an enum type and not a standard type such as eg. void* or int.
To solve the issue, turn virFirewallAddRule() from a very thin
wrapper around virFirewallAddRuleFullV() to a macro that expands
to a call to virFirewallAddRuleFull() - itself a very thin wrapper
around the aforementioned virFirewallAddRuleFullV() - with no loss
of functionality or type safety.
https://bugzilla.redhat.com/show_bug.cgi?id=1405269
If a secret was not provided for what was determined to be a LUKS
encrypted disk (during virStorageFileGetMetadata processing when
called from qemuDomainDetermineDiskChain as a result of hotplug
attach qemuDomainAttachDeviceDiskLive), then do not attempt to
look it up (avoiding a libvirtd crash) and do not alter the format
to "luks" when adding the disk; otherwise, the device_add would
fail with a message such as:
"unable to execute QEMU command 'device_add': Property 'scsi-hd.drive'
can't find value 'drive-scsi0-0-0-0'"
because of assumptions that when the format=luks that libvirt would have
provided the secret to decrypt the volume.
Access to unlock the volume will thus be left to the application.
After 478ddedc12 a bug is fixed where we wrongly presumed loopack
device name on non-Linux systems. It's lo0. However, the fix is
not reflected in the tests which are failing now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Due to nature of operations we do over the string list (more
precisely due to how virStringListRemove() works), it is not the
best idea to use dataFree callback. Problem is, on MAC address
remove, the string list remove function modifies the original
list in place. Then, virHashUpdateEntry() is called which frees
all the data stored in the list rendering @newMacsList point to
freed data.
==16002== Invalid read of size 8
==16002== at 0x50BC083: virFree (viralloc.c:582)
==16002== by 0x513DC39: virStringListFree (virstring.c:251)
==16002== by 0x51089B4: virMacMapHashFree (virmacmap.c:67)
==16002== by 0x50EF30B: virHashAddOrUpdateEntry (virhash.c:352)
==16002== by 0x50EF4FD: virHashUpdateEntry (virhash.c:415)
==16002== by 0x5108BED: virMacMapRemoveLocked (virmacmap.c:129)
==16002== by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002== by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002== by 0x403F15: virTestRun (testutils.c:180)
==16002== by 0x4032C4: mymain (virmacmaptest.c:205)
==16002== by 0x405A3B: virTestMain (testutils.c:992)
==16002== by 0x403D87: main (virmacmaptest.c:237)
==16002== Address 0xdd5a4d0 is 0 bytes inside a block of size 24 free'd
==16002== at 0x4C2AD6F: realloc (vg_replace_malloc.c:693)
==16002== by 0x50BB99B: virReallocN (viralloc.c:245)
==16002== by 0x513DC0B: virStringListRemove (virstring.c:235)
==16002== by 0x5108BA6: virMacMapRemoveLocked (virmacmap.c:124)
==16002== by 0x51092D5: virMacMapRemove (virmacmap.c:346)
==16002== by 0x402F02: testMACRemove (virmacmaptest.c:107)
==16002== by 0x403F15: virTestRun (testutils.c:180)
==16002== by 0x4032C4: mymain (virmacmaptest.c:205)
==16002== by 0x405A3B: virTestMain (testutils.c:992)
==16002== by 0x403D87: main (virmacmaptest.c:237)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In virMacMapRemoveLocked() we have two variables: @macsList and
@newMacsList. Obviously, @newMacsList is supposed to hold pointer
to modified list but in fact it holds pointer to the old list.
It's confusing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Before, boot devices information for CTs was always empty and we
didn't indicate that containers can boot from disk.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as disks.
Before, we always mistakenly presented disks based on files as filesystems, which is
not completely correct. Now we are going to show either disks or filesystems depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a hint
for libvirt representation of disks. Since now, if we have filesystems in input xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
Implicit devices like controllers are confusing for CTs and
function virDomainDefAddImplicitDevices never intended to be called
for CTs.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>
virQEMUCapsSupportsChardev existing checks returns true
for spapr-vty alone. Instead verify spapr-vty validity
and let the logic to return true for other device types
so that virtio-console passes.
The non-pseries machines dont have spapr-vio-bus. So, the
function always returned false for them before.
Fixes - https://bugzilla.redhat.com/show_bug.cgi?id=1257813
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1332019
This function will essentially be a wrapper to virStorageVolInfo in order
to provide a mechanism to have the "physical" size of the volume returned
instead of the "allocation" size. This will provide similar capabilities to
the virDomainBlockInfo which can return both allocation and physical of a
domain storage volume.
NB: Since we're reusing the _virStorageVolInfo and not creating a new
_virStorageVolInfoFlags structure, we'll need to generate the rpc APIs
remoteStorageVolGetInfoFlags and remoteDispatchStorageVolGetInfoFlags
(although both were originally created from gendispatch.pl and then
just copied into daemon/remote.c and src/remote/remote_driver.c).
The new API will allow the usage of a VIR_STORAGE_VOL_GET_PHYSICAL flag
and will make the decision to return the physical or allocation value
into the allocation field.
In order to get that physical value, virStorageBackendUpdateVolTargetInfoFD
adds logic to fill in physical value matching logic in qemuStorageLimitsRefresh
used by virDomainBlockInfo when the domain is inactive.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Although the virStorageBackendUpdateVolTargetInfo will update the
target.physical value, there is no way to provide that information
via the virStorageGetVolInfo API since it only returns the capacity
and allocation of a volume. So as described in commit id '0282ca45',
it should be possible to generate an output only <physical> value
for that purpose.
This patch generates the <physical> value in the volume XML output
for the sole purpose of being able to view/see the value to allow
someone to parse the XML in order to obtain the value.
Update the documentation to describe the output only nature.
Signed-off-by: John Ferlan <jferlan@redhat.com>
According to commit id '0282ca45a' the 'physical' value should
essentially be the last offset of the image or the host physical
size in bytes of the image container. However, commit id '15fa84ac'
refactored the GetBlockInfo to use the same returned data as the
GetStatsBlock API for an active domain. For the 'entry->physical'
that would end up being the "actual-size" as set through the
qemuMonitorJSONBlockStatsUpdateCapacityOne (commit '7b11f5e5').
Digging deeper into QEMU code one finds that actual_size is
filled in using the same algorithm as GetBlockInfo has used for
setting the 'allocation' field when the domain is inactive.
The difference in values is seen primarily in sparse raw files
and other container type files (such as qcow2), which will return
a smaller value via the stat API for 'st_blocks'. Additionally
for container files, the 'capacity' field (populated via the
QEMU "virtual-size" value) may be slightly different (smaller)
in order to accomodate the overhead for the container. For
sparse files, the state 'st_size' field is returned.
This patch thus alters the allocation and physical values for
sparse backed storage files to be more appropriate to the API
contract. The result for GetBlockInfo is the following:
capacity: logical size in bytes of the image (how much storage
the guest will see)
allocation: host storage in bytes occupied by the image (such
as highest allocated extent if there are no holes,
similar to 'du')
physical: host physical size in bytes of the image container
(last offset, similar to 'ls')
NB: The GetStatsBlock API allows a different contract for the
values:
"block.<num>.allocation" - offset of the highest written sector
as unsigned long long.
"block.<num>.capacity" - logical size in bytes of the block device
backing image as unsigned long long.
"block.<num>.physical" - physical size in bytes of the container
of the backing image as unsigned long long.
This patch detects a misconfiguration between the disk bus type and disk
address type for controller based disk buses (SATA, SCSI, FDC and
IDE). The addresses of these bus types are all managed in common code so
it's possible to decide in common code whether the disk address and bus
type are compatible or not.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Disk->info is not live updatable so add a check for this. Otherwise
libvirt reports success even though no data was updated.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
This function will be needed by the QEMU driver in an upcoming
patch. Additionally, removed a useless empty line.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
This patch reduces the complexity of the filtering algorithm in
virCgroupDetect by first correcting the controller mask and then
checking for potential co-mounts without any correlating
controller mask modifications.
If you agree that this patch removes complexity and improves
readability it could simply be squashed into the first patch
of this series.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
The cgroup controller filtering in virCgroupDetect does not work
properly if the following conditions are met:
1) the host system does not have a cgroup controller which
libvirt requests (unavailable controller) and
2) libvirt is configured to disable a controller (disabled controller) and
3) the disabled controller is located before the unavailable controller
in virCgroupController.
As an example: The memory controller is unavailable and the cpuset
controller is configured to be disabled.
In this scenario trying to start a domain results in the error
error: Controller 'cpuset' is not wanted, but 'memory' is co-mounted: Invalid argument
This error occurs when virCgroupDetect is called with a valid parent group.
The resulting group created by virCgroupCopyMounts holds for cpuset and
memory controller empty mount points. The filtering of disabled controllers
checks for co-mounts by comparing the mount points. The cpuset controller
causes the filtering to occur before the memory controller is marked as to be
ignored by modifying the controller mask since it is unavailable.
Therefore the co-mount detection logic compares the cpuset and memory controller
mount points and since both are empty the memory controller is regarded
erroneously as being co-mounted.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to localOnly DNS domain, localPtr attribute can be used to
tell the DNS server not to forward reverse lookups for unknown IPs which
belong to the virtual network.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The API creates PTR domain which corresponds to a given addr/prefix.
Both IPv4 and IPv6 addresses are supported, but the prefix must be
divisible by 8 for IPv4 and divisible by 4 for IPv6.
The generated PTR domain has the following format
IPv4: 1.2.3.4.in-addr.arpa
IPv6: 0.1.2.3.4.5.6.7.8.9.a.b.c.d.e.f.0.1.2.3.4.5.6.7.8.9.a.b.c.d.e.f.ip6.arpa
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Iterating over all child nodes when we only support one instance of each
child is pretty weird. And it would even cause memory leaks if more
than one <tftp> element was specified.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The PERF_COUNT_HW_REF_CPU_CYCLES constant is not available
on all Linux distros libvirt targets, so its use must be
made conditional. Other constant have existed long enough
that we can assume they exist, as we don't support very
old distros like RHEL-5 any more.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Enable libvirt users to modify daemon's logging output settings from outside.
If either an empty string or NULL is passed, a default logging output will be
used the same way as it would be in case writing an empty string to the
libvirtd.conf
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Now that virLog{Get,Set}DefaultOutput routines are introduced we can wire them
up to the daemon's logging initialization code. Also, change the order of
operations a bit so that we still strictly honor our precedence of settings:
cmdline > env > config now that outputs and filters are not appended anymore.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Along with an empty string, it should also be possible for users to pass
NULL to the public APIs which in turn would trigger a routine(future
work) responsible for defining an appropriate default logging output
given the current circumstances.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
These helpers will manage the log destination defaults (fetch/set). The reason
for this is to stay consistent with the current daemon's behaviour with respect
to /etc/libvirt/<daemon>.conf file, since both assignment of an empty string
or not setting the log output variable at all trigger the daemon's decision on
the default log destination which depends on whether the daemon runs daemonized
or not.
This patch also changes the logic of the selection of the default
logging output compared to how it is done now. The main difference though is
that we should only really care if we're running daemonized or not, disregarding
the fact of (not) having a TTY completely (introduced by commit eba36a3878) as
that should be of the libvirtd's parent concern (what FD it will pass to it).
Before:
if (godaemon || !hasTTY):
if (journald):
use journald
if (godaemon):
if (privileged):
use SYSCONFIG/libvirtd.log
else:
use XDG_CONFIG_HOME/libvirtd.log
else:
use stderr
After:
if (godaemon):
if (journald):
use journald
else:
if (privileged):
use SYSCONFIG/libvirtd.log
else:
use XDG_CONFIG_HOME/libvirtd.log
else:
use stderr
Signed-off-by: Erik Skultety <eskultet@redhat.com>
External disk-only snapshots with recent enough qemu don't require
libvirt to pause the VM. The logic determining when to resume cpus was
slightly flawed and attempted to resume them even if they were not
paused by the snapshot code. This normally was not a problem, but with
locking enabled the code would attempt to acquire the lock twice.
The fallout of this bug would be a error from the API, but the actual
snapshot being created. The bug was introduced with when adding support
for external snapshots with memory (checkpoints) in commit f569b87.
Resolves problems described by:
https://bugzilla.redhat.com/show_bug.cgi?id=1403691
After qemu delivers the resume event it's already running and thus it's
too late to enter lockspaces since it may already have modified the
disk. The code only creates false log entries in the case when locking
is enabled. The lockspace needs to be acquired prior to starting cpus.
