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>
Andrea Bolognani [Thu, 27 Jan 2022 16:17:36 +0000 (17:17 +0100)]
qemu: Rename path-building functions
Using the word "create" can give users the impression that disk
operations will be performed, when in reality all these functions
do is string formatting.
Follow the naming convention established by virBuildPath(),
virFileBuildPath() and virPidFileBuildPath().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Mon, 31 Jan 2022 11:55:47 +0000 (12:55 +0100)]
qemu: Validate domain definition even on migration
When we are about to spawn QEMU, we validate the domain
definition against qemuCaps. Except when domain is/was already
running before (i.e. on incoming migration, snapshots, resume
from a file). However, especially on incoming migration it may
happen that the destination QEMU is different to the source
QEMU, e.g. the destination QEMU may have some devices disabled.
And we have a function that validates devices/features requested
in domain XML against the desired QEMU capabilities (aka
qemuCaps) - it's virDomainDefValidate() which calls
qemuValidateDomainDef() and qemuValidateDomainDeviceDef()
subsequently.
But the problem here is that the validation function is
explicitly skipped over in specific scenarios (like incoming
migration, restore from a snapshot or previously saved file).
This in turn means that we may spawn QEMU and request
device/features it doesn't support. When that happens QEMU fails
to load migration stream:
qemu-kvm: ... 'virtio-mem-pci' is not a valid device model name
(NB, while the example shows one particular device, the problem
is paramount)
This problem is easier to run into since we are slowly moving
validation from qemu_command.c into said validation functions.
The solution is simple: do the validation in all cases. And while
it may happen that users would be unable to migrate/restore a
guest due to a bug in our validator, spawning QEMU without
validation is worse (especially when you consider that users can
supply their own XMLs for migrate/restore operations - these were
never validated).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2048435 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:17 +0000 (17:28 +0100)]
qemu: gpu: Get pid without binary validation
The binary validation in virPidFileReadPathIfAlive may fail with EACCES
if the calling process does not have CAP_SYS_PTRACE capability.
Therefore instead do only the check that the pidfile is locked by the
correct process.
Fixes the same issue as with swtpm.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:16 +0000 (17:28 +0100)]
qemu: tpm: Get swtpm pid without binary validation
Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
in containers) and any attempt to stat(2) or readlink(2) the file will
result in 'permission denied' error if the calling process does not have
CAP_SYS_PTRACE capability. According to proc(5) manpage:
Permission to dereference or read (readlink(2)) this symbolic link is
governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
ptrace(2).
The binary validation in virPidFileReadPathIfAlive may fail with EACCES.
Therefore instead do only the check that the pidfile is locked by the
correct process. To ensure this is always the case the daemonization and
pidfile handling of the swtpm command is now controlled by libvirt.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Vasiliy Ulyanov [Wed, 2 Feb 2022 16:28:15 +0000 (17:28 +0100)]
virpidfile: Add virPidFileReadPathIfLocked func
The function will attempt to read a pid from @path, and store it in
@pid. The @pid will only be set, however, if @path is locked by
virFileLock() at byte 0 and the pid in @path is running.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
conf: Fix documentation for virDomainDefGetShortName()
The @unique argument didn't exist even when the function was
introduced in a042275a396e, and the @vm argument was not renamed
when the function was changed to take a virDomainDef* instead of
a virDomainObj* in 7ed6934f3b92.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
SmartNIC DPUs may not expose some privileged eswitch operations
to the hypervisor hosts. For example, this happens with Bluefield
devices running in the ECPF (default) mode for security reasons. While
VF MAC address programming is possible via an RTM_SETLINK operation,
trying to set a VLAN ID in the same operation will fail with EPERM.
The equivalent ip link commands below provide an illustration:
1. This works:
sudo ip link set enp130s0f0 vf 2 mac de:ad:be:ef:ca:fe
2. Setting (or clearing) a VLAN fails with EPERM:
sudo ip link set enp130s0f0 vf 2 vlan 0
RTNETLINK answers: Operation not permitted
3. This is what Libvirt attempts to do today (when trying to clear a
VF VLAN at the same time as programming a VF MAC).
sudo ip link set enp130s0f0 vf 2 vlan 0 mac de:ad:be:ef:ca:fe
RTNETLINK answers: Operation not permitted
If setting an explicit VLAN ID results in an EPERM, clearing a VLAN
(setting a VLAN ID to 0) can be handled gracefully by ignoring the
EPERM error with the rationale being that if we cannot set this state
in the first place, we cannot clear it either.
In order to keep explicit clearing of VLAN ID working as it used to
be passing a NULL pointer for VLAN ID is used.
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There should be a way to show no intent in programming a VLAN at all
(including clearing it). This allows handling error conditions
differently when VLAN clearing is explicit (vlan id == 0) vs implicit
(vlanid == NULL - try to clear it if possible).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Set VF MAC and VLAN ID in two different operations
This has a benefit of being able to handle error codes for those
operations separately which is useful when drivers allow setting a MAC
address but do not allow setting a VLAN (which is the case with some
SmartNIC DPUs).
Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Most people will want to use isa-debugcon to obtain debug output
for SeaBIOS / EDK II, so let's include a ready-made example for
that scenario in our documentation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The current implementation of the workaround for yajl's broken
pkg-config file accidentally overwrites the value of includedir
that is later used by the installation process. Rename the
local variable to avoid this issue.
Fixes: c97075e1e46e9305d62620d8b05046aae0139438 Closes: https://gitlab.com/libvirt/libvirt/-/issues/271 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
which is used as a way to receive debug messages from the
firmware on x86 platforms.
Note that the default port is hypervisor specific, with QEMU
currently using 0xe9 since that's the original Bochs debug port.
For use with SeaBIOS/OVMF, the iobase port needs to be explicitly
set to 0x402.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virnodedeviceobj: Don't unlock virNodeDeviceObj in virNodeDeviceObjListRemove()
When virNodeDeviceObjListRemove() is called, the passed
virNodeDeviceObj is removed from internal list of node devices
and then unrefed and unlocked. While the former is warranted (the
object was refed at the beginning of the function) the unlock is
not. In fact, it's wrong from conceptual POV. We still want
threads working on the object tu mutually exclude each other.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Andrea Bolognani [Mon, 17 Jan 2022 15:07:38 +0000 (16:07 +0100)]
qemu: Drop qemuAudioDriver enumeration
This mostly overlaps with virDomainAudioType, but in a couple of
cases the string representations are different.
Right now we're doing that in a somewhat sketchy way, in that we
store values of one enumeration and then convert them to strings
using TypeToString() implementation for the other enumeration;
when converting from string, we open-code the handling of the
special values mentioned above.
Drop the second enumeration and introduce two helpers to deal
with conversion. Most calling sites don't need to be changed, and
one can even be simplified significantly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We recently started listing these in the spec file and, since we
were not creating them during the installation phase, that broke
RPM builds.
Fixes: 4b43da0bff9b78dcf1189388d4c89e524238b41d Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Thu, 27 Jan 2022 10:13:53 +0000 (11:13 +0100)]
qemu_command: Generate memory only after controllers
Currently, memory device (def->mems) part of cmd line is
generated before any controller. In majority of cases it doesn't
matter because neither of memory devices live on a bus that's
created by an exposed controller (e.g. there's no DIMM
controller, at least not exposed). Except for virtio-mem and
virtio-pmem, which do have a PCI address. And if it so happens
that the device goes onto non-default bus (pci.0) starting such
guest fails, because the controller that creates the desired bus
wasn't processed yet. QEMU processes arguments in order.
For instance, if virtio-mem has address with bus='0x01' QEMU
refuses to start with the following message:
Bus 'pci.1' not found
Similarly for virtio-pmem. I've successfully tested migration and
changing the order does not affect migration stream.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2047271 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Mon, 24 Jan 2022 18:05:44 +0000 (19:05 +0100)]
rpm: List more directories
This has two advantages: it makes it possible for the admin to
ask rpm what package they belong to, and results in them ending
up with stricter permissions than they would have if we let
libvirt create them at runtime.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Mon, 24 Jan 2022 15:54:01 +0000 (16:54 +0100)]
rpm: Move /etc/libvirt from -daemon to -libs
Files like libvirt.conf influence the behavior of the library
itself. The daemon depends on the library, so the directory is
guaranteed to be present both on the client side and on the
server side.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 28 Jan 2022 17:06:32 +0000 (18:06 +0100)]
src: Use g_steal_pointer() more
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:
@ rule1 @
expression a, b;
@@
<...
- b = a;
... when != b
- a = NULL;
+ b = g_steal_pointer(&a);
...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
projects / libvirt.git / log