The direct SCSI pool doesn't expose the volumes in the host attempting
to match it using 'virStoragePoolObjSourceMatchTypeDEVICE' which in turn
uses 'virStoragePoolSourceFindDuplicateDevices' doesn't make sense.
Remove it from the source matching completely as we can open multiple
connections to the target.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the inner loop to automatically free temporary variables and
remove unreachable error paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Value of '0' is treated equivalently to when it's not provided by the
user. Reject an explicit '0' provided by the user as it would get
ignored.
In this rare case we can make the XML parser more strict, as libvirt
would never format the '<acpi/>' element if the index is '0' thus there
are no libvirt-generated XMLs we'd not load back, as of such this is
identical to rejecting it in the validation phase.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2037146
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
No kernels supported by upstream libvirt have the feature.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The qemuFirmwareOSInterfaceTypeFromOsDefFirmware method
was added to convert from virDomainOsDefFirmware to the
qemuFirmwareOSInterface enum.
It was later also used to convert from virDomainLoader
to qemuFirmwareOSInterface in:
commit 8e1804f9f66f13ca1412d22bf1a957b6d55a2365
Author: Michal Prívozník <mprivozn@redhat.com>
Date: Tue Dec 17 17:45:50 2019 +0100
qemu_firmware: Try to autofill for old style UEFI specification
This caused compile errors with clang due to passing a
mis-matched enum type. These were later silenced by
stripping the enum types:
commit 8fcee47807d29008632a7ad918cbe93ac0a20597
Author: Michal Prívozník <mprivozn@redhat.com>
Date: Wed Jan 8 09:42:47 2020 +0100
qemu_firmware: Accept int in qemuFirmwareOSInterfaceTypeFromOsDefFirmware()
This is still rather confusing to humans reading the
code. It is clearer to just define a separate helper
method for the virDomainLoader type conversion.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'virDomainSnapshotRedefinePrep' does everything needed for a redefine
when the snapshot exists but not when we are defining metadata for a new
snapshot. This gives us weird semantics.
Extract the code for replacing the definition of an existing snapshot
into a new helper 'virDomainSnapshotReplaceDef' and refactor all
callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than callers second-guessing when the snapshot definition is
assigned turn it into a double pointer and clear it on success.
Fix callers to work with the new semantics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It is not expected that a function with 'Validate' in the name actually
modifies the validated object, even worse when it even modifies another
object and the ultimatively worst bit is that it doesn't undo the mess
if the validation fails midway.
Move the stealing of the domain definition from the definition of a
snapshot being redefined into the caller along with the call to
virDomainSnapshotAlignDisks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We use this variable name to distinguish it from the domain definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function isn't used outside of src/conf/snapshot_conf.c as of
57d252c74
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Due to historical reasons we allow users to redefine an existing
snapshot without providing the domain definition which would correspond
to it. In such case we'd use the domain definition from the snapshot
that is being redefined.
To prevent callers from doing complex moving of the domain definition
object back and forth between the snapshot definitions we can add an
argument to virDomainSnapshotAlignDisks which will allow us to pass in
the alternate definition if the one from the snapshot is missing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'require_match' set to true is only needed for internal snapshots taken
by hypervisors (qemu) which don't have a way to control which disks take
part in the snapshot (savevm).
To de-clutter callers we can change the argument to mean 'this code path
requires uniform snapshot for internal snapshots'.
Change the argument and fix the callers. For now all callers pass 'true'
but any new hypervisor or even usage in qemu is not going to share the
limitation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the appropriate type for the variable and fix all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add description of arguments, reword the description for clarity, and
fix improper argument names mentioned in the existing description.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
External snapshot with memory is created without using the
VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY flag, but rather with properly
configuring the XML. When redefining the code should be checking the
same thing as by definition an external snapshot with memory is not a
disk-only snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove error handling from the call to 'virDomainMomentObjNew' as it
can't return NULL and replace 'virHashAddEntry' by 'g_hash_table_insert'
as we've already checked that snapshot with such name doesn't exist in
the hash table. This removes handling for two impossible errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit takes care of following cases:
-> Check availability of requested ports.
->The total number of requested ports should not be more than
VIR_MAX_ISA_SERIAL_PORTS.
->The ports requested should be less than VIR_MAX_ISA_SERIAL_PORTS.
->VIR_MAX_ISA_SERIAL_PORTS should correspond to MAX_ISA_SERIAL_PORTS
specified in QEMU code commit def337ffda34d331404bd7f1a42726b71500df22.
-> Prevent duplicate device assignments to the same port.
-> In case no ports are provided in the XML, this patch scans the list of unused
isa-serial indices to automatically assign available ports for this VM.
Signed-off-by: Divya Garg <divya.garg@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These changes make the g_auto-ification in the next commit clearer.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Export 'virDomainMomentObjNew' and 'virDomainMomentObjFree' and define
the latter as autoptr cleanup function for 'virDomainMomentObj'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Format the function prototypes the same way as in the .c file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are few places in virDomainDefFormatFeatures() which can
use virXMLFormatElement() or virXMLFormatElementEmpty() instead
of writing directly into the output buffer.
After this, there are still a lot of places left, but that is
much bigger task.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The virDomainTPMDefFormat() function can't fail really. There's
no point in it returning an integer then. Make it return void and
fix both places which check for its retval.
And while at it, turn @def into a const pointer to make it
obvious the function does not modify passed struct.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The <tpm/> element formatting is handled in
virDomainTPMDefFormat() which uses the "old style" - appending
strings directly into the output buffer. With this, it's easy to
get conditions that tell when an element has ended wrong. In this
particular case, if both <encryption/> and <active_pcr_banks/>
are to be formatted the current code puts a stray '>' into the
output buffer, resulting in invalid XML.
Rewrite the function to use virXMLFormatElement() which is more
clever.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2016599#c15
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Changing <active_pcr_banks/> means changing the guest ABI and as
such must be prevented on both restoring from a file or
migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2035888
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
QCOW2 images now support 'extended_l2' which splits the default clusters
into 32 subcluster allocation units. This allows the allocation units to
be smaller without increasing the size of L2 table too much and thus also
the cache requirements for holding the full L2 table in memory.
Unfortunately it's incompatible with qemu versions older than 5.2 thus
can't be used as default.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Normally the SEV measurement only covers the firmware
loader contents. When doing a direct kernel boot, however,
with new enough OVMF it is possible to ask for the
measurement to cover the kernel, ramdisk and command line.
It can't be done automatically as that would break existing
guests using direct kernel boot with old firmware, so there
is a new XML setting allowing this behaviour to be toggled.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'virDomainDiskInsert' orders the inserted disks by target. If the target
is not provided though it would try to parse it anyways. This lead to a
crash when parsing a definition where there are multiple disks and of
two disks sharing the bus at least one also misses the target.
Since we want to actually use the parser for stuff which doesn't
necessarily need the disk target, we make virDomainDiskInsert tolerant
of missing target instead. The definition will be rejected by the
validator regardless of the order the disks were inserted in.
Fixes: 61fd7174
Closes: https://gitlab.com/libvirt/libvirt/-/issues/257
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are limits on the number of SEV/SEV-ES guests that can
be run on machines, which may be influenced by firmware
settings. This is important to expose to users.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, this attribute may either have a value of "custom", or be absent
(which defaults to "custom"), for backwards compatibility.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Dirty ring feature was introduced in qemu-6.1.0, this patch
add the corresponding feature named 'dirty-ring', which enable
dirty ring feature when starting VM.
To enable the feature, the following XML needs to be added to
the guest's domain description:
<features>
<kvm>
<dirty-ring state='on' size='xxx'>
</kvm>
</features>
If property "state=on", property "size" must be specified, which
should be power of 2 and range in [1024, 65526].
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In future commits we will need to store not just an array of
VIR_TRISTATE_SWITCH_* but also an additional integer. Follow the
example of TCG and introduce a structure where both the array an
integer can live.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It may come handy to be able to tweak TCG options, in this
specific case the size of translation block cache size (tb-size).
Since we can expect more knobs to tweak let's put them under
common element, like this:
<domain>
<features>
<tcg>
<tb-cache unit='MiB'>128</tb-cache>
</tcg>
</features>
</domain>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function can't fail at this point. Remove the return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use automatic memory clearing for the temporary strings and bitmap and
remove the cleanup section. There are multiple temporary strings added
so that we don't reuse one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
