If libvirt daemon is running unprivileged it will fail so we should not
even try to set it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The following interface XML will crash libvirt when attached to
running VM:
<interface type='network'>
<source network='hostdev_net'/>
</interface>
Instead of trying to figure out every single place where hostdev is used
as struct directly and allocating empty private data check if iommufd
is configured as that will happen only for <hostdev> device where the
private data are correctly allocated.
This patch is best viewed with `git show -w`.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
When IOMMUFD support was introduced it incorrectly tried to label
`/dev/iommu` and `/dev/vfio/devices/vfioX` but they are not added to
QEMU namespace because libvirt opens FDs and passes these FDs to QEMU.
We need to label these FDs instead.
Fixes: 7d2f91f9cb
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Current code used mix of return, goto, break and setting ret variable.
Simplify the logic to just return -1 on error.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently virDomainNetDef and virDomainActualNetDef use
virDomainHostdevDef directly as structure and the code doesn't call
virDomainHostdevDefNew() that would initialize private data.
This is hackish quick fix to solve a crash that happens in two
scenarios:
1. attaching any interface with hostdev backend
0x0000fffbfc0e2a90 in qemuDomainAttachHostPCIDevice (driver=0xfffbb4006750, vm=0xfffbf001f790, hostdev=0xfffbf400b150) at ../src/qemu/qemu_hotplug.c:1652
1652 if ((ret = qemuFDPassDirectTransferMonitor(hostdevPriv->vfioDeviceFd, priv->mon)) < 0)
2. starting VM with interface with hostdev backend using iommufd
0x00007f6638d5b9ca in qemuProcessOpenVfioDeviceFd (hostdev=hostdev@entry=0x7f6634425ee0) at ../src/qemu/qemu_process.c:7719
7719 hostdevPriv->vfioDeviceFd = qemuFDPassDirectNew(name, &vfioDeviceFd);
Proper fix for this issue is to refactor network code to use pointer and to
use virDomainHostdevDefNew().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On macOS we use flat namespace so we cannot have multiple mocks for the
same function.
Fixes: 63434db800
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For first host device we need to add iommufd object as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When removing last host device using IOMMUFD remove the iommufd object
as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We need to track if iommufd object was added to the VM when we add
hotplug support.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This cleans up creating QEMU command line and makes it easier when
adding hotplug support.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This cleans up creating QEMU command line and makes it easier when
adding hotplug support.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There is no need to call qemuDomainHostdevPrivateNew() because
privateData is allocated on creation by virDomainHostdevDefNew().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Create a single place for host device preparation code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Follow our switch pattern to simplify the code logic
and rename the function to follow our naming pattern.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We would open IOMMU device for each host device with IOMMUFD backend
leaking FD except for the last one. Since we are adding only single
iommufd object we need to open it only once.
Fixes: 2f0999a161
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If starting VM fails we would leak the FD for IOMMU device.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This will allow us reusing same code for hotplug support.
As part of refactor remove redundant checks.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 28a0621528 added support to restore
sparse images but changed the boolean that controls if we open the file
as read-only or read-write. Editing XML in the save image resulted in
following error message:
failed to write header to domain save file '/data/images/fedora40.save': Bad file descriptor
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Option --parallel-channels would require changing configuration file to
be used so introduce this option as well to make it convenient for
users.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
We should use the newest API only when user sets parallel-channels.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
We should use the newest API only when user sets parallel-channels.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
There is no need to use extra flag in addition to the new
"parallel.channels" param.
Using the flag without param would result in using uninitialized
variable. Fixing it would result in error that parallel channels cannot
be less then 1 or setting 1 as default.
Using the param without the flag is ignored.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
There is no need to have --parallel and --parallel-channels especially
when --parallel on its own is the same as not used at all. In both cases
libvirt will default to single channel.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
There is no need to have --parallel and --parallel-channels especially
when --parallel on its own is the same as not used at all. In both cases
libvirt will default to single channel.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Before this patch the code would start the revert process by destroying
the VM and preparing to revert where it would fail with following error:
error: unsupported configuration: source for disk 'sdb' is not a regular file; refusing to generate external snapshot name
and leaving user with offline VM even if it was running.
Make the check before we start the revert process to not destroy VMs.
Resolves: https://issues.redhat.com/browse/RHEL-30971
Resolves: https://issues.redhat.com/browse/RHEL-79928
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the new virXMLFormatDirect in order to remove usage of
virXMLFormatInternal.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This can be used to format XML where the element has direct value
instead of any subelement. For example:
<maxMemory slots='16' unit='KiB'>1524288</maxMemory>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This was reported on virt-manager issue tracker as it was possible to
provide `listen` attribute with properly escaped characters but libvirt
would format XML without escaping it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now we are able to move the rest into virDomainGraphicsDefFormatVNC
without breaking order of elements in the resulting XML.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now we are able to move the rest into virDomainGraphicsDefFormatSpice
without breaking order of elements in the resulting XML.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only VNC, RDP and Spice graphics types are using listen elements so call
the function only where it is needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This will be used in specific graphics types that are using listen
elements.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainGraphicsDefFormat function was way too long so split it into
separate functions for each graphics type.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use separate buffers for attributes and children elements to make the
code cleaner and to use the virXMLFormatElement() function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On current Fedora libvirt uses nftables by default but the libvirt-tck
tests are not ready for it and most of the nwfilter tests fail. We need
to keep using iptables for now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The commit in question made an incorrect change that resulted in getting
O_RDONLY FD instead of O_RDWR preventing any writes to happen with the
following error:
virQEMUSaveDataWrite:176 : failed to write header to domain save file '/path/to/save.img': Bad file descriptor
Pass 'bypass_cache' as proper bool as the original code did.
Fixes: 517248e239
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When snapshot is created with disk-only flag it is always external
snapshot without memory state. Historically when there was not support
to revert external snapshots this produced error message.
error: Failed to revert snapshot s1
error: internal error: Invalid target domain state 'disk-snapshot'. Refusing snapshot reversion
Now we can simply consider this as reverting to offline snapshot as the
possible damage to file system is already done at the point of snapshot
creation.
Resolves: https://issues.redhat.com/browse/RHEL-21549
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When we are deleting external snapshot that is not active we only need
to delete overlay disk image of the parent snapshot. This works
correctly even if parent snapshot is external and active as it will have
another overlay created when user reverted to that snapshot.
In case the parent snapshot is internal there are no overlay disk images
created as everything is stored internally within the disk image. In
this case we would delete the actual disk image storing internal
snapshots and most likely the original disk image as well resulting in
data loss once the VM is shutoff.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/734
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Originally nicindexes were updated only for VIR_DOMAIN_NET_TYPE_BRIDGE
and VIR_DOMAIN_NET_TYPE_DIRECT. The mentioned commit adds support for
NAT network mode and changes the code to update nicindexes for
VIR_DOMAIN_NET_TYPE_ETHERNET and VIR_DOMAIN_NET_TYPE_NETWORK as well.
It doesn't work as intended and after the change nicindexes are updated
only for VIR_DOMAIN_NET_TYPE_ETHERNET and VIR_DOMAIN_NET_TYPE_NETWORK.
Fixes: aa64209073
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
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>
When reverting to inactive snapshot updating the domain definition needs
to happen after the new overlays are created otherwise qemu-img will
correctly fail with error:
Trying to create an image with the same filename as the backing file
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When we revert to non-leaf snapshot and create new branch or branches
the overlay in snapshot metadata is no longer usable as a disk source
for deletion of that snapshot. We need to use other places to figure out
the correct storage source.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/534
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We need to skip all disks that have snapshot type other than 'external'.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When used with internal snapshots there is no memory state file so we
have no data to load and decompression is not needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When called from snapshot code we will need to pass snapshot object in
order to make internal snapshots work correctly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When called by snapshot code we will need to use different reason.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function will no longer be used only when restoring VM as it will
be used when reverting snapshot as well so move it to qemu_process
and rename it accordingly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
These new helpers separates the code from the logic used to start new
QEMU process with memory state and will make it easier to move
qemuSaveImageStartProcess() into qemu_process.c file.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Part of qemuSaveImageStartVM() function will be used when reverting
external snapshots. To avoid duplicating code and logic extract the
shared bits into separate function.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Function virGetConnectSecret() can return NULL so we need to check it
since in virSecretGetSecretString() it gets dereferenced.
Reported-by: coverity
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reverting external snapshot for running VM doesn't work correctly so we
should not report this capability until it is fixed.
This reverts commit de71573bfe.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Libvirt supports creating snapshots for a long time but the wording of
the feature may imply that libvirt supports external snapshots in
general but that is not true as users were not able to use APIs to
delete or revert external snapshots.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that the support to revert external snapshots is implemented we can
drop this check.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With the introduction of external snapshot revert support we need to
error out in some cases when trying to delete some snapshots.
If users reverts to non-leaf snapshots and would try to delete it after
the revert is done it would not work currently as this operation would
require using block-stream which is not implemented for now as in this
case the snapshot has two children so the disk files have multiple
overlays.
Similarly if user reverts to non-leaf snapshot and would try to delete
snapshot that is non-leaf but not in currently active snapshot chain we
would still need to use block-commit operation. The issue here is that
in order to do that we would have to start new qemu process with
different domain definition than what is currently used by the domain.
If the current domain would be running it would complicate things even
more so this operation is not yet supported.
If user creates new snapshot after reverting to non-leaf snapshot it
creates a new branch. Deleting snapshot with multiple children will
require block-stream which is not implemented for now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Pavel Hrdina