Some variables are not used outside of the for() loop. Move their
declaration to clean up the code a bit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
When using the monolithic daemon, then dom->conn has all driver
tables filled in properly and thus it's safe to call an API other
than virDomain*(). However, when using split daemons then
dom->conn has only hypervisor driver table set
(dom->conn->driver) and the rest is NULL. Therefore, if we want
to call a non-domain API (virNetworkLookupByName() in this case),
we have obtain the cached connection object accessible via
virGetConnectNetwork().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
If we place qemuDomainInterfaceAddresses() a few lines below the
two functions its using then we can drop forward declarations of
those functions.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
While at bugfixing, convert the whole function to the new-style memory
allocation handling.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Pavel Mores <pmores@redhat.com>
With NVMe disks, one can start a blockjob with a NVMe disk
that is not visible in domain XML (at least right away). Usually,
it's fairly easy to override this limitation of
qemuDomainGetMemLockLimitBytes() - for instance for hostdevs we
temporarily add the device to domain def, let the function
calculate the limit and then remove the device. But it's not so
easy with virStorageSourcePtr - in some cases they don't
necessarily are attached to a disk. And even if they are it's
done later in the process and frankly, I find it too complicated
to be able to use the simple trick we use with hostdevs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
At the very beginning of the attach function the
qemuDomainStorageSourceChainAccessAllow() is called which
modifies CGroups, locks and seclabels for new disk and its
backing chain. This must be followed by a counterpart which
reverts back all the changes if something goes wrong. This boils
down to calling qemuDomainStorageSourceChainAccessRevoke() which
is done under 'error' label. But not all failure branches jump
there. They just jump onto 'cleanup' label where no revoke is
done. Such mistake is easy to do because 'cleanup' label does
exist. Therefore, dissolve 'error' block in 'cleanup' and have
everything jump onto 'cleanup' label.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Because this is a HMP we're dealing with, there is nothing like
class of reply message, so we have to do some string comparison
to guess if the command fails. Well, with NVMe disks whole new
class of errors comes to play because qemu needs to initialize
IOMMU and VFIO for them. You can see all the messages it may
produce in qemu_vfio_init_pci().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Now, that we have everything prepared, we can generate command
line for NVMe disks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This capability tracks if qemu is capable of:
-drive file.driver=nvme
The feature was added in QEMU's commit of v2.12.0-rc0~104^2~2.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
If a domain has an NVMe disk configured, then we need to allow it
on devices CGroup so that qemu can access it. There is one caveat
though - if an NVMe disk is read only we need CGroup to allow
write too. This is because when opening the device, qemu does
couple of ioctl()-s which are considered as write.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
There are couple of places where a domain with a VFIO device gets
special treatment: in CGroups when enabling/disabling access to
/dev/vfio/vfio, and when creating/removing nodes in domain mount
namespace. Well, a NVMe disk is a VFIO device too. Fortunately,
we have this qemuDomainNeedsVFIO() function which is the only
place that needs adjustment.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
If a domain has an NVMe disk configured, then we need to create
/dev/vfio/* paths in domain's namespace so that qemu can open
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
We have this beautiful function that does crystal ball
divination. The function is named
qemuDomainGetMemLockLimitBytes() and it calculates the upper
limit of how much locked memory is given guest going to need. The
function bases its guess on devices defined for a domain. For
instance, if there is a VFIO hostdev defined then it adds 1GiB to
the guessed maximum. Since NVMe disks are pretty much VFIO
hostdevs (but not quite), we have to do the same sorcery.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The qemu driver has its own wrappers around virHostdev module (so
that some arguments are filled in automatically). Extend these to
include NVMe devices too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
To simplify implementation, some restrictions are added. For
instance, an NVMe disk can't go to any bus but virtio and has to
be type of 'disk' and can't have startupPolicy set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
There are going to be more disk types that are considered unsafe
with respect to migration. Therefore, move the error reporting
call outside of if() body and rework if-else combo to switch().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Sometimes, we have a PCI address and not fully allocated
virPCIDevice and yet we still want to know its /dev/vfio/N path.
Introduce virPCIDeviceAddressGetIOMMUGroupDev() function exactly
for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Previous patches rendered some of 'cleanup' labels needless.
Drop them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Now that all callers of qemuDomainGetHostdevPath() handle
/dev/vfio/vfio on their own, we can safely drop handling in this
function. In near future the decision whether domain needs VFIO
file is going to include more device types than just
virDomainHostdev.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
There are several variables which could be automatically freed
upon return from the function. I'm not changing @tmpPaths (which
is a string list) because it is going to be removed in next
commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
In near future, the decision what to do with /dev/vfio/vfio with
respect to domain namespace and CGroup is going to be moved out
of qemuDomainGetHostdevPath() because there will be some other
types of devices than hostdevs that need access to VFIO.
All functions that I'm changing (except qemuSetupHostdevCgroup())
assume that hostdev we are adding/removing to VM is not in the
definition yet (because of how qemuDomainNeedsVFIO() is written).
Fortunately, this assumption is true.
For qemuSetupHostdevCgroup(), the worst thing that may happen is
that we allow /dev/vfio/vfio which was already allowed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
qemuBuildSoundCodecStr() validates if a given QEMU binary
supports the sound codec. This validation can be moved to
qemu_domain.c to be executed in domain define time.
The codec validation was moved to the existing
qemuDomainDeviceDefValidateSound() function.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move QEMU caps validation of QEMU_CAPS_OBJECT_USB_AUDIO and
QEMU_CAPS_DEVICE_ICH9_INTEL_HDA to a new function in qemu_domain.c,
qemuDomainDeviceDefValidateSound(). This function is called by
qemuDomainDeviceDefValidate() to validate the sound device
in domain define time.
qemuxml2xmltest.c was adjusted to add the now required caps for
domain definition.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemuBuildTPMDevStr() does TPM model validation that can be moved to
qemu_domain.c, allowing validation in domain define time. This patch
moves it to the existing qemuDomainDeviceDefValidateTPM() function.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Console validation is currently being done by qemuBuildConsoleCommandLine().
This patch moves it to a new qemuDomainDefValidateConsole() function. This
new function is then called by qemuDomainDefValidate(), validating the
console in domain define time.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move the SPICE caps validation from qemuBuildGraphicsSPICECommandLine()
to a new function called qemuDomainDeviceDefValidateSPICEGraphics().
This function is called by qemuDomainDeviceDefValidateGraphics(),
which in turn is called by qemuDomainDefValidate(), validating the graphics
parameters in domain define time.
This validation move exposed a flaw in the 'default-video-type' tests
for PPC64, AARCH64 and s390 archs. The XML was considering 'spice' as
the default video type, which isn't true for those architectures.
This was flying under the radar until now because the SPICE validation
was being made in 'virsh start' time, while the XML validation done in
qemuxml2xmltest.c considers define time.
All other tests were adapted to consider SPICE validation in this
earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move the VNC cap validation from qemuBuildGraphicsVNCCommandLine()
to qemuDomainDeviceDefValidateGraphics(). This function is called by
qemuDomainDefValidate(), validating the graphics parameters in domain
define time.
Tests were adapted to consider SDL validation in this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There are validations for SDL, VNC, SPICE and EGL_HEADLESS
around several BuildGraphics*CommandLine in qemu_command.c. This
patch starts to move all of them to qemu_domain.c, inside the
existent qemuDomainDeviceDefValidateGraphics() function. This
function is called by qemuDomainDefValidate(), validating the
graphics parameters in domain define time.
In this patch we'll move the SDL validation code from
qemuBuildGraphicsSDLCommandLine(). Tests were adapted to consider
SDL validation in this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move the pcihole64 validation being done by
qemuBuildGlobalControllerCommandLine() to the existing function
qemuDomainDeviceDefValidateControllerPCI(), which provides
domain define time validation.
The existing pcihole64 validations in qemu_domain.c were replaced
by the ones moved from qemu_command.c. The reason is that they
are more specific, allowing VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT
and VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT to have distinct validation,
with exclusive QEMU caps and machine types.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move the boot validation being done by qemuBuildBootCommandLine()
to to a new qemuDomainDefValidateBoot() function. This new function
is called by qemuDomainDefValidate(), allowing boot validation in
domain define time.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move the PM validation being done by qemuBuildPMCommandLine() to
to a new qemuDomainDefValidatePM() function. This new function
is called by qemuDomainDefValidate(), promoting PM validation in
domain define time.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
@def->clock validation is done by qemuBuildClockCommandLine() and
qemuBuildClockArgStr(). This patch centralize the validation done
in both these functions to a new qemuDomainDefValidateClockTimers()
function. This new function is then called by qemuDomainDefValidate(),
promoting clock validation in domain define time.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
QEMU_CAPS_DEVICE_VMGENID is now being validated by
qemuDomainDefValidate().
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move QEMU caps validation of qemuBuildHostdevCommandLine() to
qemuDomainDeviceDefValidateHostdev() and qemuDomainMdevDefValidate(),
allowing them to be validated at domain define time.
Tests were adapted to consider the new caps being needed in
this earlier stage.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move QEMU caps validation of QEMU_CAPS_CHARDEV_FILE_APPEND and
QEMU_CAPS_CHARDEV_LOGFILE to qemuDomainChrSourceDefValidate().
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Move QEMU caps validation of QEMU_CAPS_USB_HUB to a new function in
qemu_domain.c, qemuDomainDeviceDefValidateHub(). This function is
called by qemuDomainDeviceDefValidate() to validate the sound device
in domain define time.
qemuxml2xmltest.c was adjusted to add the now required caps for
domain definition.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
A new function qemuDomainDeviceDefValidateNVRAM() was created
to validate the NVRAM in domain define time. Unit test was
adjusted to account for the extra QEMU_CAPS_DEVICE_NVRAM required
during domain define.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
A new qemuDomainDefValidateNuma() function was created to host
all the QEMU caps validation being done inside qemuBuildNumaArgStr().
This new function is called by qemuDomainValidateCpuCount()
to allow NUMA validation in domain define time.
Tests were changed to account for the QEMU capabilities
that need to be present at domain define time.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Next patch will validate QEMU_CAPS_NUMA_DIST in a new qemu_domain.c
function. The code to verify if a NUMA node distance is being
set will still be needed in qemuBuildNumaArgStr() though.
To avoid code repetition, let's put this logic in a helper to be
used in qemuBuildNumaArgStr() and in the new function.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Validation of MACHINE_KERNEL_IRQCHIP and MACHINE_KERNEL_IRQCHIP_SPLIT
QEMU caps are now being done in qemuDomainDefValidateFeatures().
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
virQEMUCapsSupportsVmport() is now being called inside
qemuDomainDefValidateFeatures() for VIR_DOMAIN_FEATURE_VMPORT
feature.
qemuxml2xmltest.c was changed to account for this caps being
now validated at domain define time.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Introduce a new function called qemuDomainDefValidatePSeriesFeature()
that will center all the PSeries validation done in qemu_command.c.
qemuDomainDefValidatePSeriesFeature() is then called during domain
define time, in qemuDomainDefValidateFeatures().
qemuxml2argvtest.c is also changed to include all the caps that now
are being validated in define time.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is plenty of distributions that haven't switched to
systemd nor they force their users to (Gentoo, Alpine Linux to
name a few). With the daemon split merged their only option is to
still use the monolithic daemon which will go away eventually.
Provide init scripts for these distros too.
For now, I'm not introducing config files which would correspond
to the init files except for libvirtd and virtproxyd init scripts
where it might be desirable to tweak the command line of
corresponding daemons.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
As of commit 2a00ef6e71 which
was released in v5.2.0, we require YAJL to build the QEMU driver.
Remove the checks from code that requires the QEMU driver
or checks that also check for WITH_QEMU.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
To allow backups work across external snapshots we need to improve the
algorithm which calculates which bitmaps to merge.
The algorithm must look for appropriately named bitmaps in the image and
possibly descend into a backing image if the current image does not have
the bitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This function looks up a named bitmap for a virStorageSource in the data
returned from query-named-block-nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The function will require the bitmap topology for the full
implementation. To facilitate testing, add the propagation of the
necessary data beforehand so that the test code can stay unchanged
during the changes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Separate the for now incomplete code that collects the bitmaps to be
merged for an incremental backup into a separate function. This will
allow adding testing prior to the improvement of the algorithm to
include snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The object itself has no extra value and it would make testing the code
harder. Refactor it to remove just the definition pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Re-create any active persistent bitmap in the snapshot overlay image so
that tracking for a checkpoint is persisted. While this basically
duplicates data in the allocation map it's currently the only possible
way as qemu can't mirror the allocation map into a dirty bitmap if we'd
ever want to do a backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuDomainSnapshotDiskPrepareOne is already called for each disk which
is member of the snapshot so we don't need to iterate through the
snapshot list again to generate members of the 'transaction' command for
each snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
For testing purposes it will be beneficial to be able to parse the data
from JSON directly rather than trying to simulate the monitor. Extract
the worker bits and export them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We will need to inspect the presence and attributes for dirty bitmaps.
Extract them when processing reply of query-named-block-nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The use of the parseOpaque parameter was mistakenly removed in
commit 4a4132b462
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Dec 3 10:49:49 2019 +0000
conf: don't use passed in caps in post parse method
causing the method to re-fetch qemuCaps that were already just
fetched and put into parseOpaque.
This is inefficient when parsing incoming XML, but for live
XML this is more serious as it means we use the capabilities
for the current QEMU binary on disk, rather than the running
QEMU.
That commit, however, did have a useful side effect of fixing
a crasher bug in the qemu post parse callback introduced by
commit 5e939cea89
Author: Jiri Denemark <jdenemar@redhat.com>
Date: Thu Sep 26 18:42:02 2019 +0200
qemu: Store default CPU in domain XML
The qemuDomainDefSetDefaultCPU() method in that patch did not
allow for the possibility that qemuCaps would be NULL and thus
resulted in a SEGV.
This shows a risk in letting each check in the post parse
callback look for qemuCaps == NULL. The safer option is to
check once upfront and immediately stop (postpone) further
validation.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Don't check os type / virt type / arch in the post-parse callback
because we can't assume qemuCaps is non-NULL at this point. It
also conceptually belongs to the validation callback.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This function will be removed in a future commit because it allows the
caller to acquire both monitor and agent jobs at the same time. Holding
both job types creates a vulnerability to denial of service from a
malicious guest agent.
qemuDomainSetVcpusFlags() always passes NONE for either the monitor job
or the agent job (and thus is not vulnerable to the DoS), so we can
simply replace this function with the functions for acquiring the
appropriate type of job.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding
a monitor job while we're querying the agent, we open ourselves up to a
DoS.
Split the function so that the portion issuing the agent command only
holds an agent job and the portion issuing the monitor command holds
only a monitor job.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding a
monitor job while we're querying the agent, we open ourselves up to a
DoS.
So split the function up a bit to only hold the monitor job while
querying qemu for whether the domain supports suspend. Then acquire only
an agent job while issuing the agent suspend command.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding
a monitor job while we're querying the agent, we open ourselves up to a
DoS.
Split the function so that we only hold the appropriate type of job
while rebooting.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding
a monitor job while we're querying the agent, we open ourselves up to a
DoS. So split the function into separate parts: one that does the agent
shutdown and one that does the monitor shutdown. Each part holds only a
job of the appropriate type.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace all the uses passing a single parameter as the length.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
My hesitation to remove VIR_STRDUP without VIR_STRNDUP resulted
in these being able to sneak in.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This reverts commit 7be5fe66cd.
This commit broke resctrl, because it missed the fact that the
virResctrlInfoGetCache() has side-effects causing it to actually
change the virResctrlInfo parameter, not merely get data from
it.
This code will need some refactoring before we can try separating
it from virCapabilities again.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit aims to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1610207
The cause was apparently incorrect handling of jobs in snapshot
revert code which allowed a thread executing snapshot delete to
begin job while snapshot revert was still running on another
thread. The snapshot delete thread then waited on a condition
variable in qemuMonitorSend() while the revert thread finished,
changing (and effectively corrupting) the qemuMonitor structure
under the delete thread which led to its crash.
The incorrect handling of jobs in revert code was due to the fact
that although qemuDomainRevertToSnapshot() correctly begins a job
at the start, the job was implicitly ended when qemuProcessStop()
was called because the job lives in the QEMU driver's private
data (qemuDomainObjPrivate) that was purged during
qemuProcessStop().
This fix prevents qemuProcessStop() from clearing jobs as the
idea of qemuProcessStop() clearing jobs seems wrong in the first
place. It was (inadvertently) introduced in commit
888aa4b6b9, which is effectively reverted by
the second hunk of this commit. To preserve the desired effects
of the faulty commit, the first hunk is included as suggested by
Michal.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When QEMU uid/gid is set to non-root this is pointless as if we just
used a regular setuid/setgid call, the process will have all its
capabilities cleared anyway by the kernel.
When QEMU uid/gid is set to root, this is almost (always?) never
what people actually want. People make QEMU run as root in order
to access some privileged resource that libvirt doesn't support
yet and this often requires capabilities. As a result they have
to go find the qemu.conf param to turn this off. This is not
viable for libguestfs - they want to control everything via the
XML security label to request running as root regardless of the
qemu.conf settings for user/group.
Clearing capabilities was implemented originally because there
was a proposal in Fedora to change permissions such that root,
with no capabilities would not be able to compromise the system.
ie a locked down root account. This never went anywhere though,
and as a result clearing capabilities when running as root does
not really get us any security benefit AFAICT. The root user
can easily do something like create a cronjob, which will then
faithfully be run with full capabilities, trivially bypassing
the restriction we place.
IOW, our clearing of capabilities is both useless from a security
POV, and breaks valid use cases when people need to run as root.
This removes the clear_emulator_capabilities configuration
option from qemu.conf, and always runs QEMU with capabilities
when root. The behaviour when non-root is unchanged.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With all plumbing in place, we can now enable the new functionality.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since blockcommit is asynchronous, libvirtd can be restarted while the
operation runs. To ensure the information necessary to finish up the job
is not lost, serialisation to and deserialisation from the status XML is
added.
To unittest this, the new element was only added to the active commit test,
the non-active commit test doesn't have the new element so as to test its
absence.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When blockcommit finishes successfully, one of the
qemuBlockJobProcessEventCompletedCommit() and
qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
This is where the delete flag (stored in qemuBlockJobCommitData since the
previous commit) can actually be used to delete the committed snapshot
images if requested.
We use virFileRemove() instead of a simple unlink() to cover the case where
the image to be removed is on an NFS volume.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Propagate the delete flag from qemuDomainBlockCommit() (which was just
ignoring it until now) to qemuBlockJobDiskNewCommit() where it can be
stored in the qemuBlockJobCommitData structure which holds information
necessary to finish the job asynchronously.
In the actual qemuBlockJobDiskNewCommit() in this commit, we temporarily
pass a literal 'false' to preserve the current behaviour until the whole
implementation of the feature is in place.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After the individual sub-blockjobs of a backup libvirt job finish we
must detect it and notify the parent job, so that it can be properly
terminated.
Since we update job information to determine success of a blockjob we
can directly report back also statistics of the blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the helper which cancels all blockjobs to perform the backup job
cancellation in qemuDomainAbortJob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We can use the output of 'query-jobs' to figure out some useful
information about a backup job. That is progress in case of a push job
and scratch file use in case of a pull job.
Add a worker which will total up the data and call it from
qemuDomainGetJobStatsInternal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A backup blockjob needs to be able to notify the parent backup job as
well as track all data to be able to clean up the bitmap and blockdev
used for the backup.
Add the data structure, job allocation function and status XML formatter
and parser.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Store the data of a backup job along with the index counter for new
backup jobs in the status XML. Currently we will support only one
backup job and thus there's no necessity to add arrays of jobs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Implement the transaction actions generator for blockdev-backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A backup job may consist of many backup sub-blockjobs. Add the new
blockjob type and add all type converter strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We will want to use the async job infrastructure along with all the APIs
and event for the backup job so add the backup job as a new async job
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Introduce QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP and the convertors and other
plumbing to be able to report statistics for the backup job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
$ cat f | grep -e arch -e emulator
<type arch='mipsel'>hvm</type>
$ sudo virsh define f
error: Failed to define domain from f
error: An error occurred, but the cause is unknown
After:
$ sudo virsh define f
error: Failed to define domain from f
error: unsupported configuration: No emulator found for arch 'mipsel'
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
'cfg' is never initialized here, which causes a crash
later in qemuCheckpointCreateFinalize
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
For any backing file we set 'read-only' to true, but didn't do this when
modifying the recorded backing store when creating external snapshots.
This meant that qemu would attempt to open the backing-file read-write.
This would fail for example when selinux is used as qemu doesn't have
write permission for the backing file.
https://bugzilla.redhat.com/show_bug.cgi?id=1781079
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that nearly all internal APIs use the QEMU capabilities or other
QEMU driver data directly, there's no compelling benefit to create
virCapsPtr at driver startup.
Skipping this means we don't probe capabilities for all 30 system
emulator targets at startup, only those emulators which are referenced
by an XML doc. This massively improves libvirtd startup time when the
capabilities cache is not populated. It even improves startup time
when the cache is up to date, as we don't bother to load files from
the cache until we need them.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We always refresh the capabilities object when using virResctrlInfo
during process startup. This is undesirable overhead, because we can
just directly create a virResctrlInfo instead.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Avoid grabbing the whole virCapsPtr object when we only need the
host CPU information.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Annoyingly there was no existing constructor, and identifying all the
places which do a VIR_ALLOC(cpu) is a bit error prone. Hopefully this
has found & converted them all.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Avoid grabbing the whole virCapsPtr object when we only need the
NUMA information.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The NUMA cells are stored directly in the virCapsHostPtr
struct. This moves them into their own struct allowing
them to be stored independantly of the rest of the host
capabilities. The change is used as an excuse to switch
the representation to use a GPtrArray too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the domain XML APIs don't use virCapsPtr we can stop passing it
around many QEMU driver methods.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This parameter is now unused and can be removed entirely.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
None of the impls of this callback require the virCapsPtr param.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
None of the impls of this callback require the virCapsPtr param.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
No impl of this callback requires the virCapsPtr anymore.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The only user of this callback did not require the virCapsPtr parameter.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU impl of the callback can directly use the QEMU capabilities
cache to resolve the emulator binary name, allowing virCapsPtr to be
dropped.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virCapsPtr param is not used by any of the virt drivers providing
this callback.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of using the virCapsPtr to get the default security model,
pass this in via the parser config.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the disk and chardev seclabels are validated immediately at
the time their data is parsed. This forces the parser to fill in the
top level secmodel at time of parsing which is an undesirable thing.
This validation conceptually should be done in the post-parse phase
instead.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To enable the virCapsPtr parameter to the post parse method to be
eliminated, the drivers must fetch the virCapsPtr from their own
driver via the opaque parameter, or use an alternative approach
to validate the parsed data.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The XML parser currently calls virCapabilitiesDomainDataLookup during
parsing to find the domain capabilities matching the triple
(virt type, os type, arch)
This is, however, bogus with the QEMU driver as it assumes that there
is an emulator known to the default driver capabilities that matches
this triple. It is entirely possible for the driver to be parsing an
XML file with a custom emulator path specified pointing to a binary
that doesn't exist in the default driver capabilities. This will,
for example be the case on a RHEL host which only installs the host
native emulator to /usr/bin. The user can have built a custom QEMU
for non-native arches into $HOME and wish to use that.
Aside from validation, this call is also used to fill in a machine type
for the guest if not otherwise specified. Again, this data may be
incorrect for the QEMU driver because it is not taking account of
the emulator binary that is referenced.
To start fixing this, move the validation to the post-parse callbacks
where more intelligent driver specific logic can be applied.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Moving their instance parameter to be the first one, and give consistent
ordering of other parameters across all functions. Ensure that the xml
options are passed into both functions in prep for future work.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Our normal practice is for the object type to be the name prefix, and
the object instance be the first parameter passed in.
Rename these to virDomainObjSave and virDomainDefSave moving their
primary parameter to be the first one. Ensure that the xml options
are passed into both functions in prep for future work.
Finally enforce checking of the return type and mark all parameters
as non-NULL.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the virQEMUCapsPtr objects are just empty. Future patches are
going to expect them to contain real data. Start off by populating the
machine types and arch information.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As part of a goal to eliminate the need to use virCapsPtr for anything
other than the virConnectGetCapabilies() API impl, cache the host arch
against the QEMU driver struct and use that field directly.
In the tests we move virArchFromHost() globally in testutils.c so that
every test runs with a fixed default architecture reported.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The functions for converting migration typed parameters to QEMU
migration parameters and back were only implemented for integer types.
This patch adds support for string parameters.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
With blockdev we need to refer to the nodename of the disk source image
as the source argument for the blockdev-mirror operation while still
keeping the old job name. With blockdev we must also persist the job in
qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Separate out allocation of the virStorageSource corresponding to the
target NBD export of the migration.
As part of the splitout we allocate the export name explicitly as that
one must not change regardless whether blockdev is used or not to
provide compatibility.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The non-shared-storage migration tracks the storage source used
explicitly in the migration data so we must allow for processing of the
block job which has NULL mirror as the mirror will not be populated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Now that the cleanup section does not exist remove the label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuMigrationSrcNBDCopyCancelOne uses the block job data structure but
generated it's own job name rather than taking it from the block job
data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
With -blockdev we must use the nodename as the export but we must keep
the name of the export as it was before to ensure compatiblity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Declare the variable inside the loop with automatic clearing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuDomainBlockJobSetSpeed was not converted to get the job name from
the block job data. This means that after enabling blockdev the API call
would fail as we wouldn't use the appropriate name.
https://bugzilla.redhat.com/show_bug.cgi?id=1780497
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Where appropriate replace the open coded call with the qemu wrapper
which already reports the error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
With this patch users can cold unplug some sound devices.
use "virsh detach-device vm sound.xml --config" command.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com>
This is a follow-up to patch series posted in
https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html
It implements a suggestion made by Cole in
https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html
and discussed in follow-up messages as there were no objections to the
change.
The aim is to make the code more readable by replacing nested branching
with a flat structure.
Signed-off-by: Pavel Mores <pmores@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In v5.8.0-rc1~122 we've removed the only use of @safename in
qemuMonitorTextLoadSnapshot(). What we are left with is an
declared but not initialized variable that is passed to
VIR_FREE().
Caught by libvirt-php test suite.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In v5.9.0-370-g8fa0374c5b I've tried to fix a bug by removing
some stale XATTRs in qemuProcessStop(). However, I forgot to
do nothing when the VIR_QEMU_PROCESS_STOP_NO_RELABEL flag was
specified.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Until now we only really aborted migration via qemuDomainAbortJob. This
will change with the upcoming addition of the backup job. Additionally
there were a bunch of if statements checking various aspects of the
current job.
To make it more obvious convert qemuDomainAbortJob to use a switch
statement and move the individual conditions to the appropriate job
type.
Every job type has now it's own case despite multiple job types just
plainly cancelling the job for clarity and future extension.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Following patch will refactor qemuDomainAbortJob to use a per-job-type
switch where we will need to abort a migration job in various branches.
Save some code duplication by introducing a helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
We clear some capabilities here so the lockouts need to be
re-evaluated.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Blockdev is required to do incremental backups properly. Add a helper
function for locking out capabilities and export it to allow re-doing
the processing if a different code path modifies capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Checking whether a qemu capability set right before clearing it without
any other logic doesn't make sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Do all post-processing of capabilities in qemuProcessPrepareQEMUCaps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Move the post-processing of the QEMU_CAPS_CHARDEV_FD_PASS flag to the
new function.
The clearing of the capability is based on the presence of
VIR_QEMU_PROCESS_START_STANDALONE so we must also pass in the process
start flags.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Start aggregating all capability post-processing code in one place.
The comment was modified while moving it as it was mentioning floppies
which are no longer clearing the blockdev capability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is now used only in qemu_process.c so move it there and
name it 'qemuProcessPrepareQEMUCaps' which is more appropriate to what
it's doing.
The reworded comment now mentions that it will also post-process the
caps for VM startup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The redetection was originally added in 43c01d3838 as a way to recover
from libvirtd upgrade from the time when we didn't persist the qemu
capabilities in the status XML. Also this the oldest supported qemu by
more than two years.
Even if somebody would have a running VM running at least qemu 1.5 with
such an old libvirt we certainly wouldn't do the right thing by
redetecting the capabilities and then trying to communicate with qemu.
For now it will be the best to just stop considering this scenario any
more and error out for such VM.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit c90fb5a828 added explicit use of the private copy of the qemu
capabilities to various places. The change to qemuProcessInit was bogus
though as at the point where we re-initiate the post parse callbacks
priv->qemuCaps is still NULL as we clear it after shutdown of the VM and
don't initiate it until a later point.
Using the value from priv->qemuCaps might mislead readers of the code
into thinking that something useful is being passed at that point so go
with an explicit NULL instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainGetJobInfo didn't always reset the return data in @info.
Thankfully this wouldn't be a problem as the RPC layer does it but we
should do it anyways.
Since we reset the struct we don't have to set the type to
VIR_DOMAIN_JOB_NONE as the value is 0.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
For managed save we can choose between various compression
methods. I randomly tested the 'xz' program on a 8 GB guest
and was surprised to have to wait > 50 minutes for it to
finish compressing, with 'xz' burning 100% cpu for the
entire time. Despite the impressive compression, this is
completely useless in the real world as it is far too long
to wait to save the VM.
The 'xz' binary defaults to '-6' optimization level which
aims for high compression, with moderate memory usage,
at the expense of speed.
This change switches it to use the '-3' optimization level
which is documented as being the one that optimizes speed
at expense of compression. Even with this, it will still
outperform all the other options in terms of compression
level. It is a little less than x4 faster than '-6' which
means it starts to be a viable choice to use 'xz' for
people who really want best compression.
The test results on a 1 GB, fairly freshly booted VM are
as follows
format | save | restore size
=======+=======+=============
raw | 05s | 1s | 428 MB
lzop | 05s | 3s | 160 MB
gzip | 29s | 5s | 118 MB
bz2 | 54s | 22s | 114 MB
xz | 4m37s | 13s | 86 MB
xz -3 | 1m20s | 12s | 95 MB
Based on this we can say
* For moderate compression with no noticable loss in speed
=> use lzop
* For high compression with moderate loss in speed
=> use gzip
* For best compression with significant loss in speed
=> use xz
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 4b58fdf280 which enabled block copy also for network
destinations needed to limit when the 'mirror' storage source is
initialized in cases when we e.g. don't have an appropriate backend.
Limiting it just to virStorageFileSupportsCreate is too restrictive as
for example we can't precreate block devices and thus wouldn't
initialize the 'mirror' but since it's a local source we'd try to
examine it. This would fail since it wouldn't be initialized.
Fix it by introducing a more granular check whether certain operations
are supported and fix the check interlocks.
https://bugzilla.redhat.com/show_bug.cgi?id=1778058
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We tolerate image format detection during block copy in very specific
circumstances, but the code didn't error out on failure of the format
detection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In commit 2ccb5335dc I've refactored how we fill the typed parameters
for domain statistics. The commit introduced a regression in the
formating of stats for IOthreads by using the array index to label the
entries as it's common for all other types of statistics rather than
the iothread IDs used for iothreads.
Since only the design of iothread deviates from the common approach used
in all other statistic types this was not caught.
https://bugzilla.redhat.com/show_bug.cgi?id=1778014
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that we have a separate job type which will not trigger normal code
paths for terminating job we can remove the ad-hoc handling.
This possibly fixes the issue of a broken job inheriting the disk and
then finishing in which case we'd not detach the backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
To better track jobs we couldn't parse let's introduce a new job type
which will clarify semantics internally in few places.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
We will need to clear per-job type data when we will be marking a
blockjob as broken in the new way. Extract the code for future reuse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Both failure to refresh and to dismiss the job are very unlikely but if
they happen there's not much we can do about the blockjob.
The concluded job handlers treat it as if the job failed if we don't
update the state to 'QEMU_BLOCKJOB_STATE_COMPLETED' which is probably
the safest thing to do here.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Otherwise it would get dropped later on as untracked despite us knowing
about it. Additionally since we cancelled it we must wait to dismiss it
which would not be possible if we unregister it. This also opened a
window for a race condition since the job state change event of the
just-cancelled job might be delivered prior to us unregistering the job
in which case everything would work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Since we don't know what happened to the job we can't do much about it
but we can at least log that this happened.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
We must exit the monitor prior to refusing other work, otherwise the VM
object will become unusable.
This bug was introduced in commit v5.5.0-244-gc412383796 but thankfully
the code path was not excercised without QEMU_CAPS_BLOCKDEV.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Block jobs may be members of async jobs so it makes more sense to
refresh block job state after we do steps for async job recovery.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
qemu returns an error message in the job statistics even if the job was
cancelled to emphasize it was not successful. Libvirt didn't properly
transform it into QEMU_BLOCKJOB_STATE_CANCELLED though.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Commit ed56851f1b didn't wire up fetching of the statistics for the
job which are reported by 'query-jobs'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This reverts commit 421c9550f5
qemuDomainBlockPullCommon calls virDomainObjEndAPI internally so the
original commit made us shed two references of @vm instead of one
getting us into a premature free of @vm.
This is not a straight revert as qemuDomainBlockPull was modified
meanwhile. I've also added a warning comment that @vm is consumed.
https://bugzilla.redhat.com/show_bug.cgi?id=1777230
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
<interface> devices (virDomainNetDef) are a bit different from other
types of devices in that their actual type may come from a network (in
the form of a port connection), and that doesn't happen until the
domain is started. This means that any validation of an <interface> at
parse time needs to be a bit liberal in what it accepts - when
type='network', you could think that something is/isn't allowed, but
once the domain is started and a port is created by the configured
network, the opposite might be true.
To solve this problem hypervisor drivers need to do an extra
validation step when the domain is being started. I recently (commit
3cff23f7, libvirt 5.7.0) added a function to peform such validation
for all interfaces to the QEMU driver -
qemuDomainValidateActualNetDef() - but while that function is a good
single point to call for the multiple places that need to "start" an
interface (domain startup, device hotplug, device update), it can't be
called by the other hypervisor drivers, since 1) it's in the QEMU
driver, and 2) it contains some checks specific to QEMU. For
validation that applies to network devices on *all* hypervisors, we
need yet another interface validation function that can be called by
any hypervisor driver (not just QEMU) right after its network port has
been created during domain startup or hotplug. This patch adds that
function - virDomainActualNetDefValidate(), in the conf directory,
and calls it in appropriate places in the QEMU, lxc, and libxl
drivers.
This new function is the place to put all network device validation
that 1) is hypervisor agnostic, and 2) can't be done until we know the
"actual type" of an interface.
There is no framework for validation at domain startup as there is for
post-parse validation, but I don't want to create a whole elaborate
system that will only be used by one type of device. For that reason,
I just made a single function that should be called directly from the
hypervisors, when they are initializing interfaces to start a domain,
right after conditionally allocating the network port (and regardless
of whether or not that was actually needed). In the case of the QEMU
driver, qemuDomainValidateActualNetDef() is already called in all the
appropriate places, so we can just call the new function from
there. In the case of the other hypervisors, we search for
virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic
function that calls virNetworkPortCreateXML()), and add the call to our
new function right after that.
The new function itself could be plunked down into many places in the
code, but we already have 3 validation functions for network devices
in 2 different places (not counting any basic validation done in
virDomainNetDefParseXML() itself):
1) post-parse hypervisor-agnostic
(virDomainNetDefValidate() - domain_conf.c:6145)
2) post-parse hypervisor-specific
(qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498)
3) domain-start hypervisor-specific
(qemuDomainValidateActualNetDef() - qemu_domain.c:5390)
I placed (3) right next to (2) when I added it, specifically to avoid
spreading validation all over the code. For the same reason, I decided
to put this new function right next to (1) - this way if someone needs
to add validation specific to qemu, they go to one location, and if
they need to add validation applying to everyone, they go to the
other. It looks a bit strange to have a public function in between a
bunch of statics, but I think it's better than the alternative of
further fragmentation. (I'm open to other ideas though, of course.)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This also isn't required (due to the vportprofile being stored in the
NetDef as a pointer rather than being directly contained), but it
seemed dishonest to not mark it as const (and thus permit users to
modify its contents)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
In this case, the virNetDevBandwidthPtr that is returned is not to a
region within the virDomainNetDef arg, but points elsewhere (the
NetDef has the pointer, not the entire object), so technically it's
not necessary to make the return value a const, but it's a bit
disingenuous to *not* do it.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
This makes it easier to understand which interface's config caused the
error.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
The cpuModels member of _virQEMUCapsAccel struct is not a
virObject but regular struct with a free function defined:
qemuMonitorCPUDefsFree(). Use that when clearing parent structure
instead of virObjectUnref() to avoid a memleak:
==212322== 57,275 (48 direct, 57,227 indirect) bytes in 3 blocks are definitely lost in loss record 623 of 627
==212322== at 0x4838B86: calloc (vg_replace_malloc.c:762)
==212322== by 0x554A158: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.6)
==212322== by 0x17B14BF5: qemuMonitorCPUDefsNew (qemu_monitor.c:3587)
==212322== by 0x17B27BA7: qemuMonitorJSONGetCPUDefinitions (qemu_monitor_json.c:5616)
==212322== by 0x17B14B0B: qemuMonitorGetCPUDefinitions (qemu_monitor.c:3559)
==212322== by 0x17A6AFBB: virQEMUCapsFetchCPUDefinitions (qemu_capabilities.c:2571)
==212322== by 0x17A6B2CC: virQEMUCapsProbeQMPCPUDefinitions (qemu_capabilities.c:2629)
==212322== by 0x17A70C00: virQEMUCapsInitQMPMonitorTCG (qemu_capabilities.c:4769)
==212322== by 0x17A70DDF: virQEMUCapsInitQMPSingle (qemu_capabilities.c:4820)
==212322== by 0x17A70E99: virQEMUCapsInitQMP (qemu_capabilities.c:4848)
==212322== by 0x17A71044: virQEMUCapsNewForBinaryInternal (qemu_capabilities.c:4891)
==212322== by 0x17A7119C: virQEMUCapsNewData (qemu_capabilities.c:4923)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On s390 machines host-passthrough and host-model CPUs result in the same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a graphics device was added to XML that had no video device, libvirt
automatically added a video device which was always of type 'cirrus' on
x86_64, even if the underlying qemu didn't support cirrus.
This patch refines a bit the decision about the type of the video device.
Based on QEMU capabilities, cirrus is still preferred but only added if
QEMU supports it, otherwise VGA is used if supported by QEMU. There is now
no fallback as libvirt only aspires to generate a basic working config and
leaves anything more specific up to higher-level management tools.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Pavel Mores <pmores@redhat.com>
The default video device type selection algorithm we're about to deploy will
increase the amount of code dedicated to the task by amount enough to warrant
factoring the whole thing into its own function so as not to pollute the
caller qemuDomainDeviceVideoDefPostParse(). Do it now so that the actual
algorithm change later on is in a clean commit by itself and easy to review.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Pavel Mores <pmores@redhat.com>
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:
commit 49288fac96
Author: Peter Krempa <pkrempa@redhat.com>
Date: Wed Oct 9 15:26:37 2019 +0200
util: hash: Add possibility to use simpler data free function in virHash
It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.
After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.
We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that all pieces are in place (hopefully) let's enable -blockdev.
We base the capability on presence of the fix for 'auto-read-only' on
files so that blockdev works properly, mandate that qemu supports
explicit SCSI id strings to avoid ABI regression and that the fix for
'savevm' is present so that internal snapshots work.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The 'savevm' HMP command didn't work properly with blockdev as it tried
to do snapshot of everything including the protocol nodes accessing
files which are not snapshottable. Qemu fixed this bug so now we need to
detect it to allow enabling blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The top level commands now can have 'feature' flags for fixes so add
support for querying those as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Initial implementation of 'auto-read-only' didn't reopen the backing
files when needed. For '-blockdev' to work we need to be able to tel
qemu to open a file read-only and change it during blockjobs as we label
backing chains with a sVirt label which does not allow writing. The
dynamic auto-read-only supports this as it reopens files when writing
is demanded.
Add a capability to detect that the posix file based backends support
the dynamic part.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The qemu driver will obey <backingStore> when we support blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If user starts a blockcommit or a blockcopy then we modify access
for qemu on both images and leave it like that until the job
terminates. So far so good. Problem is, if user instead of
terminating the job (where we would modify the access again so
that the state before the job is restored) calls destroy on the
domain or if qemu dies whilst executing the block job. In this
case we don't ever clear the access we granted at the beginning.
To fix this, maybe a bit harsh approach is used, but it works:
after all labels were restored (that is after
qemuSecurityRestoreAllLabel() was called), we iterate over each
disk in the domain and remove XATTRs from the whole backing chain
and also from any file the disk is being mirrored to.
This would have been done at the time of pivot, but it isn't
because user decided to kill the domain instead. If we don't do
this and leave some XATTRs behind the domain might be unable to
start.
Also, secdriver can't do this because it doesn't know if there is
any job running. It's outside of its scope - the hypervisor
driver is responsible for calling secdriver's APIs.
Moreover, this is safe to call because we don't remember labels
for any member of a backing chain except of the top layer. But
that one was restored in qemuSecurityRestoreAllLabel() call done
earlier. Therefore, not only we don't remember labels (and thus
this is basically a NOP for other images in the backing chain) it
is also safe to call this when no blockjob was started in the
first place, or if some parts of the backing chain are shared
with some other domains - this is NOP, unless a block job is
active at the time of domain destroy.
https://bugzilla.redhat.com/show_bug.cgi?id=1741456#c19
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There are four places where we remove image XATTRs and in all of
them we have the same for() loop with the same body. Move it into
a separate function because I'm about to introduce fifth place
where the same needs to be done.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Install the convertor function which enables the internals that will use
-blockdev to make qemu open the firmware image and stop using -drive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The old way to instantiate a pflash device via -drive was a hack since
it's a platform device.
The modern approach calls for configuring it via -machine and takes the
node name as an argument.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
As a first step we will build the blockdevs which will be supposed to
back the pflash drives when moving away from -drive.
This code is similar to the way we build the blockdevs for the disk, but
skips the copy-on-read layer and doesn't implement any legacy approach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Add a helper which will covert the PFLASH code file and variable file
into the virStorageSource objects stored in private data so that we can
use them with -blockdev while keeping the infrastructure to determine
the path to the loaders intact.
This is a temporary solution until we will want to do snapshots of the
pflash where we will be forced do track the full backing chain in the
XML.
In the meanwhile just convert it partially so that we can stop using
-drive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
To allow converting the pflash drives to blockdev we will need a
virStorageSource to allow using our helpers. Temporarily prior to
coverting loader data to a virStorageSoruce add private data which will
house this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Extract the old way to instantiate pflash devices to hold the firmware
via -drive to a separate function so that it can later be conditionally
disabled when -blockdev will be used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 5751a0b6b1 added a helper function
called virDomainCapsFeaturesInitUnsupported which initialized all domain
capability features as unsupported.
When adding a new feature this would initialize it as unsupported also
for hypervisor drivers which the original author possibly didn't intend
to modify. To prevent accidental wrong value being reported in such case
revert back to initializing individual features in the hypervisor
drivers themselves.
This is not a straight revert as additonal patches modified how we store
the capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting a domain without a CPU model specified in the domain XML,
QEMU will choose a default one. Which is fine unless the domain gets
migrated to another host because libvirt doesn't perform any CPU ABI
checks and the virtual CPU provided by QEMU on the destination host can
differ from the one on the source host.
With QEMU 4.2.0 we can probe for the default CPU model used by QEMU for
a particular machine type and store it in the domain XML. This way the
chosen CPU model is more visible to users and libvirt will make sure
the guest will see the exact same CPU after migration.
Architecture specific notes
- aarch64: We only set the default CPU for TCG domains as KVM requires
explicit "-cpu host" to work.
- ppc64: The default CPU for KVM is "host" thanks to some hacks in QEMU,
we will translate the default model to the model corresponding to the
host CPU ("POWER8" on a Power8 host, "POWER9" on Power9 host, etc.).
This is not a problem as the corresponding CPU model is in fact an
alias for "host". This is probably not ideal, but it's not wrong and
the default virtual CPU configured by libvirt is the same QEMU would
use. TCG uses various CPU models depending on machine type and its
version.
- s390x: The default CPU for KVM is "host" while TCG defaults to "qemu".
- x86_64: The default CPU model (qemu64) is not runnable on any host
with KVM, but QEMU just disables unavailable features and starts
happily.
https://bugzilla.redhat.com/show_bug.cgi?id=1598151https://bugzilla.redhat.com/show_bug.cgi?id=1598162
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU 4.2.0 will report default CPU types used by each machine type and
we will want to start using it.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Almost all TCG query-machines replies match KVM. The only exceptions are
4.2.0 replies on s390x which differ in the reported default CPU type.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some specifics of machine types may depend on the accelerator and thus
the data should be moved to virQEMUCapsAccel. The TCG machine types are
just copied from the ones probed for KVM to simplify the changes to
qemucapabilitiestest data files.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function copies machine type data from one QEMU caps structure to
another.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In preparation for making machine types dependent on the accelerator,
the <machine> elements are formatted between <cpu type='kvm'> and
<cpu type='tcg'>.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the code for formatting machine type data was moved to a standalone
virQEMUCapsFormatMachines function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All the code for loading machine type data was moved to a standalone
virQEMUCapsLoadMachines function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To avoid duplicating code which selects the right virQEMUCapsAccel data
to be filled during probing.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It is a tiny wrapper around virQEMUCapsProbeQMPCPUDefinitions which will
soon get private parameters and thus it cannot be exposed outside
qemu_capabilities.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
And make it use virQEMUCapsGetAccel once rather than repeating the same
code in all functions called from virQEMUCapsFormatAccel.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
And make it use virQEMUCapsGetAccel once rather than repeating the same
code in all functions called from virQEMUCapsLoadAccel.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can be used to get the pointer to all data which depend on
the accelerator.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is container for capabilities data that depend on the accelerator.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The new functions are designed to load and format capabilities which
depend on the accelerator (host CPU expansion and CPU models).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to create a mapping between CPU model names and their
corresponding QOM types.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We will need to keep some QEMU-specific data for each CPU model
supported by a QEMU binary. Instead of complicating the generic
virDomainCapsCPUModelsPtr, we can just directly store
qemuMonitorCPUDefsPtr returned by the capabilities probing code.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Most of the code moved to a new virQEMUCapsFetchCPUDefinitions function
and the existing virQEMUCapsFetchCPUModels just becomes a small wrapper
around virQEMUCapsFetchCPUDefinitions and virQEMUCapsCPUDefsToModels.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The functions return virDomainCapsCPUModelsPtr and thus they should be
called *CPUModels for consistency. Functions called *CPUDefinitions will
work on qemuMonitorCPUDefsPtr.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function translates qemuMonitorCPUDefsPtr (used by QEMU caps probing
code) into virDomainCapsCPUModelsPtr used by domain capabilities.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While virDomainCapsCPUModel structure contains 'usable' field with
virDomainCapsCPUUsable type, the lower level structure specific to QEMU
driver used virTriStateBool for the same thing and we had to translate
between them.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Let's store qemuMonitorCPUDefInfo directly in the array of CPUs in
qemuMonitorCPUDefs rather then using an array of pointers.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It is a container for a CPU models list (qemuMonitorCPUDefInfo) and a
number of elements in this list.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function would return a valid virDomainCapsCPUModelsPtr with empty
CPU models list if query-cpu-definitions exists in QEMU, but returns
GenericError meaning it's not in fact implemented. This behaviour is a
bit strange especially after such virDomainCapsCPUModels structure is
stored in capabilities XML and parsed back, which will result in NULL
virDomainCapsCPUModelsPtr rather than a structure containing nothing.
Let's just keep virDomainCapsCPUModelsPtr NULL if the QMP command is not
implemented and change the return value to int so that callers can
easily check for failure or success.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some callers of virQEMUCapsGetCPUDefinitions will need to filter the
returned list of CPU models. Let's add the filtering parameters directly
to virQEMUCapsGetCPUDefinitions to avoid copying the CPU models list
twice.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than returning a direct pointer the list stored in qemuCaps the
function now creates a new copy of the CPU models list.
The main purpose of this seemingly useless change is to update callers
to free the result returned by virQEMUCapsGetCPUDefinitions because the
internals of this function will change significantly in the following
patches.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new helper in qemuCheckpointDiscard rather than constructing the
array manually.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Another weird bug appeared concerning qemu namespaces. Basically
the problem is as follows:
1) Issue an API that causes libvirt to create a node in domain's
namespace, say /dev/nvme0n1 with 8:0 as major:minor (the API can
be attach-disk for instance). Or simply create the node from a
console by hand.
2) Detach the disk from qemu.
3) Do something that makes /dev/nvme0n1 change it's minor number.
4) Try to attach the disk again.
The problem is, in a few cases - like disk-detach - we don't
remove the corresponding /dev node from the mount namespace
(because it may be used by some other disk's backing chain). But
this creates a problem, because if the node changes its MAJ:MIN
numbers we don't propagate the change into the domain's
namespace. We do plain mknod() and ignore EEXIST which obviously
is not enough because it doesn't guarantee that the node has
updated MAJ:MIN pair.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1752978
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Last usage was removed by commit
<41f88886198e231285cc813f8c0687c8ec5c9488> and commit
<0f4d31720430b4e3735064cc0d8f88a1a438e154> forgot to drop include.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We replaced them by use of transaction to simplify possible failure
scenarios.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Delete/merge bitmaps when deleting checkpoints using a 'transaction' so
that we don't have to deal with halfway-failed scenarios and also fix
access to 'vm' while in the monitor lock.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The @def variable holds pointer to the domain defintion, but is
set only somewhere in the middle of the function. This is
suboptimal.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This flag is not implied by g_mkstemp_full, only by g_mkstemp.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Bjoern Walk <bwalk@linux.ibm.com>
Fixes: 4ac4773040
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemuDomainDefFormatBufInternal function wasn't testing whether the CPU
was actually defined in the XML and saving such a domain resulted in the
following backtrace:
0 in qemuDomainMakeCPUMigratable (cpu=0x0)
1 in qemuDomainDefFormatBufInternal()
2 in qemuDomainDefFormatXMLInternal()
3 in qemuDomainDefFormatLive()
4 in qemuDomainSaveInternal()
5 in qemuDomainSaveFlags()
6 in qemuDomainSave()
7 in virDomainSave()
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit <f136b83139c63f20de0df3285d9e82df2fb97bfc> reworked process
affinity setting but did not take cgroups into account which introduced
an issue when starting VM with custom cpuset.cpus for the whole machine
group.
If the machine group is limited to some pCPUs libvirt should not try to
set a VM to run on all pCPUs as it will result in permission denied when
writing to cpuset.cpus.
To fix this the affinity has to be set separately from cgroups cpuset.
Resolves: <https://bugzilla.redhat.com/show_bug.cgi?id=1746517>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Some layered products such as oVirt have requested a way to avoid being
blocked by guest agent commands when querying a loaded vm. For example,
many guest agent commands are polled periodically to monitor changes,
and rather than blocking the calling process, they'd prefer to simply
time out when an agent query is taking too long.
This patch adds a way for the user to specify a custom agent timeout
that is applied to all agent commands.
One special case to note here is the 'guest-sync' command. 'guest-sync'
is issued internally prior to calling any other command. (For example,
when libvirt wants to call 'guest-get-fsinfo', we first call
'guest-sync' and then call 'guest-get-fsinfo').
Previously, the 'guest-sync' command used a 5-second timeout
(VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT), whereas the actual command that
followed always blocked indefinitely
(VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK). As part of this patch, if a
custom timeout is specified that is shorter than
5 seconds, this new timeout is also used for 'guest-sync'. If there is
no custom timeout or if the custom timeout is longer than 5 seconds, we
will continue to use the 5-second timeout.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
With g_mkstemp_full, there is no need to distinguish between
mkostemp and mkostemps (no suffix vs. a suffix of a fixed length),
because the GLib function looks for the XXXXXX pattern everywhere
in the string.
Use S_IRUSR | S_IWUSR for the permissions and do not pass O_RDWR
in flags since it's implied.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The 'ramfb' attribute provides a framebuffer to the guest that can be
used as a boot display for the vgpu
For example, the following configuration can be used to provide a vgpu
with a boot display:
<hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on' ramfb='on'>
<source>
<address uuid='$UUID'/>
</source>
</hostdev>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
As suggested by Cole, this patch uses the domain capabilities to
validate the supported video model types. This allows us to remove the
model type validation from qemu_process.c and qemu_domain.c and
consolidates it all in a single place that will automatically adjust
when new domain capabilities are added.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Continue consolidation of video device validation started in previous
patch.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
The goal is to move all of the video device validation to a single place
and use domain caps to validate the supported video device models. Since
qemuDomainDeviceDefValidateVideo() is called from
qemuProcessStartValidate(), these changes should not change anny
behavior.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
In a follow-up commit, we will use the domain capabilities to validate
video device configurations, which means that we also need to make sure
that the domain capabilities include the "none" video device.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
commit 9bfcf0f62d added the
QEMU_CAPS_DEVICE_RAMFB capability but did not set the domain capability.
This patch sets the domain capability for the ramfb device and updates
the tests.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
This allows us to simplify the function and avoid jumping to 'cleanup'.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
