There is no need for a temporary variable in this function, and ever
since we switched to glib for memory allocation, there is no possibility
it can return NULL, so callers don't need to check for it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When this function was recently changed to add in parsing of
IFLA_VF_STATS, I noticed that the checks for existence of IFLA_VF_MAC
and IFLA_VF_VLAN were looking in the *wrong array*. The array that
contains the results of parsing each IFLA_VFINFO in
tb[IFLA_VFINFO_LIST] is tb_vf, but we were checking for these in tb
(which is the array containing the results of the toplevel parsing of
the netlink message, *not* the results of parsing one of the nested
IFLA_VFINFO's.
This incorrect code has been here since the function was originally
written in 2012. It has only worked all these years due to coincidence
- the items at those indexes in tb are IFLA_ADDRESS and IFLA_BROADCAST
(of the *PF*, not of any of its VFs), and those happen to always be
present in the toplevel netlink message; since we are only looking in
the incorrect place to check for mere existence of the attribute (but
are doing the actual retrieval of the attribute from the correct
place), this bug has no real consequences other than confusing anyone
trying to understand the code.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virNetDevParseVfConfig has became a multifunctional helper function,
rename it to virNetDevParseVfInfo.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Laine Stump <laine@redhat.com>
libvirt can retrieve traffic stats for emulated interfaces that are
backed by tap or macvtap devices, but this information wasn't
available for hostdev interfaces (those that are implemented by
assigning an SR-IOV VF device to a guest using vfio):
#virsh domifstat instance --interface=52:54:00:2d:b2:35
error: Failed to get interface stats instance 52:54:00:2d:b2:35
error: internal error: Interface name not provided
For some SR-IOV VF devices this information is available via the
netlink VFINFO_LIST request/response, and that is what this patch uses
to implement stats retrieval for VF. Not that this is dependent on
support in the PF driver - for example, the Mellanox ConnectX-4 Lx
(mlx5) driver reports usable stats, while Intel 82599 (ixgbe) and
82576 (igb) just report all stats as 0. (this is the same result as
"ip -s link show").
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Laine Stump <laine@redhat.com>
By default, pfifo_fast queueing discipline (qdisc) is set on
newly created interfaces (including TAPs). This qdisc has three
queues and packets that want to be sent through given NIC are
placed into one of the queues based on TOS field. Queues are then
emptied based on their priority allowing interactive sessions
stay interactive whilst something else is downloading a large
file.
Obviously, this means that kernel has to be involved and some
locking has to happen (when placing packets into queues). If
virtualization is taken into account then the above algorithm
happens twice - once in the guest and the second time in the
host.
This is arguably not optimal as it burns host CPU cycles
needlessly. Guest already made it choice and sent packets in the
order it wants.
To resolve this, Linux kernel offers 'noqueue' qdisc which can be
applied on virtual interfaces and in fact for 'lo' it is by
default:
lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue
Set it for other TAP devices we create for domains too. With this
change I was able to squeeze 1Mbps more from a macvtap attached
to a guest and to my 1Gbps LAN (as measured by iperf3).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1329644
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This helper changes the root qdisc on given interface.
Ideally, it would be written using netlink but my attempts to
write the code were not successful and thus I've fallen back to
virCommand() + tc.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently setting max_len=0 causes virtlogd to spin in a busy loop. It
is natural to allow this to disable log rollover which can be useful for
developers debugging things.
Note disabling rollover exposes the host to denial of service from a
malicious guest, so must be used with care.
Closes https://gitlab.com/libvirt/libvirt/-/issues/85
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The aim of virSocketAddrPrefixToNetmask() is to initialize passed
virSocketAddr structure based on prefix length and family.
However, it doesn't set all members in the struct which may lead
to reads of uninitialized values:
==15421== Use of uninitialised value of size 8
==15421== at 0x50F297A: _itoa_word (in /lib64/libc-2.31.so)
==15421== by 0x510C8FE: __vfprintf_internal (in /lib64/libc-2.31.so)
==15421== by 0x5120295: __vsnprintf_internal (in /lib64/libc-2.31.so)
==15421== by 0x50F8969: snprintf (in /lib64/libc-2.31.so)
==15421== by 0x51BB602: getnameinfo (in /lib64/libc-2.31.so)
==15421== by 0x496DEE0: virSocketAddrFormatFull (virsocketaddr.c:486)
==15421== by 0x496DD9F: virSocketAddrFormat (virsocketaddr.c:444)
==15421== by 0x11871F: networkDnsmasqConfContents (bridge_driver.c:1404)
==15421== by 0x1118F5: testCompareXMLToConfFiles (networkxml2conftest.c:48)
==15421== by 0x111BAF: testCompareXMLToConfHelper (networkxml2conftest.c:112)
==15421== by 0x112679: virTestRun (testutils.c:142)
==15421== by 0x111D09: mymain (networkxml2conftest.c:144)
==15421== Uninitialised value was created by a stack allocation
==15421== at 0x1175D2: networkDnsmasqConfContents (bridge_driver.c:1056)
All callers expect the function to initialize the structure
fully.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
As preparation for g_autoptr() we need to change the function to take
only virCgroupPtr.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
We don't use the lib prefix for all libraries but in these cases it
makes sense to use the prefix.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Validation is usually performed on an entire document. If we are only
interested in validating a single nested node that can occur in
different contexts, this would require writing different schemas for
any of those different contexts.
By temporarily replacing the document's root node, we can validate the
relevant node only.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There are only 3 places using the function. Two can use virBitmapNewCopy
directly. In case of the qemu capabilities code we need to free the old
bitmap first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapCopy has a failure condition, which is impossible to meet when
creating a new copy. Copy the contents directly to make it obvious that
virBitmapNewCopy can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We no longer report any errors so all callers can be replaced by
virBitmapNew. Additionally virBitmapNew can't return NULL now so error
handling is not necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now always return a valid pointer or crash so the return value
doesn't need to be checked.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the condition which would make virBitmapNewQuiet fail to possibly
overallocate by 1 rather than failing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now have APIs which automatically expand the bitmap and also API
which allocates a 0 size bitmap. Remove the condition from virBitmapNew.
Effectively reverts ce49cfb48a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapNewEmpty() can create a bitmap with 0 length. With such a
bitmap virBitmapToString will return NULL rather than an empty string.
Initialize the buffer to avoid that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Clarify which bit is considered most significant in the bitmap and
resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's only one combination used so we can remove the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When VIR_EXEC_DAEMON is true and cmd->pidfile exists, the parent
will expect the pidfile to be written before exiting, sitting
tight in a saferead() call waiting.
The child then does process tuning (via virProcessSet* functions)
before writing the pidfile. Problem is that these tunings can
fail, and trigger a 'fork_error' jump, before cmd->pidfile is
written. The result is that the process was aborted in the
child, but the parent is still hang in the saferead() call.
This behavior can be reproduced by trying to create and execute
a QEMU guest in user mode (e.g. using qemu:///session as non-root).
virProcessSetMaxMemLock() will fail if the spawned libvirtd user
process does not have CAP_SYS_RESOURCE capability. setrlimit() will
fail, and a 'fork_error' jump is triggered before cmd->pidfile
is written. The parent will hung in saferead() indefinitely. From
the user perspective, 'virsh start <guest>' will hang up
indefinitely. CTRL+C can be used to retrieve the terminal, but
any subsequent 'virsh' call will also hang because the previous
libvirtd user process is still there.
We can fix this by moving all virProcessSet*() tuning functions
to be executed after cmd->pidfile is taken care of. In the case
mentioned above, this would be the result of 'virsh start'
after this patch:
error: Failed to start domain vm1
error: internal error: Process exited prior to exec: libvirt: error :
cannot limit locked memory to 79691776: Operation not permitted
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1882093
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Lack of this one function (which is called for each active tap device
every time libvirtd is started) is the one thing preventing a
"WITHOUT_LIBNL" build of libvirt from being useful. With this
alternate implementation, guests using standard tap devices will work
properly even when libvirt is built without libnl support.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was one stray bit of code in virnetdev.c that required libnl to
build, but wasn't qualified by defined(WITH_LIBNL). Adding that, plus
putting a similar check around a static function only used by that
aforementioned code, makes libvirt build properly without libnl3-devel
installed.
How useful it is in that state is a separate issue :-)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag was originally created to indicate that either 1) the build
platform wasn't linux, 2) the build platform was linux, but the kernel
was too old to have macvtap support. Since there was already a switch
there, the ability to also disable it when 3) the kernel supports
macvtap but the user doesn't want it, was added in. I don't think that
(3) was ever an intentional goal, just something that grew naturally
out of having the flag there in the first place (unless possibly the
original author wanted a way to quickly disable their new code in case
it caused regressions elsewhere).
Now that the check for (2) has been removed, WITH_MACVTAP is just
checking (1) and (3), but (3) is pointless (because the extra code in
libvirt itself is miniscule, and the only external library needed for
it is libnl, which is also required for other unrelated features (and
itself has no subordinate dependencies and takes up < 1MB on
disk)). We can therfore eliminate the WITH_MACVTAP flag, as it is
functionally equivalent to WITH_LIBNL (which implies __linux__).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
macvlan support was added to the Linux kernel in 2.6.33, but
MACVLAN_MODE_PASSTHRU wasn't added until 2.6.38, so a workaround had
been put in place to define that constant on those few systems where
it was missing. It's useful like was probably 6 months at most, but
it's been there for over 10 years.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
WITH_VIRTUALPORT just checks that we are building on Linux and that
IFLA_PORT_MAX is defined in linux/if_link.h. Back when 802.11Qb[gh]
support was added, the IFLA_* stuff was new (introduced in kernel
2.6.35, backported to RHEL6 2.6.32 kernel at some point), and so this
extra check was necessary, because libvirt was being built on Linux
distros that didn't yet have IFLA_* (e.g. older RHEL6, all
RHEL5). It's been in the kernel for a *very* long time now, so all
supported versions of all Linux platforms libvirt builds on have it.
Note that the above paragraph implies that the conditional compilation
should be changed to #if defined(__linux__). However, the astute
reader will notice that the code in question is sending and receiving
netlink messages, so it really should be conditional on WITH_LIBNL
(which implies __linux__) instead, so that's what this patch does.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
WITH_LIBNL will only be defined on Linux platforms (because libnl is a
library written to encapsulate parts of netlink, which is a Linux-only
API), so it's redundant to write:
#if defined(__linux__) && defined(WITH_LIBNL)
We can just check for WITH_LIBNL.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
IFLA_VF_MAX was introduced to the Linux kernel in 2.6.35, and was even
backported to the RHEL*6* 2.6.32 kernel downstream, so it is present
in all supported versions of all Linux distros that libvirt builds
on. Additionally, it can't be conditionally compiled out of a
kernel. There is no reason to conditionalize any piece of code on
presence of IFLA_VF_MAX - if the platform is Linux, it is supported.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
