Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts

[Michael S. Tsirkin]

On Wed, Mar 11, 2015 at 12:06:48PM +0100, Andreas Färber wrote:
> Am 11.03.2015 um 09:56 schrieb Michael S. Tsirkin:
> > On Tue, Mar 10, 2015 at 10:36:56PM +0100, Andreas Färber wrote:
> >> Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
> >>> On Tue, Mar 10, 2015 at 06:50:24PM +0100, Andreas Färber wrote:
> >>>> Hi,
> >>>>
> >>>> Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
> >>>>> Fixes a QEMU crash when passing dump_guest_core parameter in command 
> >>>>> line.
> >>>>
> >>>> Explain that, please?
> >>>
> >>> Pls note the submission date.  It's 1 month late to ask for
> >>> basic clarifications.
> >>>
> >>> I've merged the patches, I'll fix up issues such as prettifying
> >>> includes by adding patches on top.
> >>
> >> No, since the patch is not in qemu.git (it builds!) it is not too late
> >> to fix it, nor too late to ask why a patch that introduces a breakage
> >> does what it does.
> > 
> > 
> > I tried to say that I'm not holding this patch set up
> > because there are some basic questions. Paolo reviewed
> > it and gave an ack. If others want to re-start review 1 month
> > afterwards, that's fine, but I don't want to defer pull
> > request with this any longer. If someone can quickly spot
> > a serious non-cosmetic problem there, that's another
> > matter, and would make me defer the pull request.
> > 
> > 
> >> (Moving the info from the cover letter into the
> >> commit message would've been a good idea, Marcel.)
> > 
> > I can tweak commit messages, sure, since that does not require
> > re-testing it all.
> > 
> >> All QEMU patches are supposed to be bisectable. It's our job as
> >> maintainers to build-test each. If you do that 1 month later, that's not
> >> my fault.
> >>
> >> Regards,
> >> Andreas
> > 
> > I have this patch in my tree and there's
> > no bisect issue, just test-built before and after this patch.
> > That's because I had the ifdefs in boards.h which you and
> > Peter objected to, but that is about cosmetics, I fixed that
> > with a patch on top to hopefully make you both happy.
> 
> All I was asking for is, please squash the patch(es) that fix(es) the
> build issue.

Hmm as far as I can see, there's no build issue.
My patch is required before Marcel's one.
Then there's another one by me to address the comments
you had.

I could squash them all but this just messes up attribution,
bisect build is fine, and I verified that.

> In particular if you applied the patch just yesterday when
> we complained. We've been required to, so I expect the same rules to
> apply to everyone.
> 
> In order to propose a better fix I tried to understand what the patch is
> fixing, that's all. If an improvement of the commit message comes out of
> that, good, but that was not the main purpose.
> 
> Thanks,
> Andreas
> 
> P.S. I was sick most of February and my Chromebook has a broken DRM
> driver, not allowing for much bedside-hacking. ;)

I certainly didn't intend to blame anyone, sorry if it
sounded like that. I was merely saying, it's been on review
for a while, got ack and not objections, so let's apply it unless
someone sees significant issues, I don't want to hold it up
until all questions are answered.

> > 
> > Don't take my word for it, you can check out my tree and verify,
> > that would be very wellcome.
> > 
> >> -- 
> >> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
> >> Graham Norton; HRB 21284 (AG Nürnberg)
> 
> 
> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
> Graham Norton; HRB 21284 (AG Nürnberg)