11:25 a.m.
See 2/2 for info.
Michal Prívozník (2):
qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
qemu_firmware: Try to autofill for old style UEFI specification
src/qemu/qemu_firmware.c | 90 +++++++++++++++++++++++++++++++++-------
src/qemu/qemu_firmware.h | 2 +-
src/qemu/qemu_process.c | 2 +-
3 files changed, 78 insertions(+), 16 deletions(-)
--
2.24.1
11:25 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
This function needs domain definition really, we don't need to
pass the whole domain object. This saves couple of dereferences
and characters esp. in more checks to come.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 12 ++++++------
src/qemu/qemu_firmware.h | 2 +-
src/qemu/qemu_process.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index f62ce90ac9..96058c9b45 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1204,7 +1204,7 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
int
qemuFirmwareFillDomain(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+ virDomainDefPtr def,
unsigned int flags)
{
VIR_AUTOSTRINGLIST paths = NULL;
@@ -1217,7 +1217,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
if (!(flags & VIR_QEMU_PROCESS_START_NEW))
return 0;
- if (vm->def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
+ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
return 0;
if ((nfirmwares = qemuFirmwareFetchParsedConfigs(driver->privileged,
@@ -1225,7 +1225,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
return -1;
for (i = 0; i < nfirmwares; i++) {
- if (qemuFirmwareMatchDomain(vm->def, firmwares[i], paths[i])) {
+ if (qemuFirmwareMatchDomain(def, firmwares[i], paths[i])) {
theone = firmwares[i];
VIR_DEBUG("Found matching firmware (description path
'%s')",
paths[i]);
@@ -1236,7 +1236,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
if (!theone) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("Unable to find any firmware to satisfy
'%s'"),
- virDomainOsDefFirmwareTypeToString(vm->def->os.firmware));
+ virDomainOsDefFirmwareTypeToString(def->os.firmware));
goto cleanup;
}
@@ -1245,10 +1245,10 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
* likely that admin/FW manufacturer messed up. */
qemuFirmwareSanityCheck(theone, paths[i]);
- if (qemuFirmwareEnableFeatures(driver, vm->def, theone) < 0)
+ if (qemuFirmwareEnableFeatures(driver, def, theone) < 0)
goto cleanup;
- vm->def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
+ def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_firmware.h b/src/qemu/qemu_firmware.h
index 4be65bc664..37cbfae39d 100644
--- a/src/qemu/qemu_firmware.h
+++ b/src/qemu/qemu_firmware.h
@@ -45,7 +45,7 @@ qemuFirmwareFetchConfigs(char ***firmwares,
int
qemuFirmwareFillDomain(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+ virDomainDefPtr def,
unsigned int flags);
int
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7e1db50e8f..6d4511a49f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6344,7 +6344,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
goto cleanup;
VIR_DEBUG("Prepare bios/uefi paths");
- if (qemuFirmwareFillDomain(driver, vm, flags) < 0)
+ if (qemuFirmwareFillDomain(driver, vm->def, flags) < 0)
goto cleanup;
if (qemuDomainInitializePflashStorageSource(vm) < 0)
goto cleanup;
--
2.24.1
4:29 p.m.
New subject: [libvirt] [PATCH 1/2] qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
On 12/17/19 12:25 PM, Michal Privoznik wrote:
This function needs domain definition really, we don't need to
pass the whole domain object. This saves couple of dereferences
and characters esp. in more checks to come.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 12 ++++++------
src/qemu/qemu_firmware.h | 2 +-
src/qemu/qemu_process.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
11:25 a.m.
New subject: [libvirt] [PATCH 2/2] qemu_firmware: Try to autofill for old style UEFI specification
While we discourage people to use the old style of specifying
UEFI for their domains (the old style is putting path to the FW
image under /domain/os/loader/ whilst the new one is using
/domain/os/@firmware), some applications might have not adopted
yet. They still rely on libvirt autofilling NVRAM path and
figuring out NVRAM template when using the old way (notably
virt-install does this). And in a way they are right. However,
since we really want distro maintainers to leave
--with-loader-nvram configure option and rely on JSON
descriptors, we need to implement autofilling of NVRAM template
for the old way too.
Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778
RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 82 +++++++++++++++++++++++++++++++++++-----
1 file changed, 72 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 96058c9b45..a31bde5d04 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -935,17 +935,53 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
const qemuFirmware *fw,
const char *path)
{
- size_t i;
+ qemuFirmwareOSInterface want = QEMU_FIRMWARE_OS_INTERFACE_NONE;
bool supportsS3 = false;
bool supportsS4 = false;
bool requiresSMM = false;
bool supportsSEV = false;
+ size_t i;
+
+ switch (def->os.firmware) {
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
+ want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ break;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
+ want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ break;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
+ break;
+ }
+
+ if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
+ def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE &&
+ def->os.loader) {
+ switch (def->os.loader->type) {
+ case VIR_DOMAIN_LOADER_TYPE_ROM:
+ want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ break;
+ case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+ want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ break;
+ case VIR_DOMAIN_LOADER_TYPE_NONE:
+ case VIR_DOMAIN_LOADER_TYPE_LAST:
+ break;
+ }
+
+ if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
+ STRNEQ(def->os.loader->path,
fw->mapping.data.flash.executable.filename)) {
+ VIR_DEBUG("Not matching FW interface %s or loader "
+ "path '%s' for user provided path '%s'",
+ qemuFirmwareDeviceTypeToString(fw->mapping.device),
+ fw->mapping.data.flash.executable.filename,
+ def->os.loader->path);
+ return false;
+ }
+ }
for (i = 0; i < fw->ninterfaces; i++) {
- if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_BIOS) ||
- (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_UEFI))
+ if (fw->interfaces[i] == want)
break;
}
@@ -1211,14 +1247,33 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
qemuFirmwarePtr *firmwares = NULL;
ssize_t nfirmwares = 0;
const qemuFirmware *theone = NULL;
+ bool needResult = true;
size_t i;
int ret = -1;
if (!(flags & VIR_QEMU_PROCESS_START_NEW))
return 0;
- if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
- return 0;
+ /* Fill in FW paths if either os.firmware is enabled, or
+ * loader path was provided with no nvram varstore. */
+ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
+ /* This is horrific check, but loosely said, if UEFI
+ * image was provided by the old method (by specifying
+ * its path in domain XML) but no template for NVRAM was
+ * specified ... */
+ if (!(def->os.loader &&
+ def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
+ def->os.loader->path &&
+ def->os.loader->readonly == VIR_TRISTATE_BOOL_YES &&
+ !def->os.loader->templt &&
+ def->os.loader->nvram &&
+ !virFileExists(def->os.loader->nvram)))
+ return 0;
+
+ /* ... then we want to consult JSON FW descriptors first,
+ * but we don't want to fail if we haven't found a match. */
+ needResult = false;
+ }
if ((nfirmwares = qemuFirmwareFetchParsedConfigs(driver->privileged,
&firmwares, &paths)) <
0)
@@ -1234,9 +1289,16 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
}
if (!theone) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Unable to find any firmware to satisfy
'%s'"),
- virDomainOsDefFirmwareTypeToString(def->os.firmware));
+ if (needResult) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Unable to find any firmware to satisfy
'%s'"),
+ virDomainOsDefFirmwareTypeToString(def->os.firmware));
+ } else {
+ VIR_DEBUG("Unable to find NVRAM template for '%s', "
+ "falling back to old style",
+ NULLSTR(def->os.loader ? def->os.loader->path : NULL));
+ ret = 0;
+ }
goto cleanup;
}
--
2.24.1
4:50 p.m.
New subject: [libvirt] [PATCH 2/2] qemu_firmware: Try to autofill for old style UEFI specification
On 12/17/19 12:25 PM, Michal Privoznik wrote:
While we discourage people to use the old style of specifying
UEFI for their domains (the old style is putting path to the FW
image under /domain/os/loader/ whilst the new one is using
/domain/os/@firmware), some applications might have not adopted
yet. They still rely on libvirt autofilling NVRAM path and
figuring out NVRAM template when using the old way (notably
virt-install does this). And in a way they are right. However,
since we really want distro maintainers to leave
--with-loader-nvram configure option and rely on JSON
descriptors, we need to implement autofilling of NVRAM template
for the old way too.
Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778
RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
So this uses firmware.json to info to turn this input XML
<loader readonly="yes" type="pflash">/CODE.fd</loader>
into this output XML
<loader readonly="yes" type="pflash">/CODE.fd</loader>
<nvram template="/VARS.fd"/>
independent of --with-loader-nvram, which was previously used to handle
that case. And virt-install still uses this method by default. Is that
correct?
I'm surprised we don't have an an XML test case to cover this. How hard
is it to add?
src/qemu/qemu_firmware.c | 82
+++++++++++++++++++++++++++++++++++-----
1 file changed, 72 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 96058c9b45..a31bde5d04 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -935,17 +935,53 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
const qemuFirmware *fw,
const char *path)
{
- size_t i;
+ qemuFirmwareOSInterface want = QEMU_FIRMWARE_OS_INTERFACE_NONE;
bool supportsS3 = false;
bool supportsS4 = false;
bool requiresSMM = false;
bool supportsSEV = false;
+ size_t i;
+
+ switch (def->os.firmware) {
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
+ want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ break;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
+ want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ break;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
+ break;
+ }
+
This is a refactoring that could have been done first. It's hard to
digest all the details in this patch.
+ if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
+ def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE &&
+ def->os.loader) {
+ switch (def->os.loader->type) {
+ case VIR_DOMAIN_LOADER_TYPE_ROM:
+ want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ break;
+ case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+ want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ break;
+ case VIR_DOMAIN_LOADER_TYPE_NONE:
+ case VIR_DOMAIN_LOADER_TYPE_LAST:
+ break;
+ }
+
And it might be overkill but it would help readability if these switches
were moved to their own functions, like
qemuFirmwareTypeFromInterfaceType or similar
+ if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
+ STRNEQ(def->os.loader->path,
fw->mapping.data.flash.executable.filename)) {
+ VIR_DEBUG("Not matching FW interface %s or loader "
+ "path '%s' for user provided path
'%s'",
+ qemuFirmwareDeviceTypeToString(fw->mapping.device),
+ fw->mapping.data.flash.executable.filename,
+ def->os.loader->path);
+ return false;
+ }
+ }
for (i = 0; i < fw->ninterfaces; i++) {
- if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_BIOS) ||
- (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_UEFI))
+ if (fw->interfaces[i] == want)
break;
}
@@ -1211,14 +1247,33 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
qemuFirmwarePtr *firmwares = NULL;
ssize_t nfirmwares = 0;
const qemuFirmware *theone = NULL;
+ bool needResult = true;
size_t i;
int ret = -1;
if (!(flags & VIR_QEMU_PROCESS_START_NEW))
return 0;
- if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
- return 0;
+ /* Fill in FW paths if either os.firmware is enabled, or
+ * loader path was provided with no nvram varstore. */
+ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
+ /* This is horrific check, but loosely said, if UEFI
+ * image was provided by the old method (by specifying
+ * its path in domain XML) but no template for NVRAM was
+ * specified ... */
+ if (!(def->os.loader &&
+ def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
+ def->os.loader->path &&
+ def->os.loader->readonly == VIR_TRISTATE_BOOL_YES &&
+ !def->os.loader->templt &&
+ def->os.loader->nvram &&
+ !virFileExists(def->os.loader->nvram)))
+ return 0;
+
This loader checking logic is duplicated in a few places already, see
all 4 of 5 PFLASH uses in qemu_domain.c and similar checks in xen code.
IMO it should be factored out first
- Cole
10:23 a.m.
New subject: [libvirt] [PATCH 2/2] qemu_firmware: Try to autofill for old style UEFI specification
On 12/18/19 11:50 PM, Cole Robinson wrote:
On 12/17/19 12:25 PM, Michal Privoznik wrote:
> While we discourage people to use the old style of specifying
> UEFI for their domains (the old style is putting path to the FW
> image under /domain/os/loader/ whilst the new one is using
> /domain/os/@firmware), some applications might have not adopted
> yet. They still rely on libvirt autofilling NVRAM path and
> figuring out NVRAM template when using the old way (notably
> virt-install does this). And in a way they are right. However,
> since we really want distro maintainers to leave
> --with-loader-nvram configure option and rely on JSON
> descriptors, we need to implement autofilling of NVRAM template
> for the old way too.
>
> Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778
> RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
So this uses firmware.json to info to turn this input XML
<loader readonly="yes"
type="pflash">/CODE.fd</loader>
into this output XML
<loader readonly="yes"
type="pflash">/CODE.fd</loader>
<nvram template="/VARS.fd"/>
independent of --with-loader-nvram, which was previously used to handle
that case. And virt-install still uses this method by default. Is that
correct?
Use of --with-loader-nvram will trigger a warning at compile time and
use of nvram=[] in qemu.conf will trigger a warning at runtime.
And we need to fill nvram template (in the code it's loader->templt) to
avoid looking it up qemuPrepareNVRAM(). This is what <os
firmware='uefi'/> does too.
So the way this is supposed to work is:
- FW descriptors are preferred, if a NVRAM template is found in a FW
descriptor file it's used,
- to give distro maintainers time to switch, --with-loader-nvram is not
going away just yet, and will be consulted if the previous point failed,
- nvram=[] from qemu.conf is ignored loudly.
I think this is reasonable order. Also, note that the template is
generated into live XML only, and will be generated only once - when
NVRAM template is needed, i.e. at the first boot. The second time the
domain is started the horrific check I'm introducing in
qemuFirmwareFillDomain() will trigger 'return 0' and thus no template
will be filled in.
I'm surprised we don't have an an XML test case to cover this. How hard
is it to add?
The thing is, we don't hit the problem when generating the command line
but later in the process in qemuPrepareNVRAM() which is called from
qemuProcessPrepareHost() which is not called from tests. And I don't
think we have a test that checks if live XML generated from an inactive
one matches expected one.
> src/qemu/qemu_firmware.c | 82 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index 96058c9b45..a31bde5d04 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -935,17 +935,53 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
> const qemuFirmware *fw,
> const char *path)
> {
> - size_t i;
> + qemuFirmwareOSInterface want = QEMU_FIRMWARE_OS_INTERFACE_NONE;
> bool supportsS3 = false;
> bool supportsS4 = false;
> bool requiresSMM = false;
> bool supportsSEV = false;
> + size_t i;
> +
> + switch (def->os.firmware) {
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
> + want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
> + break;
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
> + want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
> + break;
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
> + break;
> + }
> +
This is a refactoring that could have been done first. It's hard to
digest all the details in this patch.
> + if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
> + def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE &&
> + def->os.loader) {
> + switch (def->os.loader->type) {
> + case VIR_DOMAIN_LOADER_TYPE_ROM:
> + want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
> + break;
> + case VIR_DOMAIN_LOADER_TYPE_PFLASH:
> + want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
> + break;
> + case VIR_DOMAIN_LOADER_TYPE_NONE:
> + case VIR_DOMAIN_LOADER_TYPE_LAST:
> + break;
> + }
> +
And it might be overkill but it would help readability if these switches
were moved to their own functions, like
qemuFirmwareTypeFromInterfaceType or similar
> + if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
> + STRNEQ(def->os.loader->path,
fw->mapping.data.flash.executable.filename)) {
> + VIR_DEBUG("Not matching FW interface %s or loader "
> + "path '%s' for user provided path
'%s'",
> + qemuFirmwareDeviceTypeToString(fw->mapping.device),
> + fw->mapping.data.flash.executable.filename,
> + def->os.loader->path);
> + return false;
> + }
> + }
>
> for (i = 0; i < fw->ninterfaces; i++) {
> - if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS &&
> - fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_BIOS) ||
> - (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
> - fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_UEFI))
> + if (fw->interfaces[i] == want)
> break;
> }
>
> @@ -1211,14 +1247,33 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
> qemuFirmwarePtr *firmwares = NULL;
> ssize_t nfirmwares = 0;
> const qemuFirmware *theone = NULL;
> + bool needResult = true;
> size_t i;
> int ret = -1;
>
> if (!(flags & VIR_QEMU_PROCESS_START_NEW))
> return 0;
>
> - if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
> - return 0;
> + /* Fill in FW paths if either os.firmware is enabled, or
> + * loader path was provided with no nvram varstore. */
> + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
> + /* This is horrific check, but loosely said, if UEFI
> + * image was provided by the old method (by specifying
> + * its path in domain XML) but no template for NVRAM was
> + * specified ... */
> + if (!(def->os.loader &&
> + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH
&&
> + def->os.loader->path &&
> + def->os.loader->readonly == VIR_TRISTATE_BOOL_YES &&
> + !def->os.loader->templt &&
> + def->os.loader->nvram &&
> + !virFileExists(def->os.loader->nvram)))
> + return 0;
> +
This loader checking logic is duplicated in a few places already, see
all 4 of 5 PFLASH uses in qemu_domain.c and similar checks in xen code.
IMO it should be factored out first
Okay, I will try to work these into v2.
Thanks,
Michal
1784
days inactive
1804
days old
5 comments
2 participants
participants (2)
-
Cole Robinson -
Michal Privoznik