A wrapper around xmlXPathRegisterNs that will save us
from having to include xpathInternals.h everywhere
we want to use a custom namespace and open-coding
the strings already contained in virXMLNamespace.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
A function to automatically format the xmlns:<prefix>='<uri>'
attribute for per-driver namespaces.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
We have hardcoded the namespace prefix in various places:
1) the xmlns string stored in the 'href' function
2) the xmlXPathRegisterNs call in each parser
3) all the parsing and formatting code actually dealing
with these elements
While eliminating the third one is probably a job for an
actual XML-aware formatter, let's store the prefix separately
here in the virXMLNamespace structure so that future patches
can get rid of the first two bullets.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
For various XMLs, we allow a custom namespace for passing unsupported
configurations.
Introduce a single structure to hold all the driver-specific functions
to remove duplication.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If the first value in cpu.max is "max" return from function.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741837
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Our virStrToLong* helpers converts string to integers where it wraps
strtol standard function. After the conversion happens and there are
some remaining invalid characters our helpers will fail if the second
argument is NULL.
We need to pass pointer to string in cases where there are multiple
values in a single file.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741825
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The virHostdevPreparePCIDevices() function works in several
steps. In the very first one, it checks if devices we want to
detach from the host are not taken already by some other domain.
However, this piece of code returns different results depending
on the stub driver used (which is not wrong per se, but keep on
reading). If the stub driver is KVM then
virHostdevIsPCINodeDeviceUsed() is called which basically checks
if a PCI device from the detach list is not used by any domain
(including the one we are preparing the device for). If that is
the case, an error is reported ("device in use") and -1 is
returned.
However, that is not what happens if the stub driver is VFIO. If
the stub driver is VFIO, then we iterate over all PCI devices
from the same IOMMU group and check if they are taken by some
other domain (because a PCI device, well IOMMU group, can't be
shared between two or more qemu processes). But we fail to check,
if the device we are trying to detach from the host is not
already taken by a domain. That is, calling
virHostdevPreparePCIDevices() over a hostdev device twice
succeeds the first time and fails too late in the second run
(fortunately, virHostdevResetAllPCIDevices() will throw an error,
but this is already too late because the PCI device in question
was moved to the list of inactive PCI devices and now it appears
in both lists).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
It may happen that there are two domains with the same name in
two separate drivers (e.g. qemu and lxc). That is why for PCI
devices we track both names of driver and domain combination
which has taken the device. However, when we check if given PCI
device is in use (or PCI devices from the same IOMMU group) we
compare only domain name. This means that we can mistakenly claim
device as free to use while in fact it isn't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
virStorageSourceUpdatePhysicalSize is called only from
qemuDomainStorageUpdatePhysical and all callers of it reset the libvirt
error if -1 is returned.
Don't bother setting the error in the first place.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function will be reused in the qemu snapshot code. The argument is
turned into const similarly to the other virStorageFileSupports*
functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the nbd export name contains a colon, our parser would not parse it
properly as we split the string by colons. Modify the code to look up
the exportname and copy any trailing characters as the export name is
supposed to be at the end of the string.
https://bugzilla.redhat.com/show_bug.cgi?id=1733044
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The parent bridge configuration of the current device
should be read and reset, instead of reading the current
device configuration.
Signed-off-by: He Xin <hexin15@baidu.com>
Signed-off-by: Liu Qi <liuqi16@baidu.com>
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add virStorageFileSupportsCreate which allows silent check whether
virStorageFileCreate is implemented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the return value so that callers don't have to repeat logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the callers left require virPCIDeviceConfigOpen to be fatal
and only use read-only access to the config file.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For callers that only need read-only access and don't want
an error reported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Only a handful of function need write access to the PCI config
space. Create a wrapper function for those so that we can
open it read only by default.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As a side effect, this also silences the possible:
internal error: Unable to get DBus system bus connection:
Failed to connect to socket /run/dbus/system_bus_socket:
No such file or directory
error, since we check upfront whether dbus is available.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Look up the binary name upfront to avoid the error:
Cannot find 'pm-is-supported' in path: No such file or directory
In that case, we just assume nodesuspend is not available.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Get rid of the ret variable as well as the cleanup label.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Back in July 2010, commit 6ea90b84 (meant to resolve
https://bugzilla.redhat.com/571991 ) added code to set the MAC address
of any tap device to the associated guest interface's MAC, but with
the first byte replaced with 0xFE. This was done in order to assure
that
1) the tap MAC and guest interface MAC were different (otherwise L2
forwarding through the tap would not work, and the kernel would
repeatedly issue a warning stating as much).
2) any bridge device that had one of these taps attached would *not*
take on the MAC of the tap (leading to network instability as
guests started and stopped)
A couple years later, https://bugzilla.redhat.com/798467 was filed,
complaining that a user could configure a tap-based guest interface to
have a MAC address that itself had a first byte of 0xFE, silently
(other than the kernel warning messages) resulting in a non-working
configuration. This was fixed by commit 5d571045, which logged an
error and failed the guest start / interface attach if the MAC's first
byte was 0xFE.
Although this restriction only reduces the potential pool of MAC
addresses from 2^46 (last two bits of byte 1 must be set to 10) by
2^32 (still 4 orders of magnitude larger than the entire IPv4 address
space), it also means that management software that autogenerates MAC
addresses must have special code to avoid an 0xFE prefix. Now after 7
years, someone has noticed this restriction and requested that we
remove it.
So instead of failing when 0xFE is found as the first byte, this patch
removes the restriction by just replacing the first byte in the tap
device MAC with 0xFA if the first byte in the guest interface is
0xFE. 0xFA is the next-highest value that still has 10 as the lowest
two bits, and still
2) meets the requirement of "tap MAC must be different from guest
interface MAC", and
3) is high enough that there should never be an issue of the attached
bridge device taking on the MAC of the tap.
The result is that *any* MAC can be chosen by management software
(although it would still not work correctly if a multicast MAC (lowest
bit of first byte set to 1) was chosen), but that's a different
issue).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com
The various distros have the following libxml2 vesions:
CentOS 7: 2.9.1
Debian Stretch: 2.9.4
FreeBSD Ports: 2.9.9
Ubuntu 16.04 LTS: 2.9.3
Based on this sampling, we can reasonably bump libxml2 min
version to 2.9.1
The 'query_raw' struct field was added in version 2.6.28,
so can be assumed to exist.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Requires adjustments to use verify_expr() which replaces
verify_true(), and to disable the new syntax check
'sc_prohibit_gnu_make_extensions' since we require GNU make.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit fed58d83 was a hack to fix a mingw build failure due to header
inclusion order resulting in a clash over the use of DATADIR,
repeating a trick made several other times in the past. Better is to
revert that, and instead use pragmas to avoid the clash in the first
place, regardless of header ordering, solving it for everyone.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the code does not refer to any libvirt headers,
except internal.h macros, it does not need to link to
any libvirt code, nor gnulib either. The only thing it
needs is yajl.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.
Thus we only need one API for env passthrough in virCommand.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that none of the libvirt.so code will ever run in a setuid
context, we can remove the virIsSUID() method. The global
initializer function can just inline the check itself. The new
inlined check is slightly stronger as it also looks for a
setgid situation.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virt-login-shell setuid program is now a tiny piece of code
that only uses standard libc functions, and santizes the execution
environment before invoking the real virt-login-shell-helper.
The latter is thus able to use the normal libvirt.so build,
allowing us to delete the special cut down setuid library build.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The previous bump to 4.4 was done in:
commit 24241c236e
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Wed Jul 5 10:35:32 2017 +0100
Require use of GCC 4.4 or CLang compilers
with 4.4 picked due to RHEL-6. Since we dropped RHEL-6, the
next oldest distro is RHEL-7 (4.8.5), and thus we pick 4.8
as the new min.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This function does not change any of the passed addresses. It
just reads them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
This function does not change any of the passed addresses. It
just reads them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
This code that executes virPCIDeviceReattach in all
virPCIDevicePtr objects of a given virPCIDeviceListPtr
list is replicated twice in the code. Putting it in a helper
function helps with readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virHostdevReattachPCIDevice() is a static that simply does
a wait loop with virPCIDeviceWaitForCleanup() before
calling virPCIDeviceReattach().
This loop traces back to commit d1e5676c0d, aiming to
solve a race condition between Libvirt returning the
device back to the host and QEMU trying to access it in
the meantime, which resulted in QEMU exiting on error
and killing the guest. This happens because device_del
is asynchronous, returning OK even if the guest didn't
release the device. Commit 01abc8a1b8 moved this code
to qemu_hostdev.c, 82e8dd4cf8 added the pci-stub conditional
for the loop, 899b261127 moved the code to virhostdev.c
where it stood until now.
The intent of this wait loop is still valid: device_del
is still not bullet proof into preventing the conditions
that commit d1e5676c0d aimed to fix, especially when considering
all the architectures we must support. However, this loop
is executed only in virHostdevReattachPCIDevice(), leaving
every other virPCIDeviceReattach() call prone to that error.
Let's move the wait loop code to virPCIDeviceReattach(). This
will:
- make every reattach call safe from this race condition
with the pci-stub;
- allow for a bit of code cleanup (virHostdevReattachPCIDevice()
can be erased, and virHostdevReAttachPCIDevices() can use
virPCIDeviceReattach() directly);
- make it easier to understand the overall reattach mechanisms in
Libvirt, without the risk of a newcomer wondering why reattach
is done slightly different in some instances.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This code that executes virPCIDeviceReset in all virPCIDevicePtr
objects of a given virPCIDeviceListPtr list is replicated twice
in the code. Putting it in a helper function helps with
readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is no restriction on maximum value of PCI domain. In fact,
Linux kernel uses plain atomic inc when assigning PCI domains:
drivers/pci/pci.c:static int pci_get_new_domain_nr(void)
drivers/pci/pci.c-{
drivers/pci/pci.c- return atomic_inc_return(&__domain_nr);
drivers/pci/pci.c-}
Of course, this function is called only if kernel was compiled
without PCI domain support or ACPI did not provide PCI domain.
However, QEMU still has the same restriction as us: in
set_pci_host_devaddr() QEMU checks if domain isn't greater than
0xffff. But one can argue that that's a QEMU limitation. We still
want to be able to cope with other hypervisors that don't have
this limitation (possibly).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, the way we format PCI address is using printf-s
precision, e.g. "%.4x". This works if we don't want to print any
value outside of bounds (which is usually the case). However,
turns out, PCI domain can be 0x10000 which doesn't work well with
our format strings. However, if we change the format string to
"%04x" then we still pad small values with zeroes but also we are
able to print values that are larger than four digits. In fact,
this format string used by kernel to print a PCI address:
"%04x:%02x:%02x.%d"
The other three format strings (for bus, device and function) are
changed too, so that we use the same format string as kernel.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The format string for a PCI address is copied over and over
again, often with slight adjustments. Introduce global
VIR_PCI_DEVICE_ADDRESS_FMT macro that holds the formatting string
and use it wherever possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In near future, the length restriction of PCI domain is going to
be lifted. This means that our assumption that PCI address is 13
bytes long is no longer true. We can avoid this problem by making
@name dynamically allocated and thus not bother with actual
length of stringified PCI address.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function declares @ret variable and then uses
VIR_STEAL_PTR() to avoid freeing temporary variable @dev which is
constructed. Well, as of 267f1e6da5 we have VIR_RETURN_PTR()
macro so that we can avoid this pattern.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Export virResctrlMonitorGetStats and make
virResctrlMonitorGetCacheOccupancy obsoleted.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor 'virResctrlMonitorStats' to track multiple statistical
records.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor and rename 'virResctrlMonitorFreeStats' to
'virResctrlMonitorStatsFree' to free one
'virResctrlMonitorStatsPtr' object.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
