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

Re: [libvirt] [PATCH v3 4/4] add dump_memory_format in qemu.conf



On 02/27/2014 07:55 AM, Daniel P. Berrange wrote:

>> +        else {
>> +            /* when --compress option is specified, qemu.conf will not work */
>> +            cfg = virQEMUDriverGetConfig(driver);
>> +            if (cfg->dumpMemoryFormat) {
>> +                dump_memory_format = qemuDumpMemoryFormatTypeFromString(
>> +                                                        cfg->dumpMemoryFormat);
>> +                if (dump_memory_format == QEMU_DUMP_MEMORY_FORMAT_KDUMP_ZLIB)
>> +                    memory_dump_format = "kdump-zlib";
>> +                else if (dump_memory_format ==
>> +                                            QEMU_DUMP_MEMORY_FORMAT_KDUMP_LZO)
>> +                    memory_dump_format = "kdump-lzo";
>> +                else if (dump_memory_format ==
>> +                                        QEMU_DUMP_MEMORY_FORMAT_KDUMP_SNAPPY)
>> +                    memory_dump_format = "kdump-snappy";
>> +            }
>> +        }
> 
> NACK to this change. We shouldn't  invisibly change the behaviour of
> public APIs based on qemu.conf configuration parameters. The app should
> always be explicit about the format they want to produce.

Arguably, if you support value '0' as an explicit "use the default
format from the .conf file", and '1' as "raw", then you could set it up
so that applications have explicit control over all formats when
desired, but can use the .conf file default when they don't care.  But
again, this would have to be explicit by having an enum value in the .h
file that explicitly states the user wants to use a .conf file setting
(without regards to its value) instead of their own explicit value.

> 
> Where a qemu.conf parameter could be valid is wrt automatically
> triggered core dumps, which don't involve public API calls.

And even though I suggested the possibility of an explicit "use the
.conf file default", it sounds more complex; sometimes simpler is
better, and Dan's suggestion of not allowing the explicit API to ever be
influenced by the .conf file is also reasonable.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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