| Message ID | 1332606390-3605-1-git-send-email-lee.essen@nowonline.co.uk |
|---|---|
| State | New |
| Headers | show |
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 <lee.essen@nowonline.co.uk> > --- > 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? Stefan
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 <lee.essen@nowonline.co.uk> >> --- >> 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. Andreas
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<lee.essen@nowonline.co.uk> >>> --- >>> 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. Lee.
On Tue, Mar 27, 2012 at 1:01 PM, Lee Essen <lee.essen@nowonline.co.uk> 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<lee.essen@nowonline.co.uk> >>>> --- >>>> 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? AFAICT Lee has been putting in good effort. If he forgot to address review comments, we've all done that by mistake. Stefan
Am 27.03.2012 14:01, schrieb Lee Essen: > 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<lee.essen@nowonline.co.uk> >>>> --- >>>> 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. Sorry if this was harsh for you, you have indeed been trying and improving things, but my issue is this: <rant> Apart from the C99 patch that has been committed now, QEMU has been working fine for me as inofficial maintainer of Solaris host support. KVM was never supported on illumos in upstream QEMU and it's not even in upstream KVM AFAIK. It might even never be merged due to licensing issues. So this is a new, optional feature and not a breakage. Yet you keep pushing for this. You send patches on Friday afternoon and on Monday noon do a slightly improved repost. This is my job now and I do not work on it every weekend. I would rather see you not rush things so much and put more emphasis on quality of submission and investigation of why, what and how. People like you have occasionally appeared out of nowhere, submitted a few patches and left again, leaving two hands full of core contributors with the code. So it must be easily maintainable for us. Especially code that does #if oneplatform||anotherplatform is really bad because it will mean that someone else will soon come and want to add ||thirdplatform. My main point however is that you keep sending patches in an egocentrical rather than maintainer-centric way, which we have already discussed recently with David for pseries. I would've preferred that you not send everything *you* need for your goal of SmartOS support in one large series, but a patch to Paolo about qemu-timer (and I was serious about the prefix notation, there's many good example on the list and I made it really easy for you to just copy&paste) that I could just ack and maybe apply through qemu-trivial, a patch about the KVM stuff that Jan/Marcello et al. could handle, and qemu-ga in a small series that Michael could handle and I would ack (qemu-ga being unneeded for most use cases, easy to disable and therefore even less inconvenient than our broken Darwin host support). Your saying that you will maintain this in a separate tree now shows me even more that you have not yet understood what the problem with your submissions is that I have been trying to guide you to tackle. Maybe someone else can explain better, e.g. on IRC where some of the discussions would be much easier to conduct. </rant> Andreas
Am 27.03.2012 15:06, schrieb Stefan Hajnoczi: > On Tue, Mar 27, 2012 at 1:01 PM, Lee Essen <lee.essen@nowonline.co.uk> 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<lee.essen@nowonline.co.uk> >>>>> --- >>>>> 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
On Tue, Mar 27, 2012 at 13:14, Andreas Färber <andreas.faerber@web.de> wrote: > Am 27.03.2012 14:01, schrieb Lee Essen: >> 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<lee.essen@nowonline.co.uk> >>>>> --- >>>>> 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. > > Sorry if this was harsh for you, you have indeed been trying and > improving things, but my issue is this: > > <rant> > > Apart from the C99 patch that has been committed now, QEMU has been > working fine for me as inofficial maintainer of Solaris host support. Unofficial maintainer? git log --format="%aN: %s" | grep -i solaris tells a different story. But please propose yourself as the official maintainer, for example I'm no longer interested in Solaris and the other Solaris patch submitters have also stopped contributing. > KVM was never supported on illumos in upstream QEMU and it's not even in > upstream KVM AFAIK. It might even never be merged due to licensing > issues. So this is a new, optional feature and not a breakage. > > Yet you keep pushing for this. You send patches on Friday afternoon and > on Monday noon do a slightly improved repost. This is my job now and I > do not work on it every weekend. I would rather see you not rush things > so much and put more emphasis on quality of submission and investigation > of why, what and how. > People like you have occasionally appeared out of nowhere, submitted a > few patches and left again, leaving two hands full of core contributors > with the code. So it must be easily maintainable for us. > > Especially code that does #if oneplatform||anotherplatform is really bad > because it will mean that someone else will soon come and want to add > ||thirdplatform. > > My main point however is that you keep sending patches in an > egocentrical rather than maintainer-centric way, which we have already > discussed recently with David for pseries. I would've preferred that you > not send everything *you* need for your goal of SmartOS support in one > large series, but a patch to Paolo about qemu-timer (and I was serious > about the prefix notation, there's many good example on the list and I > made it really easy for you to just copy&paste) that I could just ack > and maybe apply through qemu-trivial, a patch about the KVM stuff that > Jan/Marcello et al. could handle, and qemu-ga in a small series that > Michael could handle and I would ack (qemu-ga being unneeded for most > use cases, easy to disable and therefore even less inconvenient than our > broken Darwin host support). > > Your saying that you will maintain this in a separate tree now shows me > even more that you have not yet understood what the problem with your > submissions is that I have been trying to guide you to tackle. Maybe > someone else can explain better, e.g. on IRC where some of the > discussions would be much easier to conduct. > > </rant> > > Andreas
On Tue, Mar 27, 2012 at 13:56, Andreas Färber <andreas.faerber@web.de> wrote: > Am 27.03.2012 15:06, schrieb Stefan Hajnoczi: >> On Tue, Mar 27, 2012 at 1:01 PM, Lee Essen <lee.essen@nowonline.co.uk> 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<lee.essen@nowonline.co.uk> >>>>>> --- >>>>>> 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 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 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. 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? > 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. If you became the official maintainer for Solaris host, you could just send a pull request. > Andreas
Am 27.03.2012 19:06, schrieb Blue Swirl: > On Tue, Mar 27, 2012 at 13:14, Andreas Färber <andreas.faerber@web.de> wrote: >> Am 27.03.2012 14:01, schrieb Lee Essen: >>> 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<lee.essen@nowonline.co.uk> >>>>>> --- >>>>>> 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. >> >> Sorry if this was harsh for you, you have indeed been trying and >> improving things, but my issue is this: >> >> <rant> >> >> Apart from the C99 patch that has been committed now, QEMU has been >> working fine for me as inofficial maintainer of Solaris host support. > > Unofficial maintainer? > git log --format="%aN: %s" | grep -i solaris > tells a different story. And what is that command supposed to show? I've been working with Solaris/amd64 since University until the unfortunate Oracle thing happened, and when you search for afaerber@opensolaris.org on qemu-devel and in history you should find some results. I'm not claiming to have done the original port, nor to have posted loads of patches (which you seem to be checking on above) but I certainly did take care of breakages on OpenSolaris including most importantly reworking our Makefiles to no longer rely on static libraries after device_init() and friends were introduced. Denying that would be an insult. Andreas
Am 27.03.2012 19:24, schrieb Blue Swirl: > On Tue, Mar 27, 2012 at 13:56, Andreas Färber <andreas.faerber@web.de> wrote: >> Am 27.03.2012 15:06, schrieb Stefan Hajnoczi: >>> On Tue, Mar 27, 2012 at 1:01 PM, Lee Essen <lee.essen@nowonline.co.uk> 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<lee.essen@nowonline.co.uk> >>>>>>> --- >>>>>>> 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
Am 27.03.2012 19:24, schrieb Blue Swirl: > On Tue, Mar 27, 2012 at 13:56, Andreas Färber <andreas.faerber@web.de> wrote: >> [...] 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 think I haven't been aware of > this route. When did Solaris patches go through you, could you name > some? Just stumbled over this branch with e88131d2177cf12dba99c13fe5ff6a21d8fb7dc1 from Sep 19, 2010 at its head: http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/solaris Guess whom it was committed by? http://repo.or.cz/w/qemu.git/commit/e78815a554adaa551d62a71be10ee2fcf128e473 As you can see, the more recent C99 patch - which Avi offered to write in response to my(!) report - is not in there, since this branch is still from my old OpenSolaris machine that I screwed up during an attempted migration to OpenIndiana. But like I said elsewhere, further patches are not strictly needed when building without guest agent and tracing and running in a bash environment, thus no need for tons of patches for a working platform. So had you wanted to, it would've been really easy to find evidence that I was the last one around here working on Solaris/illumos support. Andreas
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" ;; AIX) aix="yes"
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 <lee.essen@nowonline.co.uk> --- configure | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)