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

[Maxim Nestratov]

23.05.2016 19:37, Michal Privoznik пишет:

> 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
>
> Michal

As far as I understand the proposed flag would eliminate the necessity 
to specify all flags (SNAPSHOT, SAVE_STATE, NVRAM and some future ones) 
to force undefining.
But this won't allow us to undefine a domain and left a nvram file 
untouched.

Maxim