[Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Peter Maydell]

On 9 February 2012 13:46, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>> You buried the one truly important sentence, let me dig it out for you:
>>
>>         *** Patches should always go to the mailing list ***
>>
>> Exceptions need justification.  Responsible handling embargoed security
>> issues may qualify.  Style fixes certainly not.
>
> 100% agreed.

I don't see anything in the mailing list archives corresponding
to commits f05ae537, f6af014e.

No unreviewed patches should go double when we're in hardfreeze!

-- PMM

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Anthony Liguori]

On 05/03/2012 02:58 PM, Peter Maydell wrote:
> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>>> You buried the one truly important sentence, let me dig it out for you:
>>>
>>>          *** Patches should always go to the mailing list ***
>>>
>>> Exceptions need justification.  Responsible handling embargoed security
>>> issues may qualify.  Style fixes certainly not.
>>
>> 100% agreed.
>
> I don't see anything in the mailing list archives corresponding
> to commits f05ae537, f6af014e.
>
> No unreviewed patches should go double when we're in hardfreeze!

These patches are admittedly trivial but it is important to stress the point 
that all patches need to go on the mailing list before being committed.

It's an important part of keeping the development process inclusive.  I don't 
think it's reasonable to ask for an Acked-by on something as simple as 
indentation changes but at the same time, there's no reason not to just post 
patches.

Regards,

Anthony Liguori

>
> -- PMM
>

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Andreas Färber]

Am 04.05.2012 02:41, schrieb Anthony Liguori:
> On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>>>> You buried the one truly important sentence, let me dig it out for you:
>>>>
>>>>          *** Patches should always go to the mailing list ***
>>>>
>>>> Exceptions need justification.  Responsible handling embargoed security
>>>> issues may qualify.  Style fixes certainly not.
>>>
>>> 100% agreed.
>>
>> I don't see anything in the mailing list archives corresponding
>> to commits f05ae537, f6af014e.
>>
>> No unreviewed patches should go double when we're in hardfreeze!
> 
> These patches are admittedly trivial but it is important to stress the
> point that all patches need to go on the mailing list before being
> committed.
> 
> It's an important part of keeping the development process inclusive.  I
> don't think it's reasonable to ask for an Acked-by on something as
> simple as indentation changes but at the same time, there's no reason
> not to just post patches.

The second patch is far from trivial!

It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
so that I can no longer compile-test my patch series against PowerKVM.

Please revert immediately and either use a warning or a runtime abort.
And please use a proper commit message indicating that it affects "tcg/ppc".

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[malc]

On Fri, 4 May 2012, Andreas F?rber wrote:

> Am 04.05.2012 02:41, schrieb Anthony Liguori:
> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
> >>>> You buried the one truly important sentence, let me dig it out for you:
> >>>>
> >>>>          *** Patches should always go to the mailing list ***
> >>>>
> >>>> Exceptions need justification.  Responsible handling embargoed security
> >>>> issues may qualify.  Style fixes certainly not.
> >>>
> >>> 100% agreed.
> >>
> >> I don't see anything in the mailing list archives corresponding
> >> to commits f05ae537, f6af014e.
> >>
> >> No unreviewed patches should go double when we're in hardfreeze!
> > 
> > These patches are admittedly trivial but it is important to stress the
> > point that all patches need to go on the mailing list before being
> > committed.
> > 
> > It's an important part of keeping the development process inclusive.  I
> > don't think it's reasonable to ask for an Acked-by on something as
> > simple as indentation changes but at the same time, there's no reason
> > not to just post patches.
> 
> The second patch is far from trivial!
> 
> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
> so that I can no longer compile-test my patch series against PowerKVM.

As discussed on IRC, the feature does not work on PPC32, hence it's
violently disabled, what's needed is a black/white list of AREG0 ready
targets.

> 
> Please revert immediately and either use a warning or a runtime abort.
> And please use a proper commit message indicating that it affects "tcg/ppc".
>

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Alexander Graf]

On 04.05.2012, at 04:37, malc wrote:

> On Fri, 4 May 2012, Andreas F?rber wrote:
> 
>> Am 04.05.2012 02:41, schrieb Anthony Liguori:
>>> On 05/03/2012 02:58 PM, Peter Maydell wrote:
>>>> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>>>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>>>>>> You buried the one truly important sentence, let me dig it out for you:
>>>>>> 
>>>>>>         *** Patches should always go to the mailing list ***
>>>>>> 
>>>>>> Exceptions need justification.  Responsible handling embargoed security
>>>>>> issues may qualify.  Style fixes certainly not.
>>>>> 
>>>>> 100% agreed.
>>>> 
>>>> I don't see anything in the mailing list archives corresponding
>>>> to commits f05ae537, f6af014e.
>>>> 
>>>> No unreviewed patches should go double when we're in hardfreeze!
>>> 
>>> These patches are admittedly trivial but it is important to stress the
>>> point that all patches need to go on the mailing list before being
>>> committed.
>>> 
>>> It's an important part of keeping the development process inclusive.  I
>>> don't think it's reasonable to ask for an Acked-by on something as
>>> simple as indentation changes but at the same time, there's no reason
>>> not to just post patches.
>> 
>> The second patch is far from trivial!
>> 
>> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
>> so that I can no longer compile-test my patch series against PowerKVM.
> 
> As discussed on IRC, the feature does not work on PPC32, hence it's
> violently disabled, what's needed is a black/white list of AREG0 ready
> targets.

While I agree that it's broken, a runtime error would be a lot better than a compile time one. The way it's now, it only makes our automated compile tests fail. And since nobody realized until now that sparc and alpha don't work on ppc hosts, I don't think having a compile time failure is warranted.


Alex

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Blue Swirl]

On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
> On Fri, 4 May 2012, Andreas F?rber wrote:
>
>> Am 04.05.2012 02:41, schrieb Anthony Liguori:
>> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>> >>>> You buried the one truly important sentence, let me dig it out for you:
>> >>>>
>> >>>>          *** Patches should always go to the mailing list ***
>> >>>>
>> >>>> Exceptions need justification.  Responsible handling embargoed security
>> >>>> issues may qualify.  Style fixes certainly not.
>> >>>
>> >>> 100% agreed.
>> >>
>> >> I don't see anything in the mailing list archives corresponding
>> >> to commits f05ae537, f6af014e.
>> >>
>> >> No unreviewed patches should go double when we're in hardfreeze!
>> >
>> > These patches are admittedly trivial but it is important to stress the
>> > point that all patches need to go on the mailing list before being
>> > committed.
>> >
>> > It's an important part of keeping the development process inclusive.  I
>> > don't think it's reasonable to ask for an Acked-by on something as
>> > simple as indentation changes but at the same time, there's no reason
>> > not to just post patches.
>>
>> The second patch is far from trivial!
>>
>> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
>> so that I can no longer compile-test my patch series against PowerKVM.
>
> As discussed on IRC, the feature does not work on PPC32, hence it's
> violently disabled, what's needed is a black/white list of AREG0 ready
> targets.

I think disabling was a poor decision, didn't this code already work
in some cases? What's really needed is to shuffle the registers
according to ABI and this shouldn't be much different to what was
already in.

I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
in preparation. When (if) these and maybe further conversions are
committed for 1.2, PPC host support will be practically nonexistent.
Is this what you want?

>
>>
>> Please revert immediately and either use a warning or a runtime abort.
>> And please use a proper commit message indicating that it affects "tcg/ppc".
>>
>
> --
> mailto:av1474@comtv.ru
>

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2594 bytes --]

On Sun, 6 May 2012, Blue Swirl wrote:

> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
> > On Fri, 4 May 2012, Andreas F?rber wrote:
> >
> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
> >> >>>> You buried the one truly important sentence, let me dig it out for you:
> >> >>>>
> >> >>>>          *** Patches should always go to the mailing list ***
> >> >>>>
> >> >>>> Exceptions need justification.  Responsible handling embargoed security
> >> >>>> issues may qualify.  Style fixes certainly not.
> >> >>>
> >> >>> 100% agreed.
> >> >>
> >> >> I don't see anything in the mailing list archives corresponding
> >> >> to commits f05ae537, f6af014e.
> >> >>
> >> >> No unreviewed patches should go double when we're in hardfreeze!
> >> >
> >> > These patches are admittedly trivial but it is important to stress the
> >> > point that all patches need to go on the mailing list before being
> >> > committed.
> >> >
> >> > It's an important part of keeping the development process inclusive.  I
> >> > don't think it's reasonable to ask for an Acked-by on something as
> >> > simple as indentation changes but at the same time, there's no reason
> >> > not to just post patches.
> >>
> >> The second patch is far from trivial!
> >>
> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
> >> so that I can no longer compile-test my patch series against PowerKVM.
> >
> > As discussed on IRC, the feature does not work on PPC32, hence it's
> > violently disabled, what's needed is a black/white list of AREG0 ready
> > targets.
> 
> I think disabling was a poor decision, didn't this code already work
> in some cases? What's really needed is to shuffle the registers

It didn't on Linux and BSDs, might have worked on Darwin and AIX.

> according to ABI and this shouldn't be much different to what was
> already in.

The code that was commited was
a. Pathetically inneficient everywhere
b. Wrong for SysV ABI

> 
> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
> in preparation. When (if) these and maybe further conversions are
> committed for 1.2, PPC host support will be practically nonexistent.
> Is this what you want?

What i do not want is code that doesn't work. And i take non-existant
over wrong any day. I also would prefer to be notified when code which
i maintain is modified.

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Blue Swirl]

On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
> On Sun, 6 May 2012, Blue Swirl wrote:
>
>> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
>> > On Fri, 4 May 2012, Andreas F?rber wrote:
>> >
>> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
>> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>> >> >>>> You buried the one truly important sentence, let me dig it out for you:
>> >> >>>>
>> >> >>>>          *** Patches should always go to the mailing list ***
>> >> >>>>
>> >> >>>> Exceptions need justification.  Responsible handling embargoed security
>> >> >>>> issues may qualify.  Style fixes certainly not.
>> >> >>>
>> >> >>> 100% agreed.
>> >> >>
>> >> >> I don't see anything in the mailing list archives corresponding
>> >> >> to commits f05ae537, f6af014e.
>> >> >>
>> >> >> No unreviewed patches should go double when we're in hardfreeze!
>> >> >
>> >> > These patches are admittedly trivial but it is important to stress the
>> >> > point that all patches need to go on the mailing list before being
>> >> > committed.
>> >> >
>> >> > It's an important part of keeping the development process inclusive.  I
>> >> > don't think it's reasonable to ask for an Acked-by on something as
>> >> > simple as indentation changes but at the same time, there's no reason
>> >> > not to just post patches.
>> >>
>> >> The second patch is far from trivial!
>> >>
>> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
>> >> so that I can no longer compile-test my patch series against PowerKVM.
>> >
>> > As discussed on IRC, the feature does not work on PPC32, hence it's
>> > violently disabled, what's needed is a black/white list of AREG0 ready
>> > targets.
>>
>> I think disabling was a poor decision, didn't this code already work
>> in some cases? What's really needed is to shuffle the registers
>
> It didn't on Linux and BSDs, might have worked on Darwin and AIX.

Then fix it, please!

>> according to ABI and this shouldn't be much different to what was
>> already in.
>
> The code that was commited was
> a. Pathetically inneficient everywhere
> b. Wrong for SysV ABI

Yes, that's what I told back then. There are too many ABIs for various
architectures, the maintainers should know these much better.

>
>>
>> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
>> in preparation. When (if) these and maybe further conversions are
>> committed for 1.2, PPC host support will be practically nonexistent.
>> Is this what you want?
>
> What i do not want is code that doesn't work. And i take non-existant
> over wrong any day. I also would prefer to be notified when code which
> i maintain is modified.

But your approach is not OK in any sense, now we have a failed build.
Before, we had code that could work in some cases and the other cases
could be probably easily fixed.

>
> [..snip..]
>
> --
> mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3450 bytes --]

On Sun, 6 May 2012, Blue Swirl wrote:

> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
> > On Sun, 6 May 2012, Blue Swirl wrote:
> >
> >> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
> >> > On Fri, 4 May 2012, Andreas F?rber wrote:
> >> >
> >> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
> >> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
> >> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
> >> >> >>>> You buried the one truly important sentence, let me dig it out for you:
> >> >> >>>>
> >> >> >>>>          *** Patches should always go to the mailing list ***
> >> >> >>>>
> >> >> >>>> Exceptions need justification.  Responsible handling embargoed security
> >> >> >>>> issues may qualify.  Style fixes certainly not.
> >> >> >>>
> >> >> >>> 100% agreed.
> >> >> >>
> >> >> >> I don't see anything in the mailing list archives corresponding
> >> >> >> to commits f05ae537, f6af014e.
> >> >> >>
> >> >> >> No unreviewed patches should go double when we're in hardfreeze!
> >> >> >
> >> >> > These patches are admittedly trivial but it is important to stress the
> >> >> > point that all patches need to go on the mailing list before being
> >> >> > committed.
> >> >> >
> >> >> > It's an important part of keeping the development process inclusive.  I
> >> >> > don't think it's reasonable to ask for an Acked-by on something as
> >> >> > simple as indentation changes but at the same time, there's no reason
> >> >> > not to just post patches.
> >> >>
> >> >> The second patch is far from trivial!
> >> >>
> >> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
> >> >> so that I can no longer compile-test my patch series against PowerKVM.
> >> >
> >> > As discussed on IRC, the feature does not work on PPC32, hence it's
> >> > violently disabled, what's needed is a black/white list of AREG0 ready
> >> > targets.
> >>
> >> I think disabling was a poor decision, didn't this code already work
> >> in some cases? What's really needed is to shuffle the registers
> >
> > It didn't on Linux and BSDs, might have worked on Darwin and AIX.
> 
> Then fix it, please!

WTF? You commit broken code that is used by 9/10 of all PPC users (yes
all 9 of them) and _then_, not before, demand to fix it.. shrug.

> 
> >> according to ABI and this shouldn't be much different to what was
> >> already in.
> >
> > The code that was commited was
> > a. Pathetically inneficient everywhere
> > b. Wrong for SysV ABI
> 
> Yes, that's what I told back then. There are too many ABIs for various
> architectures, the maintainers should know these much better.

Told whom?

> 
> >
> >>
> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
> >> in preparation. When (if) these and maybe further conversions are
> >> committed for 1.2, PPC host support will be practically nonexistent.
> >> Is this what you want?
> >
> > What i do not want is code that doesn't work. And i take non-existant
> > over wrong any day. I also would prefer to be notified when code which
> > i maintain is modified.
> 
> But your approach is not OK in any sense, now we have a failed build.
> Before, we had code that could work in some cases and the other cases
> could be probably easily fixed.
> 

Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Blue Swirl]

On Sun, May 6, 2012 at 9:46 AM, malc <av1474@comtv.ru> wrote:
> On Sun, 6 May 2012, Blue Swirl wrote:
>
>> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
>> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >
>> >> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
>> >> > On Fri, 4 May 2012, Andreas F?rber wrote:
>> >> >
>> >> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
>> >> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> >> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>> >> >> >>>> You buried the one truly important sentence, let me dig it out for you:
>> >> >> >>>>
>> >> >> >>>>          *** Patches should always go to the mailing list ***
>> >> >> >>>>
>> >> >> >>>> Exceptions need justification.  Responsible handling embargoed security
>> >> >> >>>> issues may qualify.  Style fixes certainly not.
>> >> >> >>>
>> >> >> >>> 100% agreed.
>> >> >> >>
>> >> >> >> I don't see anything in the mailing list archives corresponding
>> >> >> >> to commits f05ae537, f6af014e.
>> >> >> >>
>> >> >> >> No unreviewed patches should go double when we're in hardfreeze!
>> >> >> >
>> >> >> > These patches are admittedly trivial but it is important to stress the
>> >> >> > point that all patches need to go on the mailing list before being
>> >> >> > committed.
>> >> >> >
>> >> >> > It's an important part of keeping the development process inclusive.  I
>> >> >> > don't think it's reasonable to ask for an Acked-by on something as
>> >> >> > simple as indentation changes but at the same time, there's no reason
>> >> >> > not to just post patches.
>> >> >>
>> >> >> The second patch is far from trivial!
>> >> >>
>> >> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
>> >> >> so that I can no longer compile-test my patch series against PowerKVM.
>> >> >
>> >> > As discussed on IRC, the feature does not work on PPC32, hence it's
>> >> > violently disabled, what's needed is a black/white list of AREG0 ready
>> >> > targets.
>> >>
>> >> I think disabling was a poor decision, didn't this code already work
>> >> in some cases? What's really needed is to shuffle the registers
>> >
>> > It didn't on Linux and BSDs, might have worked on Darwin and AIX.
>>
>> Then fix it, please!
>
> WTF? You commit broken code that is used by 9/10 of all PPC users (yes
> all 9 of them) and _then_, not before, demand to fix it.. shrug.

The same approach worked fine on x86. I don't know all architectures
and their ABIs, so I can't fix all back ends. You should be able to do
this much better. Is fixing the register order that hard?

>
>>
>> >> according to ABI and this shouldn't be much different to what was
>> >> already in.
>> >
>> > The code that was commited was
>> > a. Pathetically inneficient everywhere
>> > b. Wrong for SysV ABI
>>
>> Yes, that's what I told back then. There are too many ABIs for various
>> architectures, the maintainers should know these much better.
>
> Told whom?

The list at least, there were plenty of people involved in the discussions.

>
>>
>> >
>> >>
>> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
>> >> in preparation. When (if) these and maybe further conversions are
>> >> committed for 1.2, PPC host support will be practically nonexistent.
>> >> Is this what you want?
>> >
>> > What i do not want is code that doesn't work. And i take non-existant
>> > over wrong any day. I also would prefer to be notified when code which
>> > i maintain is modified.
>>
>> But your approach is not OK in any sense, now we have a failed build.
>> Before, we had code that could work in some cases and the other cases
>> could be probably easily fixed.
>>
>
> Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".

Then fix the misbehaviour instead of this error approach, please.

>
> --
> mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4388 bytes --]

On Sun, 6 May 2012, Blue Swirl wrote:

> On Sun, May 6, 2012 at 9:46 AM, malc <av1474@comtv.ru> wrote:
> > On Sun, 6 May 2012, Blue Swirl wrote:
> >
> >> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
> >> > On Sun, 6 May 2012, Blue Swirl wrote:
> >> >
> >> >> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
> >> >> > On Fri, 4 May 2012, Andreas F?rber wrote:
> >> >> >
> >> >> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
> >> >> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
> >> >> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >> >> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
> >> >> >> >>>> You buried the one truly important sentence, let me dig it out for you:
> >> >> >> >>>>
> >> >> >> >>>>          *** Patches should always go to the mailing list ***
> >> >> >> >>>>
> >> >> >> >>>> Exceptions need justification.  Responsible handling embargoed security
> >> >> >> >>>> issues may qualify.  Style fixes certainly not.
> >> >> >> >>>
> >> >> >> >>> 100% agreed.
> >> >> >> >>
> >> >> >> >> I don't see anything in the mailing list archives corresponding
> >> >> >> >> to commits f05ae537, f6af014e.
> >> >> >> >>
> >> >> >> >> No unreviewed patches should go double when we're in hardfreeze!
> >> >> >> >
> >> >> >> > These patches are admittedly trivial but it is important to stress the
> >> >> >> > point that all patches need to go on the mailing list before being
> >> >> >> > committed.
> >> >> >> >
> >> >> >> > It's an important part of keeping the development process inclusive.  I
> >> >> >> > don't think it's reasonable to ask for an Acked-by on something as
> >> >> >> > simple as indentation changes but at the same time, there's no reason
> >> >> >> > not to just post patches.
> >> >> >>
> >> >> >> The second patch is far from trivial!
> >> >> >>
> >> >> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
> >> >> >> so that I can no longer compile-test my patch series against PowerKVM.
> >> >> >
> >> >> > As discussed on IRC, the feature does not work on PPC32, hence it's
> >> >> > violently disabled, what's needed is a black/white list of AREG0 ready
> >> >> > targets.
> >> >>
> >> >> I think disabling was a poor decision, didn't this code already work
> >> >> in some cases? What's really needed is to shuffle the registers
> >> >
> >> > It didn't on Linux and BSDs, might have worked on Darwin and AIX.
> >>
> >> Then fix it, please!
> >
> > WTF? You commit broken code that is used by 9/10 of all PPC users (yes
> > all 9 of them) and _then_, not before, demand to fix it.. shrug.
> 
> The same approach worked fine on x86. I don't know all architectures
> and their ABIs, so I can't fix all back ends. You should be able to do
> this much better. Is fixing the register order that hard?

Yet you commit broken code without consulting the person who does know
it, that's the gist of the matter.

> 
> >
> >>
> >> >> according to ABI and this shouldn't be much different to what was
> >> >> already in.
> >> >
> >> > The code that was commited was
> >> > a. Pathetically inneficient everywhere
> >> > b. Wrong for SysV ABI
> >>
> >> Yes, that's what I told back then. There are too many ABIs for various
> >> architectures, the maintainers should know these much better.
> >
> > Told whom?
> 
> The list at least, there were plenty of people involved in the discussions.

Myself excluded for whatever reason.

> 
> >
> >>
> >> >
> >> >>
> >> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
> >> >> in preparation. When (if) these and maybe further conversions are
> >> >> committed for 1.2, PPC host support will be practically nonexistent.
> >> >> Is this what you want?
> >> >
> >> > What i do not want is code that doesn't work. And i take non-existant
> >> > over wrong any day. I also would prefer to be notified when code which
> >> > i maintain is modified.
> >>
> >> But your approach is not OK in any sense, now we have a failed build.
> >> Before, we had code that could work in some cases and the other cases
> >> could be probably easily fixed.
> >>
> >
> > Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".
> 
> Then fix the misbehaviour instead of this error approach, please.
> 

Please do read your e-mail, in particular messages from Andreas.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Blue Swirl]

On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
> On Sun, 6 May 2012, Blue Swirl wrote:
>
>> On Sun, May 6, 2012 at 9:46 AM, malc <av1474@comtv.ru> wrote:
>> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >
>> >> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
>> >> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >> >
>> >> >> On Fri, May 4, 2012 at 2:37 AM, malc <av1474@comtv.ru> wrote:
>> >> >> > On Fri, 4 May 2012, Andreas F?rber wrote:
>> >> >> >
>> >> >> >> Am 04.05.2012 02:41, schrieb Anthony Liguori:
>> >> >> >> > On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> >> >> >> >> On 9 February 2012 13:46, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >> >> >> >>> On 02/09/2012 03:48 AM, Markus Armbruster wrote:
>> >> >> >> >>>> You buried the one truly important sentence, let me dig it out for you:
>> >> >> >> >>>>
>> >> >> >> >>>>          *** Patches should always go to the mailing list ***
>> >> >> >> >>>>
>> >> >> >> >>>> Exceptions need justification.  Responsible handling embargoed security
>> >> >> >> >>>> issues may qualify.  Style fixes certainly not.
>> >> >> >> >>>
>> >> >> >> >>> 100% agreed.
>> >> >> >> >>
>> >> >> >> >> I don't see anything in the mailing list archives corresponding
>> >> >> >> >> to commits f05ae537, f6af014e.
>> >> >> >> >>
>> >> >> >> >> No unreviewed patches should go double when we're in hardfreeze!
>> >> >> >> >
>> >> >> >> > These patches are admittedly trivial but it is important to stress the
>> >> >> >> > point that all patches need to go on the mailing list before being
>> >> >> >> > committed.
>> >> >> >> >
>> >> >> >> > It's an important part of keeping the development process inclusive.  I
>> >> >> >> > don't think it's reasonable to ask for an Acked-by on something as
>> >> >> >> > simple as indentation changes but at the same time, there's no reason
>> >> >> >> > not to just post patches.
>> >> >> >>
>> >> >> >> The second patch is far from trivial!
>> >> >> >>
>> >> >> >> It unneededly breaks the build on ppc hosts (during the Hard Freeze!),
>> >> >> >> so that I can no longer compile-test my patch series against PowerKVM.
>> >> >> >
>> >> >> > As discussed on IRC, the feature does not work on PPC32, hence it's
>> >> >> > violently disabled, what's needed is a black/white list of AREG0 ready
>> >> >> > targets.
>> >> >>
>> >> >> I think disabling was a poor decision, didn't this code already work
>> >> >> in some cases? What's really needed is to shuffle the registers
>> >> >
>> >> > It didn't on Linux and BSDs, might have worked on Darwin and AIX.
>> >>
>> >> Then fix it, please!
>> >
>> > WTF? You commit broken code that is used by 9/10 of all PPC users (yes
>> > all 9 of them) and _then_, not before, demand to fix it.. shrug.
>>
>> The same approach worked fine on x86. I don't know all architectures
>> and their ABIs, so I can't fix all back ends. You should be able to do
>> this much better. Is fixing the register order that hard?
>
> Yet you commit broken code without consulting the person who does know
> it, that's the gist of the matter.

It was not broken code. Did anyone report problems during these months
until now? We need a bug fix, not violent disabling acts.

>
>>
>> >
>> >>
>> >> >> according to ABI and this shouldn't be much different to what was
>> >> >> already in.
>> >> >
>> >> > The code that was commited was
>> >> > a. Pathetically inneficient everywhere
>> >> > b. Wrong for SysV ABI
>> >>
>> >> Yes, that's what I told back then. There are too many ABIs for various
>> >> architectures, the maintainers should know these much better.
>> >
>> > Told whom?
>>
>> The list at least, there were plenty of people involved in the discussions.
>
> Myself excluded for whatever reason.

Are you not subscribed to the list?

>
>>
>> >
>> >>
>> >> >
>> >> >>
>> >> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
>> >> >> in preparation. When (if) these and maybe further conversions are
>> >> >> committed for 1.2, PPC host support will be practically nonexistent.
>> >> >> Is this what you want?
>> >> >
>> >> > What i do not want is code that doesn't work. And i take non-existant
>> >> > over wrong any day. I also would prefer to be notified when code which
>> >> > i maintain is modified.
>> >>
>> >> But your approach is not OK in any sense, now we have a failed build.
>> >> Before, we had code that could work in some cases and the other cases
>> >> could be probably easily fixed.
>> >>
>> >
>> > Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".
>>
>> Then fix the misbehaviour instead of this error approach, please.
>>
>
> Please do read your e-mail, in particular messages from Andreas.

Which messages?

>
> --
> mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[malc]

On Sun, 6 May 2012, Blue Swirl wrote:

> On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
> > On Sun, 6 May 2012, Blue Swirl wrote:
> >
> >> On Sun, May 6, 2012 at 9:46 AM, malc <av1474@comtv.ru> wrote:
> >> > On Sun, 6 May 2012, Blue Swirl wrote:
> >> >
> >> >> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
> >> >> > On Sun, 6 May 2012, Blue Swirl wrote:
> >> >> >

[..snip..]

> >>
> >> The same approach worked fine on x86. I don't know all architectures
> >> and their ABIs, so I can't fix all back ends. You should be able to do
> >> this much better. Is fixing the register order that hard?
> >
> > Yet you commit broken code without consulting the person who does know
> > it, that's the gist of the matter.
> 
> It was not broken code. Did anyone report problems during these months
> until now? We need a bug fix, not violent disabling acts.

Yes, Alexander told me, that's how i became aware of the issue.

> 
> >
> >>
> >> >
> >> >>
> >> >> >> according to ABI and this shouldn't be much different to what was
> >> >> >> already in.
> >> >> >
> >> >> > The code that was commited was
> >> >> > a. Pathetically inneficient everywhere
> >> >> > b. Wrong for SysV ABI
> >> >>
> >> >> Yes, that's what I told back then. There are too many ABIs for various
> >> >> architectures, the maintainers should know these much better.
> >> >
> >> > Told whom?
> >>
> >> The list at least, there were plenty of people involved in the discussions.
> >
> > Myself excluded for whatever reason.
> 
> Are you not subscribed to the list?

And what do rethorical questions have to do with it? Next thing you will
demand that i thoroughly study every mail even when not CC-ed or
something?

> 
> >
> >>
> >> >
> >> >>
> >> >> >
> >> >> >>
> >> >> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
> >> >> >> in preparation. When (if) these and maybe further conversions are
> >> >> >> committed for 1.2, PPC host support will be practically nonexistent.
> >> >> >> Is this what you want?
> >> >> >
> >> >> > What i do not want is code that doesn't work. And i take non-existant
> >> >> > over wrong any day. I also would prefer to be notified when code which
> >> >> > i maintain is modified.
> >> >>
> >> >> But your approach is not OK in any sense, now we have a failed build.
> >> >> Before, we had code that could work in some cases and the other cases
> >> >> could be probably easily fixed.
> >> >>
> >> >
> >> > Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".
> >>
> >> Then fix the misbehaviour instead of this error approach, please.
> >>
> >
> > Please do read your e-mail, in particular messages from Andreas.
> 
> Which messages?

This one http://www.mail-archive.com/qemu-devel@nongnu.org/msg110283.html

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Blue Swirl]

On Sun, May 6, 2012 at 11:02 AM, malc <av1474@comtv.ru> wrote:
> On Sun, 6 May 2012, Blue Swirl wrote:
>
>> On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
>> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >
>> >> On Sun, May 6, 2012 at 9:46 AM, malc <av1474@comtv.ru> wrote:
>> >> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >> >
>> >> >> On Sun, May 6, 2012 at 9:03 AM, malc <av1474@comtv.ru> wrote:
>> >> >> > On Sun, 6 May 2012, Blue Swirl wrote:
>> >> >> >
>
> [..snip..]
>
>> >>
>> >> The same approach worked fine on x86. I don't know all architectures
>> >> and their ABIs, so I can't fix all back ends. You should be able to do
>> >> this much better. Is fixing the register order that hard?
>> >
>> > Yet you commit broken code without consulting the person who does know
>> > it, that's the gist of the matter.
>>
>> It was not broken code. Did anyone report problems during these months
>> until now? We need a bug fix, not violent disabling acts.
>
> Yes, Alexander told me, that's how i became aware of the issue.
>
>>
>> >
>> >>
>> >> >
>> >> >>
>> >> >> >> according to ABI and this shouldn't be much different to what was
>> >> >> >> already in.
>> >> >> >
>> >> >> > The code that was commited was
>> >> >> > a. Pathetically inneficient everywhere
>> >> >> > b. Wrong for SysV ABI
>> >> >>
>> >> >> Yes, that's what I told back then. There are too many ABIs for various
>> >> >> architectures, the maintainers should know these much better.
>> >> >
>> >> > Told whom?
>> >>
>> >> The list at least, there were plenty of people involved in the discussions.
>> >
>> > Myself excluded for whatever reason.
>>
>> Are you not subscribed to the list?
>
> And what do rethorical questions have to do with it? Next thing you will
> demand that i thoroughly study every mail even when not CC-ed or
> something?

No, that would be unreasonable on a high volume list like we have. But
I think expecting that all maintainers roughly follow what happens on
the list is not unreasonable. I don't read all messages myself, but at
least the subject of every message.

Perhaps there should be two lists, one for patches and the other for
general discussion. Though patch discussions can engage general issues
too.

>
>>
>> >
>> >>
>> >> >
>> >> >>
>> >> >> >
>> >> >> >>
>> >> >> >> I have sent out AREG0 patches for ARM and PPC, also I have x86 patches
>> >> >> >> in preparation. When (if) these and maybe further conversions are
>> >> >> >> committed for 1.2, PPC host support will be practically nonexistent.
>> >> >> >> Is this what you want?
>> >> >> >
>> >> >> > What i do not want is code that doesn't work. And i take non-existant
>> >> >> > over wrong any day. I also would prefer to be notified when code which
>> >> >> > i maintain is modified.
>> >> >>
>> >> >> But your approach is not OK in any sense, now we have a failed build.
>> >> >> Before, we had code that could work in some cases and the other cases
>> >> >> could be probably easily fixed.
>> >> >>
>> >> >
>> >> > Well, here's a "sense", code that _silently_ misbehaves is NOT "OK".
>> >>
>> >> Then fix the misbehaviour instead of this error approach, please.
>> >>
>> >
>> > Please do read your e-mail, in particular messages from Andreas.
>>
>> Which messages?
>
> This one http://www.mail-archive.com/qemu-devel@nongnu.org/msg110283.html

I think that is not related, but I'll check.

>
> [..snip..]
>
> --
> mailto:av1474@comtv.ru

Re: [Qemu-devel] unreviewed commits

[Andreas Färber]

Am 06.05.2012 13:24, schrieb Blue Swirl:
> On Sun, May 6, 2012 at 11:02 AM, malc <av1474@comtv.ru> wrote:
>> On Sun, 6 May 2012, Blue Swirl wrote:
>>> On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
>>>> On Sun, 6 May 2012, Blue Swirl wrote:
>>>>
>>>>> Then fix the misbehaviour instead of this error approach, please.
>>>>
>>>> Please do read your e-mail, in particular messages from Andreas.
>>>
>>> Which messages?
>>
>> This one http://www.mail-archive.com/qemu-devel@nongnu.org/msg110283.html
> 
> I think that is not related, but I'll check.

It's related to us testing the pending ppc fix at least.

Does anyone know of an alpha image to test? AFAIU sparc, sparc64 and
alpha are the only applied AREG0 conversions so far, with alpha being
64-bit like apparently-working sparc64.

I'm still trying to rebase the ppc AREG0 series; if you've already
succeeded, it would be nice if you could push to your personal repo. The
April 22 version was broken by my QOM Makefile.target changes as well as
by some ppc MMU change that I'm looking into right now.

Do note that the above sparc breakage is about tcg/ppc (32-on-32), which
seems particularly odd, given that the TCG code reads so "trivial".

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] unreviewed commits

[Alexander Graf]

On 06.05.2012, at 16:41, Andreas Färber wrote:

> Am 06.05.2012 13:24, schrieb Blue Swirl:
>> On Sun, May 6, 2012 at 11:02 AM, malc <av1474@comtv.ru> wrote:
>>> On Sun, 6 May 2012, Blue Swirl wrote:
>>>> On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
>>>>> On Sun, 6 May 2012, Blue Swirl wrote:
>>>>> 
>>>>>> Then fix the misbehaviour instead of this error approach, please.
>>>>> 
>>>>> Please do read your e-mail, in particular messages from Andreas.
>>>> 
>>>> Which messages?
>>> 
>>> This one http://www.mail-archive.com/qemu-devel@nongnu.org/msg110283.html
>> 
>> I think that is not related, but I'll check.
> 
> It's related to us testing the pending ppc fix at least.
> 
> Does anyone know of an alpha image to test?

Richard, I'm sure you have one, right? :)

> AFAIU sparc, sparc64 and
> alpha are the only applied AREG0 conversions so far, with alpha being
> 64-bit like apparently-working sparc64.

Also IIRC Debian used to have an Alpha build, so it might be possible to grab something from them.


Alex

Re: [Qemu-devel] unreviewed commits

[Richard Henderson]

On 05/06/2012 09:22 AM, Alexander Graf wrote:
> Richard, I'm sure you have one, right? :)
>
>> AFAIU sparc, sparc64 and
>> alpha are the only applied AREG0 conversions so far, with alpha being
>> 64-bit like apparently-working sparc64.
>
> Also IIRC Debian used to have an Alpha build, so it might be possible to grab something from them.

That's exactly what I test with, the last debian image.


r~

Re: [Qemu-devel] unreviewed commits

[Blue Swirl]

On Sun, May 6, 2012 at 2:41 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 06.05.2012 13:24, schrieb Blue Swirl:
>> On Sun, May 6, 2012 at 11:02 AM, malc <av1474@comtv.ru> wrote:
>>> On Sun, 6 May 2012, Blue Swirl wrote:
>>>> On Sun, May 6, 2012 at 10:17 AM, malc <av1474@comtv.ru> wrote:
>>>>> On Sun, 6 May 2012, Blue Swirl wrote:
>>>>>
>>>>>> Then fix the misbehaviour instead of this error approach, please.
>>>>>
>>>>> Please do read your e-mail, in particular messages from Andreas.
>>>>
>>>> Which messages?
>>>
>>> This one http://www.mail-archive.com/qemu-devel@nongnu.org/msg110283.html
>>
>> I think that is not related, but I'll check.
>
> It's related to us testing the pending ppc fix at least.
>
> Does anyone know of an alpha image to test? AFAIU sparc, sparc64 and
> alpha are the only applied AREG0 conversions so far, with alpha being
> 64-bit like apparently-working sparc64.
>
> I'm still trying to rebase the ppc AREG0 series; if you've already
> succeeded, it would be nice if you could push to your personal repo. The
> April 22 version was broken by my QOM Makefile.target changes as well as
> by some ppc MMU change that I'm looking into right now.

URL	git://repo.or.cz/qemu/blueswirl.git
	http://repo.or.cz/r/qemu/blueswirl.git

These are still stgit patches, so it's not safe to base work on these
because they will be rebased.

>
> Do note that the above sparc breakage is about tcg/ppc (32-on-32), which
> seems particularly odd, given that the TCG code reads so "trivial".
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] unreviewed commits (was: Re: Restore consistent formatting)

[Peter Maydell]

On 4 May 2012 01:41, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 05/03/2012 02:58 PM, Peter Maydell wrote:
>> No unreviewed patches should go double when we're in hardfreeze!
>
> These patches are admittedly trivial but it is important to stress the point
> that all patches need to go on the mailing list before being committed.
>
> It's an important part of keeping the development process inclusive.  I
> don't think it's reasonable to ask for an Acked-by on something as simple as
> indentation changes but at the same time, there's no reason not to just post
> patches.

Yes, it is specifically the failure to follow the standard process
of posting patches and allowing the opportunity for review before commit
that I'm complaining about. Obviously if nobody cares to actually do
the review that's their lookout.

-- PMM