Currently we apply a 1:1 mapping between loader type and firmware
type: ROM can only match BIOS and pflash can only match UEFI.
That was accurate at the time when the check was introduced, but
is no longer the case today: the Intel TDX build of edk2, for
example, is loaded as a ROM but it still provides an UEFI
implementation to the guest.
Tweak the matching logic so that a ROM loader is allowed to match
both BIOS and UEFI firmware descriptors.
The firmware-manual-efi-tdx test case benefits from this change,
as all the missing information is now correctly filled in.
This will also solve an issue reported to the list, where
firmware builds targeting the confidential VM use case on aarch64
would not be usable at all, due to the way UEFI and ACPI are
depending on each other on the architecture.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It's possible to have firmware descriptors for builds intended to
be loaded as ROM, as is the case for those loaded as pflash.
There is no reason to skip firmware autoselection in those cases,
and doing so prevents useful information from being filled in.
After this change, the firmware-manual-efi-tdx test case is
augmented with some additional information. Even more information
will be filled in later, when we improve the matching logic.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the hack is gone, a few test cases that were failing
before start succeeding instead.
The firmware-auto-efi-format-nvramtemplate-qcow2 test case
originally passed but produced wrong results, then started
failing once we began taking templateFormat into account, and now
passes once again, finally producing the correct results.
The firmware-auto-efi-format-nvram-raw-loader-path and
firmware-auto-efi-format-nvram-raw-nvramtemplate-path test cases,
on the other hand, never passed before now, because the hack
resulted in information contradicting those provided by the user
being injected into the configuration, which in turn made it
impossible to successfully pick a firmware build. With the hack
gone they can finally succeed, as they should have in the first
place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit d3016e47be removed a hunk very similar to the one we're
adding with the rationale that there is no actual requirement for
the NVRAM file and the NVRAM template to have the same format,
which is completely correct: while libvirt will not perform the
format conversion itself, the user can do that on their own and
everything (except RESET_NVRAM) will work just fine.
That said, we also need <nvram format='foo'/> specified on its
own with no <loader> element to result in a firmware build with a
foo-formatted NVRAM template to be picked. Right now this works
thanks to the hack at the top of qemuFirmwareFillDomain() which
copies nvram.format to loader.format, but we want to get rid of
that because it has additional side effects that can lead to
confusing outcomes in certain specific scenarios.
So reintroduce this check, but make it extremely narrow: if any
other information that can influence firmware selection is
present in the domain XML, ignore the NVRAM format entirely; if
however the NVRAM format is the only information that was
provided, consider it when looking for a match.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the user has specified a desired format for the NVRAM
template, we should take that information into account when
looking for a suitable firmware build instead of ignoring it.
Two test cases start failing as a result of this change.
For firmware-auto-efi-format-nvramtemplate-qcow2, the failure
is temporary and the test case will pass once again with an
upcoming commit. It should be noted that, until now, the selected
firmware used raw, not qcow2, as the NVRAM template format,
meaning that though the test case passed the outcome was not the
desired one.
For firmware-auto-efi-format-mismatch-nvramtemplate, the failure
is desired and the test case should not have succeeded in the
first place, as there are no firmware descriptors for a build
that uses raw format for the executable and qcow2 format for the
NVRAM template.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Right now we throw the entire definition away if the path is
not present, including the format. This effectively results in
discarding user-provided information.
This change fixes the firmware-auto-efi-format-mismatch test
case. Until now, the NVRAM format ended up being raw (matching
the NVRAM template) despite the user explicitly asking for it
to be qcow2 instead.
While this means that libvirt will no longer be able to start
such a VM without user intervention, since it does not
automatically perform conversion between formats, that's still
preferrable to silently overriding an explicit user's request.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In the vast majority of cases they will match, but it just makes
more logical sense to copy the format from the NVRAM template to
the NVRAM file itself.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Instead of setting the format every single time, knowing that we
might throw away the entire definition immediately afterwards,
and duplicating a check, only set it if we are going to perform
an early return due to the rest of the definition being properly
filled in already.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently we're doing a weird dance to avoid overwriting the
user-provided path to the NVRAM template, which might potentially
be one we actually know about but just so happens not to be
listed first. Explaining why we're doing things this way requires
a fairly long comment.
We can make things simpler: if the NVRAM template path is present
in the domain XML, include it into the matching criteria. This is
consistent with how we match firmware descriptors.
Handling of format, both for the firmware executable and the
NVRAM template, is improved too. Legacy paths were used before
non-raw firmware builds existed, so we can set the format to raw
unconditionally.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If an NVRAM template is used, its format should be set too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Simple helper for the case where completely custom firmware paths
are in use. It's quite trivial right now, but it will be expanded
slightly in an upcoming commit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to how we allow the format for the loader and the NVRAM
file to be specified without the corresponding path being present,
we should allow that to happen for the NVRAM template too.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If this information is missing, the parsing code will consider
the firmware descriptor to be invalid and matching against it will
not even be attempted. So we can safely drop this redundant
fallback.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For combined firmware builds, the variable storage is part of the
same image as the executable code, whereas stateless builds don't
support variable storage at all.
In both cases, the use of a separate NVRAM storage area is not
supported, so if attributes connected to one are present in the
domain XML, firmware descriptors for stateless/combined builds
should be ignored.
ROM firmware builds are stateless by definition, so the same
handling applies to them as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The combination doesn't make sense.
After this change the firmware-auto-efi-format-loader-qcow2-rom
test case starts failing, as it should have in the first place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The combination doesn't make sense.
After this change the firmware-manual-bios-rw test case starts
failing, as it should have in the first place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The combination doesn't make sense.
After this change the firmware-manual-bios-rw test cases starts
failing, as it should have in the first place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As explained in the comment that comes along with it, this code
ensures that the user's preference is taken into account when
nvram.format is the only information that's provided. Currently
it lives in the parser, but it makes more sense for it to be
together with the rest of the firmware selection code instead.
Note that this move is not completely seamless: once the code
is moved outside of the parser, it can no longer reliably know
whether the <loader> element actually existed in the domain
XML. The difference is subtle enough that the test suite is
completely unaffected, and we are going to rework the handling
of this scenario in a way that restores the original behavior
later anyway, so it ultimately doesn't matter.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We currently apply this exception, which is critical to ensure
that the correct firmware is selected when working with older VMs,
in the postparse callback.
Move it to the firmware selection process instead, where it should
have been added in the first place.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We access the NVRAM information via the 'loader' local variable
throughout the file, and this is the only spot where the 'nvram'
local variable exists. It makes things inconsistent and opens up
the possibility of the values for 'loader' and 'nvram' going out
of sync, especially after a future commit will introduce the
need to set the former. Just get rid of the additional variable.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I have been able to find exactly zero evidence of this type of
firmware descriptor actually existing in the wild, so this is
essentialy dead code. Dropping it simplifies the task of further
tweaking the firmware selection code.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Per our coding style, function shall be separated by either a
single line or two lines. But in esx_util.c functions are
separated by three or even four blank lines. Drop excessive
spacing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The aim of this helper function is to URI-encode given string
twice. There's a bug (fixed in next commit) in which we're unable
to fetch .vmx file for a domain if corresponding datastore
contains some special characters (like +). Cole Robinson
discovered that encoding datastore twice enables libvirt to work
around the issue [2]. Well, this function does exactly that.
It was tested with the following inputs and all worked
flawlessly: "datastore", "datastore2", "datastore2+",
"datastore3+-@", "data store2+".
1: https://issues.redhat.com/browse/RHEL-134127
2: https://issues.redhat.com/browse/RHEL-133729#comment-28604072
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Currently, domain capabilities reporting is limited to X86.
Enable it for ARM as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Just like consoles, bootloader is handled differently on arm64.
It also does not used the LPC bus, and is configured with:
-o bootrom=/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin
Additionally, fill firmware inforamtion only for amd64.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These arguments control IA32 HLT and PAUSE instructions, so
there are supported only on amd64.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Console device handling in bhyve is different for amd64 and arm64.
On amd64, it's configured as an LPC device, and multiple consoles are
supported.
On arm64, only a single console can be configured, and the syntax is
different:
-o console=/dev/nmdmguest0A
Update the bhyve command generation accordingly.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, guest capabilities have VIR_ARCH_X86_64 hard-coded
for supported guest. As bhyve supports amd64 and arm64 hosts,
and guests' arch must match host's arch, reporting VIR_ARCH_X86_64
for arm64 is wrong.
Set supported guest arch to the same value as the host arch.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Both qemuSaveImageCreate and qemuSnapshotCreateActiveExternalDisks
follow the usual convention of returning 0 on success and -1 on error so
there's no need store the values in 'ret'.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Bhyve supports virtio-scsi devices using the following syntax:
bhyve ... -s N,virtio-scsi,/dev/cam/ctl[pp.vp][,scsi-device-options]
Where /dev/cam/ctl is a ctl(4) device path.
The optional "scsi-device-options" include "iid" (Initiator ID)
and "bootindex", which are currently not used by libvirt.
Model this device using:
<disk type='ctl'>
<source dev='/dev/cam/ctl'/>
<target dev='sda' bus='scsi'/>
</disk>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
CTL stands for CAM Target Layer, and CAM stands for
Common Access Method Storage subsystem, and is available
on FreeBSD.
Quoting the ctl(4) manual page:
The ctl subsystem provides SCSI target devices emulation. It supports
features such as:
• Disk, CD-ROM and processor device emulation
• Tagged queueing
• SCSI task attribute support (ordered, head of queue, simple tags)
• SCSI implicit command ordering support
• Full task management support (abort, query, reset, etc.)
• Support for multiple ports, initiators, targets and backing stores
• Support for VMWare VAAI and Microsoft ODX offload (COMPARE AND WRITE,
XCOPY, POPULATE TOKEN/WRITE USING TOKEN, WRITE SAME and UNMAP)
• Persistent reservation support
• Extensive VPD/mode/log pages support
• Featured error reporting, error injection and basic SMART support
• High Availability clustering support with ALUA
• All I/O handled in-kernel, no userland context switch overhead
This is a preparation for implementing virtio-scsi support for the bhyve
driver.
Co-authored-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The original commit (v11.9.0-rc1~84) added a dependency checking
of VIR_DOMAIN_HYPERV_STIMER on VIR_DOMAIN_HYPERV_VPINDEX
(meaning, if stimer is on then vpindex must also be on). It
justified this by citing QEMU documentation:
Per QEMU documentation (docs/system/i386/hyperv.rst):
``hv-stimer``
Enables Hyper-V synthetic timers. <snip/>
Requires: ``hv-vpindex``, ``hv-synic``, ``hv-time``
While the documentation is almost correct (see previous commit
when it's incorrect), the code express no dependency on vpindex
(kvm_hyperv_properties[] array from target/i386/kvm/kvm.c):
[HYPERV_FEAT_STIMER] = {
.desc = "synthetic timers (hv-stimer)",
.flags = {
{.func = HV_CPUID_FEATURES, .reg = R_EAX,
.bits = HV_SYNTIMERS_AVAILABLE}
},
.dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
},
If transitivity is taken into account then the documentation is
of course correct (minus that one aforementioned special case).
Well, there's no need for us to implement transitional checks.
VIR_DOMAIN_HYPERV_STIMER requires VIR_DOMAIN_HYPERV_SYNIC and
whether that requires VIR_DOMAIN_HYPERV_VPINDEX is another
question.
Just drop the transitive check.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: da261327ea
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Turns out, that synic hyperv enlightenment not always requires
vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
pc-q35-3.0, pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
disabled. This is because they did enable 'x-hv-synic-kvm-only'
CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
property is disabled by default.
To avoid parsing machine type version, let's just drop this
dependency validation and rely on QEMU to report sensible error
message.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: 1822d030c3
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The default in json-c is 32 which is too low to accomodate the 200
snapshot layers we supported historically in the qemu driver (200 is
picked based on the 256 layer limit in libxml).
The response to 'query-block' is otherwise too low and we fail to start
the VM when there's around 26 images in a backing chain.
'json_tokener_new_ex' is supported since json-c 0.11 and we require at
least 0.14.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, to probe PCI devices, for each device libvirt
calls "bhyve -s 0,<device_type>" and parses the error message to check
if this specific device is supported.
For quite some time, bhyve reports the list of devices using:
bhyve -s help
where it prints all supported devices, one device per line.
Update the code to use this command:
* It is more accurate as we don't need to rely on the error message
parsing.
* It's faster as we get all the devices in one run instead of
running bhyve for every device type.
* The code is simpler.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani
Michal Privoznik
Thomas Prescher
Philipp Schuster
Roman Bogorodskiy
Peter Krempa
Nikolay Shirokovskiy