Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris

[Andreas Färber]

Am 27.03.2012 15:06, schrieb Stefan Hajnoczi:
> On Tue, Mar 27, 2012 at 1:01 PM, Lee Essen <address@hidden> wrote:
>> On 27/03/2012 12:31, Andreas Färber wrote:
>>>
>>> Am 27.03.2012 09:23, schrieb Stefan Hajnoczi:
>>>>
>>>> On Sat, Mar 24, 2012 at 04:26:27PM +0000, Lee Essen wrote:
>>>>>
>>>>> libsocket and libxnet are required for base network functionality
>>>>> used in os_dep.c, qemu-socket.c, qga/commands-posix.c and cutils.c
>>>>>
>>>>> Signed-off-by: Lee Essen<address@hidden>
>>>>> ---
>>>>>  configure |    1 +
>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 8b4e3c1..152adaa 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -471,6 +471,7 @@ SunOS)
>>>>>    QEMU_CFLAGS="-D__EXTENSIONS__ $QEMU_CFLAGS"
>>>>>    QEMU_CFLAGS="-std=gnu99 $QEMU_CFLAGS"
>>>>>    LIBS="-lsocket -lnsl -lresolv $LIBS"
>>>>> +  libs_qga="-lsocket -lxnet $lib_qga"
>>>>
>>>>
>>>> s/lib_qga/libs_qga/
>>>>
>>>> BTW this typo is also present in mingw32 libs_qga, I have sent a patch
>>>> to fix it.
>>>>
>>>> So -lxnet isn't required in plain old LIBS?
>>>
>>>
>>> It's a question of generation AFAIU, I didn't like it either. By using
>>> the old libs, then due to Solaris' backwards compatibility we are able
>>> to run them on older Solaris versions in theory. We should be using the
>>> same libs consistently in QEMU, and I don't like double-coding them.
>>> Those comments were not yet addressed, just as my suggested subject for
>>> the timer patch and the ordering of the patches was deliberately
>>> ignored. :/ Since my patience is limited, I plan to fix them up myself
>>> before applying them to my Solaris branch and sending a PULL.
>>
>>
>> <rant>
>>
>> What?  I'm trying here ... I don't understand the ordering comment, your
>> suggestion was about putting more meaningful titles, I've tried to do that.
>>
>> Blimey ... this isn't my job, this is my own time ... I'm doing this because
>> I want to try to make things better and it feels like I'm having to jump
>> through ever decreasing hoops.
>>
>> I'm new to the whole git patch submission thing (as is obviously apparent)
>> ... so give me a break.
>>
>> And let's be clear here ... at the moment there is no support for Solaris,
>> there are countless fundamental fixes that need to go in before it will even
>> get close ... let alone thinking about kvm.
>>
>> I've tried very hard not to break any other platform, but still I can't even
>> get a single thing applied.
>>
>> </rant>
>>
>> Ok, since I'm obviously incapable of providing patches in the right form,
>> let me know if I can help in any other way. For now I will just maintain a
>> separate tree.
> 
> Not sure how the discussion got here.  As far as I'm concerned there's
> no reason to throw in the towel.
> 
> Andreas: Were you just stressed out or are you being serious?

Bit of both:

In a SUSE capacity my interest is handling such platform differences in
a sane, maintainable way. I have pointed out some issues there that we
might or might not want to do differently there. Pending feedback.

Then in a personal capacity, I get the impression that a felt 50% of my
comments do not make it into the next patches, especially concerning
formal and organizational matters. While the MAINTAINER host support
sections do not list me (they're still new in there), Solaris patches
have traditionally gone through me, so that is not a particular reaction
to the contents of form of Lee's patches, I am serious.

I do however not feel qualified for nor am I interested much in
reviewing KVM-backend patches (yet) for illumos, so I expect
Avi+Marcello and/or Jan to handle that, which Lee is not cc'ing either.
The patch submission does not reflect this yet, which had been a core
point I had implied when requesting how to split up the patch into three
series.

Concerning the timer, I was expecting review from Paolo, especially
since I raised the issue of why this was Linux-only. This is a red flag
for me, since it would indicate that Darwin, BSDs, Win32, Haiku do not
have it - possibly because no one noticed, as seen elsewhere in the code
where for, e.g., pty we have insane lists of BSD variants all added
individually and applied by Blue despite my criticism instead of having
it fixed in a better way - so history shows if we don't fix it right
away, it will always be extended and never fixed properly, that's why
I'm pressing on this where today it was just Linux and now Sun/Solaris.

Again in a personal capacity I am "stressed" by the fact that Lee is
wasting my time with too early and "incomplete" patch resubmissions that
need to be reviewed and commented on again (copy&paste?), not to mention
that most of us have other tasks to handle besides his illumos issues.
If he's ignoring my comments and not looking at previous discussions in
the archive (e.g., concerning O_ASYNC, for which we had a different
suggestion previously), why do I spend the time on patch review in the
first place.

Thus I am looking for a time-efficient way to get things fixed in
upstream, and if that requires me fixing minor nits myself rather than
going through hoops with resubmission+review cycles then so be it,
that's what Signed-off-by and From are for (cf. Jonathan Corbet's
keynote on issues with Linux kernel contributions at FOSDEM 2011). If
Lee fixes some more things and becomes a bit more patient with our
review and testing, that's fine with me as well, as long as at least one
of us that are around some longer tests the resulting patches and
verifies that we're not missing a better solution. In particular I want
to test them on Solaris 10 before Blue (whom he has cc'ed) commits them.

Andreas