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>
This test case demonstrates a flaw in the XML validation process.
ROM images are by definition in raw format, so attempting to use
any other format should have resulted in the domain XML being
rejected.
The issue will be addressed in an upcoming commit.
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>
This test case demonstrates a flaw in the XML validation process.
ROM images are by definition read-only, so attempting to use one
as read/write should have resulted in the domain XML being
rejected.
The issue will be addressed in an upcoming commit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This test cases demonstrates that the firmware autoselection
process is unable to find a BIOS image that is read/write.
This is expected, as BIOS is loaded as ROM and is thus by
definition read-only.
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>
This test case demonstrates a flaw in the XML validation process.
Read/write firmware images already contain an area dedicated to
variable storage, which they use, so attempting to use a separate
NVRAM file together with them should have resulted in the domain
XML being rejected.
The issue will be addressed in an upcoming commit.
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>
QEMU 10.2 is released so we can bump our dump to the final state.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'rdseed' CPU features stopped to be presented after updating my host
despite building the same qemu. Re-generate the dump separately from the
update to shield out this difference.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
Type=oneshot means that the service is only considered started after the
main process exits. In setups where libvirt-guests is configured to be
used and imposes a delay on the startup of guests, this will cause
systemd to report a quite high boot time as it will wait until all
guests have booted, plus all the delay between them. High boot times are
often reported as issues in monitoring systems, and require operator
attention, while in this case there is not really an issue with the
system, it's just doing what it was configured to do.
With Type=exec, the service is considered started right after the main
process is *started*, but the boot process will not wait for it.
Having other units depend on libvirt-guests.service finishing was
already not reliable because one cannot know how long the OS inside each
guest takes to boot, and libvirt-guests already does not wait for them
to be in any particular state.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Antonio Terceiro <terceiro@debian.org>
The g_base64_decode function will return a valid pointer, but with
length of zero when it fails to decode data. Report an error in that
scenario, so avoid a later more obsecure error.
eg old behaviour
# virsh secret-set-value f52a81b2-424e-490c-823d-6bd4235bc507 foo
warning: Passing secret value as command-line argument is insecure!
error: Failed to set secret value
error: value in virSecretSetValue must not be NULL
new behaviour
# /home/berrange/virsh secret-set-value f52a81b2-424e-490c-823d-6bd4235bc507 foo
warning: Passing secret value as command-line argument is insecure!
error: Secret value is not valid base64
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The UUID had only 11 characters in the last component, instead of the
required 12. Add the missing trailing '2' which is present in all the
other matching example UUIDs.
Reviewed-by: Arun Menon <armenon@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add an example of 250 layer deep nested JSON to make sure the parser
supports it. This is in order to maintain compatibility with external
snapshots in qemu, where such a deeply nested document is returned with
a 'query-block' QMP call.
I've used a fake JSON as a real reply from qemu is around 1.4MiB for a
200 deep image chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani
Zhenzhong Duan
Praveen K Paladugu
Peter Krempa
Michal Privoznik
Thomas Prescher
Philipp Schuster
Roman Bogorodskiy
Nikolay Shirokovskiy
Antonio Terceiro
Daniel P. Berrangé