A recent commit caused the virFDStreamRead method to loop reading data
until the provided buffer is full. Unfortunately the EOF handling was
not quite correct.
* When seeing a virFDStreamMsg with length zero, it would still
loop trying to read more and then get an error that the thread
has quit.
* When seeing a virFDStreamMsg with length zero on subsequent
iterations, it would discard this message, which would in turn
prevent the caller from ever seeing the 'ret == 0' return value
indicating EOF. The caller would then try to read again and get
an error about the stream being closed.
Fixes: e23fd0b7fd
Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The 'ret' variable should only have a value assigned once we have
completely finished reading data, otherwise an error on a subsequent
iteration will report an error but not return a negative value.
Fixes: e23fd0b7fd
Reported-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are some features/improvements/bug fixes I've either
contributed or reviewed/merged. Document them for upcoming
release.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently virDomainNetDef and virDomainActualNetDef use
virDomainHostdevDef directly as structure and the code doesn't call
virDomainHostdevDefNew() that would initialize private data.
This is hackish quick fix to solve a crash that happens in two
scenarios:
1. attaching any interface with hostdev backend
0x0000fffbfc0e2a90 in qemuDomainAttachHostPCIDevice (driver=0xfffbb4006750, vm=0xfffbf001f790, hostdev=0xfffbf400b150) at ../src/qemu/qemu_hotplug.c:1652
1652 if ((ret = qemuFDPassDirectTransferMonitor(hostdevPriv->vfioDeviceFd, priv->mon)) < 0)
2. starting VM with interface with hostdev backend using iommufd
0x00007f6638d5b9ca in qemuProcessOpenVfioDeviceFd (hostdev=hostdev@entry=0x7f6634425ee0) at ../src/qemu/qemu_process.c:7719
7719 hostdevPriv->vfioDeviceFd = qemuFDPassDirectNew(name, &vfioDeviceFd);
Proper fix for this issue is to refactor network code to use pointer and to
use virDomainHostdevDefNew().
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We have a g_autoptr ret in the virIdentityGetSystem function. In the
happy path it is properly returned by doing: return g_steal_pointer(&ret);
There are 2 early return paths, were we do the following: "return ret;"
This leads to the g_autoptr being cleaned up after we leave the
function, as we do not properly "steal" it.
When later using the return value we have a use-after-free, which has
led to segfaults in some cases.
As this is a regression introduced in v5.9.0-rc1~269, we change
the behavior to properly return NULL in those cases.
Fixes: c6825d8813
On-behalf-of: SAP stefan.kober@sap.com
Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We can kinda triangulate it from virConf debugging lines,
but this is nicer IMO
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
When a domain is in post-copy migration phase, we need to keep the job
active if something fails to protect the domain from changes.
Unfortunately, there is a race between migration code and
qemuProcessStop that can cause the job to stay active even when the
domain is gone and thus preventing the domain from being started again
(until virtqemud is restarted). The race is caused by unlocking the vm
object when calling virConnectUnregisterCloseCallback. While the domain
is unlocked qemuProcessStop can finish its work and the domain may no
longer be active when we get the lock back. The post-copy path does not
properly check if a domain is still active.
Instead of adding the virDomainObjIsActive check in all places where
this could happen, we can add it in virDomainObjIsPostcopy and
virDomainObjIsFailedPostcopy and let the code take the pre-copy cleanup
path. Clearly an inactive domain can never be in (failed) post-copy
migration.
https://issues.redhat.com/browse/RHEL-145179
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
A previous commit had moved this logic from the generic
postparse code to drivers but failed to update the bhyve driver
accordingly in the process.
Fixes: 1504b7f687
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Document the fact that the existing flags which apply to
NVRAM files also do the right thing when varstore files are
used instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We are not introducing additional API flags for varstore
handling since that would require unnecessary churn in all
libvirt-based apps, and the intent is the same: recreate
the UEFI variable storage, be it NVRAM or varstore, from its
template.
In order to clarify that the existing flags affect varstore
too, update their documentation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that everything else is in place, we can finally add the
firmware descriptors for the edk2 builds that use the uefi-vars
QEMU device.
Several existing test cases that were failing up until this
point can pass now. This includes firmware-auto-efi-varstore-q35,
firmware-auto-efi-varstore-aarch64 and
firmware-auto-efi-enrolled-keys-aarch64, which were only failing
because a matching firmware descriptor could not be found.
firmware-manual-efi-varstore-aarch64 also passes now, because
with the firmware descriptor in place libvirt is able to figure
out that the manually-provided path corresponds to a UEFI
firmware build, which means that the use of ACPI is fine.
The test cases using older version of QEMU still fail, as is
expected, though the error message is now slightly different and
reflect the actual reason why that is.
The qemufirmware and domaincaps tests are updated in the
expected ways. In particular, versions QEMU 10.0 and newer now
advertise varstore support as available.
https://issues.redhat.com/browse/RHEL-82645
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Before this, attempting to use a ROM that was not explictly
marked at read only resulted in an error at startup time.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Simply mimicking what is currently done for NVRAM files does
the trick. A few user-visible messages are updated to reflect
the fact that they apply both to NVRAM and varstore.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Most of the code in the qemuPrepareNVRAMFile() function can
be reused to create a varstore file from template. Move the
common parts to a generic helper, leaving only the parts
that are NVRAM-specific in the original function.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce qemuFirmwareEnsureVarstore(), which performs the same
task as the existing qemuFirmwareEnsureNVRAM() but for the
varstore element.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is the same as the existing nvramDir, except it will be
used to store the files used with the uefi-vars QEMU device.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
If the matching firmware requires the use of varstore, we
have to bubble up information about it, namely the path to
the template. If the struct member doesn't exist yet, we need
to allocate it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Stateful ROMs are those that use the uefi-vars QEMU device to
implement access to UEFI variable storage.
Matching works much the same as it does for pflash-based
firmware images. Notably, the <varstore> element is only
allowed for ROM and the <nvram> element is only allowed for
pflash.
The firmware-auto-efi-varstore-q35 and
firmware-auto-efi-varstore-aarch64 fail in a different way
after this change: the input XML is now considered valid, and
the only remaining issue is that the firmware autoselection
process is unable to find a match.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently we only look at the value for the stateless attribute
itself when matching, but the <nvram> element being included in
the input XML is likewise a clear sign that a stateless firmware
build will not satisfy the user's requirements.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The element should only be advertised as supported if the QEMU
binary contains the necessary device and a suitable JSON firmware
descriptor is found on the system. Right now the latter
requirement is not satisfied, so it's marked as not supported
across the board.
The qemufirmware test is extended to cover the new attribute.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This was not necessary until now since ROMs couldn't have an
associate NVRAM template, and technically speaking they still
can't; however, the varstore template serves essentialy the
same purpose.
The qemuFirmwareGetSupported() helper is used in two places:
one is the code that is responsible for filling in domaincaps,
where templates are ignored so this change has no impact on it;
the other is the qemufirmware test program, where this value
being reported is useful as it will allow us to confirm that
the JSON firmware descriptors for uefi-vars enabled builds are
parsed correctly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The existing syntax can only describe stateless firmware builds,
while the extended one can additionally describe builds intended
for use with the uefi-vars device. This involves including the
path to the corresponding varstore template.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Just like with firmware builds targeting the confidential use
case, use of the uefi-vars device obviates the need to have SMM
emulation enabled while still guaranteeing that protected EFI
variables work as intended.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The two checks are semantically different, so it makes sense to
perform them separately. We will soon extend the first one.
While at it, start printing out the value of isConfidential. We
could print the value of each firmware feature it's derived from,
but that would make things unnecessarily verbose; at the same
time, knowing that libvirt believes that the firmware build is
targeting the confidential use case can be useful for debugging
so it's worth including it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When present in a firmware descriptor, this feature indicates that
the corresponding executable expects to access variable storage
through the uefi-vars QEMU device.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jiri Denemark
Peter Krempa
Andi Chandler
Daniel P. Berrangé
Roman Bogorodskiy
Michal Privoznik
Jonathon Jongsma
Pavel Hrdina
Stefan Kober
Cole Robinson
Haorong Xu
Fco. Javier F. Serrador
Weblate
Pavel Borecki
Jim Spentzos
Remus-Gabriel Chelu
Luna Jernberg
Daniel Nylander
Hosted Weblate
Languages add-on
Andrea Bolognani