On 03/29/2017 10:08 AM, Andrea Bolognani wrote:
> Depending on the architecture, requirements for ACPI and UEFI can
> be different; more specifically, while on x86 UEFI requires ACPI,
> on aarch64 it's the other way around.
>
> Enforce these requirements when validating the domain, and make
> the error message more accurate by mentioning that they're not
> necessarily applicable to all architectures.
>
> Several aarch64 test cases had to be tweaked because they would
> have failed the validation step otherwise.
> ---
> src/qemu/qemu_command.c | 20 ++++----------------
> src/qemu/qemu_domain.c | 20 ++++++++++++++++++++
> .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args | 1 +
> .../qemuxml2argv-aarch64-aavmf-virtio-mmio.xml | 1 -
> .../qemuxml2argv-aarch64-cpu-passthrough.args | 1 +
> .../qemuxml2argv-aarch64-cpu-passthrough.xml | 1 -
> .../qemuxml2argv-aarch64-video-virtio-gpu-pci.args | 1 +
> .../qemuxml2argv-aarch64-video-virtio-gpu-pci.xml | 3 ---
> ...xml2argv-aarch64-virt-2.6-virtio-pci-default.args | 1 +
> ...uxml2argv-aarch64-virt-2.6-virtio-pci-default.xml | 1 -
> .../qemuxml2argv-aarch64-virt-default-nic.args | 1 +
> .../qemuxml2argv-aarch64-virt-default-nic.xml | 3 ---
> .../qemuxml2argv-aarch64-virt-virtio.args | 1 +
> .../qemuxml2argv-aarch64-virt-virtio.xml | 1 -
> .../qemuxml2argv-aarch64-virtio-pci-default.args | 1 +
> .../qemuxml2argv-aarch64-virtio-pci-default.xml | 1 -
> ...xml2argv-aarch64-virtio-pci-manual-addresses.args | 1 +
> ...uxml2argv-aarch64-virtio-pci-manual-addresses.xml | 1 -
> .../qemuxml2argv-balloon-mmio-deflate.args | 1 +
> .../qemuxml2argv-balloon-mmio-deflate.xml | 1 -
> .../qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml | 1 -
> .../qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml | 1 -
> .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 1 -
> ...ml2xmlout-aarch64-virtio-pci-manual-addresses.xml | 1 -
> 24 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 64d2d71..5cf383a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9324,18 +9324,16 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
> }
>
>
> -static int
> +static void
> qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
> - virDomainDefPtr def,
> - virQEMUCapsPtr qemuCaps)
> + virDomainDefPtr def)
> {
> - int ret = -1;
> virDomainLoaderDefPtr loader = def->os.loader;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> int unit = 0;
>
> if (!loader)
> - return 0;
> + return;
>
> switch ((virDomainLoader) loader->type) {
> case VIR_DOMAIN_LOADER_TYPE_ROM:
> @@ -9344,12 +9342,6 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
> break;
>
> case VIR_DOMAIN_LOADER_TYPE_PFLASH:
> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI) &&
> - def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("ACPI must be enabled in order to use UEFI"));
> - goto cleanup;
> - }
>
> if (loader->secure == VIR_TRISTATE_BOOL_YES) {
> virCommandAddArgList(cmd,
> @@ -9387,10 +9379,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
> break;
> }
>
> - ret = 0;
> - cleanup:
> virBufferFreeAndReset(&buf);
> - return ret;
> }
>
>
> @@ -9827,8 +9816,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps) < 0)
> goto error;
>
> - if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0)
> - goto error;
> + qemuBuildDomainLoaderCommandLine(cmd, def);
>
> if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0)
> goto error;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 458bb5f..e41e8e4 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2884,6 +2884,26 @@ qemuDomainDefValidate(const virDomainDef *def,
> goto cleanup;
> }
>
> + /* On x86, UEFI requires ACPI */
> + if (def->os.loader &&
> + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
> + ARCH_IS_X86(def->os.arch) &&
> + def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("UEFI requires ACPI on this architecture"));
> + goto cleanup;
> + }
> +
> + /* On aarch64, ACPI requires UEFI */
> + if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON &&
> + def->os.arch == VIR_ARCH_AARCH64 &&
> + (!def->os.loader ||
> + def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("ACPI requires UEFI on this architecture"));
> + return -1;
goto cleanup;
> + }
> +
So as I said in my last response to v1 - are you sure there couldn't be
a guest running that now disappears because of this check?
IOW should this move to post parse processing...
ACK - if you're positive this won't affect the running guest causing it
to disappear.
John
> if (def->os.loader &&
> def->os.loader->secure == VIR_TRISTATE_BOOL_YES) {
> /* These are the QEMU implementation limitations. But we
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
