Re: [libvirt] [PATCH] qemu: don't refuse to undefine a guest with NVRAM file

On 23.05.2016 11:16, Maxim Nestratov wrote: > 24.02.2015 13:12, Daniel P. Berrange пишет: > >> The undefine operation should always be allowed to succeed >> regardless of whether any NVRAM file exists. ie we should >> not force the application to use the VIR_DOMAIN_UNDEFINE_NVRAM >> flag. It is valid for the app to decide it wants the NVRAM >> file left on disk, in the same way that disk images are left >> on disk at undefine. >> --- >> src/qemu/qemu_driver.c | 20 +++++++------------- >> 1 file changed, 7 insertions(+), 13 deletions(-) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index bec05d4..302bf48 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -6985,19 +6985,13 @@ qemuDomainUndefineFlags(virDomainPtr dom, >> if (!virDomainObjIsActive(vm) && >> vm->def->os.loader && vm->def->os.loader->nvram && >> - virFileExists(vm->def->os.loader->nvram)) { >> - if (!(flags & VIR_DOMAIN_UNDEFINE_NVRAM)) { >> - virReportError(VIR_ERR_OPERATION_INVALID, "%s", >> - _("cannot delete inactive domain with >> nvram")); >> - goto cleanup; >> - } >> - >> - if (unlink(vm->def->os.loader->nvram) < 0) { >> - virReportSystemError(errno, >> - _("failed to remove nvram: %s"), >> - vm->def->os.loader->nvram); >> - goto cleanup; >> - } >> + virFileExists(vm->def->os.loader->nvram) && >> + (flags & VIR_DOMAIN_UNDEFINE_NVRAM) && >> + (unlink(vm->def->os.loader->nvram) < 0)) { >> + virReportSystemError(errno, >> + _("failed to remove nvram: %s"), >> + vm->def->os.loader->nvram); >> + goto cleanup; >> } >> if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, >> vm) < 0) > > As I found out the discussion followed this patch didn't come to a > conclusion and this or any other patches on the matter weren't commited. > We hit the problem with inability to undefine a domain leaving nvram > untouched recently and this patch would solve it perfectly. > I think it's worth commiting IMHO and maybe the documentation should > reflect this slight change in behavior. > Any new thoughts? Does this Cole's suggestion sounds reasonable? https://www.redhat.com/archives/libvir-list/2015-February/msg00974.html