A function that returns -1 for multiple possible failures, but only
raises a libvirt error for some of those failures, can be hard to
use correctly. Yet both of our JSON object/array appenders fall in
that pattern. True, the silent errors represent coding bugs that
none of the callers should ever trigger, while the noisy errors
represent memory failures that can happen anywhere, so we happened
to never end up failing without an error. But it is better to
either use the _QUIET memory allocation variants, and make callers
decide to report failure; or make all failure paths noisy. This
patch takes the latter approach.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similar to what commit 86dba8f3 did for virPortAllocatorRelease,
ignore port 0 in virPortAllocatorSetUsed.
For all the reasonable use cases the callers already check that
the port is non-zero, however if the port from the XML overflows
unsigned short and turns into 0, it can be set as used by
virPortAllocatorSetUsed but not released by virPortAllocatorRelease.
Also skip port '0' in virPortAllocatorSetUsed to make this behavior
symmetric.
The serenity was disturbed by commit 5dbda5e9 which started using
virPortAllocatorRelease instead of virPortAllocatorSetUsed (false).
https://bugzilla.redhat.com/show_bug.cgi?id=1591645
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This partially reverts 00dc991ca1.
2,030 (1,456 direct, 574 indirect) bytes in 14 blocks are definitely lost in loss record 77 of 80
at 0x4C30E96: calloc (vg_replace_malloc.c:711)
by 0x50F83AA: virAlloc (viralloc.c:143)
by 0x5178DFA: virPCIDeviceNew (virpci.c:1753)
by 0x51753E9: virPCIDeviceIterDevices (virpci.c:468)
by 0x5175EB5: virPCIDeviceGetParent (virpci.c:759)
by 0x517AB55: virPCIDeviceIsBehindSwitchLackingACS (virpci.c:2476)
by 0x517AC24: virPCIDeviceIsAssignable (virpci.c:2494)
by 0x10BF27: testVirPCIDeviceIsAssignable (virpcitest.c:229)
by 0x10D14C: virTestRun (testutils.c:174)
by 0x10C535: mymain (virpcitest.c:422)
by 0x10F1B6: virTestMain (testutils.c:1112)
by 0x10CF93: main (virpcitest.c:455)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This is a return argument that is to be compared against NULL on
successful return. However, it is not initialized and therefore
relies on callers setting it to NULL prior calling the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The @linkdev is In/Out function parameter as second order
reference pointer so requires first order dereference for
checking NULL which can be the result of virPCIGetNetName().
Fixes: d6ee56d723 (util: change virPCIGetNetName() to not return error if device has no net name)
Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Removing redundant sections of the code
Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
libvirt wrongly assumes that VF netdev has to have the
netdev assigned to PF. There is no such requirement in SRIOV standard.
This patch change the virNetDevSwitchdevFeature() function to deal
with SRIOV devices which does not have netdev on PF. Also corrects
one comment about PF netdev assumption.
One example of such devices is ThunderX VNIC.
By applying this change, VF device is used for virNetlinkCommand() as
it is the only netdev assigned to VNIC.
Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
Signed-off-by: dann frazier <dann.frazier@canonical.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit 7282f455a got rid of the VIR_WARNINGS_NO_CAST_ALIGN macro
when refactoring the code and broke the build with clang.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
I had intended to make these changes to commit d40b820c before
pushing, but forgot about it during the day between the initial review
and ACK.
Neither change is significant - just returning immediately when
virNetDevGetName() fails (instead of logging a debug message first)
and eliminating a comment that adds to confusion rather than
eliminating it. Still, the changes should be made to be more
consistent with nearly identical code just a few lines up (added in
commit 7282f455)
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When checking the setting of accept_ra, we have assumed that all
routes have a single nexthop, so the interface of the route would be
in the RTA_OIF attribute of the netlink RTM_NEWROUTE message. But
multipath routes don't have an RTA_OIF; instead, they have an
RTA_MULTIPATH attribute, which is an array of rtnexthop, with each
rtnexthop having an interface. This patch adds a loop to look at the
setting of accept_ra of the interface for every rtnexthop in the
array.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This is about the same number of code lines, but is simpler, and more
consistent with what will be added to check another attribute in a
coming patch.
As a side effect, it
Resolves: https://bugzilla.redhat.com/1583131
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This same operation needs to be done in multiple places, so move the
inline code into a separate function.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This is problematic if a callback function wants to send the nlmsghdr
to a library function that has no "const" in its prototype
(e.g. nlmsg_find_attr())
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Essentially, bring back the old behaviour as of commit eba36a38 which
was later changed by commit ae06048bf5. Even though all the stderr
messages will eventually end up in the journal, we're not making use of
the fields journald provides.
https://bugzilla.redhat.com/show_bug.cgi?id=1592644
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
In 600462834f we've tried to remove Author(s): lines
from comments at the beginning of our source files. Well, in some
files while we removed the "Author" line we did not remove the
actual list of authors.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This test checks if security label remembering works correctly.
It uses qemuSecurity* APIs to do that. And some mocking (even
though it's not real mocking as we are used to from other tests
like virpcitest). So far, only DAC driver is tested.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The arguments to the N_() macro must only ever be a literal string. It
is not possible to use macro arguments, or use macro string
concatenation in this context. The N_() macro is a no-op whose only
purpose is to act as a marker for xgettext when it extracts translatable
strings from the source code. Anything other than a literal string will
be silently ignored by xgettext.
Unfortunately this means that the clever MSG, MSG2 & MSG_EXISTS macros
used for building up error message strings have prevented any of the
error messages getting marked for translation. We must sadly, revert to
a more explicit listing of strings for now.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU command line arguments are very long and currently all written
on a single line to /var/log/libvirt/qemu/$GUEST.log. This introduces
logic to add line breaks after every env variable and "-" optional
argument, and every positional argument. This will create a clearer log
file, which will in turn present better in bug reports when people cut +
paste from the log into a bug comment.
An example log file entry now looks like this:
2018-12-14 12:57:03.677+0000: starting up libvirt version: 5.0.0, qemu version: 3.0.0qemu-3.0.0-1.fc29, kernel: 4.19.5-300.fc29.x86_64, hostname: localhost.localdomain
LC_ALL=C \
PATH=/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin \
HOME=/home/berrange \
USER=berrange \
LOGNAME=berrange \
QEMU_AUDIO_DRV=none \
/usr/bin/qemu-system-ppc64 \
-name guest=guest,debug-threads=on \
-S \
-object secret,id=masterKey0,format=raw,file=/home/berrange/.config/libvirt/qemu/lib/domain-33-guest/master-key.aes \
-machine pseries-2.10,accel=tcg,usb=off,dump-guest-core=off \
-m 1024 \
-realtime mlock=off \
-smp 1,sockets=1,cores=1,threads=1 \
-uuid c8a74977-ab18-41d0-ae3b-4041c7fffbcd \
-display none \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,fd=23,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
-boot strict=on \
-device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
2018-12-14 12:57:03.730+0000: shutting down, reason=failed
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virCommand APIs do not expect to be given a NULL value for an arg
name or value. Such a mistake can lead to execution of the wrong
command, as the NULL may prematurely terminate the list of args.
Detect this and report suitable error messages.
This identified a flaw in the storage test which was passing a NULL
instead of the volume path. This flaw was then validated by an incorrect
set of qemu-img args as expected data.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Simplify adding of new errors by just adding them to the array of
messages rather than having to add conversion code.
Additionally most of the messages add the format string part as a suffix
so we can avoid some of the duplication by using a macro which adds the
suffix to the original string. This way most messages fit into the 80
column limit and only 3 exceed 100 colums.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Erik Skultety <eskultet@redhat.com>
Clarify how @info is used and what the returned values look like.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Simplify wording of the error string for VIR_ERR_OPEN_FAILED and
VIR_ERR_CALL_FAILED. The error codes itself are currently unused so it
will not impact any client.
This will simplify upcomming patch which refactors how we convert these.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Few error codes were missing the version of the message with additional
info. In case of the modified messages it's not very likely they'll ever
report any additional data, but for the sake of consistency we should
provide them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Additional information for an error message is either in form of a
string or empty. Fix two offenders. One used %d as the format modifier
and the second one always expected a string.
Thankfully, neither of the offenders are currently in effect.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We do have one for the error domain but not for the error number itself.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Require that all headers are guarded by a symbol named
LIBVIRT_$FILENAME
where $FILENAME is the uppercased filename, with all characters
outside a-z changed into '_'.
Note we do not use a leading __ because that is technically a
namespace reserved for the toolchain.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This introduces a syntax-check script that validates header files use a
common layout:
/*
...copyright header...
*/
<one blank line>
#ifndef SYMBOL
# define SYMBOL
....content....
#endif /* SYMBOL */
For any file ending priv.h, before the #ifndef, we will require a
guard to prevent bogus imports:
#ifndef SYMBOL_ALLOW
# error ....
#endif /* SYMBOL_ALLOW */
<one blank line>
The many mistakes this script identifies are then fixed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When libvirt is reconnecting to running domain that uses cgroup v2
the QEMU process reports cgroup for the emulator directory because the
main thread is in that cgroup. We need to remove the "/emulator" part
in order to match with the root cgroup directory name for that domain.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The rewrite to support cgroup v2 missed this function. In cgroup v2
we have different files to track tasks.
We would fail to remove cgroup on non-systemd OSes if there is any
extra process assigned to guest cgroup because we would not kill any
process form the guest cgroup.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Most of the iptables APIs share code for the add/delete paths, but a
couple were separated. Merge the remaining APIs to facilitate future
changes.
Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The function clears and frees the passed buffers on success, but not in
one case of failure. Modify the control flow that the args are always
consumed, record it in the docs and remove few pointless cleanup paths
in callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Up until now, we formatted 'rendernode=' onto QEMU cmdline only if the
user specified it in the XML, otherwise we let QEMU do it for us. This
causes permission issues because by default the /dev/dri/renderDX
permissions are as follows:
crw-rw----. 1 root video
There's literally no reason why it shouldn't be libvirt picking the DRM
render node instead of QEMU, that way (and because we're using
namespaces by default), we can safely relabel the device within the
namespace.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the first step towards libvirt picking the first available
render node instead of QEMU. It also makes sense for us to be able to do
that, since we allow specifying the node directly for SPICE, so if
there's no render node specified by the user, we should pick the first
available one. The algorithm used for that is essentially the same as
the one QEMU uses.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The call of virResctrlMonitorGetStats will allocate the memory for
holding cache occupancy or memory bandwidth statistics.
This patch adds the function virResctrlMonitorFreeStats as the
opposing action of virResctrlMonitorGetStats to free the memory.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Return a list of virResctrlMonitorStatsPtr instead of
a virResctrlMonitorStats array in virResctrlMonitorGetStats.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
There are certain cases e.g. containers where the sysfs path might
exists, but might fail. Unfortunately the exact restrictions are only
known to libvirt when trying to write to it so we need to try it.
But in case it fails there is no need to fully abort, in those cases try
to fall back to the older ioctl interface which can still work.
That makes setting up a bridge in unprivileged LXD containers work.
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1802906
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Reported-by: Brian Candler <b.candler@pobox.com>
The path argument of virFileIsDir should be a full name
of file, pathname and filename. Fixed it by passing the
full path name to virFileIsDir.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Since the functions only return 0 or 1, they should return bool. I missed the
change when "refactoring" the first commit.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Both virProcessRunInMountNamespace() and virProcessRunInFork()
look very similar. De-duplicate the code and make
virProcessRunInMountNamespace() call virProcessRunInFork().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This new helper can be used to spawn a child process and run
passed callback from it. This will come handy esp. if the
callback is not thread safe.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We were mistakenly skipping virZPCIDeviceAddressIsEmpty() and
virZPCIDeviceAddressIsValid() when compiling on non-Linux,
which unsurprisingly ended up causing linking failures later
in the build process.
Clue-stick-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are the two attributes of zpci
which is introduced as PCI address element. Zpci element is parsed and
formatted along with PCI address. And add the related test cases.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This patch introduces PCI address extension flag for virDomainDeviceInfo
and virPCIDeviceAddress. The extension flag in virDomainDeviceInfo is
used internally during calculating PCI extension flag. The one in
virPCIDeviceAddress is the duplicate to indicate extension address is
being used. Currently only zPCI extension address is introduced to deal
with 'uid' and 'fid' on the S390 platform.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Add zPCI definitions in preparation of extending the PCI address
with parameters uid (user-defined identifier) and fid (PCI function
identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
