There was a couple of problems with the style fixes applied to the original
patch:
1.) virFileReadAllQuiet comparison was incorrectly parenthesized when moved
into a condition, causing the len to be set to the result of comparison. This,
together with the removed underflow check would underflow the phy buffer.
2.) The logic was broken. Failure to call "ip" would abort the function, thus
the "iw" branch would never be reached.
This aims to fix the issues and work around possible style complains :)
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Refactor the function to return the bitmap instead of an integer and the
inner workings so that they make more sense.
This patch also fixes possible segfault on old systems that was
introduced by commit:
commit f1a43a8e41
Author: Hu Tao <hutao@cn.fujitsu.com>
Date: Fri Sep 14 15:46:59 2012 +0800
use virBitmap to store cpu affinity info
Since commit 55ace7c478, the sockettest
fails without VIR_TEST_DEBUG set. The problem is found by test number
42 (co-incidence?), which tests range '192.168.122.1' -
'192.168.122.255' in network '192.168.122.0/24'. That is supposed to
fail because the end address is equal to the broadcast address.
When comparing these two in 'virSocketAddrEqual(end, &broadcast)',
there is a check for sin_addr as well as for sin_port. That port,
however, is different when we do not enable test debugging. With the
testing enabled, the port is 0 (correctly initialized), but without that
it has a random number there. And that's because the structure is not
initialized anywhere.
By zeroing the structure before filling in the info, we make sure we
return only the address and not any information that was not requested.
And the test work once again.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Since some functions can be optimized by reusing the buffers that they
already have instead of allocating and copying new ones, lets split
virBitmapToData to two functions where one only converts the data and
the second one is a wrapper that allocates the buffer if necessary.
There are now many more reasons that virSocketAddrGetRange() could
fail, so it is much more informative to report the error there instead
of in the caller. (one of the two callers was previously assuming
success, which is almost surely safe based on the parsing that has
already happened to the config by that time, but it still is nicer to
account for an error "just in case")
Part of fix for: https://bugzilla.redhat.com/show_bug.cgi?id=985653
virSocketAddrGetRange() has been updated to take the network address
and prefix, and now checks that both the start and end of the range
are within that network, thus validating that the entire range of
addresses is in the network. For IPv4, it also checks that ranges to
not start with the "network address" of the subnet, nor end with the
broadcast address of the subnet (this check doesn't apply to IPv6,
since IPv6 doesn't have a broadcast or network address)
Negative tests have been added to the network update and socket tests
to verify that bad ranges properly generate an error.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=985653
When we change system clock to years ago, a certain CPU may use up 100% cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign the
unsigned long long result to an INT variable,
*timeout = then - now; // timeout is INT, and then/now are long long
if (*timeout < 0)
*timeout = 0;
there's a chance that variable @then minus variable @now may be a very large number
that overflows INT value expression, then *timeout will be negative and be assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running, but it does
seems to have no harm to make the codes more robust.
Signed-off-by: Wang Yufei <james.wangyufei@huawei.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
If the <sysinfo type='smbios'...> ends up not formatting any sub-elements,
then rather than formatting as:
<sysinfo type='smbios'>
</sysinfo>
Just format it more cleanly as:
<sysinfo type='smbios'/>
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Only set directory permissions at pool build time, if:
- User explicitly requested a mode via the XML
- The directory needs to be created
- We need to do the crazy NFS root-squash workaround
This allows qemu:///session to call build on an existing directory
like /tmp.
If the firewalld backend wasn't available and libvirt decides to try
setting up a "direct" backend, it checks for the presence of iptables,
ip6tables, and ebtables. If they are not found, a message like this is logged:
error : virFirewallValidateBackend:193 : direct firewall backend
requested, but /usr/sbin/ip6tables is not available:
No such file or directory
But then at a later time if an attempt is made to use the virFirewall
API, failure will be indicated with:
error : virFirewallApply:936 : out of memory
This patch changes virFirewallApply to first check if a firewall
backend hadn't been successfully setup, and logs a slightly more
informative message in that case:
error : virFirewallApply:940 : internal error:
Failed to initialize a valid firewall backend
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223876
If an SRIOV PF is offline, the kernel won't complain if you set the
mac address and vlan tag for a VF via this PF, and it will even let
you assign the VF to a guest using PCI device assignment or macvtap
passthrough. But in this case (the PF isn't online), the device won't
be usable in the guest.
Silently setting the PF online would solve the connectivity problem,
but as pointed out by Dan Berrange, when an interface is set online
with no associated config, the kernel will by default turn on IPv6
autoconf, which could create unexpected security problems for the
host. For this reason, this patch instead logs an error and fails the
operation.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738
Originally filed against RHEL6, but present in every version of
libvirt until today.
Due to a kernel commit (b4b8f770e), cpuinfo format has changed on
ARMs. Firstly, 'Processor: ...' may not be reported, it's
replaced by 'model name: ...'. Secondly, the "Processor" string
may occur in CPU name, e.g. 'ARMv7 Processor rev 5 (v7l)'.
Therefore, we must firstly look for 'model name' and then for
'Processor' if not found.
Moreover, lines in the cpuinfo file are shuffled, so we better
not manipulate the pointer to start of internal buffer as we may
lost some info.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Old compilers whine:
src/util/virutil.c: In function 'virMemoryMaxValue':
src/util/virutil.c:2612: error: declaration of 'ulong' shadows a global declaration [-Wshadow]
/usr/include/sys/types.h:151: error: shadowed declaration is here [-Wshadow]
s/ulong/capped/ to work around the problem
Using joinable threads does not help anything, but it can lead to memory
leaks.
When a worker thread exits, it decreases nWorkers or nPrioWorkers and
once both nWorkers and nPrioWorkers are zero (i.e., the last worker is
gone), quit_cond is signaled. When freeing the pool we first tell all
threads to die and then we are waiting for both nWorkers and
nPrioWorkers to become zero. At this point we already know all threads
are gone. So the only reason for calling virThreadJoin of all workers is
to free the memory allocated for joinable threads. If we avoid
allocating this memory, we don't need to take care of freeing it.
Moreover, any memory associated with a worker thread which died before
we asked it to die (e.g., because virCondWait failed in the thread)
would be lost anyway since virThreadPoolFree calls virThreadJoin only
for threads which were running at the time virThreadPoolFree was called.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The APIs take the memory value in KiB and we store it in KiB
internally, but we cannot parse the whole ULONG_MAX range
on 64-bit systems, because virDomainParseScaledValue
needs to fit the value in bytes in an unsigned long long.
https://bugzilla.redhat.com/show_bug.cgi?id=1176739
Extend it to a universal helper used for clearing lists of any objects.
Note that the argument type is specifically void * to allow implicit
typecasting.
Additionally add a helper that works on non-NULL terminated arrays once
we know the length.
Currently we try to chown any directory passed to virDirCreate,
even if the user didn't request any explicit owner/group via the
pool/vol XML.
This causes issues with qemu:///session: try to build a pool of
a root owned directory like /tmp, and it fails trying to chown the
directory to the session user. Instead it should just leave things
as they are, unless the user requests changing permissions via
the pool XML.
Similarly this is annoying if creating a storage pool via system
libvirtd of an existing directory in user $HOME, it's now owned
by root.
The virDirCreate function is pretty convoluted, since it needs to
fork off in certain specific cases. Try to document that, to make
it clear where exactly we are changing behavior.
The current code attempts to handle this, but it only catches mkdir
failing with EEXIST. However if say trying to build /tmp for an
unprivileged qemu:///session, mkdir will fail with EPERM.
Rather than catch any errors, just don't attempt mkdir if the directory
already exists.
Fix for such a case:
1. Domain A and B xml contain the same SRIOV net hostdev(<interface
type='hostdev' /> with same pci address).
2. virsh start A (Successfully, and configure the SRIOV net with
custom mac)
3. virsh start B (Fail because of the hostdev used by domain A or other
reason.)
In step 3, 'virHostdevNetConfigRestore' is called for the hostdev
which is still used by domain A. It makes the mac/vlan of the SRIOV net
change.
Code Change in this fix:
1. As the pci used by other domain have been removed from
'pcidevs' in previous loop, we only restore the nic config for
the hostdev still in 'pcidevs'(used by this domain)
2. update the comments to make it more clear
Signed-off-by: Huanle Han <hanxueluo@gmail.com>
Refactor some code to create a static function virHostdevIsPCINetDevice
which will detect whether the hostdev is a pci net device or not.
Signed-off-by: Huanle Han <hanxueluo@gmail.com>
Commit 1268820a removed obsolete index() function and replaced it by
strchr. Few versions of gcc has a bug and reports a warning about
strchr:
../../src/util/virstring.c:1006: error: logical '&&' with non-zero
constant will always evaluate as true [-Wlogical-op]
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit 2a530a3e5 is not portable to mingw, which intentionally
avoids declaring the obsolete index(). See also:
https://bugzilla.redhat.com/show_bug.cgi?id=1214605
* src/util/virstring.c (virStringStripControlChars): Use strchr.
Signed-off-by: Eric Blake <eblake@redhat.com>
When running on FreeBSD, there's a bug in virCommandProcessIO
polling that is triggered by the commandtest.
A test that triggers EPIPE in commandtest (named "test20") hungs
forever on FreeBSD.
Apparently, this happens because FreeBSD sets POLLHUP flag on revents
when stdin in closed. And as the current implementation only checks for
POLLOUT and POLLERR, it ends up looping forever inside
virCommandProcessIO and not trying to do one more write() that would
trigger EPIPE.
To fix that check for the POLLHUP flag along with POLLOUT and POLLERR.
When a user would specify a backing chain index that is above the start
point libvirt would report a rather unhelpful error:
invalid argument: could not find backing store 1 in chain for 'sub/link2'
This patch adds an explicit check that the index is below start point in
the backing store and reports the following error if not:
invalid argument: requested backing store index 1 is above 'sub/../qcow2' in chain for 'sub/link2'
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177062
Some storage protocols allow to have the @path field in struct
virStorageSource set to NULL. Add NULLSTR() wrappers to handle this
possibility until I finish the storage source error formatter.
Build fails on non-Linux systems with this error:
CC util/libvirt_util_la-virnetdev.lo
util/virnetdev.c:364:1: error: unused function 'virNetDevReplaceMacAddress' [-Werror,-Wunused-function]
virNetDevReplaceMacAddress(const char *linkdev,
^
util/virnetdev.c:406:1: error: unused function 'virNetDevRestoreMacAddress' [-Werror,-Wunused-function]
virNetDevRestoreMacAddress(const char *linkdev,
^
2 errors generated.
The virNetDev{Restore,Replace}MacAddress() functions are only used
by VF-related routines that are available on Linux only. So move these
functions under the same #ifdef.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474
When we set the MAC address of a network device as a part of setting
up macvtap "passthrough" mode (where the domain has an emulated netdev
connected to a host macvtap device that has exclusive use of the
physical device, and sets the device MAC address to match its own,
i.e. "<interface type='direct'> <source mode='passthrough' .../>"), we
use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is
true even if it is an SRIOV Virtual Function (VF).
But, when we are setting the MAC address / vlan ID of a VF in
preparation for "hostdev network" passthrough (this is where we set
the MAC address and vlan id of the VF after detaching the host net
driver and before assigning the device to the domain with PCI
passthrough, i.e. "<interface type='hostdev'>", we do the setting via
a netlink RTM_SETLINK message for that VF's Physical Function (PF),
telling it the VF# we want to change. This sets an "administratively
changed MAC" flag for that VF in the PF's driver, and from that point
on (until the PF driver is reloaded, *not* merely the VF driver) that
VF's MAC address can't be changed using ioctl(SIOCSIFHWADDR) - the
only way to change it is via the PF with RTM_SETLINK.
This means that if a VF is used for hostdev passthrough, it will have
the admin flag set, and future attempts to use that VF for macvtap
passthrough will fail.
The solution to this problem is to check if the device being used for
macvtap passthrough is actually a VF; if so, we use the netlink
RTM_SETLINK message to the PF to set the VF's mac address instead of
ioctl(SIOCSIFHWADDR) directly to the VF; if not, behavior does not
change from previously.
There are three pieces to making this work:
1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call
virNetDev(Replace|Restore)NetConfig() rather than
virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF#
and vlanid).
2) virNetDev(Replace|Restore)NetConfig() check to see if the device is
a VF. If so, they find the PF's name and VF#, allowing them to call
virNetDev(Replace|Restore)VfConfig().
3) To prevent mixups when detaching a macvtap passthrough device that
had been attached while running an older version of libvirt,
virNetDevRestoreVfConfig() is potentially given the preserved name
of the VF, and if the proper statefile for a VF can't be found in
the stateDir (${stateDir}/${pfname}_vf${vfid}),
virNetDevRestoreMacAddress() is called instead (which will look in
the file named ${stateDir}/${vfname}).
This problem has existed in every version of libvirt that has both
macvtap passthrough and interface type='hostdev'. Fortunately people
seem to use one or the other though, so it hasn't caused any real
world problem reports.
This is a simple wrapper around virNetDevBandwidthManipulateFilter() that
will update the desired filter on an interface (usually a network bridge)
with a new MAC address. Although, the MAC address in question usually
refers to some other interface - the one that the filter is constructed
for. Yeah, hard to parse. Thing is, our NATed network has a bridge where
some part of QoS takes place. And vNICs from guests are plugged into
the bridge. However, if a guest decides to change the MAC of its vNIC,
the corresponding qemu process emits an event which we can use to
update the QoS configuration based on the new MAC address.. However,
our QoS hierarchy is currently not notified, therefore it falls apart.
This function (when called in response to the aforementioned event)
will update our QoS hierarchy and duct tape it together again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not only this simplifies the code a bit, it prepares the
environment for upcoming patches. The new
virNetDevBandwidthManipulateFilter() function is capable of both
removing a filter and adding a new one. At the same time! Yeah,
this is not currently used anywhere but look at the next commit
where you'll see it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, when constructing traffic shaping rules, the ingress
filter is created without any priority specified on the command
line. This makes kernel to make up one. While this works, it
simplifies things a bit if we provide the filter priority. In
this case, since it's the root filter lets give it the highest
priority of number 1.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The 802.11 interfaces can not be moved by themselves, their Phy has to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
On rhel-6 is broken gcc that reports this warning:
util/virbuffer.c:500: error: logical '&&' with non-zero constant will
always evaluate as true [-Wlogical-op]
Move the pragma directive before function virBufferEscapeString because
since commit aeb5262e this function uses 'strchr' too.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
The comment is describing arguments passed to the function.
However, there's no @ifmac argument. In 955af4d4 it was replaced
with @ifmac_ptr. Unfortunately, the comment wasn't updated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add virStringHasControlChars that checks if the string has
any control characters other than \t\r\n,
and virStringStripControlChars that removes them in-place.
Throughout the code, we have several places need to construct a path
somewhere in /sys/class/net/... They are not consistent and nearly
each code piece invents its own way how to do it. So unify this by:
1) use virNetDevSysfsFile() wherever possible
2) At least use common macro SYSFS_NET_DIR declared in virnetdev.h at
the rest of places which can't go with 1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If a virAsprintf() within the function fails, we call VIR_FREE()
over @rundir variable and jump onto cleanup label, where it is
freed again. It doesn't hurt, but not make much sense too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
