Peter Krempa [Wed, 9 Feb 2022 11:30:25 +0000 (12:30 +0100)]
tests: Move testPrepareHostBackendChardevOne into test utils
Move the function doing the fake setup of chardev backend for FD passing
into the collection of qemu test helpers so that it can be used in
qemumonitorjsontest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The main objective of this patch is to use a proper instance of
virDomainChrSourceDef allocated with the private data.
To achieve this the test cases are grouped into blocks by how much they
fill in the chardev definition. Some test cases are moved around so
that the resulting sequence doesn't need extra clearing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 3 Feb 2022 09:35:03 +0000 (10:35 +0100)]
qemuDomainRemoveChrDevice: Detach fdset after chardev hot-unplug
Our code uses fdsets for the pipe passed from virtlogd to qemu, but the
chardev hot-unplug code neglected to detach the fdset after the chardev
was removed. This kept the FDs open by qemu even after they were not
used any more.
After the refactor to use qemuFDPass for chardevs we now configure the
'opaque' field for fdsets used for chardevs so we can use
qemuHotplugRemoveFDSet to remove the unused fdset.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 2 Feb 2022 16:31:29 +0000 (17:31 +0100)]
qemu: Rewrite chardev startup code to use qemuFDPass
Rewrite the parts which already pass FDs via fdset or directly to use
the new infrastructure.
Apart from simpler code this also adds the appropriate names to the fds
in the fdsets which will allow us to properly remove the fdsets won
hot-unplug of chardevs, which we didn't do for now and resulted in
leaking the FDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 2 Feb 2022 15:30:36 +0000 (16:30 +0100)]
QEMUHotplugRemoveFDSet: Prepare for proper FD unplug handling
For now we have only one code path ('vdpa' interface) which actually
cleans up the fdset after it's done, but there are more device types
using fdsets.
In order to unify the handling of fdsets the removal code will now be
able to remove fdsets based on a prefix of the 'opaque' field, which
we'll always prefix with a device alias or e.g. node name once fdsets
are also used for disk backing.
To keep compatibility with old QEMUs, retain the possibility for the
VDPA interface to use the path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 24 Jan 2022 16:24:55 +0000 (17:24 +0100)]
qemu: Introduce helper functions for passing FDs to qemu
The existing helpers we have are very clumsy and there's no integration
with the monitor.
This patch introduces new helpers to bridge the gap and simplify handing
of fdsets and classic FD passing when generating commandline/hotplug
arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 24 Jan 2022 15:53:27 +0000 (16:53 +0100)]
qemu: domain: Add helper for generating 'fdset' ids for VM startup
When starting a VM we must assign unique IDs for fdsets we add via
'-add-fd'. For now it was done by using the index of the filedescriptor
passed to the virCommand. That approach is not very flexible, because
you need to have already passed the 'fd' to virCommand before generating
the fdset path, and also won't nicely work with fdsets containing two or
more fds.
This patch introduces a counter into the private data of a qemu domain
so that we can allocate unique ids without relying on virCommand.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 24 Jan 2022 14:58:37 +0000 (15:58 +0100)]
qemuMonitorRemoveFdset: Convert @fdset to unsigned int to avoid error
'qemuMonitorRemoveFdset' validates that the 'fdset' argument isn't less
than 0. We can turn it to unsigned and thus avoid the error message
completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 3 Feb 2022 10:00:29 +0000 (11:00 +0100)]
qemuProcessPrepareHostBackendChardev: Drop unneeded arguments
Caller passes 'driver->securityManager', and 'priv->qemuCaps' as
arguments along with 'vm', but both aforementioned objects are
accessible directly from 'vm'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 14 Feb 2022 09:56:41 +0000 (10:56 +0100)]
qemu: domain: Change 'Id' to 'ID' in qemuDomainStorageIdNew/Reset
In a patch adding similarly named APIs I was asked to use 'ID' instead
of 'Id'. Since the code is being put together fix
qemuDomainStorageIdNew/Reset first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add test cases for quotes appearing in the netcat parameter,
for the default behavior of proxy=auto where virt-ssh-helper
is used if available, and for proxy=native.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 10 Feb 2022 08:42:01 +0000 (09:42 +0100)]
util: Fix getting CPU frequency on Apple Silicon
The hw.cpufrequency sysctl, which we use to obtain the CPU
frequency on macOS, is not available when running on Apple
Silicon, and as a consequence we currently report an error
whenever such information is requested.
The virNodeInfo.mhz field, where the CPU frequency gets stored,
is documented as being zero when the information could not be
obtained, and we already do that for Linux on aarch64. Extend
this behavior to macOS on Apple Silicon.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 10 Feb 2022 09:19:57 +0000 (10:19 +0100)]
qemuxml2xmltest: Remove unnecessary versioned invocation of 'disk-cache'
The 'disk-cache' output file is identical in the interesting parts
(everything besides CPU config) to the '-latest' version, so the
versioned invocation can be dropped.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Fri, 11 Feb 2022 12:56:29 +0000 (13:56 +0100)]
conf: Avoid NULL-dereference in virDomainObjGetMessages
All callers currently guarantee flags passed to virDomainObjGetMessages
are either zero or contain at least one of the supported flags. But it
doesn't mean we should not check for the possibility an unknown flag was
the only one passed to virDomainObjGetMessages.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 10 Feb 2022 15:16:48 +0000 (16:16 +0100)]
qemu_migration_cookie: Properly fetch cert DN
If 1024 was not enough to fit the DN, gnutls_x509_crt_get_dn would store
the required size in subjectlen. And since we're not checking the return
value of this function, we would happily overwrite some random memory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 10 Feb 2022 15:00:52 +0000 (16:00 +0100)]
qemu_migration_cookie: Rename ret in qemuDomainExtractTLSSubject
We use 'ret' for storing values to be returned from a function. Return
values from called functions that are not supposed to be returned
further are usually called 'rv' (or 'rc').
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Thu, 10 Feb 2022 10:32:00 +0000 (11:32 +0100)]
tests: Fix fd leaks in virpcivpdtest
Tests testVirPCIVPDReadVPDBytes and testVirPCIVPDParseFullVPDInvalid
failed to properly close open fildescriptors in some cases. Let's fix it
by switching to VIR_AUTOCLOSE in the whole file.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few places where a virPCIDeviceAddress typed variable
is allocated on the stack but it's not initialized. This can lead
to random values of its members which in turn can lead to a
random behaviour.
And then fixing bhyveAssignDevicePCISlots() which does declare
the variable and then explicitly zero it by calling memset() only
to set a specific member afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Fri, 11 Feb 2022 08:28:11 +0000 (09:28 +0100)]
qemuPrepareNVRAM: Switch to VIR_AUTOCLOSE
Nothing inside the qemuPrepareNVRAM function relies on @srcFD
being closed early and nothing closes it early. It's okay then to
close it automatically when leaving the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virFileRewrite: Move error reporting into callback
When rewriting a file using virFileRewrite() and error occurs
while writing into a temporary file it's actually the callback
that can report the most accurate error. Move error reporting
into very few callback we have currently. Those callbacks are
trivial so the benefit of this change is not obvious, but this
will change shortly when slightly more complicated callback is
introduced.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, due to the way virFileRewrite() works, the rewritten
file is owned by user and group that the daemon runs under. So
far, this is not a problem, because the function is used to write
XML files or secrets for persistent objects (domains, networks,
etc.) and we don't need other users to read/write those files.
But shortly, this function is going to be used for creating files
for QEMU domains. There we want the QEMU process (i.e. different
user) to read the file.
Therefore, introduce two new arguments: @uid and @gid that allow
setting desired owner of the file. Pass -1 to preserve current
behaviour (i.e. create the file owned by the user running the
daemon).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
ch: Report VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER as supported
Even though the CH driver doesn't implement virNetworkUpdate()
API, when it does it will see the arguments in correct order.
This is similar to other drivers that don't implement the API,
like ESX, libxl, LXC, etc. Enabling this driver feature stops
clients from swapping the arguments (see comment in the API for
more info).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 15:17:20 +0000 (16:17 +0100)]
ch_process: Check whether domain is already running before starting it
There are two places where a domain can be started in CH driver:
chDomainCreateXML() and chDomainCreateWithFlags(). Both acquire a
job (good), but neither of them checks whether the domain isn't
already running. This is wrong. Fortunately, both function call
the very same virCHProcessStart() rendering it the best place for
such check.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 15:10:13 +0000 (16:10 +0100)]
ch_driver: Introduce and use virCHDomainRemoveInactive()
There are few places where a call to virDomainObjListRemove() is
guarded with !vm->persistent check. And there are some places
which are missing this check completely (leading us to losing a
domain). To prevent such mistakes introduce
virCHDomainRemoveInactive() which does the check for us. Also
replace all occurrences of virDomainObjListRemove() with the call
to the new function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 14:50:34 +0000 (15:50 +0100)]
ch_driver: Don't lock driver when looking up domains
There is no need to lock whole driver when accessing
virDomainObjList. Those APIs were specifically tailored to be
thread safe (when we were dropping QEMU driver lock). Don't
resurrect old history.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 14:28:47 +0000 (15:28 +0100)]
ch_driver: Don't lock driver when getting version
In chConnectGetVersion() the CH driver is locked in order to read
driver->version. This is needless, because not only is the
version set with driver unlocked (chStateInitialize() calls
chExtractVersion() which sets the version), but the version is
practically immutable. Once driver initialized itself it's never
changed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 09:52:47 +0000 (10:52 +0100)]
qemuFirmwareFillDomain: Use FW descriptors to lookup template when resetting NVRAM
If VIR_QEMU_PROCESS_START_RESET_NVRAM flag is passed when
starting a domain, then user requested to overwrite the domain
specific NVRAM with the one from template. But it is very likely
that the path to the template is not stored in the domain
definition, which in turn makes the copy function
(qemuPrepareNVRAM()) fail.
The solution is simple - when preparing domain, specifically when
deciding whether the path to the template should be autofilled,
ignore any existing NVRAM file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Thu, 10 Feb 2022 09:52:37 +0000 (10:52 +0100)]
qemu_process: Accept VIR_QEMU_PROCESS_START_RESET_NVRAM flag when starting a domain
In one of my previous commits I've fixed the value of
VIR_QEMU_PROCESS_START_RESET_NVRAM flag (which was masking
another value). But what I forgot to do is update virCheckFlags()
calls in two places where the flag is passed: qemuProcessLaunch()
and qemuProcessStart().
Fixes: 1b636593c76f443169ef7bdb3644fd670379d04e Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Thu, 10 Feb 2022 14:11:47 +0000 (15:11 +0100)]
docs: Clarify our stance on backported packages
The repositories containing them are usually offered with lower
guarantees, so we don't consider them when it comes to figuring
out the minimum targeted version of our dependencies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
tuqiang [Wed, 9 Feb 2022 10:59:34 +0000 (18:59 +0800)]
virNetDevOpenvswitchUpdateVlan: fix vlan tag update error
We try to update vlan tag by running virsh update-device command,
libvirtd will report ovs-vsctl arguments error. Vlan tag update
funtion does't consider the xml with no vlan configured circumstances.
The steps to reproduce the problem:
1 define and start domain with its vlan configured as:
<interface type='bridge'>
<mac address='52:54:00:9e:bb:ac'/>
<source bridge='ovs-br0'/>
<vlan>
<tag id='10'/>
</vlan>
<virtualport type='openvswitch'>
</virtualport>
<target dev='vnet4.0'/>
<model type='virtio'/>
<driver name='vhost'/>
</interface>
2 define and run virsh update-device command with no vlan configured as:
<interface type='bridge'>
<mac address='52:54:00:9e:bb:ac'/>
<source bridge='ovs-br0'/>
<virtualport type='openvswitch'>
</virtualport>
<target dev='vnet4.0'/>
<model type='virtio'/>
<driver name='vhost'/>
</interface>
#virsh update-device dom-id novlan.xml
3 virsh command returned error, and we got an error in libvirtd.log:
error : virCommandWait:2584 : internal error: exit status 1: ovs-vsctl: 'set' command requires at least 3 arguments
. Child process (ovs-vsctl --timeout=5 -- --if-exists clear Port vnet4.0 tag -- --if-exists clear Port vnet4.0 trunk
-- --if-exists clear Port vnet4.0 vlan_mode -- --if-exists set Port vnet4.0) unexpected
error : virNetDevOpenvswitchUpdateVlan:540 : internal error: Unable to set vlan configuration on port vnet4.0
Signed-off-by: Tu Qiang <tu.qiang35@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Older kernels did not support this sysctl, but they did not restrict
userfaultfd in any way so everything worked as if
vm.unprivileged_userfaultfd was set to 1. Thus we can safely ignore
errors when setting the value.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu_process: Fix theoretical overflow in uint to bool typecast
The qemuPrepareNVRAM() function accepts three arguments and the
last one being a boolean type. However, when the function is
called from qemuProcessPrepareHost() the argument passed is a
result of logical and of @flags (unsigned int) and
VIR_QEMU_PROCESS_START_RESET_NVRAM value. In theory this is
unsafe to do because if the value of the flag is ever changed
then this expression might overflow. Do what we do elsewhere:
double negation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
qemu_process.c: Fix VIR_QEMU_PROCESS_START_RESET_NVRAM value
In one of recent commits qemuProcessStartFlags enum gained new
value: VIR_QEMU_PROCESS_START_RESET_NVRAM but due to a typo it
has the same value as another member of the enum. Fix that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Mon, 7 Feb 2022 20:57:07 +0000 (13:57 -0700)]
libxl: Release auto-allocated spice ports
While VNC ports auto-allocated by the libxl driver are released in
libxlDomainCleanup, spice ports are overlooked. Rework the existing
logic to release any auto-allocated graphics ports, not just the VNC
port of the first graphics device.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
When the <loader> had an explicit readonly='no' attribute we
accidentally still marked the plfash as readonly due to the
bad conversion from virTristateBool to bool. This was missed
because the test cases run with no capabilities set and thus
are validated the -drive approach for pflash configuration,
not the -blockdev approach.
for the sake of completeness, we also add a test XML config
with no readonly attribute at all, to demonstrate that the
default for pflash is intended to be r/w.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
include: define constants for resetting NVRAM state
When starting a guest with pflash based firmware, we will initialize
NVRAM from a template if it does not already exist. In theory if the
firmware code file is updated, the existing NVRAM variables should
continue to work correctly. It is inevitable that this could break
accidentally one day. Or a bug in the firmware might corrupt the
NVRAM storage. Or user might make bad changes to the settings that
prevent booting. Or the user might have re-configured the XML to
point to a different firmware file incompatible with the current
variables.
In all these cases it would be useful to delete the existing NVRAM
and initialize it from the pristine template.
To support this introduce a VIR_DOMAIN_START_RESET_NVRAM constant
for use with virDomainCreate / virDomainCreateXML, along with
VIR_DOMAIN_SAVE_RESET_NVRAM for use with virDomainRestore and
VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM for use with
virDomainSnapshotRevert.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If we crash part way through writing the NVRAM file we end up with an
unusable NVRAM on file. To avoid this we need to write to a temporary
file and fsync(2) at the end, then rename to the real NVRAM file path.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In one of my previous commits, I've changed an XPath in
virCPUDefParseXML() from "boolean(./counter...)" to
"./counter...)". Notice the dangling closing bracket? Well, I
didn't back then.
Fixes: 0fe2d8dd335054fae38b46bbbac58a4662e1a1d0 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two places where a variable passed to VBOX_RELEASE()
macro is set to NULL explicitly. There is no need for that
because the macro sets the variable to NULL already.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Wed, 2 Feb 2022 00:03:11 +0000 (17:03 -0700)]
libxl: Set auto-allocated graphics ports to used on reconnect
The libxl driver reconnects to all running VMs when libvirtd is restarted,
but it failed to mark auto-allocated graphics ports as set in the port
allocator. If many VMs are running that use port auto-allocation and
libvirtd is restarted, the port allocator is likely to hand out a port
already in use when a new VM is created that uses auto-allocation. VM
creation will fail due to the port clash.
When reconnecting to running VMs after a libvirtd restart, let the port
allocator know about previously allocated ports.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 7 Feb 2022 11:29:47 +0000 (12:29 +0100)]
qemu: process: Don't use hardcoded QOM path for cpu for probing flags
Modify 'qemuProcessGetVCPUQOMPath' to take the detected QOM path of the
first vCPU which is always present as the QOM path used our code probing
CPU flags via 'qom-get'.
This is needed as upcoming qemu will change it.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/272
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2051451 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>