GCC complains that inlining virStringTrimOptionalNewline is not
likely on some platforms:
cc1: warnings being treated as errors
../../src/util/virfile.c: In function 'virFileReadValueBitmap':
../../src/util/virstring.h:292: error: inlining failed in call to 'virStringTrimOptionalNewline': call is unlikely and code size would grow [-Winline]
../../src/util/virfile.c:3987: error: called from here [-Winline]
Inlining this function is not going to be a measurable performance
benefit either, since the time required to execute it is going to
be dominated by running of strlen() over the string, not by the
function call overhead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
New maint release version numbers of just A.B.C format, not the old
A.B.C.D format. Adjust the check that dynamically changes the Source
URL for maint releases
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Acked-by: Andrea Bolognani <abologna@redhat.com>
If __lxstat() and __xstat() functions are not available, build fails with:
CC virfilewrapper.o
virfilewrapper.c:180:5: error: no previous prototype for function '__lxstat' [-Werror,-Wmissing-prototypes]
int __lxstat(int ver, const char *path, struct stat *sb)
^
virfilewrapper.c:208:5: error: no previous prototype for function '__xstat' [-Werror,-Wmissing-prototypes]
int __xstat(int ver, const char *path, struct stat *sb)
Luckily, we already check presence of these functions in configure
using AC_CHECK_FUNCS, so just don't wrap these if they're not available.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Add info from yet another machine, this time with resctrl data so that
we can extend tests easily in a test-driven way.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Added only in drivers that were already calling
virCapabilitiesInitNUMA(). Instead of refactoring all the callers to
behave the same way in case of error, just follow what the callers are
doing for all the functions.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
We're only adding only info about L3 caches, we can add more
later (just by changing one line), but for now that's more than enough
without overwhelming anyone.
XML snippet of how this should look like (also seen as part of the commit):
<cache>
<bank id='0' level='3' type='both' size='8192' unit='KiB' cpus='0-7'/>
</cache>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
It is no longer needed thanks to the great virfilewrapper.c. And this
way we don't have to add a new set of functions for each prefixed
path.
While on that, add two functions that weren't there before, string and
scaled integer reading ones. Also increase the length of the string
being read by one to accompany for the optional newline at the
end (i.e. change INT_STRLEN_BOUND to INT_BUFSIZE_BOUND).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This mock (which is actually not mock at all, see later) can redirect
all accesses to a path into another path. There is no need to
create mocks for particular directories, you just create a directory
with all the data a redirect the test there.
In the future, this should also be able to register callbacks for
calls/paths, e.g. when the test is going to write into anything under
"/sys/devices", call function fce(); Then in the open() call we would
add information about the fd into some structure and in write() we
would call fce() with parameters like @path to write to, @data to
be written and pointer to optional return value, so that fce() itself
could stop the call from happening or change its behaviour. But
that's an idea for a latter day.
This is not a mock because it will not be preloaded, but compiled in
the test itself. See future patches for usage.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
So, because mingw is somehow OK with dereferencing a pointer within a
VIR_DEBUG macro, compared to outside of it to which it complained with a
"potential NULL pointer dereference" error (still a false positive), we
can make the code a tiny bit cleaner.
Sighed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
A previous commit changed the spec to use librbd1-devel on
RHEL-7, since this replaces ceph-devel from RHEL-6:
commit 6cfc8834c8
Author: Peter Krempa <pkrempa@redhat.com>
Date: Thu Mar 5 11:40:54 2015 +0100
spec: Enable RBD storage driver in RHEL-7
Use correct package names too as they differ.
RHEL-7 inherited this rename from Fedora though, so it should
have also made Fedora use the new names.
This was missed, because Fedora still provides a (deprecated)
back-compat RPM for ceph-devel that just pulls in librbd1-devel
(and others).
Fixing this stops libvirt pulling Java into the build root in
Fedora.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
YouCompleteMe[1] is a vim plugin that implements semantic
code completion using libclang.
For non-trivial projects such as libvirt, the plugin needs
some help figuring out where to find the various header
files: generate its configuration file at configure time
so that the plugin works out of the box.
[1] http://valloric.github.io/YouCompleteMe/
color_coded[1] is a vim plugin that implements semantic
syntax highlighting using libclang.
For non-trivial projects such as libvirt, the plugin needs
some help figuring out where to find the various header
files: generate its configuration file at configure time
so that the plugin works out of the box.
[1] https://github.com/jeaye/color_coded
The API docs extractor, ESX code generator and keycodemapdb tools
rely on python. Historically every platform that this present, but
with switch to Python3 by default, we're increasingly seeing
installs without a /usr/bin/python.
This tightens up the check during configure, so it exits immediately
if python is missing, rather than leaving an empty $(PYTHON) make
variable which leads to more obscure errors later.
Also add it as a build dep for Mingw, since Fedora build roots no
longer get python2 by default. This was not previously a major
problem, since both ESX & API generated files were included in
EXTRA_DIST, but the keycodemapdb generated files are not, so we
require python all the time now.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When adding a nwfilter onto the list in
virNWFilterObjListAssignDef() this array is re-allocated to match
demand for new size. However, it is never freed leading to a
leak:
==26535== 136 bytes in 1 blocks are definitely lost in loss record 1,079 of 1,250
==26535== at 0x4C2E2BE: realloc (vg_replace_malloc.c:785)
==26535== by 0x54BA28E: virReallocN (viralloc.c:245)
==26535== by 0x54BA384: virExpandN (viralloc.c:294)
==26535== by 0x54BA657: virInsertElementsN (viralloc.c:436)
==26535== by 0x55DB011: virNWFilterObjListAssignDef (virnwfilterobj.c:362)
==26535== by 0x55DB530: virNWFilterObjListLoadConfig (virnwfilterobj.c:503)
==26535== by 0x55DB635: virNWFilterObjListLoadAllConfigs (virnwfilterobj.c:539)
==26535== by 0x2AC5A28B: nwfilterStateInitialize (nwfilter_driver.c:250)
==26535== by 0x5621C64: virStateInitialize (libvirt.c:770)
==26535== by 0x124379: daemonRunStateInit (libvirtd.c:881)
==26535== by 0x554AC78: virThreadHelper (virthread.c:206)
==26535== by 0x8F5F493: start_thread (in /lib64/libpthread-2.23.so)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
After bdcf6e481 there is a crasher in libvirt. The commit assumes
that priv->perf is always set. That is not true. For inactive
domains, the priv->perf is not allocated as it is set in
qemuProcessLaunch(). Now, usually we differentiate between
accesses to inactive and active definition and it works just
fine. Except for 'domstats'. There priv->perf is accessed without
prior check for domain inactivity. While we could check for that,
more robust solution is to make virPerfEventIsEnabled() accept
NULL.
How to reproduce:
1) ensure you have at least one inactive domain
2) virsh domstats
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
This patch fixes the following MinGW error (although actually being a
false positive):
../../src/util/virmdev.c: In function 'virMediatedDeviceListMarkDevices':
../../src/util/virmdev.c:453:21: error: potential null pointer
dereference [-Werror=null-dereference]
const char *mdev_path = mdev->path;
^~~~~~~~~
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The problem resides in virHostdevUpdateActiveMediatedDevices which gets
called during qemuProcessReconnect. The issue here is that
virMediatedDeviceListAdd takes a pointer to the item to be added to the
list to which VIR_APPEND_ELEMENT is used, which also clears the pointer.
However, in this case only the local copy of the pointer got cleared,
leaving the original pointing to valid memory. To sum it up, during
cleanup phase, the original pointer is freed and the daemon crashes
basically any time it would access it.
Backtrace:
0x00007ffff3ccdeba in __strcmp_sse2_unaligned
0x00007ffff72a444a in virMediatedDeviceListFindIndex
0x00007ffff7241446 in virHostdevReAttachMediatedDevices
0x00007fffc60215d9 in qemuHostdevReAttachMediatedDevices
0x00007fffc60216dc in qemuHostdevReAttachDomainDevices
0x00007fffc6046e6f in qemuProcessStop
0x00007fffc6091596 in processMonitorEOFEvent
0x00007fffc6091793 in qemuProcessEventHandler
0x00007ffff7294bf5 in virThreadPoolWorker
0x00007ffff7294184 in virThreadHelper
0x00007ffff3fdc3c4 in start_thread () from /lib64/libpthread.so.0
0x00007ffff3d269cf in clone () from /lib64/libc.so.6
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446455
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Use a local variable to hold data, rather than accessing the pointer
after calling virMediatedDeviceListAdd (therefore VIR_APPEND_ELEMENT).
Although not causing an issue at the moment, this change is a necessary
prerequisite for tweaking virMediatedDeviceListAdd in a separate patch,
which will take a reference for the source pointer (instead of pointer
value) and will clear it along the way.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Even though there are several checks before calling this function
and for some scenarios we don't call it at all (e.g. on disk hot
unplug), it may be possible to sneak in some weird files (e.g. if
domain would have RNG with /dev/shm/some_file as its backend). No
matter how improbable, we shouldn't unlink it as we would be
unlinking a file from the host which we haven't created in the
first place.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cedric Bosdonnat <cbosdonnat@suse.com>
Just like in previous commit, this fixes the same issue for
hotplug.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cedric Bosdonnat <cbosdonnat@suse.com>
While the code allows devices to already be there (by some
miracle), we shouldn't try to create devices that don't belong to
us. For instance, we shouldn't try to create /dev/shm/file
because /dev/shm is a mount point that is preserved. Therefore if
a file is created there from an outside (e.g. by mgmt application
or some other daemon running on the system like vhostmd), it
exists in the qemu namespace too as the mount point is the same.
It's only /dev and /dev only that is different. The same
reasoning applies to all other preserved mount points.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cedric Bosdonnat <cbosdonnat@suse.com>
Currently, all we need to do in qemuDomainCreateDeviceRecursive() is to
take given @device, get all kinds of info on it (major & minor numbers,
owner, seclabels) and create its copy at a temporary location @path
(usually /var/run/libvirt/qemu/$domName.dev), if @device live under
/dev. This is, however, very loose condition, as it also means
/dev/shm/* is created too. Therefor, we will need to pass more arguments
into the function for better decision making (e.g. list of mount points
under /dev). Instead of adding more arguments to all the functions (not
easily reachable because some functions are callback with strictly
defined type), lets just turn this one 'const char *' into a 'struct *'.
New "arguments" can be then added at no cost.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cedric Bosdonnat <cbosdonnat@suse.com>
When setting up mount namespace for a qemu domain the following
steps are executed:
1) get list of mountpoints under /dev/
2) move them to /var/run/libvirt/qemu/$domName.ext
3) start constructing new device tree under /var/run/libvirt/qemu/$domName.dev
4) move the mountpoint of the new device tree to /dev
5) restore original mountpoints from step 2)
Note the problem with this approach is that if some device in step
3) requires access to a mountpoint from step 2) it will fail as
the mountpoint is not there anymore. For instance consider the
following domain disk configuration:
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/dev/shm/vhostmd0'/>
<target dev='vdb' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
</disk>
In this case operation fails as we are unable to create vhostmd0
in the new device tree because after step 2) there is no /dev/shm
anymore. Leave aside fact that we shouldn't try to create devices
living in other mountpoints. That's a separate bug that will be
addressed later.
Currently, the order described above is rearranged to:
1) get list of mountpoints under /dev/
2) start constructing new device tree under /var/run/libvirt/qemu/$domName.dev
3) move them to /var/run/libvirt/qemu/$domName.ext
4) move the mountpoint of the new device tree to /dev
5) restore original mountpoints from step 3)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cedric Bosdonnat <cbosdonnat@suse.com>
When we get a POLLHUP or VIR_EVENT_HANDLE_HANGUP event for a client, we
still want to read from the socket to process any accumulated data. But
doing so inevitably results in an error and a call to
virNetClientMarkClose before we get to processing the hangup event (and
another call to virNetClientMarkClose). However the close reason passed
to the second virNetClientMarkClose call is ignored because another one
was already set. We need to pass the correct close reason when marking
the socket to be closed for the first time.
https://bugzilla.redhat.com/show_bug.cgi?id=1373859
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
While fixing a bug with incorrectly freed memory in commit
v3.1.0-399-g5498aa29a, I accidentally broke persistent migration of
transient domains. Before adding qemuDomainDefCopy in the path, the code
just took NULL from vm->newDef and used it as the persistent def, which
resulted in no persistent XML being sent in the migration cookie. This
scenario is perfectly valid and the destination correctly handles it by
using the incoming live definition and storing it as the persistent one.
After the mentioned commit libvirtd would just segfault in the described
scenario.
https://bugzilla.redhat.com/show_bug.cgi?id=1446205
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
If we are encoding a block of data that is 16 bytes in length,
we cannot leave it as 16 bytes, we must pad it out to the next
block boundary, 32 bytes. Without this padding, the decoder will
incorrectly treat the last byte of plain text as the padding
length, as it can't distinguish padded from non-padded data.
The problem exhibited itself when using a 16 byte passphrase
for a LUKS volume
$ virsh secret-set-value 55806c7d-8e93-456f-829b-607d8c198367 \
$(echo -n 1234567812345678 | base64)
Secret value set
$ virsh start demo
error: Failed to start domain demo
error: internal error: process exited while connecting to monitor: >>>>>>>>>>Len 16
2017-05-02T10:35:40.016390Z qemu-system-x86_64: -object \
secret,id=virtio-disk1-luks-secret0,data=SEtNi5vDUeyseMKHwc1c1Q==,\
keyid=masterKey0,iv=zm7apUB1A6dPcH53VW960Q==,format=base64: \
Incorrect number of padding bytes (56) found on decrypted data
Notice how the padding '56' corresponds to the ordinal value of
the character '8'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When creating v3.2.0-77-g8be3ccd04 commit, I completely forgot that one
migration capability is very special. It's the "events" capability which
tells QEMU to report "MIGRATION" events. Since libvirt always wants the
events, it is enabled in qemuConnectMonitor and the rest of the code
should not touch it.
https://bugzilla.redhat.com/show_bug.cgi?id=1439841https://bugzilla.redhat.com/show_bug.cgi?id=1441165
Messed-up-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1330940
The virsh command 'domblkinfo' returns the capacity, allocation and phisycal
size of the devices attached in a domain. Usually, this sizes are very big
and hard to understand and calculate. This commits introduce a human readable
support to check the size of each field easilly.
For example, the command before:
virsh # domblkinfo my_domain hda
Capacity: 21474836480
Allocation: 14875545600
Physical: 21474836480
and after this patch:
virsh # domblkinfo my_domain hda --human
Capacity: 20.000G
Allocation: 13.900G
Physical: 20.000G
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Nested HVM support in the libxl driver is a news-worthy
improvement for libvirt 3.3.0.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
... with VIR_NET_GENERATED_MACV???_PREFIX, which is defined in
util/virnetdevmacvlan.h.
Since VIR_NET_GENERATED_PREFIX is used for plain tap devices, it is
renamed to VIR_NET_GENERATED_TAP_PREFIX and moved to virnetdev.h
The parser had been clearing out *all* suggested device names for
type='direct' (aka macvtap) interfaces. All of the code implementing
macvtap allows for a user-specified device name, so we should allow
it. In the case that an interface name starts with "macvtap" or
"macvlan" though, we do still clear it out, just as we do with "vnet"
(which is the prefix used for automatically generated tap device
names), since those are the prefixes for the names we autogenerate for
macvtap and macvlan devices.
Resolves: https://bugzilla.redhat.com/1335798
MACVTAP_NAME_PREFIX and MACVLAN_NAME_PREFIX could be useful to other
files if they were defined in virnetdevmacvlan.h instead of
virnetdevmacvlan.c, so do that (while slightly renaming them and also
adding yet another #define that chooses between macvlan/macvtap based
on flags).
This is a prerequisite to fix: https://bugzilla.redhat.com/1335798
If the network isn't active during networkNotifyActualDevice(), we
would log an error message stating that the bridge device didn't
exist. This patch adds a check to see if the network is active, making
the logs more useful in the case that it isn't.
Partially resolves: https://bugzilla.redhat.com/1442700
Nothing that could happen during networkNotifyActualDevice() could
justify unceremoniously killing the qemu process, but that's what we
were doing.
In particular, new code added in commit 85bcc022 (first appearred in
libvirt-3.2.0) attempts to reattach tap devices to their assigned
bridge devices when libvirtd restarts (to make it easier to recover
from a restart of a libvirt network). But if the network has been
stopped and *not* restarted, the bridge device won't exist and
networkNotifyActualDevice() will fail.
This patch changes networkNotifyActualDevice() and
qemuProcessNotifyNets() to return void, so that qemuProcessReconnect()
will soldier on regardless of what happens (any errors will still be
logged though).
Partially resolves: https://bugzilla.redhat.com/1442700
After 1eb6647979 nobody calls the iohelper with 6 arguments.
Everybody uses the other mode. Well, the only user of iohelper
after the previous commit is virFileWrapperFd really.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Currently we use iohelper for virFDStream implementation. This is
because UNIX I/O can lie sometimes: even though a FD for a
file/block device is set as unblocking, actual read()/write() can
block. To avoid this, a pipe is created and one end is kept for
read/write while the other is handed over to iohelper to
write/read the data for us. Thus it's iohelper which gets blocked
and not our event loop.
This approach has two problems:
1) we are spawning a new process.
2) any exchange of information between daemon and iohelper can be
done only through the pipe.
Therefore, iohelper is replaced with an implementation in thread
which is created just for the stream lifetime. The data are still
transferred through pipe (for now), but both problems described
above are solved.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
