Commit Graph

34755 Commits

Author SHA1 Message Date
Michal Privoznik
15ce9e888b src: Detect close_range syscall during virGlobalInit()
The whole purpose of virCloseRangeInit() is to be called
somewhere during initialization (ideally before first virExec()
or virCommandRun()), so that the rest of the code already knows
kernel capabilities. While I can put the call somewhere into
remote_daemon.c (when a daemon initializes), we might call
virCommand*() even from client library (i.e. no daemon).

Therefore, put it into virGlobalInit() with the rest of
initialization code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2023-08-24 12:45:17 +02:00
Michal Privoznik
520eb3e15b vircommand: Introduce virCommandMassCloseRange()
This is brand new way of closing FDs before exec(). We need to
close all FDs except those we want to explicitly pass to avoid
leaking FDs into the child. Historically, we've done this by
either iterating over all opened FDs and closing them one by one
(or preserving them), or by iterating over an FD interval [2 ...
N] and closing them one by one followed by calling closefrom(N +
1). This is a lot of syscalls.

That's why Linux kernel developers introduced new close_from
syscall. It closes all FDs within given range, in a single
syscall. Since we keep list of FDs we want to preserve and pass
to the child process, we can use this syscall to close all FDs
in between. We don't even need to care about opened FDs.

Of course, we have to check whether the syscall is available and
fall back to the old implementation if it isn't.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2023-08-24 12:45:00 +02:00
Michal Privoznik
dd2eeaad0b vircommand: Unify mass FD closing
We have two version of mass FD closing: one for FreeBSD (because
it has closefrom()) and the other for everything else. But now
that we have closefrom() wrapper even for Linux, we can unify
these two.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2023-08-24 12:44:11 +02:00
Michal Privoznik
06d0a66292 virfile: Introduce virCloseFrom()
It is handy to close all FDs from given FD to infinity. On
FreeBSD the libc even has a function for that: closefrom(). It
was ported to glibc too, but not musl. At least glibc
implementation falls back to calling:

  close_range(from, ~0U, 0);

Now that we have a wrapper for close_range() we implement
closefrom() trivially.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2023-08-24 12:42:45 +02:00
Michal Privoznik
d69237caa3 virfile: Introduce virCloseRange()
Linux gained new close_range() syscall (in v5.9) that allows
closing a range of FDs in a single syscall. Ideally, we would use
it to close FDs when spawning a process (e.g. via virCommand
module).

Glibc has close_range() wrapper over the syscall, which falls
back to iterative closing of all FDs inside the range if running
under older kernel. We don't wane that as in that case we might
just close opened FDs (see Linux version of
virCommandMassClose()). And musl doesn't have close_range() at
all. Therefore, call syscall directly.

Now, mass close of FDs happens in a fork()-ed off child. While it
could detect whether the kernel does support close_range(), it
has no way of passing this info back to the parent and thus each
child would need to query it again and again.

Since this can't change while we are running we can cache the
information - hence virCloseRangeInit().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
2023-08-24 12:41:07 +02:00
Michal Privoznik
6a0b645537 src: Rename some members of _virDomainMemoryDef struct
As advertised earlier, now that the _virDomainMemoryDef struct is
cleaned up, we can shorten some names as their placement within
unions define their use.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:29 +02:00
Michal Privoznik
7d01b67323 src: Move _virDomainMemoryDef target nodes into an union
The _virDomainMemoryDef struct is getting a bit messy. It has
various members and only some of them are valid for given model.
Worse, some are re-used for different models. We tried to make
this more bearable by putting a comment next to each member
describing what models the member is valid for, but that gets
messy too.

Therefore, do what we do elsewhere: introduce an union of structs
and move individual members into their respective groups.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:26 +02:00
Michal Privoznik
f23a991bea src: Move _virDomainMemoryDef source nodes into an union
The _virDomainMemoryDef struct is getting a bit messy. It has
various members and only some of them are valid for given model.
Worse, some are re-used for different models. We tried to make
this more bearable by putting a comment next to each member
describing what models the member is valid for, but that gets
messy too.

Therefore, do what we do elsewhere: introduce an union of structs
and move individual members into their respective groups.

This allows us to shorten some names (e.g. nvdimmPath or
sourceNodes) as their purpose is obvious due to their placement.
But to make this commit as small as possible, that'll be
addressed later.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:23 +02:00
Michal Privoznik
a03b6cedfd qemu_driver: validate mem->model on MEMORY_DEVICE_SIZE_CHANGE event
When guest acknowledges change in size of virtio-mem (portion
that's exposed to the guest), QEMU emits
MEMORY_DEVICE_SIZE_CHANGE event. We process it in
processMemoryDeviceSizeChange(). So far, QEMU emits the even only
for virtio-mem (as that's the only memory device model that
allows live changes to its size). Nevertheless, if this ever
changes, validate the memory model upon processing the event as
the rest of the code blindly expects virtio-mem model.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:21 +02:00
Michal Privoznik
29df55e74d conf: Compare memory device address in virDomainMemoryFindByDefInternal()
This is similar to one of my previous commits. Simply speaking,
users can specify address where a memory device is mapped to. And
as such, we should include it when looking up corresponding
device in domain definition (e.g. on device hot unplug).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:18 +02:00
Michal Privoznik
10d38f46c7 qemu_hotplug: Don't validate inaccessible fields in qemuDomainChangeMemoryLiveValidateChange()
The qemuDomainChangeMemoryLiveValidateChange() function is called
when a live memory device change is requested (via
virDomainUpdateDeviceFlags()). Currently, the only model that is
allowed to change is VIRTIO_MEM (and the only value that's
allowed to change is requestedsize). The aim of the function is
to check whether the change user requested follows this rule. And
in accordance with defensive programming I made the function
check all virDomainMemoryDef struct members. Even those which are
unused for VIRTIO_MEM model.

Drop these checks as the respective members will be inaccessible
soon (as the struct is refined).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:15 +02:00
Michal Privoznik
0b7af96492 qemu_hotplug: validate address on memory device change
As of v7.9.0-rc1~296 users have ability to adjust what portion of
virtio-mem is exposed to the guest. Then, as of v9.4.0-rc2~5 they
have ability to set address where the memory is mapped. But due
to a missing check it was possible to feed
virDomainUpdateDeviceFlags() API  with memory device XML that
changes the address. This is of course not possible and should be
forbidden. Add the missing check.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:13 +02:00
Michal Privoznik
f9dcf98bbd virt-aa-helper: Set label on VIRTIO_PMEM device too
Conceptually, from host POV there's no difference between NVDIMM
and VIRTIO_PMEM. Both expose a file to the guest (which is used
as a permanent storage). Other secdriver treat NVDIMM and
VIRTIO_PMEM the same. Thus, modify virt-aa-helper so that is
appends virtio-pmem backing path into the domain profile too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:39:09 +02:00
Michal Privoznik
109ae46e4a virt-aa-helper: Rework setting virDomainMemoryDef labels
Currently, inside of virt-aa-helper code the domain definition is
parsed and then all def->mems are iterated over and for NVDIMM
models corresponding nvdimmPath is set label on. Conceptually,
this code works (except the label should be set for VIRTIO_PMEM
model too, but that is addressed in the next commit), but it can
be written in more extensible way. Firstly, there's no need to
check whether def->mems[i] is not NULL because we're inside a
for() loop that's counting through def->nmems. Secondly, we can
have a helper variable ('mem') to make the code more readable
(just like we do in other loops). Then, we can use switch() to
allow compiler warn us on new memory model.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-24 12:38:56 +02:00
Ján Tomko
2bad705ebb qemu: remove pointless qemuDomainLogContextMode
Since its introduction in 4d1b771fbb
it has only been used to differentiate between START and non-START.

Last use of QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH was removed by:

  commit f709377301
    qemu: Fix qemuDomainObjTaint with virtlogd

QEMU_DOMAIN_LOG_CONTEXT_MODE_STOP is unused since:

  commit cf3ea0769c
    qemu: process: Append the "shutting down" message using the new APIs

Now, the only caller passes QEMU_DOMAIN_LOG_CONTEXT_MODE_START.
Assume that's always the case and remove the 'mode' argument.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-23 15:25:29 +02:00
Pavel Hrdina
cb3bc96e67 qemu_snapshot: allow snapshot revert for external snapshots
Now that the support to revert external snapshots is implemented we can
drop this check.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:43 +02:00
Pavel Hrdina
88d0e6748d qemu_snapshot: add checks for external snapshot deletion
With the introduction of external snapshot revert support we need to
error out in some cases when trying to delete some snapshots.

If users reverts to non-leaf snapshots and would try to delete it after
the revert is done it would not work currently as this operation would
require using block-stream which is not implemented for now as in this
case the snapshot has two children so the disk files have multiple
overlays.

Similarly if user reverts to non-leaf snapshot and would try to delete
snapshot that is non-leaf but not in currently active snapshot chain we
would still need to use block-commit operation. The issue here is that
in order to do that we would have to start new qemu process with
different domain definition than what is currently used by the domain.
If the current domain would be running it would complicate things even
more so this operation is not yet supported.

If user creates new snapshot after reverting to non-leaf snapshot it
creates a new branch. Deleting snapshot with multiple children will
require block-stream which is not implemented for now.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:42 +02:00
Pavel Hrdina
daf48171f1 qemu_snapshot: check only once if snapshot is external
There will be more external snapshot checks introduced by following
patch so group them together.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:40 +02:00
Pavel Hrdina
32cf4ed5cd qemu_snapshot: update backing store after deleting external snapshot
With introduction of external snapshot revert we will have to update
backing store of qcow images not actively used be QEMU manually.
The need for this patch comes from the fact that we stop and start QEMU
process therefore after revert not all existing snapshots will be known
to that QEMU process due to reverting to non-leaf snapshot or having
multiple branches.

We need to loop over all existing snapshots and check all disks to see
if they happen to have the image we are deleting as backing store and
update them to point to the new image except for images currently used
by the running QEMU process doing the merge operation.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:36 +02:00
Pavel Hrdina
47821b2ff5 qemuDomainGetImageIds: pass domain definition directly
We only need the domain definition from domain object. This will allow
us to use it from snapshot code where we need to pass different domain
definition.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:34 +02:00
Pavel Hrdina
f879eae3e7 virdomainmomentobjlist: introduce virDomainMomentIsAncestor
This new helper will allow us to check if we are able to delete external
snapshot after user did revert to non-leaf snapshot.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:32 +02:00
Pavel Hrdina
1392be7092 qemu_snapshot: remove revertdisks when creating new snapshot
When user creates a new snapshot after reverting to non-leaf snapshot we
no longer need to store the temporary overlays as they will be part of
the VM XMLs stored in the newly created snapshot.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:30 +02:00
Pavel Hrdina
a68b849c51 qemu_snapshot: delete: properly update parent snapshot with revert data
When deleting external snapshot and parent snapshot is the currently
active snapshot as user reverted to it we need to properly update the
parent snapshot metadata.

After the delete is done the new overlay files will be the currently
used files created when snapshot revert was done, replacing the original
overlay files.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:29 +02:00
Pavel Hrdina
7c402396ec qemu_snapshot: add support to delete external snapshot without block commit
When block commit is not needed we can just simply unlink the
disk files.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:27 +02:00
Pavel Hrdina
d84538fbad qemu_snapshot: prepare data for non-active leaf external snapshot deletion
In this case there is no need to run block commit and using qemu process
at all.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:26 +02:00
Pavel Hrdina
a3152a506b qemu_snapshot: add merge to external snapshot delete prepare data
Before external snapshot revert every delete operation did block commit
in order to delete a snapshot. But now when user reverts to non-leaf
snapshot deleting leaf snapshot will not have any overlay files so we
can just simply delete the snapshot images.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:25 +02:00
Pavel Hrdina
b0876595a3 qemu_snapshot: extract external snapshot delete prepare to function
This part of code is about to grow to make deletion work when user
reverts to non-leaf snapshot.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:23 +02:00
Pavel Hrdina
443221bbf1 qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare
The new name reflects that we prepare data for external snapshot
deletion and the old name will be used later for different part of code.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:22 +02:00
Pavel Hrdina
8e66473781 qemu_snapshot: introduce external snapshot revert support
When reverting to external snapshot we need to create new overlay qcow2
files from the disk files the VM had when the snapshot was taken.

There are some specifics and limitations when reverting to a snapshot:

1) When reverting to last snapshot we need to first create new overlay
   files before we can safely delete the old overlay files in case the
   creation fails so we have still recovery option when we error out.

   These new files will not have the suffix as when the snapshot was
   created as renaming the original files in order to use the same file
   names as when the snapshot was created would add unnecessary
   complexity to the code.

2) When reverting to any snapshot we will always create overlay files
   for every disk the VM had when the snapshot was done. Otherwise we
   would have to figure out if there is any other qcow2 image already
   using any of the VM disks as backing store and that itself might be
   extremely complex and in some cases impossible.

3) When reverting from any state the current overlay files will be
   always removed as that VM state is not meant to be saved. It's the
   same as with internal snapshots. If user want's to keep the current
   state before reverting they need to create a new snapshot. For now
   this will only work if the current snapshot is the last.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:20 +02:00
Pavel Hrdina
96fa494f88 qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot
Both creating and deleting snapshot are using VIR_ASYNC_JOB_SNAPSHOT but
reverting is using VIR_ASYNC_JOB_START. Let's unify it to make it
consistent for all snapshot operations.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:18 +02:00
Pavel Hrdina
537f9136b7 qemu_snapshot: move external disk prepare to single function
We will need to reuse the functionality when reverting external
snapshots.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:16 +02:00
Pavel Hrdina
a8f3e2f104 qemuSnapshotCreateQcow2Files: use domain definition directly
To create new overlay files when external snapshot revert support is
introduced we will be using different domain definition than what is
currently used by the domain.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:15 +02:00
Pavel Hrdina
9b94a9e8ab qemu_snapshot: introduce qemuSnapshotCreateQcow2Files
Extract creation of qcow2 files for external snapshots to separate
function as we will need it for external snapshot revert code.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:11 +02:00
Pavel Hrdina
db9e253fc4 qemu_snapshot: use virDomainDiskByName while updating domain def
When creating external snapshot this function is called only when the VM
is not running so there is only one definition to care about. However,
it will be used by external snapshot revert code for active and inactive
definition and they may be different if a disk was (un)plugged only for
the active or inactive definition.

The current code would crash so use virDomainDiskByName() to get the
correct disk from the domain definition based on the disk name and make
sure it exists.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:10 +02:00
Pavel Hrdina
4edd9e297a qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk
Extract the code that updates disks in domain definition while creating
external snapshots. We will use it later in the external snapshot revert
code.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:09 +02:00
Pavel Hrdina
8f9d96199d virDomainSnapshotAlignDisks: Allow overriding user-configured snapshot default
This new option will be used by external snapshot revert code.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:08 +02:00
Pavel Hrdina
206ff69832 snapshot_conf: introduce <revertDisks> metadata element
This new element will hold the new disk overlay created when reverting
to non-leaf snapshot in order to remember the files libvirt created.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:06 +02:00
Pavel Hrdina
a0853c89fe snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames
Commit <ef3f3884a2432958bdd4ea0ce45509d47a91a453> introduced new
argument for virDomainSnapshotAlignDisks() that allows passing alternate
domain definition in case the snapshot parent.dom is NULL.

In case of redefining snapshot it will not hit the part of code that
unconditionally uses parent.dom as there will not be need to generate
default external file names.

It should be still fixed to make it safe. Future external snapshot
revert code will use this to generate default file names and in this
case it would crash.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:05 +02:00
Pavel Hrdina
824e2a4a98 snapshot_conf: export virDomainSnapshotDiskDefClear
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:06:03 +02:00
Pavel Hrdina
aba6f2a941 libvirt_private: list virDomainMomentDefPostParse
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2023-08-22 16:05:59 +02:00
Peter Krempa
ba2b2ad1bb qemu: capabilities: Retire unused QEMU_CAPS_DEVICE_IVSHMEM
qemu removed the support for the old 'ivshmem' device in 4.0 release.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-22 14:04:41 +02:00
Peter Krempa
dc7ca40a13 qemu: Retire 'ivshmem' device
The device was removed in qemu-4.0 and is superseded by 'ivshmem-plain'
and 'ivshmem-doorbell'.

Always report error when the old version is used and drop the irrelevant
tests.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-22 14:04:41 +02:00
Peter Krempa
4bc796a82f qemuValidateDomainVCpuTopology: Always validate vcpu count against topology
Historically we've used QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS as witness
that the topology must cover the maximum number ov vcpus. qemu started
to enforce this in qemu-2.5, thus we can now always do the check.

This change also requires aligning the topology in certain test files.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-08-22 14:04:41 +02:00
Andrea Bolognani
10a8997cbb conf: Don't default to raw format for loader/NVRAM
Due to the way the information is stored by the XML parser, we've
had this quirk where specifying any information about the loader
or NVRAM would implicitly set its format to raw. That is,

  <nvram>/path/to/guest_VARS.fd</nvram>

would effectively be interpreted as

  <nvram format='raw'>/path/to/guest_VARS.fd</nvram>

forcing the use of raw format firmware even when qcow2 format
would normally be preferred based on the ordering of firmware
descriptors. This behavior can be worked around in a number of
ways, but it's fairly unintuitive.

In order to remove this quirk, move the selection of the default
firmware format from the parser down to the individual drivers.

Most drivers only support raw firmware images, so they can
unconditionally set the format early and be done with it; the
QEMU driver, however, supports multiple formats and so in that
case we want this default to be applied as late as possible,
when we have already ruled out the possibility of using qcow2
formatted firmware images.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:34 +02:00
Andrea Bolognani
b845e376a4 qemu: Match NVRAM template extension for new domains
Keep things consistent by using the same file extension for the
generated NVRAM path as the NVRAM template.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:32 +02:00
Andrea Bolognani
e96e322725 qemu: Filter firmware based on loader.readonly
If the user included loader.readonly=no in the domain XML, we
should not pick a firmware build that expects to work with
loader.readonly=yes.

https://bugzilla.redhat.com/show_bug.cgi?id=2196178

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:31 +02:00
Andrea Bolognani
ccbb987707 qemu: Generate NVRAM path in more cases
Right now, we only generate it after finding a matching entry
either among firmware descriptors or in the legacy firmware
list.

Even if the domain is configured to use a custom firmware build
that we know nothing about, however, we should still automatically
generate the NVRAM path instead of requiring the user to provide
it manually.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:30 +02:00
Andrea Bolognani
4a49114ff4 qemu: Don't overwrite NVRAM template for legacy firmware
Just because we have found a matching entry, it doesn't mean
that we should discard the information explicitly provided in
the domain XML.

https://bugzilla.redhat.com/show_bug.cgi?id=2196178
https://gitlab.com/libvirt/libvirt/-/issues/500

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:28 +02:00
Andrea Bolognani
ac76386eda qemu: Fix lookup against stateless/combined pflash
Just like the more common split builds, these are of type
QEMU_FIRMWARE_DEVICE_FLASH; however, they have no associated
NVRAM template, so we can't access the corresponding structure
member unconditionally or we'll trigger a crash.

https://bugzilla.redhat.com/show_bug.cgi?id=2196178

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:20 +02:00
Andrea Bolognani
d917883b30 qemu: Fix return value for qemuFirmwareFillDomainLegacy()
The documentation states that, just like the Modern() variant,
this function should return 1 if a match wasn't found. It
currently doesn't do that, and returns 0 instead.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2023-08-21 13:51:19 +02:00