Re: [PATCH qemu 2/2] dump: Only use the makedumpfile flattened format when necessary

On Tue, Sep 12, 2023 at 10:34:04AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Aug 23, 2023 at 2:03 PM Marc-André Lureau
> <[email protected]> wrote:
> >
> > Hi
> >
> > On Wed, Aug 23, 2023 at 4:31 AM Stephen Brennan
> > <[email protected]> wrote:
> > >
> > > Stephen Brennan <[email protected]> writes:
> > > > Marc-André Lureau <[email protected]> writes:
> > > >> I am a bit reluctant to change the dump format by default. But since the
> > > >> flatten format is more "complicated" than the "normal" format, perhaps we
> > > >> can assume all users will handle it.
> > > >>
> > > >> The change is probably late for 8.1 though..
> > > >
> > > > Thank you for your review and testing!
> > > >
> > > > I totally understand the concern about making the change by default. I
> > > > do believe that nobody should notice too much because the normal format
> > > > should be easier to work with, and more broadly compatible. I don't know
> > > > of any tool which deals with the flattened format that can't handle the
> > > > normal format, except for "makedumpfile -R" itself.
> > > >
> > > > If it's a blocker, I can go ahead and add a new flag to the command to
> > > > enable the behavior. However there are a few good justifications not to
> > > > add a new flag. I think it's going to be difficult to explain the
> > > > difference between the two formats in documentation, as the
> > > > implementation of the formats is a bit "into the weeds". The libvirt API
> > > > also specifies each format separately (kdump-zlib, kdump-lzo,
> > > > kdump-snappy) and so adding several new options there would be
> > > > unfortunate for end-users as well.
> > > >
> > > > At the end of the day, it's your judgment call, and I'll implement it
> > > > how you prefer.
> > >
> > > I just wanted to check back on this to clarify the next step. Are you
> > > satisfied with this and waiting to apply it until the right time? Or
> > > would you prefer me to send a new version making this opt-in?
> > >
> >
> > Nobody seems to raise concerns. If it would be just me, I would change
> > it. But we should rather be safe, so let's make it this opt-in please.
> 
> Alex, Daniel, Thomas .. Do you have an opinion on changing the dump format?

I think it is a bad idea for the command to output data in a different
format, silently switching based on whether it is passed a pipe or and
file FD.

Libvirt may pass either a plain file FD or a pipe FD, depending on
whether the user set the VIR_DUMP_BYPASS_CACHE API flag. IMHO it
is unacceptable for that to result in a change of data format as a
silent side effect.

Looking back at the history, the patches originally did NOT use the
flatten makedumpfile format, but to ensure it was able to work with
non-seekable files, it jumped through hoops to write to a temporary
file first process it and then write to the real file. Switching to
makedumpfile format was to enable it to avoid temp files and always
be compatible with non-seekable FDs.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|