Our coding style suggests 80 chars per line with error messages
being exception (for easier git-grep). Apply this exception onto
the newly created domain_postparse.c file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The domain post parse functions currently live in domain_conf.c
which thus grows always larger. Mimic what we've done for the
validation code and move the post parse code into a separate
file: domain_postparse.c.
I've started by moving every function with PostParse in its name
into the new file and then compile hunting for helper functions
only to move them as well.
In the end, I've moved virDomainDefPostParse symbol in
libvirt_private.syms into a new section. And while
virDomainDeviceDefPostParseOne() is made 'public' in
domain_postparse.h too, I'm not exporting it because it has no
caller outside src/conf/ and it's unlikely it ever will.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virDomainDefPostParseDeviceIteratorData struct is exported in
domain_conf.h because it's used in both domain_conf.c and
domain_validate.c. However, the latter usage is not warranted,
it's just a shortcut so that we don't have to introduce a similar
struct just for domain_validate.c. Well, do the extra step and
introduce a separate structure for domain_validate.c. This allows
us to move post parse code later on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To match the XML roots domainCapabilities and storagepoolCapabilities,
the wildcards should be *domainCap* and *storagepoolCap*.
Fixes: 7b0e2e4a55
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For a qcow2 storage volume with luks encryption created by qemu-img, its
dumped storage vol XML has no secret element in encryption:
<volume type='file'>
...
<encryption format='luks'>
</encryption>
...
</volume>
That will cause a failure in rng validation. Fix that validation failure.
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In my previous commit I've introduced virDomainIOMMUDefValidate()
function with a switch() statement. However, two cases in it,
though empty, were not terminated with a break statement which
made compiler complain.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Per v8.3.0-rc1~199 it's only a virtio IOMMU that can have
<address/>. The rest (Intel and SMMUv3) are system devices and
thus have no address associated with them. However, this
assumption is never checked for.
Fixes: b0eb1e193f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The commit v8.3.0-rc1~199 introduced <address/> to <iommu/>
device. And while it updated the RNG it forgot to update the
docs. Fix that.
Fixes: b0eb1e193f
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are two places where the @model member of
_virDomainIOMMUDef struct is typecasted to virDomainIOMMUModel
which is completely unnecessary because the struct already
defines the member of that type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The storage pool tests have host-specific versions which I neglected to
update in commit c44930d932 thus breaking
the test-suite on non-linux OSes.
Fixes: c44930d932
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Treat the 'protocolVer' field as a string so that e.g. '4.1' can be
used. Forbid only ',' in the string as it's a separator of arguments for
mount options.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Provide simple recipes for the most common high-level tasks.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
No sane firmware build will fail this check, but just to be on
the safe side let's check anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, a firmware configuration such as
<os firmware='efi'>
<firmware>
<feature enabled='yes' name='enrolled-keys'/>
</firmware>
</os>
will correctly pick a firmware that implements the Secure Boot
feature and initialize the NVRAM file so that it contains the
keys necessary to enforce the signing requirements. However, the
lack of a
<loader secure='yes'/>
element makes it possible for pflash writes to happen outside
of SMM mode. This means that the authenticated UEFI variables
where the keys are stored could potentially be overwritten by
malicious code running in the guest, thus making it possible to
circumvent Secure Boot.
To prevent that from happening, automatically turn on the
loader.secure feature whenever a firmware that implements Secure
Boot is chosen by the firmware autoselection logic. This is
identical to the way we already automatically enable SMM in such
a scenario.
Note that, while this is technically a guest-visible change, it
will not affect migration of existings VMs and will not prevent
legitimate guest code from running.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Regardless of whether firmware autoselection is in use, we
still want to parse the list of requested features. Doing this
will allow us to produce better error messages.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Generally speaking, when firmware autoselection is in use we
don't want any information to be provided manually. There are
two exceptions:
* we still want the path to the NVRAM file to be customizable;
* using <loader secure='yes'/> was how you would ask for a
firmware that implements the Secure Boot feature in the
original approach to firmware autoselection, so we want to
keep that working.
Anything else should result in a descriptive error.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/327
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This makes it explicit that there are two possible scenarios
(whether or not firmware autoselection is in use) and will make
upcoming changes cleaner to implement.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently we're simply ignoring some elements and attributes,
such as the loader path, when firmware autoselection is enabled
because we know we're not going to use them.
This makes sense, but has the unfortunate consequence of
confusing users who experience part of their configuration
simply going away for no apparent reason.
A more user-friendly approach is to produce meaningful error
messages in those scenarios. As a first step towards that goal,
stop conditionally parsing information.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This combination doesn't make sense and so the firmware
autoselection logic will not be able to find a suitable firmware,
but it's more user-friendly to report a detailed error upfront.
Note that this check would ideally happen in the validate phase,
but if we moved it there we would no longer be able to
automatically enable secure-boot when enrolled-keys=yes. Since
the combination never resulted in a working configuration, the
chances of this causing real-world VMs to disappear are
extremely low.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are currently no failure scenarios for the function, but
we're about to add one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The latter doesn't make sense without the former, so make that
visible in the XML.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently, the lack of a <loader> element results in the <nvram>
element being completely ignored, but this is unnecessarily
limiting: even when firmware autoselection is in use, it should
be possible for the user to specify a custom path for the NVRAM
file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This makes the function more consistent with
virDomainLoaderDefParseXML() by preferring the virXMLProp
class of functions to XPath access.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We're going to start passing multiple nodes to the function in
a moment, so we need a more specific name.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
All the data in the <nvram> element ends up in the same struct
as that coming from the <loader> element, so it makes sense to
have a single entry point for parsing an XML document into a
virDomainLoaderDef instance.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It belongs to virDomainLoaderDefParseXMLNvram(), where the other
parts of the <nvram> element are handled.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When the 'type' attribute is present we'd end up overwriting
this value via virDomainStorageSourceParse(). Moving this
assignment makes the current code clearer and will also help
with upcoming changes.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The previous name was identical, modulo the case, to the
completely unrelated virDomainNVRAMDefParseXML().
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pure code movement, needed to prepare for upcoming changes.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Note that some of these new tests are displaying incorrect or
suboptimal behavior. When we address those in upcoming patches,
this will be highlighted by changes in the test data.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This currently has not effect whatsoever, so it's just cluttering
the input files.
We're going to add specific handling for this scenario, as well
as a test case covering it, in an upcoming commit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This does the opposite of
commit 392292cd99
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Wed Feb 23 12:45:51 2022 +0000
tests: don't use auto-generated NVRAM path in tests
in order to minimize input files.
We're going to add a test case specifically covering the use of
custom NVRAM paths with firmware autoselection in an upcoming
commit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When testing firmware selection, we don't really care about any
of the hardware assigned to the VM, and in fact it's better to
keep it as minimal as possible to make sure that the focus
remains on the firmware bits.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Group all tests related to firmware selection together and give
them consistent names that leave room for further tests to be
added in an upcoming commit.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This was introduced in
commit 5882064084
Author: Martin Kletzander <mkletzan@redhat.com>
Date: Wed Feb 25 15:45:26 2015 +0100
tests: Add test for os interleaving
to ensure a recent change in the schema was behaving correctly.
Seven years later, it no longer seems very useful to keep it
around.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This simplifies the test data without negatively impacting test
coverage.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The pci-bridge-many-disks test case is not related to firmware
handling at all, so we can trim it without losing any coverage.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This supports sockets created by libvirt and passed by FD using the
same method as in security_dac.c.
Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
According to [1]:
Prior to GnuTLS 3.6.0 for the ephemeral or anonymous
Diffie-Hellman (DH) TLS ciphersuites the application was
required to generate or provide DH parameters. That is no
longer necessary as GnuTLS utilizes DH parameters and
negotiation from [RFC7919].
This allows us to:
a) drop the code that's setting DH params,
b) drop @dhParams member from _virNetTLSContext struct. and
c) drop gnutls_dh_params_generate2() mock.
1: https://www.gnutls.org/manual/html_node/Parameter-generation.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Released almost 5 years ago, gnutls-3.6.0 brings some important
features (which are utilized in next commit). Hence, require that
version at least.
Per repology, currently shipped versions are:
RHEL-8: 3.6.16
RHEL-9: 3.7.3
Debian 11: 3.7.1
Debian 12: 3.7.6
openSUSE Leap 15.3: 3.6.7
Ubuntu LTS 20.04: 3.6.13
Ubuntu LTS 22.04: 3.7.3
FreeBSD 12: 3.7.6
Fedora 34: 3.7.4
Fedora 35: 3.7.6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ever since v7.6.0-rc1~235 we can use ovs-vsctl to set QoS instead
of tc. However, we don't have a test that's verifying generated
cmd line for ovs-vsctl.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Our coding style expects a long line to be broken into shorter
lines which are then aligned on the first character, for
instance:
"some string that's broken "
"into multiple lines"
However, one can argue that there are few cases where shifting
the alignment makes the code more readable. And this is the case
of expected cmd line for DO_TEST_SET() where a long cmd line can
be aligned on the arguments rather than the binary:
TC " filter ..."
" police ..."
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The last usage of the testMinimalStruct struct was removed in
v1.2.2-rc1~206 which forgot to remove the struct as well. Remove
it now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
