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.