qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris
Date: Wed, 28 Mar 2012 20:41:39 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120312 Thunderbird/11.0

Am 27.03.2012 19:24, schrieb Blue Swirl:
> On Tue, Mar 27, 2012 at 13:56, Andreas Färber <address@hidden> wrote:
>> 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'm a bit surprised about this claim,

I'm surprised you are!

> I think I haven't been aware of
> this route. When did Solaris patches go through you, could you name
> some? I'm asking because the git log command I sent doesn't show your
> name very often.

I don't have a link handy right now but I was involved in upstreaming
some things from the downstream OpenSolaris QEMU project and was kind-of
taking over OpenSolaris host support caretaking (if you prefer that over
maintaince, which has become misassociated with PULLs lately) from IIRC
Ben Taylor, who is no longer around. At that time I was pretty much the
only one testing on OpenSolaris, reporting breakages and most if not all
fixes came from me, which in no way contradicts "through me". I'd be
unsurprised if not few of my patches actually were applied by you.

You might better remember that I opposed the move to drop kqemu because
I was using it on OpenSolaris/amd64 at the time!
The drop made things slower for me but some guests were still much
quicker unaccelerated on an Athlon64 X2 under OpenSolaris than on a
dual-core G5 under Darwin.

Solaris 10 I gave up when we didn't make progress with the shell issues
following my bug report.

>> 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.
> 
> What would be the proper way? For example, we have hw/usb/host-linux.c
> and hw/usb/host-linux.c, should we have pty-linux.c etc, though the
> files would be a few lines each? Could all #ifdeffery be eliminated?

With the case I have in mind - someone adding a third(?) flavor of BSD
to some pty code #ifdef, I had commented with Haiku in mind that we
should introduce a feature check in configure. The problem with you
picking up a patch in such a case is that you commit it to the
repository directly and any further cleanups are being blocked as "just
churn". That's part of why the bar for new patches has gone up so much.
(In addition to git unlike svn not allowing to fix up commit messages
after the fact due to the commit hash thing.)

>> 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.
> 
> If you became the official maintainer for Solaris host, you could just
> send a pull request.

I'm considering it and would be happy to pass that on to Lee once he's
gained some more patch handling experience, if he wants. If you want to,
that's totally fine with me as well, as long as we get indirection
towards qemu.git (a reviewable queue) and good bisect-friendly patches
or at least a predictable workflow that allows objections within a
reasonable time frame.

The way we used to handle host support issues for Solaris and Darwin in
the past was by having groups of users that would cc each other for
review (which is what I requested from Lee originally).
In the end this boils down to a discussion that I've had with Anthony
earlier today about how we interpret the MAINTAINERS file. You seem to
read it as "if no one is listed with a pattern for this patch then I can
apply it without waiting for acks, and if someone is listed then that
person needs to send a PULL", which is ignoring many shades of grey
(such as the four/six-eye scheme we're employing for ppc atm).
But that's for another thread and day.

Andreas



reply via email to

[Prev in Thread] Current Thread [Next in Thread]