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

[Daniel P. Berrange]

On Thu, Feb 27, 2014 at 08:52:19AM -0700, Eric Blake wrote:
> 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.

While you could do that, I just don't see the value in having an option
where the data you get back is in an arbitrary format you can't predict

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

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|