* [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris @ 2012-03-24 16:26 Lee Essen 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen ` (3 more replies) 0 siblings, 4 replies; 29+ messages in thread From: Lee Essen @ 2012-03-24 16:26 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andreas Färber 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" ;; AIX) aix="yes" -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-24 16:26 [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Lee Essen @ 2012-03-24 16:26 ` Lee Essen 2012-03-27 7:29 ` Stefan Hajnoczi ` (2 more replies) 2012-03-24 16:26 ` [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks " Lee Essen ` (2 subsequent siblings) 3 siblings, 3 replies; 29+ messages in thread From: Lee Essen @ 2012-03-24 16:26 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andreas Färber sigbus_reraise is used by the kvm_wait_io_event function and is needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS to the current CONFIG_LINUX only ifdef. Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> --- cpus.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/cpus.c b/cpus.c index 25ba621..6550f22 100644 --- a/cpus.c +++ b/cpus.c @@ -455,7 +455,7 @@ static void cpu_signal(int sig) exit_request = 1; } -#ifdef CONFIG_LINUX +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) static void sigbus_reraise(void) { sigset_t set; @@ -491,7 +491,9 @@ static void qemu_init_sigbus(void) action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; sigaction(SIGBUS, &action, NULL); +#if defined(CONFIG_LINUX) prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); +#endif } static void qemu_kvm_eat_signals(CPUArchState *env) -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen @ 2012-03-27 7:29 ` Stefan Hajnoczi 2012-03-27 11:41 ` Lee Essen 2012-03-27 11:41 ` Andreas Färber 2012-03-27 13:49 ` Jan Kiszka 2 siblings, 1 reply; 29+ messages in thread From: Stefan Hajnoczi @ 2012-03-27 7:29 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Andreas Färber, qemu-devel On Sat, Mar 24, 2012 at 04:26:28PM +0000, Lee Essen wrote: > sigbus_reraise is used by the kvm_wait_io_event function and is > needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS > to the current CONFIG_LINUX only ifdef. > > Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> > --- > cpus.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) I'm curious if this means Illumos KVM has MCE support? Stefan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-27 7:29 ` Stefan Hajnoczi @ 2012-03-27 11:41 ` Lee Essen 0 siblings, 0 replies; 29+ messages in thread From: Lee Essen @ 2012-03-27 11:41 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Blue Swirl, Andreas Färber, qemu-devel On 27/03/2012 08:29, Stefan Hajnoczi wrote: > On Sat, Mar 24, 2012 at 04:26:28PM +0000, Lee Essen wrote: >> sigbus_reraise is used by the kvm_wait_io_event function and is >> needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS >> to the current CONFIG_LINUX only ifdef. >> >> Signed-off-by: Lee Essen<lee.essen@nowonline.co.uk> >> --- >> cpus.c | 4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) > > I'm curious if this means Illumos KVM has MCE support? > > Stefan > KVM_CAP_MCE is defined and the supporting ioctl's seem to be there. Not having this code enabled stops it working, and it was in the original illumos port .. but that's about as far as my knowledge goes on this. Lee. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen 2012-03-27 7:29 ` Stefan Hajnoczi @ 2012-03-27 11:41 ` Andreas Färber 2012-03-27 11:45 ` Jan Kiszka 2012-03-27 13:49 ` Jan Kiszka 2 siblings, 1 reply; 29+ messages in thread From: Andreas Färber @ 2012-03-27 11:41 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Stefan Hajnoczi, Jan Kiszka, qemu-devel Am 24.03.2012 17:26, schrieb Lee Essen: > sigbus_reraise is used by the kvm_wait_io_event function and is > needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS > to the current CONFIG_LINUX only ifdef. > > Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> > --- > cpus.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 25ba621..6550f22 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -455,7 +455,7 @@ static void cpu_signal(int sig) > exit_request = 1; > } > > -#ifdef CONFIG_LINUX > +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) As asked elsewhere: Linux was the only KVM platform so far. If sigbus_reraise() is only used in some KVM function like you said, can't this we guarded with #if defined(CONFIG_KVM) or similar so that we don't have to expand this once FreeBSD etc. merge KVM support, i.e. feature-based? Andreas > static void sigbus_reraise(void) > { > sigset_t set; > @@ -491,7 +491,9 @@ static void qemu_init_sigbus(void) > action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; > sigaction(SIGBUS, &action, NULL); > > +#if defined(CONFIG_LINUX) > prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); > +#endif > } > > static void qemu_kvm_eat_signals(CPUArchState *env) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-27 11:41 ` Andreas Färber @ 2012-03-27 11:45 ` Jan Kiszka 2012-03-27 11:47 ` Jan Kiszka 0 siblings, 1 reply; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 11:45 UTC (permalink / raw) To: Andreas Färber; +Cc: Lee Essen, Blue Swirl, qemu-devel, Stefan Hajnoczi On 2012-03-27 13:41, Andreas Färber wrote: > Am 24.03.2012 17:26, schrieb Lee Essen: >> sigbus_reraise is used by the kvm_wait_io_event function and is >> needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS >> to the current CONFIG_LINUX only ifdef. >> >> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> >> --- >> cpus.c | 4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index 25ba621..6550f22 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -455,7 +455,7 @@ static void cpu_signal(int sig) >> exit_request = 1; >> } >> >> -#ifdef CONFIG_LINUX >> +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) > > As asked elsewhere: Linux was the only KVM platform so far. If Power, s390, soon also ARM? Also, this code is not KVM specific, MCE forwarding is supposed to work with TCG as well. That said, some generic HAVE_MCE_FORWARDING or so makes probably sense when there are more platform supporting it. Jan > sigbus_reraise() is only used in some KVM function like you said, can't > this we guarded with #if defined(CONFIG_KVM) or similar so that we don't > have to expand this once FreeBSD etc. merge KVM support, i.e. feature-based? > > Andreas > >> static void sigbus_reraise(void) >> { >> sigset_t set; >> @@ -491,7 +491,9 @@ static void qemu_init_sigbus(void) >> action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; >> sigaction(SIGBUS, &action, NULL); >> >> +#if defined(CONFIG_LINUX) >> prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); >> +#endif >> } >> >> static void qemu_kvm_eat_signals(CPUArchState *env) > -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-27 11:45 ` Jan Kiszka @ 2012-03-27 11:47 ` Jan Kiszka 2012-03-27 11:54 ` Jan Kiszka 0 siblings, 1 reply; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 11:47 UTC (permalink / raw) To: Andreas Färber; +Cc: Lee Essen, Blue Swirl, qemu-devel, Stefan Hajnoczi On 2012-03-27 13:45, Jan Kiszka wrote: > On 2012-03-27 13:41, Andreas Färber wrote: >> Am 24.03.2012 17:26, schrieb Lee Essen: >>> sigbus_reraise is used by the kvm_wait_io_event function and is >>> needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS >>> to the current CONFIG_LINUX only ifdef. >>> >>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> >>> --- >>> cpus.c | 4 +++- >>> 1 files changed, 3 insertions(+), 1 deletions(-) >>> >>> diff --git a/cpus.c b/cpus.c >>> index 25ba621..6550f22 100644 >>> --- a/cpus.c >>> +++ b/cpus.c >>> @@ -455,7 +455,7 @@ static void cpu_signal(int sig) >>> exit_request = 1; >>> } >>> >>> -#ifdef CONFIG_LINUX >>> +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) >> >> As asked elsewhere: Linux was the only KVM platform so far. If > > Power, s390, soon also ARM? Err, forget about this part. :) > Also, this code is not KVM specific, MCE > forwarding is supposed to work with TCG as well. > > That said, some generic HAVE_MCE_FORWARDING or so makes probably sense > when there are more platform supporting it. > > Jan > >> sigbus_reraise() is only used in some KVM function like you said, can't >> this we guarded with #if defined(CONFIG_KVM) or similar so that we don't >> have to expand this once FreeBSD etc. merge KVM support, i.e. feature-based? >> >> Andreas >> >>> static void sigbus_reraise(void) >>> { >>> sigset_t set; >>> @@ -491,7 +491,9 @@ static void qemu_init_sigbus(void) >>> action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; >>> sigaction(SIGBUS, &action, NULL); >>> >>> +#if defined(CONFIG_LINUX) >>> prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); >>> +#endif >>> } >>> >>> static void qemu_kvm_eat_signals(CPUArchState *env) >> > -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-27 11:47 ` Jan Kiszka @ 2012-03-27 11:54 ` Jan Kiszka 0 siblings, 0 replies; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 11:54 UTC (permalink / raw) To: Andreas Färber; +Cc: Lee Essen, Blue Swirl, qemu-devel, Stefan Hajnoczi On 2012-03-27 13:47, Jan Kiszka wrote: > On 2012-03-27 13:45, Jan Kiszka wrote: >> On 2012-03-27 13:41, Andreas Färber wrote: >>> Am 24.03.2012 17:26, schrieb Lee Essen: >>>> sigbus_reraise is used by the kvm_wait_io_event function and is >>>> needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS >>>> to the current CONFIG_LINUX only ifdef. >>>> >>>> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> >>>> --- >>>> cpus.c | 4 +++- >>>> 1 files changed, 3 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/cpus.c b/cpus.c >>>> index 25ba621..6550f22 100644 >>>> --- a/cpus.c >>>> +++ b/cpus.c >>>> @@ -455,7 +455,7 @@ static void cpu_signal(int sig) >>>> exit_request = 1; >>>> } >>>> >>>> -#ifdef CONFIG_LINUX >>>> +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) >>> >>> As asked elsewhere: Linux was the only KVM platform so far. If >> >> Power, s390, soon also ARM? > > Err, forget about this part. :) > >> Also, this code is not KVM specific, MCE >> forwarding is supposed to work with TCG as well. Hmm, but it's not implemented yet... Forgot about this minor detail. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise for Solaris 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen 2012-03-27 7:29 ` Stefan Hajnoczi 2012-03-27 11:41 ` Andreas Färber @ 2012-03-27 13:49 ` Jan Kiszka 2 siblings, 0 replies; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 13:49 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Andreas Färber, qemu-devel On 2012-03-24 17:26, Lee Essen wrote: > sigbus_reraise is used by the kvm_wait_io_event function and is > needed on both Linux and Solaris. This patch adds CONFIG_SOLARIS > to the current CONFIG_LINUX only ifdef. > > Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> > --- > cpus.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 25ba621..6550f22 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -455,7 +455,7 @@ static void cpu_signal(int sig) > exit_request = 1; > } > > -#ifdef CONFIG_LINUX > +#if defined(CONFIG_LINUX) || defined(CONFIG_SOLARIS) > static void sigbus_reraise(void) > { > sigset_t set; > @@ -491,7 +491,9 @@ static void qemu_init_sigbus(void) > action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler; > sigaction(SIGBUS, &action, NULL); > > +#if defined(CONFIG_LINUX) > prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0); > +#endif BTW, this looks suspicious. Are you sure Solaris delivers a compatible SIGBUS with all the information KVM needs to translate it to a MCE? That is not a KVM subsystem feature, it's a kernel feature that Solaris would either have to provide in the same way as Linux, or you need some glue code to translate the differences. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-24 16:26 [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Lee Essen 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen @ 2012-03-24 16:26 ` Lee Essen 2012-03-27 15:01 ` Paolo Bonzini 2012-03-24 16:26 ` [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC Lee Essen 2012-03-27 7:23 ` [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Stefan Hajnoczi 3 siblings, 1 reply; 29+ messages in thread From: Lee Essen @ 2012-03-24 16:26 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andreas Färber Dynticks was limited to linux. This patch adds Solaris support and ensures a CLOCK_HIGHRES clock is used which is the optimal setup for Solaris systems. Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> --- qemu-timer.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index d7f56e5..48817c9 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -77,7 +77,7 @@ struct qemu_alarm_timer { int (*start)(struct qemu_alarm_timer *t); void (*stop)(struct qemu_alarm_timer *t); void (*rearm)(struct qemu_alarm_timer *t, int64_t nearest_delta_ns); -#if defined(__linux__) +#if defined(__linux__) || defined(__sun__) int fd; timer_t timer; #elif defined(_WIN32) @@ -165,7 +165,7 @@ static int unix_start_timer(struct qemu_alarm_timer *t); static void unix_stop_timer(struct qemu_alarm_timer *t); static void unix_rearm_timer(struct qemu_alarm_timer *t, int64_t delta); -#ifdef __linux__ +#if defined(__linux__) || defined(__sun__) static int dynticks_start_timer(struct qemu_alarm_timer *t); static void dynticks_stop_timer(struct qemu_alarm_timer *t); @@ -177,7 +177,7 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t, int64_t delta); static struct qemu_alarm_timer alarm_timers[] = { #ifndef _WIN32 -#ifdef __linux__ +#if defined(__linux__) || defined(__sun__) {"dynticks", dynticks_start_timer, dynticks_stop_timer, dynticks_rearm_timer}, #endif @@ -502,7 +502,7 @@ static void host_alarm_handler(int host_signum) } } -#if defined(__linux__) +#if defined(__linux__) || defined(__sun__) #include "compatfd.h" @@ -533,7 +533,11 @@ static int dynticks_start_timer(struct qemu_alarm_timer *t) #endif /* SIGEV_THREAD_ID */ ev.sigev_signo = SIGALRM; +#if defined(__sun__) + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { +#else if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { +#endif perror("timer_create"); /* disable dynticks */ @@ -585,7 +589,7 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t, } } -#endif /* defined(__linux__) */ +#endif /* defined(__linux__) || defined(__sun__) */ #if !defined(_WIN32) -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-24 16:26 ` [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks " Lee Essen @ 2012-03-27 15:01 ` Paolo Bonzini 2012-03-27 15:08 ` Jan Kiszka 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2012-03-27 15:01 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Jan Kiszka, Andreas Färber, qemu-devel Il 24/03/2012 17:26, Lee Essen ha scritto: > Dynticks was limited to linux. This patch adds Solaris support > and ensures a CLOCK_HIGHRES clock is used which is the optimal > setup for Solaris systems. Looks good, but I would prefer if you tested for timer_create in configure and use #ifdef CONFIG_RT_TIMER instead. > +#if defined(__sun__) > + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { > +#else > if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { > +#endif This should be #ifdef CLOCK_HIGHRES. Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-27 15:01 ` Paolo Bonzini @ 2012-03-27 15:08 ` Jan Kiszka 2012-03-27 15:52 ` Paolo Bonzini 0 siblings, 1 reply; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 15:08 UTC (permalink / raw) To: Paolo Bonzini Cc: Lee Essen, Blue Swirl, Andreas Färber, qemu-devel@nongnu.org On 2012-03-27 17:01, Paolo Bonzini wrote: > Il 24/03/2012 17:26, Lee Essen ha scritto: >> Dynticks was limited to linux. This patch adds Solaris support >> and ensures a CLOCK_HIGHRES clock is used which is the optimal >> setup for Solaris systems. > > Looks good, but I would prefer if you tested for timer_create in > configure and use #ifdef CONFIG_RT_TIMER instead. > >> +#if defined(__sun__) >> + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { >> +#else >> if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { >> +#endif > > This should be #ifdef CLOCK_HIGHRES. Are we sure about this is and will remain equivalent and correct? Also, I found some man page that says CLOCK_HIGHRES is non-adjustable while CLOCK_REALTIME is. That should make a difference in QEMU. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-27 15:08 ` Jan Kiszka @ 2012-03-27 15:52 ` Paolo Bonzini 2012-03-27 16:00 ` Jan Kiszka 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2012-03-27 15:52 UTC (permalink / raw) To: Jan Kiszka Cc: Lee Essen, Blue Swirl, Andreas Färber, qemu-devel@nongnu.org Il 27/03/2012 17:08, Jan Kiszka ha scritto: >>> +#if defined(__sun__) >>> + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { >>> +#else >>> if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { >>> +#endif >> >> This should be #ifdef CLOCK_HIGHRES. > > Are we sure about this is and will remain equivalent and correct? > > Also, I found some man page that says CLOCK_HIGHRES is non-adjustable > while CLOCK_REALTIME is. That should make a difference in QEMU. Right, that's why I CCed you but then I forgot to ask the question. Does QEMU rely on CLOCK_REALTIME when "-rtc clock=host" is in use? A monotonic clock would work better when CLOCK_REALTIME jumps backwards (DST->solar). If the jump goes unnoticed, the alarm timer would have no timeout for an hour or so. Of course the opposite is true when going from solar time to DST; you move the realtime clock one hour forward and, with CLOCK_MONOTONIC, a host_clock timer to trigger an hour too late. Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-27 15:52 ` Paolo Bonzini @ 2012-03-27 16:00 ` Jan Kiszka 2012-03-27 17:49 ` Peter Portante 0 siblings, 1 reply; 29+ messages in thread From: Jan Kiszka @ 2012-03-27 16:00 UTC (permalink / raw) To: Paolo Bonzini Cc: Lee Essen, Blue Swirl, Andreas Färber, qemu-devel@nongnu.org On 2012-03-27 17:52, Paolo Bonzini wrote: > Il 27/03/2012 17:08, Jan Kiszka ha scritto: >>>> +#if defined(__sun__) >>>> + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { >>>> +#else >>>> if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { >>>> +#endif >>> >>> This should be #ifdef CLOCK_HIGHRES. >> >> Are we sure about this is and will remain equivalent and correct? >> >> Also, I found some man page that says CLOCK_HIGHRES is non-adjustable >> while CLOCK_REALTIME is. That should make a difference in QEMU. > > Right, that's why I CCed you but then I forgot to ask the question. > > Does QEMU rely on CLOCK_REALTIME when "-rtc clock=host" is in use? A > monotonic clock would work better when CLOCK_REALTIME jumps backwards > (DST->solar). If the jump goes unnoticed, the alarm timer would have no > timeout for an hour or so. > > Of course the opposite is true when going from solar time to DST; you > move the realtime clock one hour forward and, with CLOCK_MONOTONIC, a > host_clock timer to trigger an hour too late. But the latter would be fixed up when we actually check for expired timers against the proper clocks. Hmm, I'm not sure anymore if we really need CLOCK_REALTIME for the timer here. This also has no telling history. Maybe the contributor of dynticks just didn't think about this aspect of REALTIME vs. MONOTONIC. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris 2012-03-27 16:00 ` Jan Kiszka @ 2012-03-27 17:49 ` Peter Portante 0 siblings, 0 replies; 29+ messages in thread From: Peter Portante @ 2012-03-27 17:49 UTC (permalink / raw) To: Jan Kiszka Cc: Lee Essen, Paolo Bonzini, Andreas Färber, qemu-devel@nongnu.org, Blue Swirl [-- Attachment #1: Type: text/plain, Size: 1789 bytes --] There exists a simple patch that changes the wait loop timer to use CLOCK_MONOTONIC. See https://github.com/portante/qemu/commit/35c92daa9784882153c6d8b0e15e8c8f181d6e8a . -peter On Tue, Mar 27, 2012 at 12:00 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2012-03-27 17:52, Paolo Bonzini wrote: > > Il 27/03/2012 17:08, Jan Kiszka ha scritto: > >>>> +#if defined(__sun__) > >>>> + if (timer_create(CLOCK_HIGHRES, &ev, &host_timer)) { > >>>> +#else > >>>> if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { > >>>> +#endif > >>> > >>> This should be #ifdef CLOCK_HIGHRES. > >> > >> Are we sure about this is and will remain equivalent and correct? > >> > >> Also, I found some man page that says CLOCK_HIGHRES is non-adjustable > >> while CLOCK_REALTIME is. That should make a difference in QEMU. > > > > Right, that's why I CCed you but then I forgot to ask the question. > > > > Does QEMU rely on CLOCK_REALTIME when "-rtc clock=host" is in use? A > > monotonic clock would work better when CLOCK_REALTIME jumps backwards > > (DST->solar). If the jump goes unnoticed, the alarm timer would have no > > timeout for an hour or so. > > > > Of course the opposite is true when going from solar time to DST; you > > move the realtime clock one hour forward and, with CLOCK_MONOTONIC, a > > host_clock timer to trigger an hour too late. > > But the latter would be fixed up when we actually check for expired > timers against the proper clocks. Hmm, I'm not sure anymore if we really > need CLOCK_REALTIME for the timer here. This also has no telling > history. Maybe the contributor of dynticks just didn't think about this > aspect of REALTIME vs. MONOTONIC. > > Jan > > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux > > [-- Attachment #2: Type: text/html, Size: 2541 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC 2012-03-24 16:26 [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Lee Essen 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen 2012-03-24 16:26 ` [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks " Lee Essen @ 2012-03-24 16:26 ` Lee Essen 2012-03-27 14:56 ` Paolo Bonzini 2012-03-27 7:23 ` [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Stefan Hajnoczi 3 siblings, 1 reply; 29+ messages in thread From: Lee Essen @ 2012-03-24 16:26 UTC (permalink / raw) To: qemu-devel; +Cc: Blue Swirl, Andreas Färber Solaris does not support the O_ASYNC option to open, this patch adds the same functionality through the I_SETSIG ioctl. Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> --- qga/channel-posix.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index 40f7658..86245c1 100644 --- a/qga/channel-posix.c +++ b/qga/channel-posix.c @@ -3,6 +3,10 @@ #include "qemu_socket.h" #include "qga/channel.h" +#ifdef CONFIG_SOLARIS +#include <sys/stropts.h> +#endif + #define GA_CHANNEL_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ struct GAChannel { @@ -123,7 +127,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod switch (c->method) { case GA_CHANNEL_VIRTIO_SERIAL: { +#ifdef CONFIG_SOLARIS + int fd = qemu_open(path, O_RDWR | O_NONBLOCK); + if (fd == -1) { + g_critical("error opening channel: %s", strerror(errno)); + exit(EXIT_FAILURE); + } + if (ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI) < 0) { + g_critical("error with setsig on channel: %s", strerror(errno)); + exit(EXIT_FAILURE); + } +#else int fd = qemu_open(path, O_RDWR | O_NONBLOCK | O_ASYNC); +#endif if (fd == -1) { g_critical("error opening channel: %s", strerror(errno)); exit(EXIT_FAILURE); -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC 2012-03-24 16:26 ` [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC Lee Essen @ 2012-03-27 14:56 ` Paolo Bonzini 2012-03-27 15:12 ` Andreas Färber 0 siblings, 1 reply; 29+ messages in thread From: Paolo Bonzini @ 2012-03-27 14:56 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Andreas Färber, qemu-devel Il 24/03/2012 17:26, Lee Essen ha scritto: > Solaris does not support the O_ASYNC option to open, this patch > adds the same functionality through the I_SETSIG ioctl. > > Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> > --- > qga/channel-posix.c | 16 ++++++++++++++++ > 1 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/qga/channel-posix.c b/qga/channel-posix.c > index 40f7658..86245c1 100644 > --- a/qga/channel-posix.c > +++ b/qga/channel-posix.c > @@ -3,6 +3,10 @@ > #include "qemu_socket.h" > #include "qga/channel.h" > > +#ifdef CONFIG_SOLARIS > +#include <sys/stropts.h> > +#endif > + > #define GA_CHANNEL_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ > > struct GAChannel { > @@ -123,7 +127,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod > > switch (c->method) { > case GA_CHANNEL_VIRTIO_SERIAL: { > +#ifdef CONFIG_SOLARIS > + int fd = qemu_open(path, O_RDWR | O_NONBLOCK); > + if (fd == -1) { > + g_critical("error opening channel: %s", strerror(errno)); > + exit(EXIT_FAILURE); > + } > + if (ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI) < 0) { > + g_critical("error with setsig on channel: %s", strerror(errno)); > + exit(EXIT_FAILURE); > + } > +#else > int fd = qemu_open(path, O_RDWR | O_NONBLOCK | O_ASYNC); > +#endif > if (fd == -1) { > g_critical("error opening channel: %s", strerror(errno)); > exit(EXIT_FAILURE); Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC 2012-03-27 14:56 ` Paolo Bonzini @ 2012-03-27 15:12 ` Andreas Färber 0 siblings, 0 replies; 29+ messages in thread From: Andreas Färber @ 2012-03-27 15:12 UTC (permalink / raw) To: Paolo Bonzini, Lee Essen; +Cc: Blue Swirl, qemu-devel Am 27.03.2012 16:56, schrieb Paolo Bonzini: > Il 24/03/2012 17:26, Lee Essen ha scritto: >> Solaris does not support the O_ASYNC option to open, this patch >> adds the same functionality through the I_SETSIG ioctl. >> >> Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> >> --- >> qga/channel-posix.c | 16 ++++++++++++++++ >> 1 files changed, 16 insertions(+), 0 deletions(-) >> >> diff --git a/qga/channel-posix.c b/qga/channel-posix.c >> index 40f7658..86245c1 100644 >> --- a/qga/channel-posix.c >> +++ b/qga/channel-posix.c >> @@ -3,6 +3,10 @@ >> #include "qemu_socket.h" >> #include "qga/channel.h" >> >> +#ifdef CONFIG_SOLARIS >> +#include <sys/stropts.h> >> +#endif >> + >> #define GA_CHANNEL_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ >> >> struct GAChannel { >> @@ -123,7 +127,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod >> >> switch (c->method) { >> case GA_CHANNEL_VIRTIO_SERIAL: { >> +#ifdef CONFIG_SOLARIS >> + int fd = qemu_open(path, O_RDWR | O_NONBLOCK); >> + if (fd == -1) { >> + g_critical("error opening channel: %s", strerror(errno)); >> + exit(EXIT_FAILURE); >> + } >> + if (ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI) < 0) { >> + g_critical("error with setsig on channel: %s", strerror(errno)); >> + exit(EXIT_FAILURE); >> + } >> +#else >> int fd = qemu_open(path, O_RDWR | O_NONBLOCK | O_ASYNC); >> +#endif >> if (fd == -1) { >> g_critical("error opening channel: %s", strerror(errno)); >> exit(EXIT_FAILURE); > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> I would like to see this improved: This unnecessarily duplicates code into a rarely tested code path, we have the fd == -1 check twice in the suggested Solaris code path above. I would rather have the ioctl in a separate ifdef section below to avoid that. For O_ASYNC someone previously suggested to #define O_ASYNC for Solaris, for which I have an experimental osdep.h patch in my VM. The two options were either 0 or O_NDELAY(?) iirc; don't like the former and not sure about the semantics of the latter. If that's no longer desired, we can at least restrict the #ifdeffery to | O_ASYNC by breaking the line. Andreas ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-24 16:26 [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Lee Essen ` (2 preceding siblings ...) 2012-03-24 16:26 ` [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC Lee Essen @ 2012-03-27 7:23 ` Stefan Hajnoczi 2012-03-27 11:31 ` Andreas Färber 3 siblings, 1 reply; 29+ messages in thread From: Stefan Hajnoczi @ 2012-03-27 7:23 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Andreas Färber, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 7:23 ` [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Stefan Hajnoczi @ 2012-03-27 11:31 ` Andreas Färber 2012-03-27 12:01 ` Lee Essen 0 siblings, 1 reply; 29+ messages in thread From: Andreas Färber @ 2012-03-27 11:31 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Lee Essen, Blue Swirl, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 11:31 ` Andreas Färber @ 2012-03-27 12:01 ` Lee Essen 2012-03-27 13:06 ` Stefan Hajnoczi 2012-03-27 13:14 ` Andreas Färber 0 siblings, 2 replies; 29+ messages in thread From: Lee Essen @ 2012-03-27 12:01 UTC (permalink / raw) To: Andreas Färber; +Cc: Blue Swirl, Stefan Hajnoczi, qemu-devel 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 12:01 ` Lee Essen @ 2012-03-27 13:06 ` Stefan Hajnoczi 2012-03-27 13:56 ` Andreas Färber 2012-03-27 13:14 ` Andreas Färber 1 sibling, 1 reply; 29+ messages in thread From: Stefan Hajnoczi @ 2012-03-27 13:06 UTC (permalink / raw) To: Lee Essen, Andreas Färber; +Cc: Blue Swirl, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 13:06 ` Stefan Hajnoczi @ 2012-03-27 13:56 ` Andreas Färber 2012-03-27 17:24 ` Blue Swirl 0 siblings, 1 reply; 29+ messages in thread From: Andreas Färber @ 2012-03-27 13:56 UTC (permalink / raw) To: Stefan Hajnoczi, Lee Essen; +Cc: Blue Swirl, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 13:56 ` Andreas Färber @ 2012-03-27 17:24 ` Blue Swirl 2012-03-28 18:41 ` Andreas Färber 2012-03-28 19:46 ` Andreas Färber 0 siblings, 2 replies; 29+ messages in thread From: Blue Swirl @ 2012-03-27 17:24 UTC (permalink / raw) To: Andreas Färber; +Cc: Lee Essen, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 17:24 ` Blue Swirl @ 2012-03-28 18:41 ` Andreas Färber 2012-03-28 19:46 ` Andreas Färber 1 sibling, 0 replies; 29+ messages in thread From: Andreas Färber @ 2012-03-28 18:41 UTC (permalink / raw) To: Blue Swirl; +Cc: Lee Essen, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 17:24 ` Blue Swirl 2012-03-28 18:41 ` Andreas Färber @ 2012-03-28 19:46 ` Andreas Färber 1 sibling, 0 replies; 29+ messages in thread From: Andreas Färber @ 2012-03-28 19:46 UTC (permalink / raw) To: Blue Swirl; +Cc: Lee Essen, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 12:01 ` Lee Essen 2012-03-27 13:06 ` Stefan Hajnoczi @ 2012-03-27 13:14 ` Andreas Färber 2012-03-27 17:06 ` Blue Swirl 1 sibling, 1 reply; 29+ messages in thread From: Andreas Färber @ 2012-03-27 13:14 UTC (permalink / raw) To: Lee Essen; +Cc: Blue Swirl, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 13:14 ` Andreas Färber @ 2012-03-27 17:06 ` Blue Swirl 2012-03-28 17:44 ` Andreas Färber 0 siblings, 1 reply; 29+ messages in thread From: Blue Swirl @ 2012-03-27 17:06 UTC (permalink / raw) To: Andreas Färber; +Cc: Lee Essen, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris 2012-03-27 17:06 ` Blue Swirl @ 2012-03-28 17:44 ` Andreas Färber 0 siblings, 0 replies; 29+ messages in thread From: Andreas Färber @ 2012-03-28 17:44 UTC (permalink / raw) To: Blue Swirl; +Cc: Lee Essen, Stefan Hajnoczi, qemu-devel 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2012-03-28 19:47 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-24 16:26 [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Lee Essen 2012-03-24 16:26 ` [Qemu-devel] [PATCH 2/4] Enable sigbus_reraise " Lee Essen 2012-03-27 7:29 ` Stefan Hajnoczi 2012-03-27 11:41 ` Lee Essen 2012-03-27 11:41 ` Andreas Färber 2012-03-27 11:45 ` Jan Kiszka 2012-03-27 11:47 ` Jan Kiszka 2012-03-27 11:54 ` Jan Kiszka 2012-03-27 13:49 ` Jan Kiszka 2012-03-24 16:26 ` [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks " Lee Essen 2012-03-27 15:01 ` Paolo Bonzini 2012-03-27 15:08 ` Jan Kiszka 2012-03-27 15:52 ` Paolo Bonzini 2012-03-27 16:00 ` Jan Kiszka 2012-03-27 17:49 ` Peter Portante 2012-03-24 16:26 ` [Qemu-devel] [PATCH 4/4] qga/channel-posix: provide Solaris alternative to O_ASYNC Lee Essen 2012-03-27 14:56 ` Paolo Bonzini 2012-03-27 15:12 ` Andreas Färber 2012-03-27 7:23 ` [Qemu-devel] [PATCH 1/4] Add socket/xnet libs to configure for Solaris Stefan Hajnoczi 2012-03-27 11:31 ` Andreas Färber 2012-03-27 12:01 ` Lee Essen 2012-03-27 13:06 ` Stefan Hajnoczi 2012-03-27 13:56 ` Andreas Färber 2012-03-27 17:24 ` Blue Swirl 2012-03-28 18:41 ` Andreas Färber 2012-03-28 19:46 ` Andreas Färber 2012-03-27 13:14 ` Andreas Färber 2012-03-27 17:06 ` Blue Swirl 2012-03-28 17:44 ` Andreas Färber
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).