This reverts commit 06c960e477de4561c7ba956f82994fa120226397.
Turns out, this feature is not needed and QEMU will fix TSC
without any intervention from outside.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>P
QEMU 7.0.0 adds a new property tsc-clear-on-reset to x86 CPU, corresponding
to Libvirt's <tsc on_reboot="clear"/> element. Plumb it in the validation,
command line handling and tests.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It makes sense to have these in the same file as the definitions
of enums.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These enums are essentially the same and always sorted in the
same order in every hypervisor with jobs. They can be generalized
by using the qemu enums as the main ones as they are the most
extensive.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I think the code looks cleaner without else branches.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Let's generate prealloc-threads property onto the cmd line if
domain configuration requests so.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Only fairly new QEMUs are capable of user provided number of
preallocation threads. Validate this assumption.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The prealloc-threads is property of memory-backend class which is
parent to the other three classes memory-backend-{ram,file,memfd}.
Therefore the property is present for all, or none if QEMU is
older than v5.0.0-rc0~75^2~1^2~3 which introduced the property.
Anyway, the .reserve property is the same story, and we chose
memory-backend-file to detect it, so stick with our earlier
decision and use the same backend to detect this new property.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In case we are snapshotting at least one 'manual' disk we will pause the
VM and keep it paused.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1866400
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The idea of the manual mode is to allow a synchronized snapshot in cases
when the storage is outsourced to an unmanaged storage provider which
requires cooperation with snapshotting.
The mode will instruct the hypervisor to pause along when the other
components are snapshotted and the 'manual' disk can be snapshotted
along. This increases latency of the snapshot but allows them in
otherwise impossible situations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code parsing thue query-cpu-definitions response will short-circuit
the for loop in the case where usable=yes, resulting in us failing to
parse the CPU deprecation flag.
IOW, we only reported deprecations in domain capabilities for CPU models
which were not runnable on the host.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The qemuProcessQMPStop() function is intended to kill this dummy
QEMU process we started only for querying capabilities.
Nevertheless, it may be not plain QEMU binary we executed, but
in fact it may be a memcheck tool (e.g. valgrind) that executes
QEMU later. By switching to virProcessKillPainfully() we allow
this wrapper tool to exit gracefully.
Another up side is that virProcessKillPainfully() reports an
error so no need for us to VIR_ERROR() ourselves.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It does not make sense to have both of these, since one of them
is only a wrapper for the other one. I decided to preserve the
more general one, which requires only virDomainObj and rewrote it
a bit, so that it pulls the qemu driver from privateData.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This allows init job even if cb structure is not set. This patch
also includes slight rewriting of the function to make it look
cleaner when freeing resources, by allocating privateData at the
end.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We should allow resetting / freeing / restoring / parsing /
formatting qemuDomainJobObj even if 'cb' attribute is not set.
This is theoretical for now, but the attribute must not be always
set in the future. It is sufficient to check if 'cb' exists
before dereferencing it.
This commit partially reverts af16e754cd4efc3ca1.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On domain startup a couple of devices are allowed in the devices
controller no matter the domain configuration. The aim is to
allow devices crucial for QEMU or one of its libraries, or user
is passing through a device (e.g. through additional cmd line
arguments) and wants QEMU to access it.
However, during unplug it may happen that a device is configured
to use one of such devices and since we deny /dev nodes on
hotplug we would deny such device too. For example,
/dev/urandom belongs onto the list of implicit devices and users
can hotplug and hotunplug an RNG device with /dev/urandom as
backend.
The fix is fortunately simple - just consult the list of implicit
devices before removing the device from the namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In all cases virCgroupDenyDevicePath() is followed by
virDomainAuditCgroupPath(). Might as well pack that into one
function and call it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In all cases virCgroupAllowDevicePath() is followed by
virDomainAuditCgroupPath(). Might as well pack that into one
function and call it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When allowing or denying RNG device in CGroups there's a special
check if the backend device exists (errno == ENOENT) in which
case success is returned to caller. This is in contrast with the
rest of the functions and in fact wrong too - if the backend
device doesn't exist then QEMU will fail opening it. Might as
well signal error here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
When creating /dev nodes in a QEMU domain's namespace the first
thing we simply do is unlink() the path and create it again. This
aims to solve the case when a file changed type/major/minor in
the host and thus we need to reflect this in the guest's
namespace. Fair enough, except we can be a bit more clever about
it: firstly check whether the path doesn't already exist or isn't
already of the correct type/major/minor and do the
unlink+creation only if needed.
Currently, this is implemented only for symlinks and
block/character devices. For regular files/directories (which are
less common) this might be implemented one day, but not today.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When building namespace for a domain there are couple of devices
that are created independent of domain config (see
qemuDomainPopulateDevices()). The idea behind is that these
devices are crucial for QEMU or one of its libraries, or user is
passing through a device and wants us to create it in the
namespace too. That's the reason that these devices are allowed
in the devices CGroup controller as well.
However, during unplug it may happen that a device is configured
to use one of such devices and since we remove /dev nodes on
hotplug we would remove such device too. For example,
/dev/urandom belongs onto the list of implicit devices and users
can hotplug and hotunplug an RNG device with /dev/urandom as
backend.
The fix is fortunately simple - just consult the list of implicit
devices before removing the device from the namespace.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Autofree the temporary string and shuffle around the success path to
avoid the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The NBD connection for non-shared storage migration can have the same
issue regarding TLS certificate name match as the migration connection
itself.
Propagate the configured name also for the NBD connections.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1901394
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We do support non-shared storage migration with TLS now. Fix the comment
claiming otherwise.
Fixes: a8dc146a4d14b6f169f27b6d238452beb254a637
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Detect that qemu can override TLS hostname setting for NBD clients.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The block of code pausing the VM assigns 'resume' to true but it's
already true because of the previous condition.
The code is deliberately kept in two blocks as upcoming changes will
modify both conditions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All callers except the one in the 'esx' driver pass the flag. The 'esx'
driver has a check that 'def->ndisks' is zero after parsing the
definition. This means that we can simply always parse the disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to the external snapshot code the internal inactive snapshot
creation helper should act only when an internal snapshot of the disk is
required. For now the callers ensure that it's either _INTERNAL or _NO
when control reaches this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The string value associated to the enum is "no". Rename the enum
accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use an if/else branch rather than a expression with a ternary operator.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Preparation steps ensure that the 'snapshot' field can only be
'VIR_DOMAIN_SNAPSHOT_LOCATION_NONE' or
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL' at this point, but upcoming
patches will change that. Handle only external snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the virNWFilterBinding APIs are using the nwfilter
update lock directly, there is no need for the virt drivers
to do it themselves.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
That is the proper POSIX way.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add support for sending one FD from the client along with a monitor
command so that it's possible to use 'getfd' and 'add-fd' to use FDs
passed from the client with other QMP commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuDomainPrepareDiskSourceData' propagates 'detect_zeroes' only for
the disk source image, but the mirror destination has the ambition to
replace the disk source when the job is finished, so we need to
propagate the 'detect_zeroes' setting also in that case.
Unfortunately it would become very hairy to either set 'disk->mirror'
sooner or propagate that we want this done into
'qemuDomainPrepareDiskSourceData', so the most straightforward solution
is to do the propagation inside 'qemuDomainBlockCopyCommon'.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/277
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently the 'nvram_template' entry is mandatory when parsing the
firmware descriptor based on flash. QEMU is extending the firmware
descriptor spec to make the 'nvram_template' optional, depending
on the value of a new 'mode' field:
- "split"
* "executable" contains read-only CODE
* "nvram_template" contains read-write VARS
- "combined"
* "executable" contains read-write CODE and VARs
* "nvram_template" not present
- "stateless"
* "executable" contains read-only CODE and VARs
* "nvram_template" not present
In the latter case, the guest OS can write vars but the
firmware will make no attempt to persist them, so any changes
will be lost at poweroff.
For now we parse this new 'mode' but discard any firmware
which is not 'mode=split' when matching for a domain.
In the tests we have a mixture of files with and without the
mode attribute.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When undefining a VM, we must optionally delete any NVRAM that might
exist. When using firmware auto-select we always check the generated
path, ignoring any user specified path.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
After v8.0.0-466-g08101bde5d we unconditionally regenerate per
domain NVRAM path even though it might have been parsed earlier
from domain XML. The way we do that leads to a memleak:
43 bytes in 1 blocks are definitely lost in loss record 330 of 682
at 0x483F7E5: malloc (vg_replace_malloc.c:381)
by 0x50D5B18: g_malloc (in /usr/lib64/libglib-2.0.so.0.7000.2)
by 0x50EFA4F: g_strdup (in /usr/lib64/libglib-2.0.so.0.7000.2)
by 0x49E774E: virXPathString (virxml.c:88)
by 0x4A3F0E4: virDomainDefParseBootLoaderOptions (domain_conf.c:18226)
by 0x4A3F49C: virDomainDefParseBootOptions (domain_conf.c:18298)
by 0x4A448C3: virDomainDefParseXML (domain_conf.c:19598)
by 0x4A487A1: virDomainDefParseNode (domain_conf.c:20404)
by 0x117FCF: testCompareXMLToArgv (qemuxml2argvtest.c:726)
by 0x142124: virTestRun (testutils.c:142)
by 0x1423D4: virTestRunLog (testutils.c:197)
by 0x140A76: mymain (qemuxml2argvtest.c:3406)
If we parsed NVRAM path from domain XML we must refrain from
generating new path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In case when a user starts a block copy operation with
VIR_DOMAIN_BLOCK_COPY_SHALLOW and VIR_DOMAIN_BLOCK_COPY_REUSE_EXT and
both the reused image and the original disk have a backing image libvirt
specifically does not insert the backing image until after the job is
asked to be completed via virBlockJobAbort with
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT.
This is so that management applications can copy the backing image on
the background.
Now when a user aborts the block job instead of cancelling it we'd
ignore the fact that we didn't insert the backing image yet and the
cancellation would result into a 'blockdev-del' of a invalid node name
and thus an 'error' severity entry in the log.
To solve this issue we use the same conditions when the backing image
addition is avoided to remove the internal state for them prior to the
call to unplug the mirror destination.
Reported-by: Kashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When building the default memory backend (which has id='pc.ram')
and no guest NUMA is configured then
qemuBuildMemCommandLineMemoryDefaultBackend() is called. However,
its return value is ignored which means that on invalid
configuration (e.g. when non-existent hugepage size was
requested) an error is reported into the logs but QEMU is started
anyway. And while QEMU does error out its error message doesn't
give much clue what's going on:
qemu-system-x86_64: Memory backend 'pc.ram' not found
While at it, introduce a test case. While I could chose a nice
looking value (e.g. 4MiB) that's exactly what I wanted to avoid,
because while such value might not be possible on x84_64 it may
be possible on other arches (e.g. ppc is notoriously known for
supporting wide range of HP sizes). Let's stick with obviously
wrong value of 5MiB.
Reported-by: Charles Polisher <chas@chasmo.org>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is to make it explicit that the template only applies to the NVRAM
store, not the main loader binary, even if the loader is writable.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Before creating a NVRAM path, the qemuDomainNVRAMPathGenerate
method checks whether the config is using the old style
firmware approach. This check is redundant in one of the two
callers. By inlining the check into the other caller, it makes
it clearer to understand that the NVRAM path filling is done
conditionally.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
