| Message ID | 10309083-5E08-4C90-A985-79B76344867E@nowonline.co.uk |
|---|---|
| State | New |
| Headers | show |
On Sat, Mar 17, 2012 at 08:04, Lee Essen <lee.essen@nowonline.co.uk> wrote: > (third email attempt, apologies if you get duplicates) The patch does not apply: git am: Applying: Basic Illumos support fatal: corrupt patch at line 19 Patch failed at 0001 Basic Illumos support patch: patching file configure patch: **** malformed patch at line 19: ;; Unchanged lines in a patch should be preceded by a space character and these are missing from your message. Please check your mail configuration, use git-send-email or if all else fails, attach the patch. > This patch adds some basic constructs to better support Illumos/Solaris. > > I've kept away from kvm, configure etc. This just covers making sure the > right libs are used, and the code is Solaris/Illumos compatible. > > In qemu-timer.c there are lots of __linux__ || __sun__ constructs, I wanted > to make sure I didn't alter the linux behaviour and this seemed the safest > way to do it. > > Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> > > --- > > configure | 1 + > cpus.c | 4 +++- > qemu-timer.c | 14 +++++++++----- > qga/channel-posix.c | 16 ++++++++++++++++ > qga/commands-posix.c | 9 +++++++++ > 5 files changed, 38 insertions(+), 6 deletions(-) > > diff --git a/configure b/configure > index afe7395..68cc3a7 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" > 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) > 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) > > 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, GAChanne > > 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); > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 7b2be2f..67531aa 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -35,6 +35,11 @@ > #include "qemu-queue.h" > #include "host-utils.h" > > +#if defined(__sun__) > +#include <sys/sockio.h> > +extern char **environ; > +#endif > + > static void reopen_fd_to_null(int fd) > { > int nullfd; > @@ -807,7 +812,11 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) > goto error; > } > > +#if defined(__sun__) > + mac_addr = (unsigned char *) &ifr.ifr_enaddr; > +#else > mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; > +#endif > > if (asprintf(&info->value->hardware_address, > "%02x:%02x:%02x:%02x:%02x:%02x", > >
On 24 Mar 2012, at 12:56, Blue Swirl wrote: > On Sat, Mar 17, 2012 at 08:04, Lee Essen <lee.essen@nowonline.co.uk> wrote: >> (third email attempt, apologies if you get duplicates) > > The patch does not apply: > git am: > Applying: Basic Illumos support > fatal: corrupt patch at line 19 > Patch failed at 0001 Basic Illumos support > > patch: > patching file configure > patch: **** malformed patch at line 19: ;; > > Unchanged lines in a patch should be preceded by a space character and > these are missing from your message. Please check your mail > configuration, use git-send-email or if all else fails, attach the > patch. > Me and patches just aren't working well! It's fine in my outbox, it must be being stripped by my mail system or mailer somewhere, I'm having trouble getting git-send-mail to work because of auth issues, so I've attached the patch for now. Apologies. Lee.
Am 24.03.2012 15:51, schrieb Lee Essen: > On 24 Mar 2012, at 12:56, Blue Swirl wrote: > >> On Sat, Mar 17, 2012 at 08:04, Lee Essen <lee.essen@nowonline.co.uk> wrote: >>> (third email attempt, apologies if you get duplicates) I did receive all three fine btw. [...] > Me and patches just aren't working well! It's fine in my outbox, it must be being stripped by my mail system or mailer somewhere, I'm having trouble getting git-send-mail to work because of auth issues, so I've attached the patch for now. You may need to install some Perl modules via cpan, assuming you have ssl / tls and username set correctly. What's the error? I'm not yet okay with this patch: First, never put personal comments such as resends above the --- line of a [PATCH], comments can go under ---, indented by one space. Please describe in a neutral way what the patch does, how QEMU behaves with/without (warning, error, new feature?), but not what you've tried, not done, want, etc. in the course of writing the patch - that would go into a Change Log (which can go below --- or in the cover letter). This is still too many changes at once, without proper explanation. A good patch IMO would be one that, e.g., only adds the libs to configure and explains in the commit message for what functions those are needed, so that we don't get the same breakage immediately after elsewhere. Maybe we should even move them to a more central place as a precaution? qemu-timer should be one patch ("qemu-timer: Enable dynticks for Solaris"; why was dynticks limited to Linux?), cpus (where is sigbus_reraise() being used? again, it being Linux-only looks odd), qemu-ga (1. O_ASYNC, 2. mac_addr, 3. libs). You will find that the smaller a patch is, the easier it is to get it in since less different maintainers will be involved in acking it and since they are less likely to postpone review. Andreas
On 24 Mar 2012, at 15:44, Andreas Färber wrote: > Am 24.03.2012 15:51, schrieb Lee Essen: >> On 24 Mar 2012, at 12:56, Blue Swirl wrote: >> >>> On Sat, Mar 17, 2012 at 08:04, Lee Essen <lee.essen@nowonline.co.uk> wrote: >>>> (third email attempt, apologies if you get duplicates) > > I did receive all three fine btw. > > [...] >> Me and patches just aren't working well! It's fine in my outbox, it must be being stripped by my mail system or mailer somewhere, I'm having trouble getting git-send-mail to work because of auth issues, so I've attached the patch for now. > > You may need to install some Perl modules via cpan, assuming you have > ssl / tls and username set correctly. What's the error? It was having problems with auth, tis was working ok. I've resolved it now by reconfiguring my mail server to allow my illumos dev host to relay without auth. > > I'm not yet okay with this patch: > > First, never put personal comments such as resends above the --- line of > a [PATCH], comments can go under ---, indented by one space. Please > describe in a neutral way what the patch does, how QEMU behaves > with/without (warning, error, new feature?), but not what you've tried, > not done, want, etc. in the course of writing the patch - that would go > into a Change Log (which can go below --- or in the cover letter). > > This is still too many changes at once, without proper explanation. A > good patch IMO would be one that, e.g., only adds the libs to configure > and explains in the commit message for what functions those are needed, > so that we don't get the same breakage immediately after elsewhere. > Maybe we should even move them to a more central place as a precaution? > > qemu-timer should be one patch ("qemu-timer: Enable dynticks for > Solaris"; why was dynticks limited to Linux?), > cpus (where is sigbus_reraise() being used? again, it being Linux-only > looks odd), > qemu-ga (1. O_ASYNC, 2. mac_addr, 3. libs). > > You will find that the smaller a patch is, the easier it is to get it in > since less different maintainers will be involved in acking it and since > they are less likely to postpone review. Ok … I'll break them down as you suggest and provide better descriptions. Regards, Lee.
On Sat, Mar 24, 2012 at 03:50:06PM +0000, Lee Essen wrote: > > On 24 Mar 2012, at 15:44, Andreas Färber wrote: > > > Am 24.03.2012 15:51, schrieb Lee Essen: > >> On 24 Mar 2012, at 12:56, Blue Swirl wrote: > >> > >>> On Sat, Mar 17, 2012 at 08:04, Lee Essen <lee.essen@nowonline.co.uk> wrote: > >>>> (third email attempt, apologies if you get duplicates) > > > > I did receive all three fine btw. > > > > [...] > >> Me and patches just aren't working well! It's fine in my outbox, it must be being stripped by my mail system or mailer somewhere, I'm having trouble getting git-send-mail to work because of auth issues, so I've attached the patch for now. > > > > You may need to install some Perl modules via cpan, assuming you have > > ssl / tls and username set correctly. What's the error? > > It was having problems with auth, tis was working ok. I've resolved it now by reconfiguring my mail server to allow my illumos dev host to relay without auth. I had a mail server that required TLS but no actual authentication. git-send-email(1) didn't like that and it required tweaking the source slightly to skip login after the TLS session is established when no password was specified. It sounds like your issue might be slightly different, but if it's the same I can share the hack. Stefan
diff --git a/configure b/configure index afe7395..68cc3a7 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" 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) 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) 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, GAChanne 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); diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 7b2be2f..67531aa 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -35,6 +35,11 @@ #include "qemu-queue.h" #include "host-utils.h" +#if defined(__sun__) +#include <sys/sockio.h> +extern char **environ; +#endif + static void reopen_fd_to_null(int fd) { int nullfd; @@ -807,7 +812,11 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp) goto error; } +#if defined(__sun__) + mac_addr = (unsigned char *) &ifr.ifr_enaddr; +#else mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data; +#endif if (asprintf(&info->value->hardware_address, "%02x:%02x:%02x:%02x:%02x:%02x",
(third email attempt, apologies if you get duplicates) This patch adds some basic constructs to better support Illumos/Solaris. I've kept away from kvm, configure etc. This just covers making sure the right libs are used, and the code is Solaris/Illumos compatible. In qemu-timer.c there are lots of __linux__ || __sun__ constructs, I wanted to make sure I didn't alter the linux behaviour and this seemed the safest way to do it. Signed-off-by: Lee Essen <lee.essen@nowonline.co.uk> --- configure | 1 + cpus.c | 4 +++- qemu-timer.c | 14 +++++++++----- qga/channel-posix.c | 16 ++++++++++++++++ qga/commands-posix.c | 9 +++++++++ 5 files changed, 38 insertions(+), 6 deletions(-)