7:43 a.m.
v2 of:
https://www.redhat.com/archives/libvir-list/2019-December/msg01076.html
diff to v1:
- split 2/2 in the original series into 3 patches as suggested by Cole
Note, 1/4 is reviewed, but I haven't pushed it yet and sending it here
for completeness.
Michal Prívozník (4):
qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
qemu_firmware: Introduce @want variable to qemuFirmwareMatchDomain()
src: Introduce and use virDomainDefHasOldStyleUEFI() and
virDomainDefHasOldStyleROUEFI()
qemu_firmware: Try to autofill for old style UEFI specification
src/bhyve/bhyve_driver.c | 3 +-
src/conf/domain_conf.c | 16 ++++++++
src/conf/domain_conf.h | 6 +++
src/libvirt_private.syms | 2 +
src/libxl/libxl_conf.c | 3 +-
src/libxl/xen_xl.c | 8 ++--
src/qemu/qemu_domain.c | 14 ++-----
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_firmware.c | 80 +++++++++++++++++++++++++++++++++-------
src/qemu/qemu_firmware.h | 2 +-
src/qemu/qemu_process.c | 2 +-
11 files changed, 103 insertions(+), 36 deletions(-)
--
2.24.1
7:43 a.m.
New subject: [libvirt] [PATCH v2 1/4] 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>
Reviewed-by: Cole Robinson <crobinso(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 6a76d355f5..b9bb1df179 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1202,7 +1202,7 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
int
qemuFirmwareFillDomain(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+ virDomainDefPtr def,
unsigned int flags)
{
VIR_AUTOSTRINGLIST paths = NULL;
@@ -1215,7 +1215,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,
@@ -1223,7 +1223,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]);
@@ -1234,7 +1234,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;
}
@@ -1243,10 +1243,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 1e128d1d83..4195042194 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6267,7 +6267,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
return -1;
VIR_DEBUG("Prepare bios/uefi paths");
- if (qemuFirmwareFillDomain(driver, vm, flags) < 0)
+ if (qemuFirmwareFillDomain(driver, vm->def, flags) < 0)
return -1;
if (qemuDomainInitializePflashStorageSource(vm) < 0)
return -1;
--
2.24.1
8:53 a.m.
New subject: [libvirt] [PATCH v2 1/4] qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
On Tue, Jan 07, 2020 at 02:43:22PM +0100, 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>
Reviewed-by: Cole Robinson <crobinso(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: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:43 a.m.
New subject: [libvirt] [PATCH v2 2/4] qemu_firmware: Introduce @want variable to qemuFirmwareMatchDomain()
This simplifies condition when matching FW interface by having a
single line condition instead of multiline one. Also, it prepares
the code for future expansion.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index b9bb1df179..a3305d5554 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -928,22 +928,39 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
}
+static qemuFirmwareOSInterface
+qemuFirmwareOSInterfaceTypeFromOsDefFirmware(virDomainOsDefFirmware fw)
+{
+ switch (fw) {
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
+ return QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
+ return QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
+ break;
+ }
+
+ return QEMU_FIRMWARE_OS_INTERFACE_NONE;
+}
+
+
static bool
qemuFirmwareMatchDomain(const virDomainDef *def,
const qemuFirmware *fw,
const char *path)
{
size_t i;
+ qemuFirmwareOSInterface want;
bool supportsS3 = false;
bool supportsS4 = false;
bool requiresSMM = false;
bool supportsSEV = false;
+ want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware);
+
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;
}
--
2.24.1
8:54 a.m.
New subject: [libvirt] [PATCH v2 2/4] qemu_firmware: Introduce @want variable to qemuFirmwareMatchDomain()
On Tue, Jan 07, 2020 at 02:43:23PM +0100, Michal Privoznik wrote:
This simplifies condition when matching FW interface by having a
single line condition instead of multiline one. Also, it prepares
the code for future expansion.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1:08 p.m.
New subject: [libvirt] [PATCH v2 2/4] qemu_firmware: Introduce @want variable to qemuFirmwareMatchDomain()
On Tue, Jan 07, 2020 at 02:43:23PM +0100, Michal Privoznik wrote:
This simplifies condition when matching FW interface by having a
single line condition instead of multiline one. Also, it prepares
the code for future expansion.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_firmware.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index b9bb1df179..a3305d5554 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -928,22 +928,39 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
}
+static qemuFirmwareOSInterface
+qemuFirmwareOSInterfaceTypeFromOsDefFirmware(virDomainOsDefFirmware fw)
+{
+ switch (fw) {
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
+ return QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
+ return QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
+ case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
+ break;
+ }
+
+ return QEMU_FIRMWARE_OS_INTERFACE_NONE;
+}
+
+
static bool
qemuFirmwareMatchDomain(const virDomainDef *def,
const qemuFirmware *fw,
const char *path)
{
size_t i;
+ qemuFirmwareOSInterface want;
bool supportsS3 = false;
bool supportsS4 = false;
bool requiresSMM = false;
bool supportsSEV = false;
+ want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware);
clang isn't happy with this as its passing the wrong enum
../../src/qemu/qemu_firmware.c:964:77: error: implicit conversion from enumeration type
'virDomainLoader' to different enumeration type 'virDomainOsDefFirmware'
[-Werror,-Wenum-conversion]
want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~^~~~
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
3:20 a.m.
New subject: [libvirt] [PATCH v2 2/4] qemu_firmware: Introduce @want variable to qemuFirmwareMatchDomain()
On 1/7/20 8:08 PM, Daniel P. Berrangé wrote:
On Tue, Jan 07, 2020 at 02:43:23PM +0100, Michal Privoznik wrote:
> This simplifies condition when matching FW interface by having a
> single line condition instead of multiline one. Also, it prepares
> the code for future expansion.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_firmware.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index b9bb1df179..a3305d5554 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -928,22 +928,39 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
> }
>
>
> +static qemuFirmwareOSInterface
> +qemuFirmwareOSInterfaceTypeFromOsDefFirmware(virDomainOsDefFirmware fw)
> +{
> + switch (fw) {
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
> + return QEMU_FIRMWARE_OS_INTERFACE_BIOS;
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
> + return QEMU_FIRMWARE_OS_INTERFACE_UEFI;
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
> + case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
> + break;
> + }
> +
> + return QEMU_FIRMWARE_OS_INTERFACE_NONE;
> +}
> +
> +
> static bool
> qemuFirmwareMatchDomain(const virDomainDef *def,
> const qemuFirmware *fw,
> const char *path)
> {
> size_t i;
> + qemuFirmwareOSInterface want;
> bool supportsS3 = false;
> bool supportsS4 = false;
> bool requiresSMM = false;
> bool supportsSEV = false;
>
> + want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware);
clang isn't happy with this as its passing the wrong enum
../../src/qemu/qemu_firmware.c:964:77: error: implicit conversion from enumeration type
'virDomainLoader' to different enumeration type 'virDomainOsDefFirmware'
[-Werror,-Wenum-conversion]
want =
qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~^~~~
Yeah, I've noticed this in our CI and I just pushed the fix:
8fcee47807 qemu_firmware: Accept int in
qemuFirmwareOSInterfaceTypeFromOsDefFirmware()
Michal
7:43 a.m.
New subject: [libvirt] [PATCH v2 3/4] src: Introduce and use virDomainDefHasOldStyleUEFI() and virDomainDefHasOldStyleROUEFI()
These functions are meant to replace verbose check for the old
style of specifying UEFI with a simple function call.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 3 +--
src/conf/domain_conf.c | 16 ++++++++++++++++
src/conf/domain_conf.h | 6 ++++++
src/libvirt_private.syms | 2 ++
src/libxl/libxl_conf.c | 3 +--
src/libxl/xen_xl.c | 8 +++-----
src/qemu/qemu_domain.c | 14 ++++----------
src/qemu/qemu_driver.c | 3 +--
8 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index a764b4d4ed..5b8fba7467 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -704,8 +704,7 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn,
if (def->os.bootloader == NULL &&
def->os.loader) {
- if ((def->os.loader->readonly != VIR_TRISTATE_BOOL_YES) ||
- (def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH)) {
+ if (!virDomainDefHasOldStyleROUEFI(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Only read-only pflash is supported."));
goto cleanup;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ee33b7caf0..64d7af9e3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31510,6 +31510,22 @@ virDomainDefHasMdevHostdev(const virDomainDef *def)
}
+bool
+virDomainDefHasOldStyleUEFI(const virDomainDef *def)
+{
+ return def->os.loader &&
+ def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH;
+}
+
+
+bool
+virDomainDefHasOldStyleROUEFI(const virDomainDef *def)
+{
+ return virDomainDefHasOldStyleUEFI(def) &&
+ def->os.loader->readonly == VIR_TRISTATE_BOOL_YES;
+}
+
+
/**
* virDomainGraphicsDefHasOpenGL:
* @def: domain definition
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e012975fca..e6b06a8eba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3673,6 +3673,12 @@ virDomainDefHasVFIOHostdev(const virDomainDef *def);
bool
virDomainDefHasMdevHostdev(const virDomainDef *def);
+bool
+virDomainDefHasOldStyleUEFI(const virDomainDef *def);
+
+bool
+virDomainDefHasOldStyleROUEFI(const virDomainDef *def);
+
bool
virDomainGraphicsDefHasOpenGL(const virDomainDef *def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9185e49fda..ec1054cdc3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -311,6 +311,8 @@ virDomainDefHasMdevHostdev;
virDomainDefHasMemballoon;
virDomainDefHasMemoryHotplug;
virDomainDefHasNVMeDisk;
+virDomainDefHasOldStyleROUEFI;
+virDomainDefHasOldStyleUEFI;
virDomainDefHasUSB;
virDomainDefHasVcpusOffline;
virDomainDefHasVFIOHostdev;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 1be2a789d5..2488bb9d32 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -545,8 +545,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
* future, Xen will support a user-specified firmware path. See
* http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg01628.html
*/
- if (def->os.loader &&
- def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH)
+ if (virDomainDefHasOldStyleUEFI(def))
b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF;
if (def->emulator) {
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index ebcea41d1c..91b1825399 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1228,11 +1228,9 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
if (xenConfigSetString(conf, "builder", "hvm") < 0)
return -1;
- if (def->os.loader &&
- def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) {
- if (xenConfigSetString(conf, "bios", "ovmf") < 0)
- return -1;
- }
+ if (virDomainDefHasOldStyleUEFI(def) &&
+ xenConfigSetString(conf, "bios", "ovmf") < 0)
+ return -1;
if (def->os.slic_table &&
xenConfigSetString(conf, "acpi_firmware", def->os.slic_table)
< 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 065c9e97a9..788caf864c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5697,8 +5697,7 @@ qemuDomainDefValidate(const virDomainDef *def,
/* On x86, UEFI requires ACPI */
if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI ||
- (def->os.loader &&
- def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH)) &&
+ virDomainDefHasOldStyleUEFI(def)) &&
ARCH_IS_X86(def->os.arch) &&
def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -5710,8 +5709,7 @@ qemuDomainDefValidate(const virDomainDef *def,
if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON &&
def->os.arch == VIR_ARCH_AARCH64 &&
(def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
- (!def->os.loader ||
- def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH))) {
+ !virDomainDefHasOldStyleUEFI(def))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("ACPI requires UEFI on this architecture"));
goto cleanup;
@@ -16608,12 +16606,9 @@ void
qemuDomainNVRAMPathGenerate(virQEMUDriverConfigPtr cfg,
virDomainDefPtr def)
{
- if (def->os.loader &&
- def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
- def->os.loader->readonly == VIR_TRISTATE_BOOL_YES &&
+ if (virDomainDefHasOldStyleROUEFI(def) &&
!def->os.loader->nvram)
qemuDomainNVRAMPathFormat(cfg, def, &def->os.loader->nvram);
-
}
@@ -16740,8 +16735,7 @@ qemuDomainInitializePflashStorageSource(virDomainObjPtr vm)
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
return 0;
- if (!def->os.loader ||
- def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH)
+ if (!virDomainDefHasOldStyleUEFI(def))
return 0;
if (!(pflash0 = virStorageSourceNew()))
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae54c00239..34d1374d87 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15218,8 +15218,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
* Avoid the issues by forbidding internal snapshot with pflash completely.
*/
if (found_internal &&
- vm->def->os.loader &&
- vm->def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) {
+ virDomainDefHasOldStyleUEFI(vm->def)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("internal snapshots of a VM with pflash based "
"firmware are not supported"));
--
2.24.1
8:57 a.m.
New subject: [libvirt] [PATCH v2 3/4] src: Introduce and use virDomainDefHasOldStyleUEFI() and virDomainDefHasOldStyleROUEFI()
$SUBJECT could be cut down in length a bit
"src: introduce helpers for checking UEFI config"
On Tue, Jan 07, 2020 at 02:43:24PM +0100, Michal Privoznik wrote:
These functions are meant to replace verbose check for the old
style of specifying UEFI with a simple function call.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 3 +--
src/conf/domain_conf.c | 16 ++++++++++++++++
src/conf/domain_conf.h | 6 ++++++
src/libvirt_private.syms | 2 ++
src/libxl/libxl_conf.c | 3 +--
src/libxl/xen_xl.c | 8 +++-----
src/qemu/qemu_domain.c | 14 ++++----------
src/qemu/qemu_driver.c | 3 +--
8 files changed, 34 insertions(+), 21 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:43 a.m.
New subject: [libvirt] [PATCH v2 4/4] 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 | 47 +++++++++++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index a3305d5554..a835fe6b4a 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -959,6 +959,21 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware);
+ if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
+ def->os.loader) {
+ want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type);
+
+ 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 (fw->interfaces[i] == want)
break;
@@ -1226,14 +1241,29 @@ 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 and the varstore doesn't exist ... */
+ if (!virDomainDefHasOldStyleROUEFI(def) ||
+ def->os.loader->templt ||
+ 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)
@@ -1249,9 +1279,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
9 a.m.
New subject: [libvirt] [PATCH v2 4/4] qemu_firmware: Try to autofill for old style UEFI specification
On Tue, Jan 07, 2020 at 02:43:25PM +0100, 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
s/adopted/adapted/
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,
Not "in a way" - they are absolutely always right, because
thats what our API guarantee means for apps. So
s/And in a way they are right/We must preserve backcompat
for this previously supported config approach/
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 | 47 +++++++++++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1783
days inactive
1784
days old
10 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik