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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)