All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* [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 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 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 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 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 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 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 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

* 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 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 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

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 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 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

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

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 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 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 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, 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

* 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

* 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.