[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]