An incorrect check for domainRegister caused the DNS server for a
virtual domain to be registered with systemd-resolved even if
register='no' attribute was present. Only omitting the attribute
completely would disable the registration.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The nodedev code unhelpfully reports
couldn't convert node device def to mdevctl JSON
which hides the actual error message
No JSON parser implementation is available
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virdrivermoduletest will attempt to dlopen() each driver module,
so they must be build before the test can run.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We fail to express an ordering between the custom target that
generates the combined augeas test input file, and the meson
test command.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In case when the interface is being detached/reattached it may happen
that udev will return NULL from 'udev_device_get_sysname()'.
As the RPC code requires nonnull strings in the return array it fails to
serialize such reply:
libvirt: XML-RPC error : Unable to encode message payload
Fix this by simply ignoring such interfaces as there's nothing we can
report in such case.
A similar fix was done to 'udevConnectListAllInterfaces' in commit
2ca94317ac.
Resolves: https://issues.redhat.com/browse/RHEL-34615
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make the array-filling operation of udevListInterfacesByStatus optional
and replace the completely redundant udevNumOfInterfacesByStatus by it.
Further patches fixing the listing will not need to be duplicated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some public objects (like virDomain, virInterface, and so on) are
missing g_autoptr() cleanup functions. Provide missing
declarations. Note, this is only for our internal use - hence
datatypes.h.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When starting a domain and there's no vCPU/emulator pinning set,
we query the list of all online physical CPUs and set affinity of
the child process (which eventually becomes QEMU) to that list.
We can't assume libvirtd itself had affinity to all online CPUs
and since affinity of the child process is inherited, we should
fix it afterwards. But that's not necessarily correct. Users
might isolate some physical CPUs and we should avoid touching
them unless explicitly told so (i.e. vCPU/emulator pinning told
us so).
Therefore, when attempting to set affinity to all online CPUs
subtract the isolated ones.
Before this commit:
root@localhost:~# cat /sys/devices/system/cpu/isolated
19,21,23
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 14835's current affinity list: 0-23
After:
root@virtlab414:~# taskset -cp $(pgrep qemu)
pid 17153's current affinity list: 0-18,20,22
Resolves: https://issues.redhat.com/browse/RHEL-33082
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This is a helper that parses /sys/devices/system/cpu/isolated
into a virBitmap. It's going to be needed soon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virFileReadValueBitmap() fails in the latter case, unfortunately.
Introduce a slightly modified version that accepts empty files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virBitmapParseUnlimited() fails in the latter case,
unfortunately. Introduce a slightly modified version that accepts
empty files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Extend the list of supported formats, update and clarify comment
in qemu.conf.in (removed misleading sentence about the order of
compression format types).
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/589
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Return value of a function 'virDomainChrDefNew' is dereferenced
at hyperv_driver.c without checking for NULL, which can lead to
NULL dereference immediately after.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Oleg Sviridov <oleg.sviridov@red-soft.ru>
Reviewed-by: Kristína Hanicová <khanicov@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Similar to commit 57d084febe, another case of the libxl driver not
adapting to modular daemons. When converting configuration that
contains a type='network' interface, the converter calls
virNetworkLookupByName, passing the hypervisor connection object
instead of a connection to virtnetworkd. E.g.
> cat dom.xml
...
<interface type='network'>
<source network='default'/>
</interface>
...
> virsh net-info default
Name: default
UUID: 25a5b089-1e71-4956-99aa-df2213bbb407
Active: yes
Persistent: no
Autostart: no
Bridge: virbr0
> virsh domxml-to-native xen-xl dom.xml
error: Network not found: default
Acquire a connection to virtnetworkd and use it when calling
virNetwork* APIs.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The mpx feature was removed from the corresponding qemu cpu models.
With mpx in the libvirt cpu models, libvirt believes the feature
to be implicitly enabled when creating qemu VMs, while in fact it is
disabled.
This became an issue when commit 94eacd5a5f introduced new vmx-*
features, of which some are dependent on mpx (see "feature_dependencies"
table in qemu target/i386/cpu.c), e.g. vmx-exit-clear-bndcfgs and
vmx-entry-load-bndcfgs. These features cannot be enabled by qemu
without also mpx being enabled, leading to the error message
error: Failed to create domain from testdomain.xml
error: operation failed: guest CPU doesn't match
specification: missing features: mpx,vmx-exit-clear-bndcfgs,
vmx-entry-load-bndcfgs
when trying to create a VM with a "host-model" cpu on a host that
does support mpx and the mentioned vmx-* features:
<domain>
...
<cpu mode='host-model' check='full' />
...
</domain>
Resolve the issue by removing mpx from libvirt's cpu models as well.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Features removed from a CPU model are marked with "removed='yes'"
attribute in the CPU map. Such features will always be present in a CPU
definition produced by libvirt regardless on their state. In other words
a running domain (even saved in a file) will always explicitly contain
states of all features removed from the specified CPU model. This
enables migration to older libvirt which would otherwise think the
affected features should be enabled as they are still included in the
CPU model in the older version of CPU map. Migration from an old libvirt
to a new one would be broken as the new libvirt would think the removed
features should be disabled (because they are not included in the CPU
model anymore), which might not be the case on the source host. Thus we
were refusing to remove CPU features unless they were never working and
no domain could even be running with those features enabled.
This patch removes the limitation. When handling CPU definitions with
missing features marked as removed in the specified CPU model, we know
whether it comes from a running domain, in which case it must have been
created by older libvirt where the missing CPU features were not removed
yet. This means the features must have been enabled on the source and we
can automatically fix the definition by adding the missing features with
correct states.
We can safely remove any CPU feature from our CPU models now, but it
should only be used for features removed from all versions of a given
CPU model in QEMU because unversioned models correspond to v1.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virCPUUpdate check the CPU definition for features that were marked as
removed in the specified CPU model and explicitly adds those that were
not mentioned in the definition. So far such features were added with
VIR_CPU_FEATURE_DISABLE policy, but the caller may want to use a
different policy in some situations, which is now possible via the
removedPolicy parameter.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCPUDefAddFeatureInternal helper function only fails if it is
called with VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE, which is only used in
virCPUDefAddFeature. The other callers (virCPUDefUpdateFeature and
virCPUDefAddFeatureIfMissing) will never get anything but 0 from
virCPUDefAddFeatureInternal and their return type can be changed to
void.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When using vSPC (Virtual Serial Port Concentrator) in vSphere the actual
address for it is saved in serialX.vspc in which case the
serialX.fileName is most probably something we can't get any useful
information from and we also fail during the parsing rendering any
dumpxml and similar tries unsuccessful.
Instead of parsing the vspc URL with something along the lines of
`virURIParse(vspc ? vspc : fileName)`, which could lead to us reporting
information that is very prune to misuse (the vSPC seemingly has a
protocol on top of the telnet connection; redefining the domain would
change the behaviour; the URL might have a fragment we are not saving;
etc.) or adding more XML knobs to indicate vSPC usage (which we would
not be able to configure; we'd have to properly error out everywhere;
etc.) let's just report dummy serial port that leads to nowhere (i.e.
type="null").
Resolves: https://issues.redhat.com/browse/RHEL-32182
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Users are seeing periodic segfaults from libvirt client apps,
especially thread heavy ones like virt-manager. A typical
stack trace would end up in the virNetClientIOEventFD method,
with illegal access to stale stack data. eg
==238721==ERROR: AddressSanitizer: stack-use-after-return on address 0x75cd18709788 at pc 0x75cd3111f907 bp 0x75cd181ff550 sp 0x75cd181ff548
WRITE of size 4 at 0x75cd18709788 thread T11
#0 0x75cd3111f906 in virNetClientIOEventFD /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1634:15
#1 0x75cd3210d198 (/usr/lib/libglib-2.0.so.0+0x5a198) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#2 0x75cd3216c3be (/usr/lib/libglib-2.0.so.0+0xb93be) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#3 0x75cd3210ddc6 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x5adc6) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#4 0x75cd3111a47c in virNetClientIOEventLoop /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1722:9
#5 0x75cd3111a47c in virNetClientIO /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2002:10
#6 0x75cd3111a47c in virNetClientSendInternal /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2170:11
#7 0x75cd311198a8 in virNetClientSendWithReply /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2198:11
#8 0x75cd31111653 in virNetClientProgramCall /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclientprogram.c:318:9
#9 0x75cd31241c8f in callFull /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6054:10
#10 0x75cd31241c8f in call /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6076:12
#11 0x75cd31241c8f in remoteNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/src/remote/remote_client_bodies.h:5959:9
#12 0x75cd31410ff7 in virNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/libvirt-network.c:952:15
The root cause is a bad assumption in the virNetClientIOEventLoop
method. This method is run by whichever thread currently owns the
buck, and is responsible for handling I/O. Inside a for(;;) loop,
this method creates a temporary GSource, adds it to the event loop
and runs g_main_loop_run(). When I/O is ready, the GSource callback
(virNetClientIOEventFD) will fire and call g_main_loop_quit(), and
return G_SOURCE_REMOVE which results in the temporary GSource being
destroyed. A g_autoptr() will then remove the last reference.
What was overlooked, is that a second thread can come along and
while it can't enter virNetClientIOEventLoop, it will register an
idle source that uses virNetClientIOWakeup to interrupt the
original thread's 'g_main_loop_run' call. When this happens the
virNetClientIOEventFD callback never runs, and so the temporary
GSource is not destroyed. The g_autoptr() will remove a reference,
but by virtue of still being attached to the event context, there
is an extra reference held causing GSource to be leaked. The
next time 'g_main_loop_run' is called, the original GSource will
trigger its callback, and access data that was allocated on the
stack by the previous thread, and likely SEGV.
To solve this, the thread calling 'g_main_loop_run' must call
g_source_destroy, immediately upon return, to guarantee that
the temporary GSource is removed.
CVE-2024-4418
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Martin Shirokov <shirokovmartin@gmail.com>
Tested-by: Martin Shirokov <shirokovmartin@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Allow generation of command line for virtio-sound-pci and virtio-sound-device
devices along with additional virtio options.
A new testcase is added to test virtio-sound-pci. The
arm-vexpressa9-virtio testcase is also extended to test virtio-sound-device.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch adds parsing of the virtio sound model, along with parsing
of virtio options and PCI/virtio-mmio address assignment.
A new 'streams' attribute is added for configuring number of PCM streams
(default is 2) in virtio sound devices. QEMU additionally has jacks and chmaps
parameters but these are currently stubbed, hence they are excluded in this
patch series.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability can be used to detect if the qemu binary already
supports 'ras' feature for 'virt' machine type.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The "quantum" attribute of HTB is documented as:
Number of bytes to serve from this class before the scheduler
moves to the next class.
Since v1.3.2-rc1~225 we compute what we think is the appropriate
value and pass it on the TC command line. But kernel and
subsequently TC use uint32_t to store this value. If we compute
value outside of this type then TC fails and prints usage which
we then interpret as an error message. Needlessly long error
message. While there's not much we can do about the latter, we
can put a cap on the value and stop tickling this behavior of TC.
Fixes: 065054daa7
Resolves: https://issues.redhat.com/browse/RHEL-34112
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit v10.0.0-265-ge67bca23e4 added a `active_config` and
`defined_config` to nodedev mdev internal XML handling.
`defined_config` can be filled at XML parse time, but `active_config`
must be filled in by nodedev driver. This wasn't implemented for the
test driver however, which caused virt-manager test suite regressions.
Working example:
```
$ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
<device>
<name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
<path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
<parent>css_0_0_0023</parent>
<capability type='mdev'>
<type id='vfio_ccw-io'/>
<iommuGroup number='0'/>
</capability>
</device>
```
Broken example:
```
$ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
<device>
<name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
<path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
<parent>css_0_0_0023</parent>
<capability type='mdev'>
<iommuGroup number='0'/>
</capability>
</device>
```
There's already code that does what we want in the test suite.
Move it to a shared function, and call it in test driver when
creating a nodedev from driver startup XML.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
- Error if INACTIVE requested for transient object
- Force dumping INACTIVE XML when object is inactive
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
This was the implied default before nodedevs gained a notion of
being inactive and transient. It also matches the implied default
when parsing other object types
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Rework 'virDomainUSBDeviceDefForeach' to use virDomainDeviceInfoIterate
instead of open-coding all iterators. To achieve this
'virDomainDeviceIsUSB' needs to be fixed as it didn't properly handle
'sound', 'fs', 'chr', 'ccid', and 'net' usb devices.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
While QEMU accepts and interprets an empty string in the tls-hostname
field in migration parametes as if it's unset, the same does not apply
for the 'tls-hostname' field when 'blockdev-add'-ing a NBD backend for
non-shared storage migration.
When libvirt sets up migation with TLS in 'qemuMigrationParamsEnableTLS'
the QEMU_MIGRATION_PARAM_TLS_HOSTNAME migration parameter will be set to
empty string in case when the 'hostname' argument is passed as NULL.
Later on when setting up the NBD connections for non-shared storage
migration 'qemuMigrationParamsGetTLSHostname', which fetches the value
of the aforementioned TLS parameter.
This bug was mostly latent until recently as libvirt used
MIGRATION_DEST_CONNECT_HOST mode in most cases which required the
hostname to be passed, thus the parameter was set properly.
This changed with 8d693d79c4 for post-copy migration, where libvirt now
instructs qemu to connect and thus passes NULL hostname to
qemuMigrationParamsEnableTLS, which in turn causes libvirt to try to
add NBD connection with empty string as tls-hostname resulting in:
error: internal error: unable to execute QEMU command 'blockdev-add': Certificate does not match the hostname
To address this modify 'qemuMigrationParamsGetTLSHostname' to undo the
weird semantics the migration code uses to handle TLS hostname and make
it return NULL if the hostname is an empty string.
Fixes: e8fa09d66b
Resolves: https://issues.redhat.com/browse/RHEL-32880
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Since OVS keeps desired state in a DB, upon sudden crash of the
host we may leave a port behind. There's no problem on VM
shutdown or NIC hotunplug as we call corresponding del-port
function (virNetDevOpenvswitchRemovePort()). But if the host
suddenly crashes we won't ever do that. What happens next, is
when OVS starts it finds desired state in its DB and creates a
stale port.
OVS added support for transient ports in v2.5.0 (Feb 2016) and
since its v2.9.0 it even installs a systemd service
(ovs-delete-transient-ports) that automatically deletes transient
ports on system startup. If we mark a port as transient then OVS
won't restore its state on restart after crash.
This change may render "--may-exist" argument redundant, but I'm
not sure about all the implications if it was removed. Let's keep
it for now.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/615
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The CCW variant of the 'vhost-user-fs' device in qemu doesn't
deliberately support the 'bootindex' attribute as the machine is unable
to boot from such device.
Reject '<boot order' on non-PCI virtiofs, add tests validating that it's
rejected as well as that virtiofs on PCI-based hosts but without address
specified will be accepted.
Resolves: https://issues.redhat.com/browse/RHEL-22728
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Pretty straightforward. Just put mem-reserve attribute whenever
it's set. Previous commit ensures it's set only for valid
controller models.
Resolves: https://issues.redhat.com/browse/RHEL-7461
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Only two controller models allow setting mem-reserve:
pcie-root-port and pci-bridge. Reflect this fact during
validation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
There are PCI devices with pretty large non-prefetchable memory,
for instance:
Memory at 9d800000 (64-bit, non-prefetchable) [size=8M]
Memory at a6800000 (64-bit, non-prefetchable) [size=16K]
For cold plugged devices this is not a problem, because firmware
sets PCI controllers in a way that make devices behind them just
work. Problem arises if such PCI device is to be hot plugged.
Since the PCI device wasn't present at cold boot, firmware could
not take it into calculations and the amount of reserved memory
is not sufficient.
Introduce a know that allows users overriding value computed by
FW and thus allow hot plug of such PCI devices.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ages ago origCPU in domain private data was introduced to provide
backward compatibility when migrating to an old libvirt, which did not
support fetching updated CPU definition from QEMU. Thus origCPU will
contain the original CPU definition before such update. But only if the
update actually changed anything. Let's always fill origCPU with the
original definition when starting a domain so that we can rely on it
being always set, even if it matches the updated definition.
This fixes migration or save operations with custom domain XML after
commit v10.1.0-88-g14d3517410, which expected origCPU to be always set
to the CPU definition from inactive XML to check features explicitly
requested by a user.
https://issues.redhat.com/browse/RHEL-30622
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
$ virsh --connect test:///default nodedev-list
error: Failed to list node devices
error: unsupported flags (0x80000000) in function testConnectListAllNodeDevices
The test driver handles the nodedev state flags, we just need to
allow them
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
The typed parameter array length must be non-NULL and either 0, or a
positive number.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The only thing we need to free in the cleanup code is virCPUDef and for
that we already have g_autoptr handler.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In one of my recent commits I've chopped just too much and moved
a variable declaration into a function not realizing it's still
used on FreeBSD. Bring it back but only for the FreeBSD case.
Fixes: f8b5bd855f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
clang on Fedora started to complain about some calls to g_new0()
we're making in vbox_snapshot_conf.c. Specifically, we're passing
zero as number of elements to allocate. And while usually SA
tools are not clever, in this specific case clang is right.
There are three cases where such call is made, but all of them
later use VIR_EXPAND_N() to allocate more memory (if needed). But
VIR_EXPAND_N() accepts a variable set to NULL happily.
Therefore, just drop those three calls to g_new0(..., 0) and let
VIR_EXPAND_N() allocate memory.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In QEMU and LXC drivers in a few places only
virNetDevBandwidthClear() is called. This means that if an
interface is of openvswitch vport profile, its QoS is not
removed. And to make matters worse - OVS is designed to remember
state even when corresponding interface is gone. This leads to
stale QoS settings piling up in OVS database.
To resolve this, introduce virDomainInterfaceClearQoS() which
looks at given interface and calls corresponding QoS clear
function. Then, basically replace virNetDevBandwidthClear() calls
in those hypervisor drivers with this new function.
Resolves: https://issues.redhat.com/browse/RHEL-30373
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The reason virDomainClearNetBandwidth() exists in src/conf/ is
that at the time its introduction we did not have a better place.
But now we do. Firstly, virDomainClearNetBandwidth() is
hypervisor agnostic code, but really has nothing to do with
domain configuration (it doesn't parse/format XML). Secondly, in
near future it'll call another function from src/hypervisor/ and
that's not really allowed from src/conf/.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The @brname argument of virNetDevOpenvswitchRemovePort() is and
was unused ever since its introduction in v0.9.11-rc1~257. Just
remove it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Both LXC and QEMU drivers have the same code to remove vport when
removing a domain's interface. Instead of repeating the same
pattern in both drivers, move the code into hypervisor agnostic
location (src/hypervisor/) and switch to calling this new
function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The comment to virNetDevOpenvswitchInterfaceGetMaster() contains
wrong function name. Fix this.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When performing an install, it's common for tooling such as virt-install
to remove the install kernel/initrd once they are successfully booted and
the domain has been redefined to boot without them. After the installation
is complete and the domain is rebooted/shutdown, the DAC and selinux
security drivers attempt to restore labels on the now deleted files. It's
harmles wrt functionality, but results in error messages such as
Mar 08 12:40:37 virtqemud[5639]: internal error: child reported (status=125): unable to stat: /var/lib/libvirt/boot/vir>
Mar 08 12:40:37 virtqemud[5639]: unable to stat: /var/lib/libvirt/boot/virtinst-yvp19moo-linux: No such file or directo>
Mar 08 12:40:37 virtqemud[5639]: Unable to run security manager transaction
Add a check for file existence to the virSecurity*RestoreFileLabel functions,
and avoid relabeling if the file is no longer available. Skipping the restore
caused failures in qemusecuritytest, which mocks stat, chown, etc as part of
ensuring the security drivers properly restore labels. virFileExists is now
mocked in qemusecuritymock.c to return true when passed a file previously
seen by the mocked stat, chown, etc functions.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make sure the old value in `scsi_target->wwpn` is free'd before replacing it.
While at it, simplify the code.
==9104== 38 bytes in 2 blocks are definitely lost in loss record 1,943 of 3,250
==9104== at 0x483B8C0: malloc (vg_replace_malloc.c:442)
==9104== by 0x4DFB69B: g_malloc (gmem.c:130)
==9104== by 0x4E1921D: g_strdup (gstrfuncs.c:363)
==9104== by 0x495D60B: g_strdup_inline (gstrfuncs.h:321)
==9104== by 0x495D60B: virFCReadRportValue (virfcp.c:62)
==9104== by 0x4A5F5CB: virNodeDeviceGetSCSITargetCaps (node_device_conf.c:2914)
==9104== by 0xBF62529: udevProcessSCSITarget (node_device_udev.c:657)
==9104== by 0xBF62529: udevGetDeviceDetails (node_device_udev.c:1406)
==9104== by 0xBF62529: udevAddOneDevice (node_device_udev.c:1563)
==9104== by 0xBF639B5: udevProcessDeviceListEntry (node_device_udev.c:1637)
==9104== by 0xBF639B5: udevEnumerateDevices (node_device_udev.c:1691)
==9104== by 0xBF639B5: nodeStateInitializeEnumerate (node_device_udev.c:2009)
==9104== by 0x49BDBFD: virThreadHelper (virthread.c:256)
==9104== by 0x5242069: start_thread (in /usr/lib64/libc.so.6)
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch will allow usb-net devices to be automatically assigned a USB
address (and skip any attempt to assign a PCI one).
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit 2ecdf25929 was intended to
implement two things: reduce stack usage inside ACL helpers and
minimally initialize virDomainDef object to avoid passing garbage
inside validation framework. Though original commit has not
touched other ACL helpers.
This patch adds proper clauses to
remoteRelayNetworkEventCheckACL
remoteRelayStoragePoolEventCheckACL
remoteRelayNodeDeviceEventCheckACL
remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add DMI support for risc-v and mips. Attempt to read dmidecode and
fall back to old behavior if that fails.
The SMBIOS specification[1] officially supports both RISC-V and LoongArch.
Some mips-based Loongson-3 processors also have SMBIOS.
[1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.0.pdf
Signed-off-by: Brett Holman <brett.holman@canonical.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Implement display="on" and ramfb="on" for vfio PCI host devices in qemu.
This enables passthrough PCI devices for display just like we did for
mdevs.
Resolves: https://issues.redhat.com/browse/RHEL-28808
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We already allow the user to specify display="on" and ramfb="on" for
mdev host devices. But newer GPU models will no longer use the mdev
framework, so we should enable this same functionality for other
non-mdev passthrough PCI devices.
Resolves: https://issues.redhat.com/browse/RHEL-28808
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The test simply invokes libvirtd and expects it to fail. We can do that
directly in meson without the need for a wrapper script.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
XML metadata for snapshot contains only single list of disk overlays
from the moment when the snapshot was taken. When user creates multiple
branches of snapshots the parent snapshot will still list only the
original disk overlays. This may cause an issue in a specific scenario:
s1
|
+- s2
+- s3 (active)
For this snapshot topology when we delete s2 metadata for s1 are not
updated. Now when we delete s1 the code operated with incorrect
overlays from s1 metadata in order to update s3 metadata resulting in no
changes to s3 metadata.
Now when user tries to delete s3 it fails with following error:
error: Failed to delete snapshot s3
error: operation failed: snapshot VM disk source and parent disk source are not the same
For the actual deletion there is a code to figure out the correct disk
source but it was not used to update metadata as well. Due to reasons
how block commit in libvirt works we need to create a copy of that disk
source in order to have it available when updating metadata as the
original source will be freed at that point.
Resolves: https://issues.redhat.com/browse/RHEL-26276
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Calling this function when deleting internal snapshot isn't required
because with internal snapshots all changes are done within the file
itself so there is no file deletion and no need to update snapshot
metadata.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The PCI VPD (Vital Product Data) may be missing or the kernel can report
presence but not actually have the data. Also the data is specified by
the device vendor and thus may be invalid in some cases.
To avoid log spamming, since the only usage in the node device driver is
ignoring errors, remove all error reporting.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/607
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The VPD parsing is fragile and depends on hardware vendor's adherance to
standards. Since libvirt only ever uses this data to report it in the
nodedev XML which ignores any errors there's no much point in having
error reporting which I've added recently.
Turn the errors into VIR_DEBUG statements in preparation for upcoming
patch which completely removes the expectation to report errors.
This effectively reverts commits dfc85658bd and f85a382a0e.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If the original code detected a missing or null boot index in the
new XML, it automatically added the current value. This
autocompletion was incorrect because it was impossible to
distinguish between user intent and user error - changing the
boot order itself is forbidden and should always be an error.
Resolves: https://issues.redhat.com/browse/RHEL-23416
Fixes: aa3e07caec
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Change the log level for pauses of guests due to watchdog timeouts
or io errors from debug to warn to enhance the visibility of such
events.
Signed-off-by: Lennart Fricke <lennart.fricke@drehpunkt.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implement method for loongarch to get host info, such as
cpu frequency, system info, etc.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Implement support for loongarch64 in the QEMU driver.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Add loongarch cpu support, Define new cpu type 'loongarch64'
and implement it's driver functions.
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
While the C API entry points will validate non-negative lengths
for various parameters, the RPC server de-serialization code
will need to allocate memory for arrays before entering the C
API. These allocations will thus happen before the non-negative
length check is performed.
Passing a negative length to the g_new0 function will usually
result in a crash due to the negative length being treated as
a huge positive number.
This was found and diagnosed by ALT Linux Team with AFLplusplus.
CVE-2024-2494
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Found-by: Alexandr Shashkin <dutyrok@altlinux.org>
Co-developed-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Current entries should always be listed before obsolete ones.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
The single caller for each function passes the same value
for @src and @parent, which means that we don't really need
the additional API.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Each one only has a single, trivial caller.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
It was clearly copied over from the SELinux driver without
updating its name in the process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
ch_driver expects path to be of a dir for save/restore. So, update
the documentation at global API as well.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Save & Restore are supported without any network and hostdev config
defined. So, add a validation for it before performing save.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are chances that libvirt process is killed and it resulting in
stale managed save dirs. So check for it, and cleanup it there's any.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Following callbacks have been implemented
* domainRestore
* domainRestoreFlags
The path parameter to these callbacks has to be of the directory where
libvirt has performed save. Additionally, call restore in `domainCreate`
if the domain has managedsave.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Create libvirt managed saveDir and pass it to CH to save the VM
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implemented save callbacks. CH's vmm.snapshot API is called to save the
domain state. The path passed to these callbacks has to be of directory
as CH takes dir as input to snapshot and saves multiple files under it.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pass virCHDriverConfig to VirCHMonitorNew instead of just stateDir so
that the cfg can be used for any additional purposes.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implement folowing API calls from CH monitor
* vmm.snapshot -> to save a domain
* vmm.restore -> to restore saved domain
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virProcessSetScheduler() uses not just sched_setscheduler() but
also sched_get_priority_{min,max}(). Currently we assume that
the former being available implies that the latter are as well,
but that's not the case for at least GNU/Hurd.
Make sure all functions are actually available before
attempting to use them.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit c07cf0a686 replaced this check with one for the
presence of cpu_set_t.
The idea at the time was that only sched_{get,set}affinity()
were visible by default, while making cpu_set_t visible required
defining _WITH_CPU_SET_T. So libvirt would detect the function
and attempt to use it, but the code would not compile because
the necessary data type had not been made accessible.
The commit in question brought three FreeBSD commits as evidence
of this. While [1] and [2] do indeed seem to support this
explanation, [3] from just a few days later made it so that not
just cpu_set_t, but also the functions, required user action to
be visible. This arguably would have made the change unnecessary.
However, [4] from roughly a month later changed things once
again: it completely removed _WITH_CPU_SET_T, making both the
functions and the data type visible by default.
This is the status quo that seems to have persisted until
today. If one were to check any recent FreeBSD build job
performed as part of our CI pipeline, for example [5] and [6]
for FreeBSD 13 and 14 respectively, they would be able to
confirm that in both cases cpu_set_t is detected as available.
Since there is no longer a difference between the availability
of the functions and that of the data type, go back to what we
had before.
This has the interesting side-effect of fixing a bug
introduced by the commit in question.
When detection was changed from the function to the data type,
most uses of WITH_SCHED_GETAFFINITY were replaced with uses of
WITH_DECL_CPU_SET_T, but not all of them: specifically, those
that decided whether qemuProcessInitCpuAffinity() would be
actually implemented or replaced with a no-op stub were not
updated, which means that we've been running the stub version
everywhere except on FreeBSD ever since.
The code has been copied to the Cloud Hypervisor driver in
the meantime, which is similarly affected. Now that we're
building the actual implementation, we need to add virnuma.h
to the includes.
As a nice bonus this also makes things work correctly on
GNU/Hurd, where cpu_set_t is available but
sched_{get,set}affinity() are non-working stubs.
[1] https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2
[2] https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb
[3] https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a
[4] https://cgit.freebsd.org/src/commit/?id=5e04571cf3cf4024be926976a6abf19626df30be
[5] https://gitlab.com/libvirt/libvirt/-/jobs/6266401204
[6] https://gitlab.com/libvirt/libvirt/-/jobs/6266401205
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
FreeBSD 14 implements sched_{get,set}affinity() for
compatibility with Linux, but we should still use the native
syscalls instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Depending on the situation, the IDs that we pass to these
functions can be either referring to processes or threads.
Linux doesn't have separate interfaces for one or the other,
but FreeBSD does and we're explicitly telling it that the ID
refers to a process. When it refers to a thread instead, the
call will fail, and the VM will not be able to start.
Luckily, another possible choice is CPU_WHICH_TIDPID, which
makes things behave the same as Linux.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The source tag sets the rootdir property of the device, which is
the directory exposed to the guest via the MTP device. The target
tag sets the desc property. This device supports read-only mode
as well. Like virtiofs, it does not support additional access
modes.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Expose usb-mtp device as another type of <filesystem/>.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This capability reflects presence of -device usb-mtp.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use svirt_t instead of virtd_t, since virtd_t is not available in the
session mode and qemu with svirt_t won't be able to talk to unconfined_t
socket.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On domain startup, qemuSetupCgroupForExtDevices checks
if a cgroup controller is present and skips the setup if not.
Add a similar check to qemuVirtioFSSetupCgroup to prevent
crashing when hotplugging a virtiofs filesystem.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
when the thread pool is dynamically expanded, threads should
not be created from the existing workers; they should be created
from the newly expanded workers
Signed-off-by: Wei Gong <gongwei833x@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Features marked with added='yes' in CPU model definitions have to be
removed before migration, otherwise older libvirt would complain about
unknown CPU features. We only do this for features that were enabled for
a given CPU model even with older libvirt, which just ignored the
features. And only for features we added ourselves when updating CPU
definition during domain startup, that is we do not remove features
which were explicitly mentioned by a user.
That said, this is not the safest thing we could do, but it's
effectively the same thing we did before the affected features were
added: we ignored them completely on both sides of migration.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function returns a list of explicitly mentioned features in the CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The content is arch specific and checking for Icelake-Server CPU model
on non-x86 architectures does not make sense.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This is not a good idea in general, but we can (and have to) do it in
specific cases when a feature has always been part of a CPU model in
hypervisor's definition, but we ignored it and did not include the
feature in our definition.
Blindly adding the features to the CPU map and not adding them to
existing CPU models breaks migration between old and new libvirt in both
directions. New libvirt would complain the features got unexpectedly
enabled (as they were not mentioned in the incoming domain XML) even
though they were also enabled on the source and the old libvirt just
didn't know about them. On the other hand, old libvirt would refuse to
accept incoming migration of a domain started by new libvirt because the
domain XML would contain CPU features unknown to the old libvirt.
This is exactly what happened when several vmx-* features were added a
few releases back. Migration between libvirt releases before and after
the addition is now broken.
This patch adds support for added these features to existing CPU models
by marking them with added='yes'. The features will not be considered
part of the CPU model and will be described explicitly via additional
<feature/> elements, but the compatibility check will not complain if
they are enabled by the hypervisor even though they were not explicitly
mentioned in the CPU definition and incoming migration from old libvirt
will succeed.
To fix outgoing migration to old libvirt, we also need to drop all those
features from domain XML unless they were explicitly requested by the
user. This will be handled by a later patch.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In a few cases (CH driver) we want
virCapabilitiesDomainSupported() just to check whether given
virtType is supported and report a different error message (that
suggests how to solve the problem). Introduce reportError
argument which makes the function report an error iff set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In near future we will want to check whether capabilities for
given virtType exist, but report an error on our own. Introduce
reportError argument which makes the function report an error iff
set.
In one specific case (virQEMUCapsGetDefaultVersion()) we were
even overwriting (more specific) error message reportd by
virCapabilitiesDomainDataLookup(). Drop that too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If the host doesn't have /dev/kvm nor /dev/mshv, i.e. CH driver
is unable to run any guests, then an error is reported. But the
usual thing to do here is print an info message into the logs and
return VIR_DRV_STATE_INIT_SKIPPED. It is a recoverable error
after all.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Change the default to modern qcow2 as it's supported by all qemu
versions supported by libvirt and in fact 'qemu-img' already defaults to
the new format for a long time.
Some Unittests require changes to pass, now that version 1.1 is default.
Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this
patch doesn't affect them.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/602
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Similar to other VIR_ERR_NO_* errors, we don't want to spam the daemon
log with these messages.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'display-reload' QMP command had been introduced from QEMU 6.0.0:
9cc0765165
Currently it only supports reloading TLS certificates for VNC.
Resloves: https://issues.redhat.com/browse/RHEL-16333
Signed-off-by: Zheng Yan <yanzheng759@huawei.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The new virDomainGraphicsReload API is used to make the domain reload
its certificates without restart, and avoid service interruption.
Currently, only QEMU VNC TLS certificates are supported, but
flags are also reserved for subsequent scenarios.
To reload QEMU VNC TLS certificates as an example, we can call:
virDomainGraphicsReload(domain, 0, 0);
Then the specified QMP message would be send to QEMU:
{"execute": "display-reload", "arguments":{"type": "vnc", "tls-certs": true}}
Signed-off-by: Zheng Yan <yanzheng759@huawei.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'display-reload' QMP command was introduced in QEMU 6.0.0, so we
add a compatible capability to check if target QEMU binary supports it.
{"execute":"display-reload", "arguments":{"type": "vnc", "tls-certs": true}}
The new QMP refer to:
9cc0765165
Signed-off-by: Zheng Yan <yanzheng759@huawei.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Cloud-Hypervisor is capable of running VMs with kvm or mshv as the
hypervisor on Linux Host. Guest to hypevisor ABI with mshv hypervisor is
the same as in the case of VIR_DOMAIN_VIRT_HYPERV. So, VIR_DOMAIN_VIRT_HYPERV
type will be reused to represent the config with Linux Host and mshv as the
hypervisor.
While initializing ch driver, check if either of /dev/kvm or /dev/mshv
device is present on the host. Before starting ch domains, check if the
requested hypervisor device is present on the host.
Users can specify hypervisor in ch guests's domain definitions like
below:
<domain type='kvm'>
_or_
<domain type='hyperv'>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With Unix mode, pass a socket path to cloud-hypervisor.
Cloud-Hypervisor will attach guest's serial port to this socket path.
Users can connect to the serial port using one of the following commands:
`socat -,crnl UNIX-CONNECT:<path/to/socket>`
OR
`minicom --device unix#<path/to/socket>`
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Unix Socket backend is only supported for serial port in
cloud-hypervisor. Add relevant checks in chValidateDomainDeviceDef.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently all machine types which do honour '-usb' are already covered
by code which will either select a proper controller model or would
select the same one which '-usb' would use.
Thus all of the legacy -usb controller code can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
- 'virt*' machines already don't allow downgrade
- 'versatilepb' and 'realview' machines use 'pci-ohci' controller with '-usb'
- all other machines ignore '-usb' (some have sysbus-based USB
controller which we don't even consider)
For the 'versatilepb' and 'realview' machines libvirt would already
resort to picking either an existing controller model or trying to pick
the one which '-usb' would select and thus fail either way.
All other machine types ignore it.
We can thus remove the fallback for all arm-based machines.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
- 'pseries' machines already don't allow downgrade
- 'g3beige' and 'mac99' machines use 'pci-ohci' controller with '-usb'
- all other machines ignore '-usb'
For 'g3beige' and 'mac99' libvirt already has 'pci-ohci' as contoller it
would select as one of the options when picking a model, thus it's
impossible to reach situation when '-usb' would be honoured.
All other machine types ignore it.
We can thus remove the fallback for all ppc-based machines.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The default USB device auto-selection code for 'pseries' machines picks
controller models which are also selected when '-usb' is used thus it's
impossible to end up in the case when using '-usb' would be possible:
$ qemu-system-ppc64 --machine pseries,usb=on
qemu-system-ppc64: could not find a module for type 'nec-usb-xhci'
$ qemu-system-ppc64 --machine pseries-2.5,usb=on
qemu-system-ppc64: could not find a module for type 'pci-ohci'
Remove the impossible downgrade and adjust tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
- 'q35' machine type already explicitly forbids fallback
- 'isapc' never supported USB and '-usb' is ignored
- 'i440fx' does support '-usb' and translates it into 'piix3-uhci' which
is identical to what libvirt selects
- we currently don't care about 'microvm'
Attempting to start an 'pc' (i440fx) machine with -usb when 'piix3-uhci'
is compiled out will fail and in any other case libvirt will use the
proper explicitly selected controller.
Drop the '-usb' downgrade for x86 arch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This controller is used as the default/implicit USB controller by
multiple machine types which honour the '-usb' flag of qemu. Add it as
fallback in libvirt too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The machine types historically have a default USB controller populated
via '-usb' which libvirt assumed implicitly. Qemu will use 'pci-ohci'
for both if '-usb' is used.
Unfortunately an USB controller instantiated via '-usb' is unusable as
the bus name libvirt generates doesn't reflect the real name qemu uses,
and thus no libvirt-defined USB devices can be put on the controller.
This patch will populate the default USB controller into the XML and
select it's model to 'pci-ohci' unconditionally as the machine would
fail to start with '-usb' if that controller model is not available.
This patch doesn't try to make any other assumptions about
auto-populated model of USB controllers, which means that for an
explicit USB controller without model a different model will be picked.
Note that this will likely cause ABI differences and break migration for
the two machine types, in the corner case when the default USB
controller would be populated, but given that both are obsolete board
types and USB was unusable it doesn't make sense to keep supporting this
specific case when '-usb' was formatted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Assign VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT rather than -1.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Most machine types are avaliable in all arches by qemu. This is also
true for the 'versatilepb' machine type example in the tests.
Move all the ARM architectures together so that they are handled in
sync.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Ideally check='partial' would check exactly the features QEMU would want
to enable when asked for a specific CPU model (and features). But there
is no way we could ask QEMU how a specific CPU would look like. So we
use our definition from CPU map, which may slightly differ as QEMU adds
or removes features from CPU models, and thus we may end up checking
features which QEMU would not enable while missing some required ones.
We can do better in specific cases, though. If a CPU definition uses
only a model and disabled features (or none at all), we already know
whether QEMU can enable all features required by the CPU model as that's
what we use to set usable='yes' attribute in the list of available CPU
models in domain capbilities XML. So when a usable CPU model is
requested without asking for additional features (disabling features is
fine) we can avoid our possible inaccurate check using our CPU map.
For backward compatibility we only consider usable models. If a
specified model is not usable, we still check it the old way and even
let QEMU start it (and disable some features) in case our definition
lacks some features compared to QEMU.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/608
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Recently a kernel bug caused QEMU to report a CPU feature as enabled
while listing it in the "unavailable-features" list of features that
were requested, but could not be enabled. The feature was actually
enabled, but we marked it as disabled when starting a domain. Later when
the domain is migrated, the destination requests the feature to be
disabled, which breaks the guest ABI or if we are lucky QEMU just fails
to load the migration stream.
Let's make similar bugs more visible in the future by refusing to even
start the domain.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
At the moment, any kind of issue being detected in any of the
firmware descriptor files will result in the entire process
being aborted.
In particular, installing a build of edk2 for an architecture
that libvirt doesn't yet know about, for example loongarch64,
will break most firmware-related functionality: it will no
longer be possible to define new EFI VMs, start existing ones,
or even just obtain the domcapabilities for any architecture.
This is obviously unnecessarily harsh. Adopt a more relaxed
approach and simply ignore the firmware descriptors that we
are unable to parse correctly.
https://bugzilla.redhat.com/show_bug.cgi?id=2258946
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Instead of returning the list of paths exactly as obtained
from qemuFirmwareFetchConfigs(), and allocating the list of
firmwares to be exactly that size right away, start with two
empty lists and add elements to them one by one.
At the moment this only makes things more verbose, but later
we're going to change things so that it's possible that some
of the paths/firmwares are not included in the lists returned
to the caller, and at that point the changes will pay off.
Note that we can't use g_auto() for the new list of paths,
because until the very last moment it's not null-terminated,
so g_strfreev() wouldn't be able to handle it correctly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In a couple of cases, we were reporting an error without
actually terminating the parse process.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Ever since this function was introduced in 2012 it could've tried
filling in an extra interface name. That was made worse in 2019 when
the caller functions started accepting NULL arrays of size 0.
This is assigned CVE-2024-1441.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reported-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Fixes: 5a33366f5c
Fixes: d6064e2759
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Adding 'save' ACL to REMOTE_PROC_NODE_DEVICE_DEFINE_XML to make
REMOTE_PROC_NODE_DEVICE_UPDATE ACLs meaningful.
Fixes: 69f9e7dbc2
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow to modify a node device by using virNodeDeviceDefineXML() to align
its behavior with other drivers define methods.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Implement the API functions in the node device driver by using mdevctl
modify with the options defined and live.
Instead of increasing the minimum mdevctl version to 1.3.0 in the spec
file to ensure support exists in mdevctl the support is dynamically
checked before using mdevctl.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A public API method which allows to update or modify objects is
implemented for almost all other objects that have a concept of
persistent definition and activatability. Currently node devices of type
mdev can be persistent and active. This new method allows to update
defined and active node devices as well.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow to filter node devices based on their persistent or transient
states.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The configuration of a defined mdev can be modified after the mdev is
started. The defined configuration and the active configuration can
therefore run out of sync. Handle this by storing the modifiable data
which is the mdev type and attributes in two separate active and
defined configurations. mdevctl supports with callout scripts to do an
attribute retrieval of started mdevs which is already implemented in
libvirt.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor attribute handling code into methods for easier reuse.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Create a new structure holding type and attributes as these are
modifiable in a persistent mdev configuration and run out of sync with
the active mdev configuration.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similar to when actual data is being written to the stream, it is
necessary to acknowledge handling of the client request when a hole is
encountered. This is done later in daemonStreamHandleWrite by sending a
fake zero-length reply if the status variable is set to
VIR_STREAM_CONTINUE. It seems that setting status from the message
header was missed for holes in the introduction of the sparse stream
feature.
Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As of v9.8.0-rc1~7 we check whether two <memory/> devices don't
overlap (since we allow setting where a <memory/> device should
be mapped to). We do this pretty straightforward, by comparing
start and end address of each <memory/> device combination.
But since only the start address is given (an exposed in the
XML), the end address is computed trivially as:
start + mem->size * 1024
And for majority of memory device types this works. Except for
NVDIMMs. For them the <memory/> device consists of two separate
regions: 1) actual memory device, and 2) label.
Label is where NVDIMM stores some additional information like
namespaces partition and so on. But it's not mapped into the
guest the same way as actual memory device. In fact, mem->size is
a sum of both actual memory device and label sizes. And to make
things a bit worse, both sizes are subject to alignment (either
the alignsize value specified in XML, or system page size if not
specified in XML).
Therefore, to get the size of actual memory device we need to
take mem->size and substract label size rounded up to alignment.
If we don't do this we report there's an overlap between two
NVDIMMs even when in reality there's none.
Fixes: 3fd64fb0e2
Fixes: 91f9a9fb4f
Resolves: https://issues.redhat.com/browse/RHEL-4452?focusedId=23805174#comment-23805174
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
VIR_CLOSE() sets errno on failure so it's better to use
virReportSystemError() than plain virReportError() as the former
reports errno value too.
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The current implementation sets the guest-sync timeout to the
smaller value between the default value (QEMU_AGENT_WAIT_TIME)
and agent->timeout, without considering the timeout passed
via the qga command.
This patch enhances the guest-sync timeout logic to use the
minimum value among the default value, agent->timeout, and
the timeout passed via the qga command.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/590
Signed-off-by: ray <honglei.wang@smartx.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move the assumption from the code pre-creating the storage to
qemuMigrationDstPrepareStorage where it's checked for other cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Migrating into a 'directory' won't ever work as we ask qemu to emulate a
fat filesystem, so restoring of the files won't be possible. Same for
'vhost-user' disks which don't support blockjobs as there's no block
backend used in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Check the existance of storage per-type rather than trying to come up
with a common "path".
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have a switch statement, the code adding the 'slice' for
block devices of non-equal sizes can be moved to appropriate location.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Automatically free helper variables, remove the 'cleanup' label and
use virBufferCurrentContent() to take the XML from the buffer rather
than extracting it to a separate variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow storage migration of VDPA devices by properly checking that they
exist on the destionation. Pre-creation is not supported but if the
device exists the migration should be able to succeed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Decrease the likelyhood that addition of a new storage type will be
forgotten.
This patch also unifies the type check to consult the 'actual' type of
the storage in both cases as the NVMe check looked for the XML declared
type while virStorageSourceIsLocalStorage() looks for the
actual/translated type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Previously, the script would only detect differences between
libvirt's and qemu's list of x86 features, adding those features
to libvirt was a manual and error prone procedure.
Replace with a script that can generate libvirt's feature list
directly from qemu source code.
Usage: sync_qemu_features_i386.py [--output OUTPUT] [qemu]
If not specified otherwise, "output" defaults to x86_features.xml
in the same directory as sync_qemu_features_i386.py. If a checkout
of the qemu source code resides next to the libvirt directory, it
will be found automatically and need not be specified.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use "0x%08x" as format for all values:
sed \
-e "s/'0x\(..\)'/'0x000000\\1'/g" \
-e "s/'0x\(...\)'/'0x00000\\1'/g"
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
commit v9.10.0-129-g8b93d78c83 (first appearing in libvirt-10.0.0) was
supposed to allow forcing a PCI hostdev to be bound to a particular
driver by adding <driver model='blah'/> to the XML for the
device. Unfortunately, a single line was missed during the final
changes to the patch prior to pushing, and the result was that the
driver model could be set to *anything* and it would be accepted but
just ignored.
This patch adds the missing line, which will set the stubDriverName
field of the virPCIDevice object from the hostdev object as the
virPCIDevice is being created. This ends up being used by
virPCIDeviceBindToStub() as the driver that it binds the device to.
Fixes: 8b93d78c83
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This option controls whether the sysctl config for enabling unprivileged
userfaultfd will be installed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
/dev/userfaultfd device is preferred over userfaultfd syscall for
post-copy migrations. Unless qemu driver is configured to disable mount
namespace or to forbid access to /dev/userfaultfd in cgroup_device_acl,
we will copy it to the limited /dev filesystem QEMU will have access to
and label it appropriately. So in the default configuration post-copy
migration will be allowed even without enabling
vm.unprivileged_userfaultfd sysctl.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our virSecret XML is still parsed and formatted using old way
(e.g. virXPathString() + virXXXTypeFromString() combo, or
formatting elements using plain virBufferAsprintf() instead of
virXMLFormatElement()). Modernize the code as it'll make it
easier for future expansion.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert the field and adjust the XML parsers to use
virXMLPropEnum().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virSecretDefParseUsage() function is called conditionally.
Call it unconditionally and keep pointer to the <usage/> node as
it'll come handy soon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When adding vtpm virSecret usage type (in v5.6.0-rc1~61) we
forgot to update polkit access check. This limited user's ability
to match secrets in their rules. Add missing case into switch in
virAccessDriverPolkitCheckSecret().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previously we were only starting or stopping nbdkit when the guest was
started or stopped or when hotplugging/unplugging a disk. But when doing
block operations, the disk backing store sources can also be be added or
removed independently of the disk device. When this happens the nbdkit
backend was not being handled properly. For example, when doing a
blockcopy from a nbdkit-backed disk to a new disk and pivoting to that
new location, the nbdkit process did not get cleaned up properly. Add
some functionality to qemuDomainStorageSourceAccessModify() to handle
this scenario.
Since we're now starting nbdkit from the ChainAccessAllow/Revoke()
functions, we no longer need to explicitly start nbdkit in hotplug code
paths because the hotplug functions already call these allow/revoke
functions and will start/stop nbdkit if necessary.
Add a check to qemuNbdkitProcessStart() to report an error if we
are trying to start nbdkit for a disk source that already has a running
nbdkit process. This shouldn't happen, and if it does it indicates an
error in another part of our code.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When starting nbdkit processes for the backing store of a disk, we were
returning an error if any backing store failed, but we were not cleaning
up processes that succeeded higher in the chain. Make sure that if we
return a failure status from qemuNbdkitStartStorageSource() that we roll
back any processes that had been started.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow us to start or stop nbdkit for just a single disk source
or for every source in the backing chain. This will be used in following
patches.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This patch adds a new attribute "register" to the <domain> element. If
set to "yes", the DNS server created for the virtual network is
registered with systemd-resolved as a name server for the associated
domain. The names known to the dnsmasq process serving DNS and DHCP
requests for the virtual network will then be resolvable from the host
by appending the domain name to them.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When checking for machined we do not really care whether systemd itself
is running, we just need machined to be either running or socket
activated by systemd. That is, exactly the same we do for logind.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanups, qemuMonitorIOWriteWithFD() is but a thin wrapper
over virSocketSendMsgWithFDs(). Replace the body of the former
with a call to the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanups, virSocketSendFD() is but a thin wrapper
over virSocketSendMsgWithFDs(). Replace the body of the former
with a call to the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of using strlen() to calculate length of payload we're
sending, let caller specify the size: they may want to send just
a portion of a buffer (even though the only current user
doesn't).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, virSocketSendMsgWithFDs() reports two errors:
1) if CMSG_FIRSTHDR() fails,
2) if sendmsg() fails.
Well, the latter sets an errno, so caller can just use
virReportSystemError(). And the former - it is very unlikely to
fail because memory for whole control message was allocated just
a few lines above.
The motivation is to unify behavior of virSocketSendMsgWithFDs()
and virSocketSendFD() because the latter is just a subset of the
former (will be addressed later).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'raw' driver without any special configuration is not needed and
creates overhead in qemu.
Stop using the 'raw' format driver in cases when it's not needed. A
special case when it is needed is for FD passed images with only a
single writable FD passed, where we need an overlay driver to properly
reflect the 'read-only' flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Store whether qemu supports the appropriate option for block-stream and
block-commit commands and always use it if available.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability is asserted when both block-stream and block-commit QMP
commands support the 'backing-mask-protocol' argument.
The argument causes qemu to record 'raw' as the backing file format in
case when a protocol node is used directly. This is needed to preserve
compatibility of images after a block-commit or block-pull libvirt
operation with older libvirt versions in case when we'll want to remove
the unneded 'raw' format drivers from the block graph.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Inside of virsocket.c there is an include of poll.h and
PKT_TIMEOUT_MS macro definition. Neither of these is really
needed and in fact it's a leftover after I reworked one of
previously merged commits during review.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
enable VIR_DOMAIN_NET_TYPE_ETHERNET network support for ch guests.
Tested with following interface config:
<interface type='ethernet'>
<target dev='chtap0' managed="yes"/>
<model type='virtio'/>
<driver queues='2'/>
<interface>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This capability checks if ch can receive multiple fds along with net-add
api. This capability is required to enable multiple queues for
domain/guest interfaces.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virSocketSendMsgWithFDs method send fds along with payload using
SCM_RIGHTS. virSocketRecv method polls, receives and sends the response
to callers.
These methods are required to add network suppport in ch driver.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move domain interface management methods from qemu to hypervisor. This
refactoring allows the domain management methods to be shared between CH and
qemu drivers.
This commit does not introduce any functional changes.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Drop unused parameter from virDomainNetReleaseActualDevice method.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The helper was used only in 'qemucapabilitiesnumbering' test which was
removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Instead of open-coding a partial version of it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The qemuDomainGetSCSIControllerModel() function, which is
responsible for choosing a model for a SCSI controller that
didn't have one provided by the user, considers values >0 to
mean "model has been set".
Since MODEL_SCSI_AUTO == 0, this means that such a value is
considered the same as MODEL_SCSI_DEFAULT (-1). This makes
sense, as not specifying a model name or explicitly asking for
one to be automatically chosen intuitively should result in
the same behavior.
Specifically, there is no case in which a value of
MODEL_SCSI_AUTO or MODEL_SCSI_DEFAULT is encountered after the
initial controller creation: it is either replaced with an
actual model, or an error is raised.
Despite this, there are a few places in the QEMU driver where
we incorrectly treat these values as if they were actual
model names. To reduce confusion, make sure that no longer
happens.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Remove the wannabe error reporting via 'VIR_DEBUG/VIR_INFO' in favor of
proper errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code abused 'VIR_INFO' as an attempt at error reporting. Rework the
code to return the usual 0/-1 and raise proper errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rewrite the conditions after exiting the parser so that they are easier
to understand. This partially decreases the granularity of "error"
messages as they are not strictly necessary albeit for debugging.
As it was already observed in this code the logic itself often does
something else than the comment claims, thus the code logic is
preserved.
Changes:
- any case when not all data was processed is aggregated together and
gets a common "error" message
- absence of 'checksum' field is checked separately
- helper variables are removed as they are no longer needed
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a 'switch' statement instead of a bunch of if/elseif statements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'fieldFormat' variable is guaranteed to have only the proper enum
values by virPCIVPDResourceGetFieldValueFormat.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Merge the pre-checks with the 'switch' statement which is operating on
the same values to simplify further refactoring.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Replace VIR_INFO being used as form of error reporting with proper
virReportError and the usual return values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Each caller was checking that the function read as many bytes as it
expected. Move the check inside virPCIVPDReadVPDBytes and make it report
a proper error rather than just a combination of VIR_DEBUG inside the
function and a random VIR_INFO in the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Until now 'virPCIDeviceGetVPD' couldn't reallistically raise an error,
but that will change. Handle the errors by either resetting it if we'd
be ignoring it or forward it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
- fix passing of 'errno' to 'virReportSystemError'
The 'open' syscall returns '-1' and sets 'errno' on failure. The code
passed '-fd' as 'errno' rather than errno itself, thus always reporting
EPERM.
- don't overwrite errors when closing FD
Use VIR_AUTOCLOSE to avoid overwriting the errors from virPCIVPDParse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A checker function should not raise VIR_INFO or VIR_WARN messages
especially if they contain information useful only for debugging.
Turn the message into a VIR_DEBUG with universal meaning.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function always succeeded and after the removal of programing error
checks doesn't even have a 'return false' case. Additionally one of the
tests in 'virpcivpdtest' tested that this function never failed on wrong
data. Embrace this logic and remove the return value and adjust logging
to VIR_DEBUG level to avoid spamming logs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Don't overwrite already reported errors and improve parsing of
attributes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The errors raised in virNodeDeviceCapVPDParseCustomFields were actually
ignored by continuing the parse rather than raised.
Rather than just replace 'continue' by 'return -1' this patch refactors
the whole parser to simplify it as well as report reasonable errors.
Parsing of individual fields is done without XPath and is extracted into
a common helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All callers satisfy these checks as they are just for programming
errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function are never called with NULL argument so the checks can be
removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
None of the callers pass NULL, so the NULL check is pointless. Remove it
an remove the return value.
The function is exported only for use in 'virpcivpdtest' thus marking
the arguments as NONNULL is unnecessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use VIR_DEBUG instead of VIR_INFO as that's more appropriate and report
relevant information for debugging.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virXPathNodeSet' returns -1 only when 'ctxt' or 'xpath' are NULL or
when the 'xpath' string is invalid. Both are programming errors. It
doesn't make sense for the code to overwrite the error message for
anything supposedly more relevant.
The majority of calls to 'virXPathNodeSet' already didn't do this, so
this patch fixes the rest to prevent it from spreading again.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The check in 'virPCIVPDResourceIsValidTextValue' allows any printable
characters, thus the XML schema should do the same.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to previous commit other specific fields which come from the
system data and aren't sanitized enough to be safe for XML were also
formatted via virBufferAsprintf.
Other static and safe strings used virBufferEscapeString instead of
virBufferAddLit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The custom field data is taken from PCI device data which can contain
any printable characters, and thus must be escaped when putting into
XML.
Originally, based on the comment and XML schema which was fixed in
previous commits the idea seemed to be that the parser would validate
that only characters which don't break the XML would be present but that
didn't seem to materialize.
Switch to proper escaping of the XML.
Fixes: 3954378d06
Resolves: https://issues.redhat.com/browse/RHEL-22314
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is never called with NULL argument. Remove the check and
refactor the rest including the debug statement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function does not reject '&', '<', '>' contrary to what it actually
states. Move and adjust the comment.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's nothing under the 'cleanup:' label thus the whole code can be
simplified.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Originally the migration code didn't register the NBD disk port with the
port allocator when it was manually specified. Later when commit
e74d627bb3 refactored the code and started registering it, the
old logic which was clearing 'priv->nbdPort' in case when it was manually
specified was not removed.
This caused following problems:
- the port was not released after successful migration
- the port was released even when it was not allocated on failures
regarding the NBD server start
- the port was not released on other failures of the migration after
NBD server startup
To address this we remove the assumption that 'priv->nbdPort' is used
only for auto-allocated port and fill it only once the port is
allocated and make the caller of qemuMigrationDstStartNBDServer
responsible for releasing it.
Fixes: e74d627bb3
Resolves: https://issues.redhat.com/browse/RHEL-21543
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Add a few debug statements to be able to trace lifetime of a
reserved/allocated port.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Locks in following text:
A: virNetServer
B: virNetServerClient
C: daemonClientPrivate
'virNetServerSetClientAuthenticated' locks A then B
'remoteDispatchAuthPolkit' calls 'virNetServerSetClientAuthenticated'
while holding C.
If a client closes its connection 'virNetServerProcessClients' with the
lock A and B locked will call 'virNetServerClientCloseLocked' which will
try to dispose of the 'client' private data by:
ref(b);
unlock(b);
remoteClientFreePrivateCallbacks();
lock(b);
unref(b);
Unfortunately remoteClientFreePrivateCallbacks() tries lock C.
Thus the locks are held in the following order:
polkit auth: C -> A
connection close: A -> C
causing a textbook-example deadlock. To resolve it we can simply drop
lock 'C' before calling 'virNetServerSetClientAuthenticated' as the lock
is not needed any more.
Resolves: https://issues.redhat.com/browse/RHEL-20337
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Commit 7a39b04d68 ("apparmor: Enable passt support") grants
passt(1) read-write access to /{,var/}run/libvirt/qemu/passt/* if
started by the libvirt daemon. That's the path where passt creates
PID and socket files only if the guest is started by the root user.
If the guest is started by another user, though, the path is more
commonly /var/run/user/$UID/libvirt/qemu/run/passt: add it as
read-write location. Otherwise, passt won't be able to start, as
reported by Andreas.
While at it, replace /{,var/}run/ in the existing rule by its
corresponding tunable variable, @{run}.
Fixes: 7a39b04d68 ("apparmor: Enable passt support")
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1061678
Reported-by: Andreas B. Mundt <andi@debian.org>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Historically creating offline external snapshot required disk-only flag
as well. Now when user requests new snapshot for offline VM and at least
one disk is specified to use external snapshot we will no longer require
disk-only flag as all other not specified disk will use external
snapshots as well.
Resolves: https://issues.redhat.com/browse/RHEL-22797
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Introduce new function qemuSnapshotCreateUseExternal() that will return
true if we will use external snapshots as default location.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The condition was completely wrong. As per the comment for function
virDomainMomentIsAncestor() it checks that the first argument is
descendant of the second argument.
Consider the following snapshot tree for VM:
s1
|
+- s2
| |
| +- s3
|
+- s4
|
+- s5 (current)
When deleting s2 with the original code we checked if
virDomainMomentIsAncestor(s2, s5) which would return false basically for
any snapshot as s5 is leaf snapshot so no children.
When deleting s2 with fixed code we check if
virDomainMomentIsAncestor(s5, s2) which still returns false but when
deleting s4 it will correctly return true.
Before this fix it fails with the following error:
error: Failed to delete snapshot s2
error: invalid argument: could not find base disk source in disk source chain
After the fix it fails with correct error:
error: Failed to delete snapshot s2
error: unsupported configuration: deletion of non-leaf external snapshot that is not in active chain is not supported
Resolves: https://issues.redhat.com/browse/RHEL-23212
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Rectify the condition to remove a domain only if it is not persistent.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It has nothing to do with assigning addresses, so it makes more
sense to have it in qemu_domain.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemuDomainGetSCSIControllerModel() can return -1 on failure,
but qemuDomainFindOrCreateSCSIDiskController() didn't implement
any handling for this scenario.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Group things together where it makes sense, avoid unnecessary
uses of 'else if', plus other tweaks.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The current defaults, that can be altered on a per-architecture
basis, are derived from the historical x86 behavior.
Every time support for a new architecture is added to libvirt,
care must be taken to override these default: if that doesn't
happen, guests will end up with additional hardware, which is
something that's generally undesirable.
Turn things around, and require architectures to explicitly
ask for the devices to be created by default instead. The
behavior for existing architectures is preserved.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
They reference functions that have since been renamed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
