Re: [Qemu-devel] [PATCH] qemu-clock: add an alarm timer based on timerfd

2012-09-19 Thread Peter Portante 
On Wed, Sep 19, 2012 at 3:44 AM, Jan Kiszka jan.kis...@siemens.com wrote:

 On 2012-09-19 09:26, Paolo Bonzini wrote:
  Il 18/09/2012 22:37, Anthony Liguori ha scritto:
  Unfortunately, there's a lot of Windows code in qemu-timer.c and
 main-loop.c
  right now otherwise the refactoring would be trivial.  I'll leave that
 for
  another day.
 
  Cc: Paolo Bonzini pbonz...@redhat.com
  Cc: Jan Kiszka jan.kis...@siemens.com
  Signed-off-by: Anthony Liguori aligu...@us.ibm.com
  ---
  Please note, this is lightly tested.  Since this is such a fundamental
 change,
  I'd like to do some performance analysis before committing but wanted
 to share
  early.
 
  Looks good.  I think Peter Portante tested something similar, and found
 no big
  difference between the two.  But it's a good thing and, in my opinion,
 for
  non-timerfd OSes we should simply adjust the select() timeout and not
 bother
  with signals.

 What would be the advantage of timerfd over select? On Linux, both use
 hrtimers (and low slack for RT processes).


I am not sure the comparison is timerfd v. select, but timerfd v signal
based timer (setitimer). The timerfd path allows you to integrate with
select/poll/epoll loops, where as signal based timers make that more
difficult. One can do the same thing with signalfd, but only for one
signal, where as you can setup multiple timers at the expense of file
descriptors.

Additionally, FWIW, select() has a resolution capped by its use of struct
timeval, which is microseconds, where timerfd_settime allows for nanosecond
resolution.


 I'm starting to like the
 select/WaitForMultipleObjects pattern as it would allow to consolidate
 over basically two versions of timers and simplify the code.


With timerfd, signalfd and eventfd, Linux seems to have provided all the
coverage needed to make that happen.



 Jan

 
  I'm not sure if the same can be done for Windows, but I think it's
 possible as long
  as you keep the timeBeginPeriod/timeEndPeriod calls.  As a start,
 Stefan, can you
  check if the win32 timer works for you with the calls added?  Like this:
 
  diff --git a/qemu-timer.c b/qemu-timer.c
  index c7a1551..721c769 100644
  --- a/qemu-timer.c
  +++ b/qemu-timer.c
  @@ -673,6 +673,10 @@ static int win32_start_timer(struct
 qemu_alarm_timer *t)
   HANDLE hTimer;
   BOOLEAN success;
 
  +timeGetDevCaps(mm_tc, sizeof(mm_tc));
  +
  +timeBeginPeriod(mm_tc.wPeriodMin);
  +
   /* If you call ChangeTimerQueueTimer on a one-shot timer (its period
  is zero) that has already expired, the timer is not updated.
  Since
  creating a new timer is relatively expensive, set a bogus
 one-hour
  @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer
 *t)
   if (!success) {
   fprintf(stderr, Failed to initialize win32 alarm timer: %ld\n,
   GetLastError());
  +timeEndPeriod(mm_tc.wPeriodMin);
   return -1;
   }
 
  @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer
 *t)
   if (hTimer) {
   DeleteTimerQueueTimer(NULL, hTimer, NULL);
   }
  +timeEndPeriod(mm_tc.wPeriodMin);
   }
 
   static void win32_rearm_timer(struct qemu_alarm_timer *t,
 
  Paolo
 

 --
 Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
 Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] qemu-clock: add an alarm timer based on timerfd

2012-09-19 Thread Peter Portante 
On Tue, Sep 18, 2012 at 4:37 PM, Anthony Liguori aligu...@us.ibm.comwrote:

 timerfd is a new(ish) mechanism provided in the 2.6.25+ Linux kernels that
 allows for a high resolution event timer signaled through a file
 descriptor.
 This patch adds an alarm timer implementation using timerfd.

 timerfd has a couple advantages over dynticks.  Namely, it's fd based
 instead
 of signal based which reduces the likelihood of receiving EINTR failure
 codes.

 Because we can't process timers in a signal handler, we write to a pipe()
 and
 read that in the main loop.  timerfd allows us to avoid the extra write to
 a
 pipe.

 Finally, since timerfd is just a file descriptor, it could conceivably
 used to
 implement per-thread alarm timers.  It also would integrate very easily in
 a
 glib main loop.

 Unfortunately, there's a lot of Windows code in qemu-timer.c and
 main-loop.c
 right now otherwise the refactoring would be trivial.  I'll leave that for
 another day.

 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Anthony Liguori aligu...@us.ibm.com
 ---
 Please note, this is lightly tested.  Since this is such a fundamental
 change,
 I'd like to do some performance analysis before committing but wanted to
 share
 early.
 ---
  configure|   14 +
  qemu-timer.c |   86
 ++
  2 files changed, 100 insertions(+), 0 deletions(-)

 diff --git a/configure b/configure
 index 8564142..8a3135b 100755
 --- a/configure
 +++ b/configure
 @@ -2532,6 +2532,17 @@ if compile_prog   ; then
sync_file_range=yes
  fi

 +# check for timerfd
 +timerfd=no
 +cat  $TMPC  EOF
 +#include sys/timerfd.h
 +int main(void) { return timerfd_create(CLOCK_REALTIME, 0); }
 +EOF
 +
 +if compile_prog   ; then
 +  timerfd=yes
 +fi
 +
  # check for linux/fiemap.h and FS_IOC_FIEMAP
  fiemap=no
  cat  $TMPC  EOF
 @@ -3467,6 +3478,9 @@ fi
  if test $signalfd = yes ; then
echo CONFIG_SIGNALFD=y  $config_host_mak
  fi
 +if test $timerfd = yes ; then
 +  echo CONFIG_TIMERFD=y  $config_host_mak
 +fi
  if test $tcg_interpreter = yes ; then
echo CONFIG_TCG_INTERPRETER=y  $config_host_mak
  fi
 diff --git a/qemu-timer.c b/qemu-timer.c
 index c7a1551..6f7fa35 100644
 --- a/qemu-timer.c
 +++ b/qemu-timer.c
 @@ -35,6 +35,10 @@
  #include mmsystem.h
  #endif

 +#ifdef CONFIG_TIMERFD
 +#include sys/timerfd.h
 +#endif
 +
  /***/
  /* timers */

 @@ -66,6 +70,9 @@ 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);
 +#ifdef CONFIG_TIMERFD
 +int timerfd;
 +#endif
  #if defined(__linux__)
  timer_t timer;
  int fd;
 @@ -121,6 +128,12 @@ static void qemu_rearm_alarm_timer(struct
 qemu_alarm_timer *t)
  /* TODO: MIN_TIMER_REARM_NS should be optimized */
  #define MIN_TIMER_REARM_NS 25

 +#ifdef CONFIG_TIMERFD
 +static int timerfd_start_timer(struct qemu_alarm_timer *t);
 +static void timerfd_stop_timer(struct qemu_alarm_timer *t);
 +static void timerfd_rearm_timer(struct qemu_alarm_timer *t, int64_t
 delta);
 +#endif
 +
  #ifdef _WIN32

  static int mm_start_timer(struct qemu_alarm_timer *t);
 @@ -148,6 +161,10 @@ static void dynticks_rearm_timer(struct
 qemu_alarm_timer *t, int64_t delta);
  #endif /* _WIN32 */

  static struct qemu_alarm_timer alarm_timers[] = {
 +#ifdef CONFIG_TIMERFD
 +{timerfd, timerfd_start_timer,
 + timerfd_stop_timer, timerfd_rearm_timer},
 +#endif
  #ifndef _WIN32
  #ifdef __linux__
  {dynticks, dynticks_start_timer,
 @@ -476,6 +493,75 @@ static void host_alarm_handler(int host_signum)

  #include compatfd.h

 +static void timerfd_event(void *opaque)
 +{
 +struct qemu_alarm_timer *t = opaque;
 +ssize_t len;
 +uint64_t count;
 +
 +len = read(t-timerfd, count, sizeof(count));
 +g_assert(len == sizeof(count));
 +
 +t-expired = true;
 +t-pending = true;
 +
 +/* We already process pending timers at the end of the main loop
 whereas
 + * this function is called during I/O processing.  That means we know
 that
 + * pending timers will be checked before select()'ing again which
 means we
 + * don't need to explicitly call qemu_notify_event()
 + */
 +}
 +
 +static int timerfd_start_timer(struct qemu_alarm_timer *t)
 +{
 +t-timerfd = timerfd_create(CLOCK_REALTIME, TFD_CLOEXEC);
 +if (t-timerfd == -1) {
 +return -errno;
 +}
 +
 +qemu_set_fd_handler(t-timerfd, timerfd_event, NULL, t);
 +
 +return 0;
 +}
 +
 +static void timerfd_stop_timer(struct qemu_alarm_timer *t)
 +{
 +qemu_set_fd_handler(t-timerfd, NULL, NULL, NULL);
 +close(t-timerfd);
 +t-timerfd = -1;
 +}
 +
 +static void timerfd_rearm_timer(struct qemu_alarm_timer *t,
 +int64_t nearest_delta_ns)
 +{
 +int ret;
 +struct itimerspec

Re: [Qemu-devel] [PATCH] qemu-clock: add an alarm timer based on timerfd

2012-09-19 Thread Peter Portante 
On Wed, Sep 19, 2012 at 10:27 AM, Jan Kiszka jan.kis...@siemens.com wrote:
 Please turn of HTML in you mailer. It's very hard to parse your reply.

 On 2012-09-19 16:15, Peter Portante wrote:
 On Wed, Sep 19, 2012 at 3:44 AM, Jan Kiszka 
 jan.kis...@siemens.commailto:jan.kis...@siemens.com wrote:
 On 2012-09-19 09:26, Paolo Bonzini wrote:
 Il 18/09/2012 22:37, Anthony Liguori ha scritto:
 Unfortunately, there's a lot of Windows code in qemu-timer.c and 
 main-loop.c
 right now otherwise the refactoring would be trivial.  I'll leave that for
 another day.

 Cc: Paolo Bonzini pbonz...@redhat.commailto:pbonz...@redhat.com
 Cc: Jan Kiszka jan.kis...@siemens.commailto:jan.kis...@siemens.com
 Signed-off-by: Anthony Liguori 
 aligu...@us.ibm.commailto:aligu...@us.ibm.com
 ---
 Please note, this is lightly tested.  Since this is such a fundamental 
 change,
 I'd like to do some performance analysis before committing but wanted to 
 share
 early.

 Looks good.  I think Peter Portante tested something similar, and found no 
 big
 difference between the two.  But it's a good thing and, in my opinion, for
 non-timerfd OSes we should simply adjust the select() timeout and not bother
 with signals.

 What would be the advantage of timerfd over select? On Linux, both use
 hrtimers (and low slack for RT processes).

 I am not sure the comparison is timerfd v. select, but timerfd v signal 
 based timer (setitimer). The timerfd path allows you to integrate with 
 select/poll/epoll loops, where as signal based timers make that more 
 difficult. One can do the same thing with signalfd, but only for one signal, 
 where as you can setup multiple timers at the expense of file descriptors.

 Additionally, FWIW, select() has a resolution capped by its use of struct 
 timeval, which is microseconds, where timerfd_settime allows for nanosecond 
 resolution.

  1us resolution is pointless, even on RT-hardened kernels with fast
 hardware underneath and when running natively.

Perhaps.

Under Linux, we have been unable to demonstrate sub-50us resolution
for timeouts outside of RT scheduling classes. We have been able to
get around 1us resolution using SCHED_RR or SCHED_FIFO.

However, if native clocks return timer values at nanosecond
resolution, why would you not want to maintain those units to avoid
rounding errors or simple logic errors during conversions? Not sure
how pointless all the work has been to create software APIs and
hardware interfaces that use nanosecond resolution.


 I'm starting to like the
 select/WaitForMultipleObjects pattern as it would allow to consolidate
 over basically two versions of timers and simplify the code.

 With timerfd, signalfd and eventfd, Linux seems to have provided all the 
 coverage needed to make that happen.

 The advantage is that timers based on select/poll timeouts will allow to
 unify a lot of code for _all_ host platforms, i.e. even Windows. We
 still need to evaluate the precise impact and look for potentially
 missed limitations (aka: someone has to write patches and test them).
 But if there are no relevant ones, it should be the better architecture.

 That said, a timerfd based solution for Linux may be an intermediate
 step of the select-based work takes longer.

 Jan

 --
 Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
 Corporate Competence Center Embedded Linux

Re: [Qemu-devel] KVM call agenda for May, Thrusday 1st

2012-04-30 Thread Peter Portante 
On Mon, Apr 30, 2012 at 7:58 AM, Anthony Liguori anth...@codemonkey.wswrote:

 On 04/30/2012 04:17 AM, Juan Quintela wrote:



 Hi

 Please send in any agenda items you are interested in covering.


 FYI, I won't be able to attend as I'll be at the LF End User Summit.

 Regards,

 Anthony Liguori


 Thanks, Juan.



 Just for clarity, is this for Tuesday, May 1st, or Thursday, May 3rd?

[Qemu-devel] [PATCH] Remove extra -lrt switch

2012-04-27 Thread Peter Portante 
Remove the extra -lrt switch which might be there from the package
config check for gthreads.

See also: e3c56761b465a4253871c32b06ebbc2d8b3fc3e1 for the extra
pthread switch removal.

Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 configure |   11 ++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 15bbc73..e748b06 100755
--- a/configure
+++ b/configure
@@ -2581,7 +2581,16 @@ EOF
 if compile_prog   ; then
   :
 elif compile_prog  -lrt ; then
-  LIBS=-lrt $LIBS
+  found=no
+  for lib_entry in $LIBS; do
+if test $lib_entry = -lrt; then
+  found=yes
+  break
+fi
+  done
+  if test $found = no; then
+LIBS=-lrt $LIBS
+  fi
 fi
 
 if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] Remove extra -lrt switch

2012-04-27 Thread Peter Portante 
On Fri, Apr 27, 2012 at 1:43 PM, Peter Portante
peter.porta...@redhat.comwrote:

 Remove the extra -lrt switch which might be there from the package
 config check for gthreads.

 See also: e3c56761b465a4253871c32b06ebbc2d8b3fc3e1 for the extra
 pthread switch removal.


FYI: I am not trying to dribble these in, I am just finding them as I play
around with debugging the behavior of compiler switches and these
redundancies get in the way.

Thanks, -peter

Signed-off-by: Peter Portante peter.porta...@redhat.com
 ---
  configure |   11 ++-
  1 files changed, 10 insertions(+), 1 deletions(-)

 diff --git a/configure b/configure
 index 15bbc73..e748b06 100755
 --- a/configure
 +++ b/configure
 @@ -2581,7 +2581,16 @@ EOF
  if compile_prog   ; then
   :
  elif compile_prog  -lrt ; then
 -  LIBS=-lrt $LIBS
 +  found=no
 +  for lib_entry in $LIBS; do
 +if test $lib_entry = -lrt; then
 +  found=yes
 +  break
 +fi
 +  done
 +  if test $found = no; then
 +LIBS=-lrt $LIBS
 +  fi
  fi

  if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a
 \
 --
 1.7.7.6

Re: [Qemu-devel] [PATCH] Remove extra -lrt switch

2012-04-27 Thread Peter Portante 

On 04/27/2012 02:14 PM, Peter Maydell wrote:

On 27 April 2012 18:43, Peter Portantepeter.porta...@redhat.com  wrote:

Remove the extra -lrt switch which might be there from the package
config check for gthreads.

See also: e3c56761b465a4253871c32b06ebbc2d8b3fc3e1 for the extra
pthread switch removal.

Signed-off-by: Peter Portantepeter.porta...@redhat.com
---
  configure |   11 ++-
  1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 15bbc73..e748b06 100755
--- a/configure
+++ b/configure
@@ -2581,7 +2581,16 @@ EOF
  if compile_prog   ; then
   :
  elif compile_prog  -lrt ; then
-  LIBS=-lrt $LIBS
+  found=no
+  for lib_entry in $LIBS; do
+if test $lib_entry = -lrt; then
+  found=yes
+  break
+fi
+  done
+  if test $found = no; then
+LIBS=-lrt $LIBS
+  fi
  fi

If we need this more than once it looks like a candidate for
a shell function, maybe:
   add_to_libs -lrt
?


Great idea, thanks for pointing that out. I'll re-spin this patch as a 
re-factoring of the first, and then apply the second, by using a shell 
script as proposed.


-peter



-- PMM

[Qemu-devel] [PATCH v2] Remove the extra -lrt switch

2012-04-27 Thread Peter Portante 
The package config check for gthreads might have already placed
a -lrt switch in LIBS earlier.

Refactored the code from the pthread switch removal, from commit
e3c56761b465a4253871c32b06ebbc2d8b3fc3e1, to make it work for the
more general case.

Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 configure |   26 +++---
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index 15bbc73..4c4ecfb 100755
--- a/configure
+++ b/configure
@@ -81,6 +81,19 @@ path_of() {
 return 1
 }
 
+add_to_libs() {
+  found=no
+  for lib_entry in $LIBS; do
+if test $lib_entry = $1; then
+  found=yes
+  break
+fi
+  done
+  if test $found = no; then
+LIBS=$1 $LIBS
+  fi
+}
+
 # default parameters
 source_path=`dirname $0`
 cpu=
@@ -2083,16 +2096,7 @@ else
   for pthread_lib in $PTHREADLIBS_LIST; do
 if compile_prog  $pthread_lib ; then
   pthread=yes
-  found=no
-  for lib_entry in $LIBS; do
-if test $lib_entry = $pthread_lib; then
-  found=yes
-  break
-fi
-  done
-  if test $found = no; then
-LIBS=$pthread_lib $LIBS
-  fi
+  add_to_libs $pthread_lib
   break
 fi
   done
@@ -2581,7 +2585,7 @@ EOF
 if compile_prog   ; then
   :
 elif compile_prog  -lrt ; then
-  LIBS=-lrt $LIBS
+  add_to_libs -lrt
 fi
 
 if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
-- 
1.7.7.6

[Qemu-devel] [PATCH] xtensa, hw: remove unnecessary include of dyngen-exec.h

2012-04-25 Thread Peter Portante 

Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 hw/spapr_hcall.c |1 -
 xtensa-semi.c|1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 94bb504..88c1fab 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -1,6 +1,5 @@
 #include sysemu.h
 #include cpu.h
-#include dyngen-exec.h
 #include qemu-char.h
 #include sysemu.h
 #include qemu-char.h
diff --git a/xtensa-semi.c b/xtensa-semi.c
index 5754b77..8c97a02 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@
 #include string.h
 #include stddef.h
 #include cpu.h
-#include dyngen-exec.h
 #include helpers.h
 #include qemu-log.h
 
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] xtensa, hw: remove unnecessary include of dyngen-exec.h

2012-04-25 Thread Peter Portante 

On 04/25/2012 04:07 PM, Max Filippov wrote:

On 04/25/2012 10:47 PM, Peter Portante wrote:

Signed-off-by: Peter Portantepeter.porta...@redhat.com
---
  hw/spapr_hcall.c |1 -
  xtensa-semi.c|1 -
  2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 94bb504..88c1fab 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -1,6 +1,5 @@
  #include sysemu.h
  #include cpu.h
-#include dyngen-exec.h
  #include qemu-char.h
  #include sysemu.h
  #include qemu-char.h
diff --git a/xtensa-semi.c b/xtensa-semi.c
index 5754b77..8c97a02 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@
  #includestring.h
  #includestddef.h
  #include cpu.h
-#include dyngen-exec.h
  #include helpers.h
  #include qemu-log.h



Regarding xtensa part: it does not apply now that helpers.h is renamed 
to helper.h

Hi Max,

I am not entirely sure I understand what you wrote above. I am probably 
missing a subtlety above, so pardon me ahead of time.


For the file xtensa-semi.c, the references it makes to env are 
satisfied by the argument declaration. Looking through helper.h and 
def-helper.h, for completeness, I don't see any references to env 
either. So it appears that dyngen-exec.h does not need to be included 
(and in fact, it is born out by a successful compile).


Do I understand things correctly?

Thanks, -peter


Otherwise Acked-by: Max Filippov jcmvb...@gmail.com

Re: [Qemu-devel] [PATCH] xtensa, hw: remove unnecessary include of dyngen-exec.h

2012-04-25 Thread Peter Portante 

On 04/25/2012 04:30 PM, Andreas Färber wrote:

Am 25.04.2012 20:47, schrieb Peter Portante:

Signed-off-by: Peter Portantepeter.porta...@redhat.com
---
  hw/spapr_hcall.c |1 -
  xtensa-semi.c|1 -
  2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 94bb504..88c1fab 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -1,6 +1,5 @@
  #include sysemu.h
  #include cpu.h
-#include dyngen-exec.h
  #include qemu-char.h
  #include sysemu.h
  #include qemu-char.h

Careful! This part depends on http://patchwork.ozlabs.org/patch/154522/
getting applied first - I was surprised that you didn't send it as a
two-patch series alongside the emv v2. (cc'ing qemu-ppc)
Ugh, thanks for pointing that out. I should have done that. I was too 
focused on the bug first.


I'll re-submit this first change once the above patch is in the tree.

diff --git a/xtensa-semi.c b/xtensa-semi.c
index 5754b77..8c97a02 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@
  #includestring.h
  #includestddef.h
  #include cpu.h
-#include dyngen-exec.h
  #include helpers.h
  #include qemu-log.h


This part is independent of the above. If you rebase it and split it off
into its own patch (e.g., git checkout -p) then Max can apply it to his
xtensa queue already.

Thanks, I'll do that.

-peter



Andreas

Re: [Qemu-devel] [PATCH] xtensa, hw: remove unnecessary include of dyngen-exec.h

2012-04-25 Thread Peter Portante 

On 04/25/2012 04:56 PM, Max Filippov wrote:

On 04/26/2012 12:31 AM, Peter Portante wrote:

On 04/25/2012 04:07 PM, Max Filippov wrote:

On 04/25/2012 10:47 PM, Peter Portante wrote:

Signed-off-by: Peter Portantepeter.porta...@redhat.com
---
hw/spapr_hcall.c | 1 -
xtensa-semi.c | 1 -
2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 94bb504..88c1fab 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -1,6 +1,5 @@
#include sysemu.h
#include cpu.h
-#include dyngen-exec.h
#include qemu-char.h
#include sysemu.h
#include qemu-char.h
diff --git a/xtensa-semi.c b/xtensa-semi.c
index 5754b77..8c97a02 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@
#includestring.h
#includestddef.h
#include cpu.h
-#include dyngen-exec.h
#include helpers.h
#include qemu-log.h



Regarding xtensa part: it does not apply now that helpers.h is 
renamed to helper.h

Hi Max,

I am not entirely sure I understand what you wrote above. I am 
probably missing a subtlety above, so pardon me ahead of

time.

For the file xtensa-semi.c, the references it makes to env are 
satisfied by the argument declaration. Looking through
helper.h and def-helper.h, for completeness, I don't see any 
references to env either. So it appears that
dyngen-exec.h does not need to be included (and in fact, it is born 
out by a successful compile).


Do I understand things correctly?


Yes, you're right and the change itself is fine, but the patch is 
based on the old version of xtensa-semi.c,
that had #include helpers.h. It was renamed to helper.h in the 
commit 16c1deae215d4aac5b9b4fc090844b92852a0c5b.


I see that now. You meant that the patch does not apply without 
conflicts now, not that it no longer applies in the abstract.


I am going to resubmit the patch against your changes shortly, without 
the changes to hw/spapr_hcall.c.


Thanks, -peter

[Qemu-devel] [PATCH] xtensa: remove unnecessary include of dyngen-exec.h

2012-04-25 Thread Peter Portante 
---
 xtensa-semi.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/xtensa-semi.c b/xtensa-semi.c
index b7c8c34..1c8a19e 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@
 #include string.h
 #include stddef.h
 #include cpu.h
-#include dyngen-exec.h
 #include helper.h
 #include qemu-log.h
 
-- 
1.7.7.6

[Qemu-devel] [PATCH v2] pseries: Fix use of global CPU state

2012-04-23 Thread Peter Portante 
Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA
functions for pSeries shared processor partitions) introduced the
register_dtl() function and typo emv as name of its argument.
This went unnoticed because the code in that function can access the
global variable env so that no build failure resulted.

Fix the argument to read env. Resolves LP#986241.

Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 hw/spapr_hcall.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 634763e..94bb504 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env, 
target_ulong addr)
 return H_SUCCESS;
 }
 
-static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr)
+static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr)
 {
 env-dispatch_trace_log = 0;
 env-dtl_size = 0;
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] Bug fix for #986241: spell env correctly

2012-04-22 Thread Peter Portante 
On Sun, Apr 22, 2012 at 3:56 PM, Andreas Färber afaer...@suse.de wrote:

 Am 20.04.2012 17:44, schrieb Peter Portante:
  Signed-off-by: Peter Portante peter.porta...@redhat.com

 Fix itself looks okay, but author, maintainer and qemu-ppc were missing
 in CC, and a better commit message would be:

 ---8---
 pseries: Fix use of global CPU state

 Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA
 functions for pSeries shared processor partitions) introduced the
 register_dtl() function and typo emv as name of its argument.
 This went unnoticed because the code in that function can access the
 global variable env so that no build failure resulted.

 Fix the argument to read env. Resolves LP#986241.
 ---8---


Thanks Andreas. I'll resubmit the patch tomorrow. -peter


 Andreas

  ---
   hw/spapr_hcall.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
  index 634763e..94bb504 100644
  --- a/hw/spapr_hcall.c
  +++ b/hw/spapr_hcall.c
  @@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env,
 target_ulong addr)
   return H_SUCCESS;
   }
 
  -static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr)
  +static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr)
   {
   env-dispatch_trace_log = 0;
   env-dtl_size = 0;

 --
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] Bug fix for #986241: spell env correctly

2012-04-22 Thread Peter Portante 
On Sun, Apr 22, 2012 at 9:40 PM, Peter Portante
peter.a.porta...@gmail.comwrote:

 On Sun, Apr 22, 2012 at 3:56 PM, Andreas Färber afaer...@suse.de wrote:

 Am 20.04.2012 17:44, schrieb Peter Portante:
  Signed-off-by: Peter Portante peter.porta...@redhat.com

 Fix itself looks okay, but author, maintainer and qemu-ppc were missing
 in CC, and a better commit message would be:

 ---8---
 pseries: Fix use of global CPU state

 Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA
 functions for pSeries shared processor partitions) introduced the
 register_dtl() function and typo emv as name of its argument.
 This went unnoticed because the code in that function can access the
 global variable env so that no build failure resulted.

 Fix the argument to read env. Resolves LP#986241.
 ---8---


 Thanks Andreas. I'll resubmit the patch tomorrow. -peter


 Andreas

  ---
   hw/spapr_hcall.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
  index 634763e..94bb504 100644
  --- a/hw/spapr_hcall.c
  +++ b/hw/spapr_hcall.c
  @@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env,
 target_ulong addr)
   return H_SUCCESS;
   }
 
  -static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr)
  +static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr)
   {
   env-dispatch_trace_log = 0;
   env-dtl_size = 0;

 --
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



And I planned on submitting the other changes to remove the unused includes
of dyngen-exec.h in a separate patch.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] Bug fix for #986241: spell env correctly

2012-04-22 Thread Peter Portante 

On 04/22/2012 09:22 PM, David Gibson wrote:

On Sun, Apr 22, 2012 at 09:56:09PM +0200, Andreas Färber wrote:

Am 20.04.2012 17:44, schrieb Peter Portante:

Signed-off-by: Peter Portantepeter.porta...@redhat.com

Fix itself looks okay, but author, maintainer and qemu-ppc were missing
in CC, and a better commit message would be:

---8---
pseries: Fix use of global CPU state

Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA
functions for pSeries shared processor partitions) introduced the
register_dtl() function and typo emv as name of its argument.
This went unnoticed because the code in that function can access the
global variable env so that no build failure resulted.

Fix the argument to read env. Resolves LP#986241.

Ouch.  Good catch.
Found by playing around with -flto with gcc. Ran into problems with link 
time common warnings due to register declaration in dyngen-exec.h being 
included in multiple .c files where they referenced env. Rebuilt with 
--enabled-tcg-interpretter (which causes dyngen-exec.h to only emit an 
extern declaration), and ifdef'd out the storage declarations in other 
.c files, and voila, undefined reference to env from within 
hw/spapr_hcall.c.


Do folks seen this as a maintenance headache to have a global variable 
name used as a parameter name in function arguments as well?


Can somebody speak to the history of using a global variable for env 
state instead of passing it around in function calls? Is this because we 
could not trust the compiler to keep it in a register between function 
calls? If so, then do you think it might be worth a look at what the 
compilers can do to help simplify somewhat complicated code and still 
maintain the performance? Or was it that env had to be added to so many 
functions that it became too tedious to maintain?


Regards,

-peter

[Qemu-devel] [PATCH] Remove extra pthread switch

2012-04-20 Thread Peter Portante 
From the Department of the Redundancy Department:
  remove the extra pthread switch which might be there
  from the package config check for gthreads.

Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 configure |   11 ++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 8b4e3c1..0038dfc 100755
--- a/configure
+++ b/configure
@@ -2039,7 +2039,16 @@ else
   for pthread_lib in $PTHREADLIBS_LIST; do
 if compile_prog  $pthread_lib ; then
   pthread=yes
-  LIBS=$pthread_lib $LIBS
+  found=no
+  for lib_entry in $LIBS; do
+if test $lib_entry = $pthread_lib; then
+  found=yes
+  break
+fi
+  done
+  if test $found = no; then
+LIBS=$pthread_lib $LIBS
+  fi
   break
 fi
   done
-- 
1.7.7.6

[Qemu-devel] [PATCH] Bug fix for #986241: spell env correctly

2012-04-20 Thread Peter Portante 
Signed-off-by: Peter Portante peter.porta...@redhat.com
---
 hw/spapr_hcall.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 634763e..94bb504 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env, 
target_ulong addr)
 return H_SUCCESS;
 }
 
-static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr)
+static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr)
 {
 env-dispatch_trace_log = 0;
 env-dtl_size = 0;
-- 
1.7.7.6

[Qemu-devel] [Bug 986241] [NEW] Misspelling of env parameter in deregister_dtl() [hw/spapr_hcall.c]

2012-04-20 Thread Peter Portante 
Public bug reported:

Looks like there is a simple misspelling of the env parameter, as emv,
in hw/spapr_hcall.c:485:

  static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong
addr)

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/986241

Title:
  Misspelling of env parameter in deregister_dtl() [hw/spapr_hcall.c]

Status in QEMU:
  New

Bug description:
  Looks like there is a simple misspelling of the env parameter, as
  emv, in hw/spapr_hcall.c:485:

static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong
  addr)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/986241/+subscriptions

[Qemu-devel] [PATCH v2] qemu-timer.c: Remove 250us timeouts

2012-04-05 Thread Peter Portante 
Basically, the main wait loop calls qemu_run_all_timers() unconditionally. The
first thing this routine used to do is to see if a timer had been serviced,
and then reset the loop timeout to the next deadline.

However, the new deadlines had not been calculated at that point, as
qemu_run_timers() had not been called yet for each of the clocks. So
qemu_rearm_alarm_timer() would end up with a negative or zero deadline, and
default to setting a 250us timeout for the loop.

As qemu_run_timers() is called for each clock, the real deadlines would be put
in place, but because a loop timeout was already set, the loop timeout would
not be changed.

Once that 250us timeout fired, the real deadline would be used for the
subsequent timeout.

For idle VMs, this effectively doubles the number of times through the loop,
doubling the number of select() system calls, timer calls, etc. putting added
scheduling pressure on the kernel. And under cgroups, this really causes a big
problem because the cgroup code does not scale well.

By simply running the timers before trying to rearm the timer, we always rearm
with a non-zero deadline, effectively halving the number of system calls.

Signed-off-by: Peter Portante pport...@redhat.com
---
 qemu-timer.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index d7f56e5..b0845f1 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -472,16 +472,16 @@ void qemu_run_all_timers(void)
 {
 alarm_timer-pending = 0;
 
+/* vm time timers */
+qemu_run_timers(vm_clock);
+qemu_run_timers(rt_clock);
+qemu_run_timers(host_clock);
+
 /* rearm timer, if not periodic */
 if (alarm_timer-expired) {
 alarm_timer-expired = 0;
 qemu_rearm_alarm_timer(alarm_timer);
 }
-
-/* vm time timers */
-qemu_run_timers(vm_clock);
-qemu_run_timers(rt_clock);
-qemu_run_timers(host_clock);
 }
 
 #ifdef _WIN32
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] Small change to remove short 250us timeouts every other time through the wait loop.

2012-03-27 Thread Peter Portante 
Should I resubmit the patch? Thanks,

-peter

On Mar 27, 2012, at 8:34 AM, Avi Kivity a...@redhat.com wrote:

 On 03/22/2012 04:59 PM, Peter Portante wrote:
 Basically, the main wait loop calls qemu_run_all_timers() unconditionally. 
 The
 first thing this routine used to do is to see if a timer had been serviced,
 and then reset the loop timeout to the next deadline.
 
 However, the new deadlines had not been calculated at that point, as
 qemu_run_timers() had not been called yet for each of the clocks. So
 qemu_rearm_alarm_timer() would end up with a negative or zero deadline, and
 default to setting a 250us timeout for the loop.
 
 As qemu_run_timers() is called for each clock, the real deadlines would be 
 put
 in place, but because a loop timeout was already set, the loop timeout would
 not be changed.
 
 Once that 250us timeout fired, the real deadline would be used for the
 subsequent timeout.
 
 For idle VMs, this effectively doubles the number of times through the loop,
 doubling the number of select() system calls, timer calls, etc. putting added
 scheduling pressure on the kernel. And under cgroups, this really causes a 
 big
 problem because the cgroup code does not scale well.
 
 By simply running the timers before trying to rearm the timer, we always 
 rearm
 with a non-zero deadline, effectively halving the number of system calls.
 
 Reviewed-by: Avi Kivity a...@redhat.com
 
 Note the canonical subject line for patches is subsystem: short
 description, in this case something like qemu-timer: remove spurious
 host alarm wakeups would be a good fit.
 
 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/4] Enable qemu-timer dynticks for Solaris

2012-03-27 Thread Peter Portante 
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.kis...@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

[Qemu-devel] [PATCH] Small change to remove short 250us timeouts every other time through the wait loop.

2012-03-22 Thread Peter Portante 
Basically, the main wait loop calls qemu_run_all_timers() unconditionally. The
first thing this routine used to do is to see if a timer had been serviced,
and then reset the loop timeout to the next deadline.

However, the new deadlines had not been calculated at that point, as
qemu_run_timers() had not been called yet for each of the clocks. So
qemu_rearm_alarm_timer() would end up with a negative or zero deadline, and
default to setting a 250us timeout for the loop.

As qemu_run_timers() is called for each clock, the real deadlines would be put
in place, but because a loop timeout was already set, the loop timeout would
not be changed.

Once that 250us timeout fired, the real deadline would be used for the
subsequent timeout.

For idle VMs, this effectively doubles the number of times through the loop,
doubling the number of select() system calls, timer calls, etc. putting added
scheduling pressure on the kernel. And under cgroups, this really causes a big
problem because the cgroup code does not scale well.

By simply running the timers before trying to rearm the timer, we always rearm
with a non-zero deadline, effectively halving the number of system calls.

Signed-off-by: Peter Portante pport...@redhat.com
---
 qemu-timer.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index d7f56e5..b0845f1 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -472,16 +472,16 @@ void qemu_run_all_timers(void)
 {
 alarm_timer-pending = 0;
 
+/* vm time timers */
+qemu_run_timers(vm_clock);
+qemu_run_timers(rt_clock);
+qemu_run_timers(host_clock);
+
 /* rearm timer, if not periodic */
 if (alarm_timer-expired) {
 alarm_timer-expired = 0;
 qemu_rearm_alarm_timer(alarm_timer);
 }
-
-/* vm time timers */
-qemu_run_timers(vm_clock);
-qemu_run_timers(rt_clock);
-qemu_run_timers(host_clock);
 }
 
 #ifdef _WIN32
-- 
1.7.7.6

[Qemu-devel] [PATCH 0/4] More whitespace and coding style clean ups ahead of future changes

2012-03-21 Thread Peter Portante 
Hi Folks,

Please forgive me if you find these changes are annoying, as I am trying to
learn the ropes of patch submission with git ahead of making a real patch.

While working on the code, I found that scripts/checkpatch.pl will flag lines
that I am changing as not adhereing to the codeing standard due to
pre-existing coding violations. So I figured I could learn a bit about how to
submit patches by fixing these files I will be touching before submitting the
code changes.

I also figure this might benefit others working on the code base a bit.

Thanks,

-peter

[Qemu-devel] [PATCH 3/4] And some final work to complete the coding style upgrade, missed by the previous commit.

2012-03-21 Thread Peter Portante 
Signed-off-by: Peter Portante pport...@redhat.com
---
 slirp/slirp.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 40f7523..fd28f17 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -457,7 +457,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
  * (and they can crash the program)
  */
 if (so-so_state  SS_NOFDREF || so-s == -1) {
-   continue;
+   continue;
 }
 
 /*
@@ -535,7 +535,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
  */
 #ifdef PROBE_CONN
 if (so-so_state  SS_ISFCONNECTING) {
-ret = qemu_recv(so-s, ret, 0,0);
+ret = qemu_recv(so-s, ret, 0, 0);
 
 if (ret  0) {
 /* XXX */
@@ -549,7 +549,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
 
 /* tcp_input will take care of it */
 } else {
-ret = send(so-s, ret, 0,0);
+ret = send(so-s, ret, 0, 0);
 if (ret  0) {
 /* XXX */
 if (errno == EAGAIN || errno == EWOULDBLOCK ||
@@ -563,7 +563,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
   so-so_state = ~SS_ISFCONNECTING;
 }
 }
-tcp_input((struct mbuf *)NULL, sizeof(struct ip),so);
+tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
 } /* SS_ISFCONNECTING */
 #endif
 }
-- 
1.7.7.6

[Qemu-devel] [PATCH] Whitespace clean up.

2012-03-21 Thread Peter Portante 
Signed-off-by: Peter Portante pport...@redhat.com
---
 block/raw-posix.c |   28 ++--
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2d1bc13..719a590 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -112,7 +112,7 @@
reopen it to see if the disk has been changed */
 #define FD_OPEN_TIMEOUT (10)
 
-#define MAX_BLOCKSIZE  4096
+#define MAX_BLOCKSIZE   4096
 
 typedef struct BDRVRawState {
 int fd;
@@ -494,7 +494,7 @@ again:
 #endif
 if (!fstat(fd, sb)  (S_IFCHR  sb.st_mode)) {
 #ifdef DIOCGMEDIASIZE
-   if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)size))
+if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)size))
 #elif defined(DIOCGPART)
 {
 struct partinfo pi;
@@ -894,7 +894,7 @@ static int hdev_has_zero_init(BlockDriverState *bs)
 
 static BlockDriver bdrv_host_device = {
 .format_name= host_device,
-.protocol_name= host_device,
+.protocol_name  = host_device,
 .instance_size  = sizeof(BDRVRawState),
 .bdrv_probe_device  = hdev_probe_device,
 .bdrv_file_open = hdev_open,
@@ -903,12 +903,12 @@ static BlockDriver bdrv_host_device = {
 .create_options = raw_create_options,
 .bdrv_has_zero_init = hdev_has_zero_init,
 
-.bdrv_aio_readv= raw_aio_readv,
-.bdrv_aio_writev   = raw_aio_writev,
-.bdrv_aio_flush= raw_aio_flush,
+.bdrv_aio_readv = raw_aio_readv,
+.bdrv_aio_writev= raw_aio_writev,
+.bdrv_aio_flush = raw_aio_flush,
 
 .bdrv_truncate  = raw_truncate,
-.bdrv_getlength= raw_getlength,
+.bdrv_getlength = raw_getlength,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
 
@@ -1015,7 +1015,7 @@ static BlockDriver bdrv_host_floppy = {
 .format_name= host_floppy,
 .protocol_name  = host_floppy,
 .instance_size  = sizeof(BDRVRawState),
-.bdrv_probe_device = floppy_probe_device,
+.bdrv_probe_device  = floppy_probe_device,
 .bdrv_file_open = floppy_open,
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
@@ -1024,10 +1024,10 @@ static BlockDriver bdrv_host_floppy = {
 
 .bdrv_aio_readv = raw_aio_readv,
 .bdrv_aio_writev= raw_aio_writev,
-.bdrv_aio_flush= raw_aio_flush,
+.bdrv_aio_flush = raw_aio_flush,
 
 .bdrv_truncate  = raw_truncate,
-.bdrv_getlength= raw_getlength,
+.bdrv_getlength = raw_getlength,
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
 
@@ -1114,7 +1114,7 @@ static BlockDriver bdrv_host_cdrom = {
 .format_name= host_cdrom,
 .protocol_name  = host_cdrom,
 .instance_size  = sizeof(BDRVRawState),
-.bdrv_probe_device = cdrom_probe_device,
+.bdrv_probe_device  = cdrom_probe_device,
 .bdrv_file_open = cdrom_open,
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
@@ -1123,7 +1123,7 @@ static BlockDriver bdrv_host_cdrom = {
 
 .bdrv_aio_readv = raw_aio_readv,
 .bdrv_aio_writev= raw_aio_writev,
-.bdrv_aio_flush= raw_aio_flush,
+.bdrv_aio_flush = raw_aio_flush,
 
 .bdrv_truncate  = raw_truncate,
 .bdrv_getlength = raw_getlength,
@@ -1233,7 +1233,7 @@ static BlockDriver bdrv_host_cdrom = {
 .format_name= host_cdrom,
 .protocol_name  = host_cdrom,
 .instance_size  = sizeof(BDRVRawState),
-.bdrv_probe_device = cdrom_probe_device,
+.bdrv_probe_device  = cdrom_probe_device,
 .bdrv_file_open = cdrom_open,
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
@@ -1242,7 +1242,7 @@ static BlockDriver bdrv_host_cdrom = {
 
 .bdrv_aio_readv = raw_aio_readv,
 .bdrv_aio_writev= raw_aio_writev,
-.bdrv_aio_flush= raw_aio_flush,
+.bdrv_aio_flush = raw_aio_flush,
 
 .bdrv_truncate  = raw_truncate,
 .bdrv_getlength = raw_getlength,
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/4] Yet another missed use of tabs.

2012-03-21 Thread Peter Portante 
Signed-off-by: Peter Portante pport...@redhat.com
---
 slirp/slirp.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index fd28f17..45a8de2 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -457,7 +457,7 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
  * (and they can crash the program)
  */
 if (so-so_state  SS_NOFDREF || so-s == -1) {
-   continue;
+continue;
 }
 
 /*
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/4] More work to fix up the formatting and the whitespace of this module to conform to the coding standard.

2012-03-21 Thread Peter Portante 
Signed-off-by: Peter Portante pport...@redhat.com
---
 slirp/slirp.c |  381 +++--
 1 files changed, 205 insertions(+), 176 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index ca0ff94..40f7523 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -295,15 +295,17 @@ void slirp_select_fill(int *pnfds,
 /*
  * See if we need a tcp_fasttimo
  */
-if (time_fasttimo == 0  so-so_tcpcb-t_flags  TF_DELACK)
-   time_fasttimo = curtime; /* Flag when we want a fasttimo */
+if (time_fasttimo == 0  so-so_tcpcb-t_flags  TF_DELACK) {
+time_fasttimo = curtime; /* Flag when we want a fasttimo */
+}
 
 /*
  * NOFDREF can include still connecting to local-host,
  * newly socreated() sockets etc. Don't want to select these.
  */
-if (so-so_state  SS_NOFDREF || so-s == -1)
-   continue;
+if (so-so_state  SS_NOFDREF || so-s == -1) {
+continue;
+}
 
 /*
  * Set for reading sockets which are accepting
@@ -336,7 +338,8 @@ void slirp_select_fill(int *pnfds,
  * Set for reading (and urgent data) if we are connected, can
  * receive more, and we have room for it XXX /2 ?
  */
-if (CONN_CANFRCV(so)  (so-so_snd.sb_cc  
(so-so_snd.sb_datalen/2))) {
+if (CONN_CANFRCV(so)
+ (so-so_snd.sb_cc  (so-so_snd.sb_datalen/2))) {
 FD_SET(so-s, readfds);
 FD_SET(so-s, xfds);
 UPD_NFDS(so-s);
@@ -357,8 +360,9 @@ void slirp_select_fill(int *pnfds,
 if (so-so_expire = curtime) {
 udp_detach(so);
 continue;
-} else
+} else {
 do_slowtimo = 1; /* Let socket expire */
+}
 }
 
 /*
@@ -377,33 +381,33 @@ void slirp_select_fill(int *pnfds,
 }
 }
 
-/*
- * ICMP sockets
- */
-for (so = slirp-icmp.so_next; so != slirp-icmp;
- so = so_next) {
-so_next = so-so_next;
-
-/*
- * See if it's timed out
- */
-if (so-so_expire) {
-if (so-so_expire = curtime) {
-icmp_detach(so);
-continue;
-} else {
-do_slowtimo = 1; /* Let socket expire */
-}
-}
+/*
+ * ICMP sockets
+ */
+for (so = slirp-icmp.so_next; so != slirp-icmp;
+ so = so_next) {
+so_next = so-so_next;
 
-if (so-so_state  SS_ISFCONNECTED) {
-FD_SET(so-s, readfds);
-UPD_NFDS(so-s);
-}
+/*
+ * See if it's timed out
+ */
+if (so-so_expire) {
+if (so-so_expire = curtime) {
+icmp_detach(so);
+continue;
+} else {
+do_slowtimo = 1; /* Let socket expire */
 }
+}
+
+if (so-so_state  SS_ISFCONNECTED) {
+FD_SET(so-s, readfds);
+UPD_NFDS(so-s);
+}
+}
 }
 
-*pnfds = nfds;
+*pnfds = nfds;
 }
 
 void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
@@ -424,9 +428,9 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
 curtime = qemu_get_clock_ms(rt_clock);
 
 QTAILQ_FOREACH(slirp, slirp_instances, entry) {
-/*
- * See if anything has timed out
- */
+/*
+ * See if anything has timed out
+ */
 if (time_fasttimo  ((curtime - time_fasttimo) = 2)) {
 tcp_fasttimo(slirp);
 time_fasttimo = 0;
@@ -437,152 +441,157 @@ void slirp_select_poll(fd_set *readfds, fd_set 
*writefds, fd_set *xfds,
 last_slowtimo = curtime;
 }
 
-/*
- * Check sockets
- */
-if (!select_error) {
 /*
- * Check TCP sockets
+ * Check sockets
  */
-for (so = slirp-tcb.so_next; so != slirp-tcb;
- so = so_next) {
-so_next = so-so_next;
-
+if (!select_error) {
 /*
- * FD_ISSET is meaningless on these sockets
- * (and they can crash the program)
+ * Check TCP sockets
  */
-if (so-so_state  SS_NOFDREF || so-s == -1)
-   continue;
+for (so = slirp-tcb.so_next; so != slirp-tcb;
+ so = so_next) {
+so_next = so

[Qemu-devel] [PATCH 1/4] More whitespace cleanup in preparation for future commits.

2012-03-21 Thread Peter Portante 
Signed-off-by: Peter Portante pport...@redhat.com
---
 block_int.h   |4 +-
 qemu-timer.c  |6 +-
 slirp/slirp.c |  526 
 3 files changed, 268 insertions(+), 268 deletions(-)

diff --git a/block_int.h b/block_int.h
index b460c36..64e8b82 100644
--- a/block_int.h
+++ b/block_int.h
@@ -31,8 +31,8 @@
 #include qemu-timer.h
 #include qapi-types.h
 
-#define BLOCK_FLAG_ENCRYPT 1
-#define BLOCK_FLAG_COMPAT6 4
+#define BLOCK_FLAG_ENCRYPT  1
+#define BLOCK_FLAG_COMPAT6  4
 
 #define BLOCK_IO_LIMIT_READ 0
 #define BLOCK_IO_LIMIT_WRITE1
diff --git a/qemu-timer.c b/qemu-timer.c
index d7f56e5..0eedf6e 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -65,7 +65,7 @@ struct QEMUClock {
 
 struct QEMUTimer {
 QEMUClock *clock;
-int64_t expire_time;   /* in nanoseconds */
+int64_t expire_time;/* in nanoseconds */
 int scale;
 QEMUTimerCB *cb;
 void *opaque;
@@ -231,7 +231,7 @@ void configure_alarms(char const *opt)
 /* Ignore */
 goto next;
 
-   /* Swap */
+/* Swap */
 tmp = alarm_timers[i];
 alarm_timers[i] = alarm_timers[cur];
 alarm_timers[cur] = tmp;
@@ -492,7 +492,7 @@ static void host_alarm_handler(int host_signum)
 {
 struct qemu_alarm_timer *t = alarm_timer;
 if (!t)
-   return;
+return;
 
 if (alarm_has_dynticks(t) ||
 qemu_next_alarm_deadline () = 0) {
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1502830..ca0ff94 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -275,107 +275,107 @@ void slirp_select_fill(int *pnfds,
 global_xfds = NULL;
 
 nfds = *pnfds;
-   /*
-* First, TCP sockets
-*/
-   do_slowtimo = 0;
-
-   QTAILQ_FOREACH(slirp, slirp_instances, entry) {
-   /*
-* *_slowtimo needs calling if there are IP fragments
-* in the fragment queue, or there are TCP connections active
-*/
-   do_slowtimo |= ((slirp-tcb.so_next != slirp-tcb) ||
-   (slirp-ipq.ip_link != slirp-ipq.ip_link.next));
-
-   for (so = slirp-tcb.so_next; so != slirp-tcb;
-so = so_next) {
-   so_next = so-so_next;
-
-   /*
-* See if we need a tcp_fasttimo
-*/
-   if (time_fasttimo == 0  so-so_tcpcb-t_flags  
TF_DELACK)
-  time_fasttimo = curtime; /* Flag when we want a 
fasttimo */
-
-   /*
-* NOFDREF can include still connecting to local-host,
-* newly socreated() sockets etc. Don't want to select 
these.
-*/
-   if (so-so_state  SS_NOFDREF || so-s == -1)
-  continue;
-
-   /*
-* Set for reading sockets which are accepting
-*/
-   if (so-so_state  SS_FACCEPTCONN) {
+/*
+ * First, TCP sockets
+ */
+do_slowtimo = 0;
+
+QTAILQ_FOREACH(slirp, slirp_instances, entry) {
+/*
+ * *_slowtimo needs calling if there are IP fragments
+ * in the fragment queue, or there are TCP connections active
+ */
+do_slowtimo |= ((slirp-tcb.so_next != slirp-tcb) ||
+(slirp-ipq.ip_link != slirp-ipq.ip_link.next));
+
+for (so = slirp-tcb.so_next; so != slirp-tcb;
+ so = so_next) {
+so_next = so-so_next;
+
+/*
+ * See if we need a tcp_fasttimo
+ */
+if (time_fasttimo == 0  so-so_tcpcb-t_flags  TF_DELACK)
+   time_fasttimo = curtime; /* Flag when we want a fasttimo */
+
+/*
+ * NOFDREF can include still connecting to local-host,
+ * newly socreated() sockets etc. Don't want to select these.
+ */
+if (so-so_state  SS_NOFDREF || so-s == -1)
+   continue;
+
+/*
+ * Set for reading sockets which are accepting
+ */
+if (so-so_state  SS_FACCEPTCONN) {
 FD_SET(so-s, readfds);
-   UPD_NFDS(so-s);
-   continue;
-   }
-
-   /*
-* Set for writing sockets which are connecting
-*/
-   if (so-so_state  SS_ISFCONNECTING) {
-   FD_SET(so-s, writefds);
-   UPD_NFDS(so-s);
-   continue;
-   }
-
-   /*
-* Set for writing if we are connected, can send more, 
and
-* we have something to send