Olaf Hering [Thu, 25 Mar 2021 16:26:07 +0000 (17:26 +0100)]
libxl: add API wrapper for libxl_domain_need_memory
Upcoming changes will use different LIBXL_API_VERSION variants.
Prepare libxl_domain_need_memory, which changed the storage size of
"need_memkb" in Xen 4.8. With Xen 4.12 the libxl_domain_config
parameter was changed
No functional change intended.
Signed-off-by: Olaf Hering <olaf@aepfle.de> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
An example test failure with the above environment
907) QEMU XML-2-XML-active video-virtio-gpu-sdl-gl
In 'libvirt/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml':
Offset 1244
Expect [v]
Actual [audio id='1' type='pulseaudio'/>
<v]
Scrub QEMU_AUDIO_DRV from the environment before executing the tests in
qemuxml2xmltest. SDL_AUDIODRIVER also needs scrubbed since it will be
examined if QEMU_AUDIO_DRV=sdl.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu: implement setting of rotation rate for SCSI/IDE disks
This is available in QEMU with "ide-hd" and "scsi-hd" device
types. It was originally mistakenly added to the "scsi-block"
device type too, but later removed. This doesn't affect libvirt
since we restrict usage to device=disk.
When this property is not set then QEMU's default behaviour
is to not report any rotation rate information, which
causes most guest OS to assume rotational storage.
conf: add support for disk "rotation_rate" property
This lets the app expose the virtual SCSI or IDE disks as solid state
devices by setting a rate of '1', or rotational media by setting a
rate between 1025 and 65534.
meson: Don't check whether /usr/local/bin/grep is GNU grep
Since /usr/local is where ports live, it's reasonable to assume
that a grep binary found in there will have been installed via
ports and will thus be GNU grep.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Wed, 24 Mar 2021 10:33:15 +0000 (11:33 +0100)]
meson: Look for GNU tools on macOS too
macOS is similar to FreeBSD in that it ships non-GNU versions
of several utilities that we need in the base system.
macOS actually includes GNU make already, but unfortunately due
to licensing reasons the tool is permanently stuck in 2006, so
even in that case users are better off installing a recent
version from Homebrew along with the dozens of other libvirt
dependencies that already need to be obtained that way.
Note that, unlike FreeBSD ports, Homebrew is fully consistent
in adding the 'g' prefix to the name of the GNU tools, so we
can detect GNU grep without additional hacks.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Wed, 24 Mar 2021 09:10:20 +0000 (10:10 +0100)]
meson: Check GNU sed's availability
As explained in the comment in build-aux/Makefile.in, the
version of sed included in the FreeBSD base system is not GNU
sed, which our syntax-check rules expect; as a result, many
checks will fail with
gmake: gsed: No such file or directory
/bin/sh: gsed: not found
Similarly to what we're already doing with GNU make and GNU
grep, look for GNU sed during the configuration step and fail
early if it's not available.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Wed, 24 Mar 2021 08:36:19 +0000 (09:36 +0100)]
meson: Reorganize looking for programs
While this change doesn't look like it would improve things and
actually introduces a tiny bit of duplication, it's necessary in
order to prepares the stage for further changes.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
XML <source bridge='VMnet0'/> update in <interface type='bridge'/>
Previously, we accepted empty bridge name, because some old versions of
VMWare Workstation did not put it into the config. But this doesn't make
much sense - to have an interface type bridge with no name. We
circumvented this problem by generating an empty name but that is
equally wrong.
Therefore, fill in missing bridge names (according to the documentation
[1] the default bridge name is VMnet0) and error out if bridge name is
missing.
build: teach run script how to temporarily stop systemd units
When testing locally built daemons on a systemd host there can be quite
a few systemd units that need temporarily stopping, and ideally
restarting after the test is complete. This becomes a massive burden
when modular daemons are running and you want to test libvirtd, as a
huge number of units need stopping.
The run script can facilitate this usage by looking at what units are
running and automatically stopping any that are known to conflict with
the daemon that is about to be run. This is only done when running as
root, since non-root libvirtd does not (currently) use systemd.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 30 Mar 2021 08:28:35 +0000 (10:28 +0200)]
NEWS: Mention fix for exec-restart of virtlo(g|ck)d and 'object_add' improvements
Mention that libvirt-7.2 will be needed to do stuff that executes
'object-add'/'object-del' QMP commands with the upcoming qemu-6.0 and
that exec-restart of virtlockd and virtlogd was fixed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
repeats the firmware attribute twice. This has no functional benefit, as
evidenced by fact that we use a single struct field to store both
attributes, while needlessly introducing an error scenario. The XML can
just be simplified to:
wangjian [Fri, 26 Mar 2021 03:21:16 +0000 (11:21 +0800)]
node_device_udev: Serialize access to pci_get_strings)_
Since the functions provided by libpciaccess are not thread-safe,
when the udev-event and nodedev-init threads of libvirt call the
pci_get_strings function provided by libpaciaccess at the same
time the following can happen:
Michal Privoznik [Wed, 24 Mar 2021 16:33:14 +0000 (17:33 +0100)]
lib: Undo some g_steal_pointer() changes
Recently, a few commits back I've switched bunch of code to
g_steal_pointer() using coccinelle. Problem was that the semantic
patch used was slightly off:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Problem is that, "... when != a" is supposed to jump over those
lines, which don't contain expression a. My idea was to replace
the following pattern too:
This is not necessarily correct - as demonstrated by our hotplug
code. The real problem is ambiguous memory ownership transfer
(functions which add device to domain def take ownership only on
success), but to not tackle the real issue let's revert those
parts.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Wed, 24 Mar 2021 15:49:39 +0000 (16:49 +0100)]
virnetsocket: Revert part of g_steal_pointer() rewrite
Turns out, the way that glib implements g_steal_pointer() is not
compatible with function callbacks. And that's what my recent
patch did in virNetSocketEventFree(). Revert that part.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Thu, 25 Mar 2021 11:12:13 +0000 (12:12 +0100)]
vz: Add case for VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER
In one of my recent patches I've introduced new connection
feature VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER.
However, I forgot to add corresponding case into a switch in
vzConnectSupportsFeature().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 16 Mar 2021 09:33:26 +0000 (10:33 +0100)]
lib: Fix calling of virNetworkUpdate() driver callback
The order in which virNetworkUpdate() accepts @section and
@command arguments is not the same as in which it passes them
onto networkUpdate() callback. Until recently, it did not really
matter, because calling the API on client side meant arguments
were encoded in reversed order (compared to the public API), but
then on the server it was fixed again - because the server
decoded RPC (still swapped), called public API (still swapped)
and in turn called the network driver callback (with reversing
the order - so magically fixing the order).
Long story short, if the public API is called even number of
times those swaps cancel each other out. The problem is when the
API is called an odd numbed of times - which happens with split
daemons and the right URI. There's one call in the client (e.g.
virsh net-update), the other in a hypervisor daemon (say
virtqemud) which ends up calling the API in the virnetworkd.
The fix is obvious - fix the order in which arguments are passed
to the callback.
But, to maintain compatibility with older, yet unfixed, daemons
new connection feature is introduced. The feature is detected
just before calling the callback and allows client to pass
arguments in correct order (talking to fixed daemon) or in
reversed order (talking to older daemon).
Unfortunately, older client talking to newer daemon can't be
fixed. Let's hope that it's less frequent scenario.
Fixes: 574b9bc66b6b10cc4cf50f299c3f0ff55f2cbefb
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 16 Mar 2021 09:32:59 +0000 (10:32 +0100)]
network: Implement virConnectSupportsFeature()
So far, it was not needed, but shortly a client will want to know
whether virNetworkUpdate() API is fixed or not. See next commits
for more info.
Side note, this driver's implementation is called only when using
sub-driver's connection, i.e. "network:///system". For any other
URI the corresponding hypervisor's driver callback is called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Tue, 23 Mar 2021 18:39:37 +0000 (14:39 -0400)]
qemu: increase locked memory limit when a vDPA device is present
Just like VFIO devices, vDPA devices may need to have all guest memory
pages locked/pinned in order to operate properly. In the case of VFIO
devices (including mdev and NVME, which also use VFIO) libvirt
automatically increases the locked memory limit when one of those
devices is present. This patch modifies that code to also increase the
limit if there are any vDPA devices present.
Resolves: https://bugzilla.redhat.com/1939776 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Laine Stump [Tue, 23 Mar 2021 18:28:04 +0000 (14:28 -0400)]
qemu: account for mdev devices in getPPC64MemLockLimitBytes()
This function is a specialized version of
qemuDomainGetMemLockLimitBytes() for PPC64. Simplifying it in the same
manner as the previous patch has the nice side effect of accounting
for the possibility of an mdev device
(I don't know if mdev devices are supported on PPC, but even if not
then a) the additional check for mdev devices gained by using
qemuDomainNeedsVFIO() in place of open coding will be an effective
NOP, and b) if mdev devices are supported on PPC64 in the future, this
function will be prepared for it).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Laine Stump [Tue, 23 Mar 2021 17:13:33 +0000 (13:13 -0400)]
qemu: simplify qemuDomainGetMemLockLimitBytes()
This function goes through a loop checking if each hostdev is a VFIO
or mdev device, and then later it calls virDomainDefHasNVMEDisk(). The
function qemuDomainNeedsVFIO() does exactly the same thing, so let's
just call that instead.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Wed, 24 Mar 2021 09:33:45 +0000 (10:33 +0100)]
esx: Fix @doms pointer steal in esxConnectListAllDomains()
The ESX implementation of virConnectListAllDomains() follows
pretty much implementations in other drivers: it has local array
of virDomainPtr-s which (if requested by caller) is filled by
actual domains or not (if the caller is interested only in the
count of domains).
Anyway, in case of the former, the passed @domains argument is
set to the local array, which is then set to NULL to prevent it
from freeing under cleanup label. Pretty standard pattern.
Except, the local array is set to NULL always. Even if the local
array is not stolen. Fortunately, this doesn't lead to a memory
leak, because if caller is not interested in the array, none is
allocated. But it doesn't set good example and also breaks my
spatch rules.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Laine Stump [Wed, 24 Feb 2021 03:19:56 +0000 (22:19 -0500)]
util: don't log error if SRIOV PF has no associated netdev
Some SRIOV PFs don't have a netdev associated with them (the spec
apparently doesn't require it). In most cases when libvirt is dealing
with an SRIOV VF, that VF must have a PF, and the PF *must* have an
associated netdev (the only way to set the MAC address of a VF is by
sending a netlink message to the netdev of that VF's PF). But there
are times when we don't need for the PF to have a netdev; in
particular, when we're just getting the Switchdev Features for a VF,
we don't need the PF netdev - the netdev of the VF (apparently) works
just as well.
Commit 6452e2f5 (libvirt 5.1.0) *kind of* made libvirt work around PFs
with no netdevs in this case - if virNetDevGetPhysicalFunction
returned an error when setting up to retrieve Switchdev feature info,
it would ignore the error, and then check if the PF netdev name was
NULL and, if so it would reset the error object and continue on rather
than returning early with a failure. The problem is that by the time
this special handling occured, the error message about missing netdev
had already been logged, which was harmless to proper operation, but
confused the user.
Fortunately there are only 2 users of virNetDevGetPhysicalFunction, so
it is easy to redefine it's API to state that a missing netdev name is
*not* an error - in that case it will still return success, but the
caller must be prepared for the PF netdev name to be NULL. After
making this change, we can modify the two callers to behave properly
with the new semantics (for one of the callers it *is* still an error,
so the error message is moved there, but for the other it is okay to
continue), and our spurious error messages are a thing of the past.
Resolves: https://bugzilla.redhat.com/1924616 Fixes: 6452e2f5e1837bd753ee465e6607ed3c4f62b815 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemu: don't raise error upon interface update without <frames/> for <rx/> in coalesce
With this, incomplete XML without <frames/> for <rx/> in coalesce
won't raise error as before. It will leave the coalesce parameter
empty, thanks to passing it as a parameter and return an integer
to indicate error state - previously it returned pointer (or NULL
for both error and incomplete XML).
I also added a test case to test this functionality in the
qemuxml2xmltest.
The code went through some refactoring:
* change of a condition
* addition of a parameter
* change of order, that allowed removal of VIR_FREE
* removal of redundant labels and variables
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1535930 Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
domain_cgroup.c: update domain after setting blkio.weight
Commit ac87d3520ad5 consolidated common cgroup code between the QEMU and
lxc drivers in domain_cgroup.c. In this process, in
virDomainCgroupSetupDomainBlkioParameters(), a call to
virCgroupGetBlkioWeight() went missing.
The result is that 'virsh blkiotune' is setting the blkio.weight for the
guest in the host cgroup, but not on the domain XML, because
virCgroupGetBlkioWeight() is also used to write the blkio.weight value
in the domain object.
Fix it by adding the virCgroupGetBlkioWeight() call in the
virDomainCgroupSetupDomainBlkioParameters() helper.
Fixes: ac87d3520ad542d558854a72b0ae0a81fddc6747
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1941407 Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Mon, 22 Mar 2021 13:44:55 +0000 (14:44 +0100)]
virDomainCheckpointRedefinePrep: Assign default bitmap names when domain XML is missing
Previously we'd assign the default checkpoint bitmap names in
virDomainCheckpointAlignDisks. In cases when the checkpoint is redefined
without a domain XML virDomainCheckpointAlignDisks is not called.
Add an explicit call to virDomainCheckpointDefAssignBitmapNames to
restore functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 22 Mar 2021 13:43:17 +0000 (14:43 +0100)]
qemuCheckpointDiscardBitmaps: Refuse to delete checkpoint with NULL bitmap name
When a checkpoint is redefined without providing the domain XML, we
might end up with a definition where the per-disk bitmap name is not
set. Trying to delete such checkpoint would lead to a crash.
Refuse such deletion.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1941600 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 24 Feb 2021 15:40:45 +0000 (16:40 +0100)]
tests: qemucapabilities: Update qemu caps for object-add qapification
qemu qapified object-add, which means that it's introspectable via
query-qmp-schema. Update the qemu-6.0 capabilities to commit v5.2.0-3205-g92566947b3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 12 Mar 2021 14:44:19 +0000 (15:44 +0100)]
qemu: command: Use JSON for QAPIfied -object directly
Skip the lossy conversion to legacy commandline arguments by using the
JSON props directly when -object is QAPIfied. This avoids issues with
conversion of bitmaps and also allows validation of the generated JSON
against the QMP schema in the tests.
Since the new approach is triggered by a qemu capability the code
from 'virQEMUBuildObjectCommandlineFromJSON' in util/virqemu.c was moved
to 'qemuBuildObjectCommandlineFromJSON' in qemu/qemu_command.c which has
the virQEMUCaps type.
Some functions needed to be modified to propagate qemuCaps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 30 Nov 2020 16:08:46 +0000 (17:08 +0100)]
qemuMonitorCreateObjectPropsWrap: Open-code in qemuBuildMemoryBackendProps
There's just one caller left. Since qemuBuildMemoryBackendProps is too
complex to be modified for now, just move the adding of 'id' and 'qom'
type directly into the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Starting from qemu-6.0 the parameters of -object/object-add are formally
described by the QAPI schema. Additionally this changes the nesting of
the properties as the 'props' nested object will be flattened to the
parent.
We'll need to detect whether qemu switched to this new approach to
generate the objects with proper nesting and also allow testing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 23 Mar 2021 10:47:39 +0000 (11:47 +0100)]
domain_conf: Separate virDomainOS clear into a function
The virDomainDefFree() function frees individual members of
virDomainDef struct. The function is already long enough, move
code that handles def->os member into a separate function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Mon, 22 Mar 2021 11:02:23 +0000 (12:02 +0100)]
ci: Drop FreeBSD 11 build
FreeBSD 12 was released in December 2018, so according to our
platform support policy we can now drop support for the previous
major release. It would be going EOL in September anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Fri, 19 Mar 2021 21:42:25 +0000 (22:42 +0100)]
qemu: Update asyncOwnerAPI when entering async job phase
In case an async job spans multiple APIs (e.g., incoming migration) the
API that started the job is recorded as the asyncOwnerAPI even though it
is no longer running and the owner thread is updated properly to the one
currently handling the job. Let's also update asyncOwnerAPI to make it
more obvious which is the current (or the most recent) API involved in
the job.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Attempting to set the memlock limit might fail if we're running
in a containerized environment where CAP_SYS_RESOURCE is not
available, and if the limit is already high enough there's no
point in trying to raise it anyway.
Now that we've implemented a fallback for the function that
obtains the information from /proc, there is no reason we would
get a failure unless there's something seriously wrong with the
environment we're running in, in which case we're better off
reporting the issue to the user rather than pretending
everything is fine.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Calling prlimit() requires elevated privileges, specifically
CAP_SYS_RESOURCE, and getrlimit() only works for the current
process which is too limiting for our needs; /proc/$pid/limits,
on the other hand, can be read by any process, so implement
parsing that file as a fallback for when prlimit() fails.
This is useful in containerized environments.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Fri, 19 Mar 2021 13:24:19 +0000 (14:24 +0100)]
syntax-check: Run flake8 on all Python scripts
Currenty we only check files that end in .py, but we have at
least a couple of scripts that don't have that suffix and we
nonetheless want to keep compliant with the code style.
Extend the sc_flake8 syntax-check rule so that any file that
contains a Python 3 shebang is fed to flake8 too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Fri, 19 Mar 2021 13:27:28 +0000 (14:27 +0100)]
gitignore: Ignore __pycache__ directory
Unfortunately running Python scripts causes this directory to
be created in the *source* tree, and there doesn't seem to be
a way to prevent that from happening.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Fri, 19 Mar 2021 15:04:37 +0000 (16:04 +0100)]
ci: Drop prefix from Dockerfiles
Since the string "ci" is already contained in the path, it
seems unnecessary to include it into the filename too: in fact,
we only do that for Dockerfiles and not for files in ci/cirrus,
even though those are generated the very same way.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Implement "<os firmware='efi'>" support for bhyve driver.
As there are not really lot of options, try to find
"BHYVE_UEFI.fd" firmware which is installed by the
sysutils/uefi-edk2-bhyve FreeBSD port.
If not found, just use the first found firmware
in the firmwares directory (which is configurable via
config file).
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>