Re: [libvirt] [PATCH v2 4/5] qemu: Enforce ACPI, UEFI requirements

On Wed, 2017-03-29 at 19:50 -0400, John Ferlan wrote: [...] >> @@ -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; Missed that when moving the code over, good catch :) [...] > 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. AIUI it's the other way around: PostParse() is called every time the XML is loaded, including daemon start / reload, so adding a new error condition there can cause existing guests to disappear. Validate() on the other hand is only called when creating, editing or starting guests, so having the check there will prevent users from misconfiguring guests but will also give them a chance to fix existing guests.

As part of preparing this respin, I created a misconfigured guest and verified that it didn't disappear after starting a build of libvirt that included this patch, so I'll take your ACK :)