Unused as of:
commit f136b831
    qemu: Rework setting process affinity
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

The current qemu driver code for changing bandwidth on a NIC first asks
the network driver if the change is supported, then changes the
bandwidth on the VIF, and then tells the network driver to update the
bandwidth on the bridge.

This is potentially racing if a parallel API call causes the network
driver to allocate bandwidth on the bridge between the check and the
update phases.

Change the code to just try to apply the network bridge update
immediately and rollback at the end if something failed.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

If libvirt receives DISCONNECTED event and prDaemonRunning is set
to false, and qemuDomainRemoveDiskDevice() is performing in the
meantime, then qemuDomainRemoveDiskDevice() will fail to remove
pr-helper object because prDaemonRunning is false. But removing
that check from qemuHotplugRemoveManagedPR() is not enough,
because after removing the object through monitor the
qemuProcessKillManagedPRDaemon() is called which contains the
same check. Thus the pr-helper process might be left behind.
Signed-off-by: NJie Wang <wangjie88@huawei.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Log the flags passed to the function in a exploded state so that it's
easily visible what's happening to the image.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The hash table returned by qemuMonitorGetAllBlockJobInfo is organized by
the frontend name (which skipps the 'drive-' prefix). While our code
properly matches the jobs to the disk, qemu needs the full job name
including the 'drive-' prefix to be able to identify jobs.

Fix this by adding an argument to qemuMonitorGetAllBlockJobInfo which
does not modify the job name before filling the hash.

This fixes a regression where users would not be able to cancel/pivot
block jobs after restarting libvirtd while a blockjob is running.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In commit 76b9aba2 I refactored how the function treats the readonly
flag which introduced a bug when we'd not allow to force read-write
state for an image.

This created problems with blockjobs where we need to temporarily
force images to have read-write permissions.

Rename QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY to
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_ONLY and also introduce
a complement QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_WRITE which
will allow to force write access.

https://bugzilla.redhat.com/show_bug.cgi?id=1717768Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In commit 76b9aba2 I tried to refactor qemuDomainStorageSourceAccessAllow
but used wrong operators for adding bitwise flags.

This way the flags would result in 0 if any of them would be applied.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The upcoming virDomainBackup() API needs to take advantage of the
ability to expose a bitmap as part of nbd-server-add for a pull-mode
backup (this is the recently-added QEMU_CAPS_NBD_BITMAP capability).
Signed-off-by: NEric Blake <eblake@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

The upcoming virDomainBackup() API needs to take advantage of various
qcow2 bitmap manipulations as the basis to virDomainCheckpoints and
incremental backups.  Add four functions to expose
block-dirty-bitmap-{add,enable,disable,merge} (this is the
recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: NEric Blake <eblake@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Add two capabilities for testing features required for the upcoming
virDomainBackupBegin: use block-dirty-bitmap-merge as the generic
witness of bitmap support needed for checkpoints (since all of the
bitmap management functionalities were finalized in the same qemu 4.0
release), and the bitmap parameter to nbd-server-add for pull-mode
backup support.  Even though both capabilities are likely to be
present or absent together (that is, it is unlikely to encounter a
qemu that backports only one of the two), it still makes sense to keep
two capabilities as the two uses are orthogonal (full backups don't
require checkpoints, push mode backups don't require NBD bitmap
support, and checkpoints can be used for more than just incremental
backups).

Existing code is not affected by the new capabilities.
Signed-off-by: NEric Blake <eblake@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Migration always uses a TCP socket for NBD servers, because we don't
support same-host migration. But upcoming pull-mode incremental backup
needs to also support a Unix socket, for retrieving the backup from
the same host. Support this by plumbing virStorageNetHostDef through
the monitor calls, since that is a nice reusable struct that can track
both TCP and Unix sockets.

Update qemumonitorjsontest to verify both forms of the QMP command.
Signed-off-by: NEric Blake <eblake@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Now that we never get to the actual snapshot code if there's nothing to
do we can remove the variable and surrounding logic.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Skip actual snapshot creation code if we have 0 disks to snapshot.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

All cases taking the 'cleanup' path can take the original 'error' path.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Introduce 'rc' for collecting state from monitor commands so that we can
initialize 'ret' to -1. This also fixes few cases which could return 0
from the function despite an error condition.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

qemuDomainSnapshotDiskDataFree also removes the resources associated
with the disk data. Move the unlinking of the just-created file so that
we can unify the cleanup paths.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In commit cbb4d229 I named the function with 'free' suffix, but at
that time it already did some non-freeing tasks. Rename it to make it
obvious that it's not just memory managemet.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The function skips disks which are not selected for snapshot. Rather
than creating a sparse array and check whether the given field is filled
compress the entries.

Note that this does not allocate a smaller array, but the memory
allocation is short-lived.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If there's an offline config definition save it unconditionally even if
it was not modified.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The error path is unlikely thus saving the status XML even if we didn't
modify it does not add much burden.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

After getting rid of pre-transaction qemu support the cleanup section is
unused.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The caller has it so there's no point in getting it again.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Commit 2f2254c7 attempted to fix a memory leak by ensuring
cpumapToSet is always a freshly allocated bitmap, but regrettably
introduced a NULL pointer access while doing so, because it called
virBitmapCopy() without allocating the destination bitmap first.

Solve the issue by using virBitmapNewCopy() instead.
Reported-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

We're using VIR_AUTOPTR() for everything now, plus the
cleanup section was not doing anything useful anyway.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

In two out of three scenarios we are cleaning up properly after
ourselves, but commit 5f2212c0 has changed the remaining one
in a way that caused it to start leaking cpumapToSet.

Refactor the logic so that cpumapToSet is always a freshly
allocated bitmap that gets cleaned up automatically thanks to
VIR_AUTOPTR(); this also allows us to remove the hostcpumap
variable.
Reported-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Ever since the feature was introduced with commit 0f8e7ae3,
it has contained a logic error in that it attempted to use a NUMA
node map where a CPU map was expected.

Because of that, guests using <numatune> might fail to start:

  # virsh start guest
  error: Failed to start domain guest
  error: cannot set CPU affinity on process 40055: Invalid argument

This was particularly easy to trigger on POWER 8 machines, where
secondary threads always show up as offline in the host: having

  <numatune>
    <memory mode='strict' placement='static' nodeset='1'/>
  </numatune>

in the guest configuration, for example, would result in libvirt
trying to set the process affinity so that it would prefer
running on CPU 1, but since that's a secondary thread and thus
shows up as offline, the operation would fail, and so would
starting the guest.

Use the newly introduced virNumaNodesetToCPUset() to convert the
NUMA node map to a CPU map, which in the example above would be
48,56,64,72,80,88 - a valid input for virProcessSetAffinity().

https://bugzilla.redhat.com/show_bug.cgi?id=1703661Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

When migrating a domain with invtsc CPU feature enabled, the TSC
frequency of the destination host must match the frequency used when the
domain was started on the source host or the destination host has to
support TSC scaling.

If the frequencies do not match and the destination host does not
support TSC scaling, QEMU will fail to set the right TSC frequency when
starting vCPUs on the destination and thus migration will fail. However,
this is quite late since both host might have spent significant time
transferring memory and perhaps even storage data.

By adding the check to libvirt we can let migration fail before any data
starts to be sent over. If for some reason libvirt is unable to detect
the host's TSC frequency or scaling support, we'll just let QEMU try and
the migration will either succeed or fail later.

Luckily, we mandate TSC frequency to be explicitly set in the domain XML
to even allow migration of domains with invtsc. We can just check
whether the requested frequency is compatible with the current host
before starting QEMU.

https://bugzilla.redhat.com/show_bug.cgi?id=1641702Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The function is renamed as virQEMUCapsProbeHostCPU and it does not get
the list of allowed CPU models from qemuCaps anymore. This is
responsibility is moved to the caller. The result is just a very thin
wrapper around virCPUGetHost mostly required mocking in tests.

The generic function is used in place of a direct call to virCPUGetHost
in virQEMUCapsInitHostCPUModel to make sure tests don't accidentally
probe host CPU.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1575526Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

All current IOMMU features are specific to Intel IOMMU, so
understandably we check for the corresponding capabilities
inside the Intel-specific switch() branch; however, we want
to make sure SMMUv3 IOMMU users get an error if they try to
enable any of those features in their guest, and performing
the capability checks unconditionally is both the easiest
way to achieve that, as well as the one least likely to
result in us inadvertently letting users enable some new
Intel-specific IOMMU feature for ARM guests later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

SMMUv3 is an IOMMU implementation for ARM virt guests.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This capability can be used to figure out whether the
QEMU binary at hand supports the machine type property
we need in order to enable SMMUv3 IOMMU support.

Unfortunately we can't avoid probing the RISC-V binaries
along with the ARM ones, since both architectures have
their own 'virt' machine type.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Current capability checks are specific to Intel IOMMU, so
we need to move them inside the switch() statement before
we can introduce more virDomainIOMMUModel values.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

This doesn't make a whole lot of difference now, but once
we introduce more virDomainIOMMUModel values the current
structure will no longer work.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>