date:20100113

Since pkgconfig can give different output for different targets,
it should be tried with the cross-compilation prefix first.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   19 ++-
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 5c056f5..f3584db 100755
--- a/configure
+++ b/configure
@@ -965,6 +965,15 @@ EOF
 fi
 
 ##
+# pkgconfig probe
+
+pkgconfig=${cross_prefix}pkg-config
+if ! test -x $(which $pkgconfig 2/dev/null); then
+  # likely not cross compiling, or hope for the best
+  pkgconfig=pkg-config
+fi
+
+##
 # Sparse probe
 if test $sparse != no ; then
   if test -x $(which cgcc 2/dev/null); then
@@ -1047,8 +1056,8 @@ if test $vnc_tls != no ; then
 #include gnutls/gnutls.h
 int main(void) { gnutls_session_t s; gnutls_init(s, GNUTLS_SERVER); return 0; 
}
 EOF
-  vnc_tls_cflags=`pkg-config --cflags gnutls 2 /dev/null`
-  vnc_tls_libs=`pkg-config --libs gnutls 2 /dev/null`
+  vnc_tls_cflags=`$pkgconfig --cflags gnutls 2 /dev/null`
+  vnc_tls_libs=`$pkgconfig --libs gnutls 2 /dev/null`
   if compile_prog $vnc_tls_cflags $vnc_tls_libs ; then
 vnc_tls=yes
 libs_softmmu=$vnc_tls_libs $libs_softmmu
@@ -1320,7 +1329,7 @@ if test $check_utests != no ; then
 #include check.h
 int main(void) { suite_create(qemu test); return 0; }
 EOF
-  check_libs=`pkg-config --libs check`
+  check_libs=`$pkgconfig --libs check`
   if compile_prog  $check_libs ; then
 check_utests=yes
 libs_tools=$check_libs $libs_tools
@@ -1339,8 +1348,8 @@ if test $bluez != no ; then
 #include bluetooth/bluetooth.h
 int main(void) { return bt_error(0); }
 EOF
-  bluez_cflags=`pkg-config --cflags bluez 2 /dev/null`
-  bluez_libs=`pkg-config --libs bluez 2 /dev/null`
+  bluez_cflags=`$pkgconfig --cflags bluez 2 /dev/null`
+  bluez_libs=`$pkgconfig --libs bluez 2 /dev/null`
   if compile_prog $bluez_cflags $bluez_libs ; then
 bluez=yes
 libs_softmmu=$bluez_libs $libs_softmmu
-- 
1.6.5.2

[Qemu-devel] [PATCH v2 0/4] pkg-config related fixes for cross compilation

This is v2 of the series to simplify cross-compiling, by using
a cross pkg-config tool whenever possible.

I'm now using pkg-config also in the native compilation case
whenever possible.  I also found and fixed a typo in the static
SDL case.

Paolo Bonzini (4):
  use cross-prefix for pkgconfig
  fixes to the static compilation case for sdl
  use pkg-config for sdl whenever available
  use pkg-config for libcurl whenever available

 configure |   50 +++--
 1 files changed, 36 insertions(+), 14 deletions(-)

[Qemu-devel] [PATCH v2 2/4] fixes to the static compilation case for sdl

After the next commit, pkg-config could be used for the shared library
configuration case and sdl-config for static libraries.  So I prepare
the test here by doing two changes:

at the same time I remove useless backslashes from the invocation of
grep;

1) fixing a typo ($sd_cflags).  The typo has been there since commit
1ac88f2 (remove sdl_static. Just do the right thing if static is yes,
2009-07-27).

2) fixing an erroneous test `... | grep  /dev/null` idiom that would
never succeed since grep's output would be empty;

3) checking the status code after executing sdl-config --static --libs;
this is needed for the next patch only.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
1 and 2 above show that the aalib-config case is not very
much used.  In addition to this, on my system aalib-config
does not support --static-libs at all.  So I was very much
tempted to nuke the aalib-config handling completely.

 configure |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index f3584db..b8641b8 100755
--- a/configure
+++ b/configure
@@ -1009,12 +1009,12 @@ EOF
   fi
 fi
 
-# static link with sdl ?
+# static link with sdl ? (note: sdl.pc's --static --libs is broken)
 if test $sdl = yes -a $static = yes ; then
   sdl_libs=`sdl-config --static-libs 2/dev/null`
-  if test `sdl-config --static-libs 2/dev/null | grep \\\-laa  
/dev/null` ; then
+  if test $? = 0  echo $sdl_libs | grep -- -laa  /dev/null; then
  sdl_libs=$sdl_libs `aalib-config --static-libs 2 /dev/null`
- sdl_cflags=$sd_cflags `aalib-config --cflags 2 /dev/null`
+ sdl_cflags=$sdl_cflags `aalib-config --cflags 2 /dev/null`
   fi
   if compile_prog $sdl_cflags $sdl_libs ; then
:
-- 
1.6.5.2

[Qemu-devel] [PATCH v2 3/4] use pkg-config for sdl whenever available

Together with the first patch this enables using the prefixed
pkg-config, thus picking up the correct flags for SDL.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index b8641b8..2553724 100755
--- a/configure
+++ b/configure
@@ -989,18 +989,24 @@ fi
 ##
 # SDL probe
 
-sdl_too_old=no
+if $pkgconfig sdl --modversion /dev/null 21; then
+  sdlconfig=$pkgconfig sdl
+  _sdlversion=`$sdlconfig --modversion 2/dev/null | sed 's/[^0-9]//g'`
+else
+  sdlconfig='sdl-config'
+  _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+fi
 
+sdl_too_old=no
 if test $sdl != no ; then
   cat  $TMPC  EOF
 #include SDL.h
 #undef main /* We don't want SDL to override our main() */
 int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
 EOF
-  sdl_cflags=`sdl-config --cflags 2 /dev/null`
-  sdl_libs=`sdl-config --libs 2 /dev/null`
+  sdl_cflags=`$sdlconfig --cflags 2 /dev/null`
+  sdl_libs=`$sdlconfig --libs 2 /dev/null`
   if compile_prog $sdl_cflags $sdl_libs ; then
-_sdlversion=`sdl-config --version | sed 's/[^0-9]//g'`
 if test $_sdlversion -lt 121 ; then
   sdl_too_old=yes
 else
-- 
1.6.5.2

[Qemu-devel] [PATCH v2 4/4] use pkg-config for libcurl whenever available

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 2553724..b3d4640 100755
--- a/configure
+++ b/configure
@@ -1309,13 +1309,19 @@ fi
 ##
 # curl probe
 
+if $pkgconfig libcurl --modversion /dev/null 21; then
+  curlconfig='$pkgconfig libcurl'
+else
+  curlconfig=curl-config
+fi
+
 if test $curl != no ; then
   cat  $TMPC  EOF
 #include curl/curl.h
 int main(void) { return curl_easy_init(); }
 EOF
-  curl_cflags=`curl-config --cflags 2/dev/null`
-  curl_libs=`curl-config --libs 2/dev/null`
+  curl_cflags=`$curlconfig --cflags 2/dev/null`
+  curl_libs=`$curlconfig --libs 2/dev/null`
   if compile_prog $curl_cflags $curl_libs ; then
 curl=yes
 libs_tools=$curl_libs $libs_tools
-- 
1.6.5.2

[Qemu-devel] [PATCH v3 0/4] pkg-config related fixes for cross compilation

This is v3 of the series to simplify cross-compiling, by using
a cross pkg-config tool whenever possible.

I'm now using pkg-config also in the native compilation case
whenever possible.  I also found and fixed a typo in the static
SDL case.

v2-v3: fix typo

v1-v2: always use pkg-config when available

Paolo Bonzini (4):
  use cross-prefix for pkgconfig
  fixes to the static compilation case for sdl
  use pkg-config for sdl whenever available
  use pkg-config for libcurl whenever available

 configure |   50 +++--
 1 files changed, 36 insertions(+), 14 deletions(-)

[Qemu-devel] [PATCH v3 1/4] use cross-prefix for pkgconfig

Since pkgconfig can give different output for different targets,
it should be tried with the cross-compilation prefix first.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   19 ++-
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 5c056f5..f3584db 100755
--- a/configure
+++ b/configure
@@ -965,6 +965,15 @@ EOF
 fi
 
 ##
+# pkgconfig probe
+
+pkgconfig=${cross_prefix}pkg-config
+if ! test -x $(which $pkgconfig 2/dev/null); then
+  # likely not cross compiling, or hope for the best
+  pkgconfig=pkg-config
+fi
+
+##
 # Sparse probe
 if test $sparse != no ; then
   if test -x $(which cgcc 2/dev/null); then
@@ -1047,8 +1056,8 @@ if test $vnc_tls != no ; then
 #include gnutls/gnutls.h
 int main(void) { gnutls_session_t s; gnutls_init(s, GNUTLS_SERVER); return 0; 
}
 EOF
-  vnc_tls_cflags=`pkg-config --cflags gnutls 2 /dev/null`
-  vnc_tls_libs=`pkg-config --libs gnutls 2 /dev/null`
+  vnc_tls_cflags=`$pkgconfig --cflags gnutls 2 /dev/null`
+  vnc_tls_libs=`$pkgconfig --libs gnutls 2 /dev/null`
   if compile_prog $vnc_tls_cflags $vnc_tls_libs ; then
 vnc_tls=yes
 libs_softmmu=$vnc_tls_libs $libs_softmmu
@@ -1320,7 +1329,7 @@ if test $check_utests != no ; then
 #include check.h
 int main(void) { suite_create(qemu test); return 0; }
 EOF
-  check_libs=`pkg-config --libs check`
+  check_libs=`$pkgconfig --libs check`
   if compile_prog  $check_libs ; then
 check_utests=yes
 libs_tools=$check_libs $libs_tools
@@ -1339,8 +1348,8 @@ if test $bluez != no ; then
 #include bluetooth/bluetooth.h
 int main(void) { return bt_error(0); }
 EOF
-  bluez_cflags=`pkg-config --cflags bluez 2 /dev/null`
-  bluez_libs=`pkg-config --libs bluez 2 /dev/null`
+  bluez_cflags=`$pkgconfig --cflags bluez 2 /dev/null`
+  bluez_libs=`$pkgconfig --libs bluez 2 /dev/null`
   if compile_prog $bluez_cflags $bluez_libs ; then
 bluez=yes
 libs_softmmu=$bluez_libs $libs_softmmu
-- 
1.6.5.2

[Qemu-devel] [PATCH v3 2/4] fixes to the static compilation case for sdl

After the next commit, pkg-config could be used for the shared library
configuration case and sdl-config for static libraries.  So I prepare
the test here by doing two changes:

at the same time I remove useless backslashes from the invocation of
grep;

1) fixing a typo ($sd_cflags).  The typo has been there since commit
1ac88f2 (remove sdl_static. Just do the right thing if static is yes,
2009-07-27).

2) fixing an erroneous test `... | grep  /dev/null` idiom that would
never succeed since grep's output would be empty;

3) checking the status code after executing sdl-config --static --libs;
this is needed for the next patch only.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
1 and 2 above show that the aalib-config case is not very
much used.  In addition to this, on my system aalib-config
does not support --static-libs at all.  So I was very much
tempted to nuke the aalib-config handling completely.

 configure |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index f3584db..b8641b8 100755
--- a/configure
+++ b/configure
@@ -1009,12 +1009,12 @@ EOF
   fi
 fi
 
-# static link with sdl ?
+# static link with sdl ? (note: sdl.pc's --static --libs is broken)
 if test $sdl = yes -a $static = yes ; then
   sdl_libs=`sdl-config --static-libs 2/dev/null`
-  if test `sdl-config --static-libs 2/dev/null | grep \\\-laa  
/dev/null` ; then
+  if test $? = 0  echo $sdl_libs | grep -- -laa  /dev/null; then
  sdl_libs=$sdl_libs `aalib-config --static-libs 2 /dev/null`
- sdl_cflags=$sd_cflags `aalib-config --cflags 2 /dev/null`
+ sdl_cflags=$sdl_cflags `aalib-config --cflags 2 /dev/null`
   fi
   if compile_prog $sdl_cflags $sdl_libs ; then
:
-- 
1.6.5.2

[Qemu-devel] [PATCH v3 3/4] use pkg-config for sdl whenever available

Together with the first patch this enables using the prefixed
pkg-config, thus picking up the correct flags for SDL.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   14 ++
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index b8641b8..2553724 100755
--- a/configure
+++ b/configure
@@ -989,18 +989,24 @@ fi
 ##
 # SDL probe
 
-sdl_too_old=no
+if $pkgconfig sdl --modversion /dev/null 21; then
+  sdlconfig=$pkgconfig sdl
+  _sdlversion=`$sdlconfig --modversion 2/dev/null | sed 's/[^0-9]//g'`
+else
+  sdlconfig='sdl-config'
+  _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+fi
 
+sdl_too_old=no
 if test $sdl != no ; then
   cat  $TMPC  EOF
 #include SDL.h
 #undef main /* We don't want SDL to override our main() */
 int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
 EOF
-  sdl_cflags=`sdl-config --cflags 2 /dev/null`
-  sdl_libs=`sdl-config --libs 2 /dev/null`
+  sdl_cflags=`$sdlconfig --cflags 2 /dev/null`
+  sdl_libs=`$sdlconfig --libs 2 /dev/null`
   if compile_prog $sdl_cflags $sdl_libs ; then
-_sdlversion=`sdl-config --version | sed 's/[^0-9]//g'`
 if test $_sdlversion -lt 121 ; then
   sdl_too_old=yes
 else
-- 
1.6.5.2

[Qemu-devel] [PATCH v3 4/4] use pkg-config for libcurl whenever available

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 configure |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 2553724..b3d4640 100755
--- a/configure
+++ b/configure
@@ -1309,13 +1309,19 @@ fi
 ##
 # curl probe
 
+if $pkgconfig libcurl --modversion /dev/null 21; then
+  curlconfig=$pkgconfig libcurl
+else
+  curlconfig=curl-config
+fi
+
 if test $curl != no ; then
   cat  $TMPC  EOF
 #include curl/curl.h
 int main(void) { return curl_easy_init(); }
 EOF
-  curl_cflags=`curl-config --cflags 2/dev/null`
-  curl_libs=`curl-config --libs 2/dev/null`
+  curl_cflags=`$curlconfig --cflags 2/dev/null`
+  curl_libs=`$curlconfig --libs 2/dev/null`
   if compile_prog $curl_cflags $curl_libs ; then
 curl=yes
 libs_tools=$curl_libs $libs_tools
-- 
1.6.5.2

Re: [Qemu-devel] Re: [RFC 0/7]: Add VNC connect/disconnect events

2010-01-13 Thread Daniel P. Berrange

On Tue, Jan 12, 2010 at 04:28:46PM -0600, Anthony Liguori wrote:
 On 01/12/2010 03:28 PM, Luiz Capitulino wrote:
 On Mon, 11 Jan 2010 13:55:19 +
 Daniel P. Berrangeberra...@redhat.com  wrote:
 

 So perhaps we should declare that the lifecycle is
 
-  CONNECT   (provide IP / port details)
-  AUTHENTICATED (provide IP / port details + authenticated ID details
  eg x509 dname, or SASL usernsmae)
-  DISCONNECT(provide IP / port details)
 
 
 Obviously AUTHENTICATED may be optional if the client goes away
 immedaitely before trying auth. The AUTHENTICATED event probably
 also ought to allow for an indication of success vs failure so
 the app can see failed login attempts
  
   I'm having an issue with the reporting of failure.
 
   Turns out we can have a few error conditions on login and they are
 auth mechanism dependent. Also, as I'm not familiar with the code,
 it's not always easy to get the ID information on failures.
 
   So, what is simple to do is to have an event called VNC_AUTHENTICATION,
 it will have a 'authenticated' key which can be true or false. If it's true
 authentication has been successful and ID information is available,
 otherwise authentication has failed and only IP/port info is available.
 
   Of course that CONNECT and DISCONNECT events are also provided.

 
 It might be worthwhile looking at the events that gtk-vnc supports.
 
 | VNC_CONNECTED,-  client has connected
   VNC_INITIALIZED,- initialized is completed
   VNC_DISCONNECTED,- client has disconnected
   VNC_AUTH_FAILURE,  - authorization has failed
   VNC_AUTH_UNSUPPORTED,- authorization has failed (could not 
   negotiate an auth type)
 
 Initialized can provide you all of the credential information.  I think 
 it's stronger than AUTHENTICATED because authentication alone does not 
 imply that a session is active.  Initialized tells a listener that at the 
 moment this is received, the VNC session is active.  If I'm a management 
 tool, that's the thing I'm likely interested in.

That is a good point, and if we have the three events at time of 'connect',
'initialized' and 'disconnected', then if a mgmt app sees a 'disconnect' but
no 'initialized' event, it knows the authentication was unsuccessful so do
not need an explicit flag for that.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 1/2 v2] block: flush backing_hd in the right place

2010-01-13 Thread Kevin Wolf

Am 12.01.2010 19:13, schrieb Christoph Hellwig:
 The backing device is only modified from bdrv_commit.  So instead of
 flushing it every time bdrv_flush is called for the front-end device
 only flush it after we're written data to it in bdrv_commit.
 
 Also flush the frontend image if we have a make_empty method that
 possibly writes to it.
 
 Signed-off-by: Christoph Hellwig h...@lst.de
 
 Index: qemu/block.c
 ===
 --- qemu.orig/block.c 2010-01-12 19:08:07.363003775 +0100
 +++ qemu/block.c  2010-01-12 19:09:10.836255948 +0100
 @@ -589,6 +589,7 @@ int bdrv_commit(BlockDriverState *bs)
  BlockDriver *drv = bs-drv;
  int64_t i, total_sectors;
  int n, j;
 +int ret = 0;
  unsigned char sector[512];
  
  if (!drv)
 @@ -620,10 +621,18 @@ int bdrv_commit(BlockDriverState *bs)
  }
  }
  
 -if (drv-bdrv_make_empty)
 - return drv-bdrv_make_empty(bs);
 +if (drv-bdrv_make_empty) {
 + ret = drv-bdrv_make_empty(bs);
 +bdrv_flush(bs);
 +}

Indentation is off here (but it already was before the patch). The logic
looks good to me now.

Kevin

Re: [Qemu-devel] [PATCHv7 3/3] virtio: add features as qdev properties

On Tue, Jan 12, 2010 at 11:06:33PM +0100, Christoph Hellwig wrote:
 On Tue, Jan 12, 2010 at 09:50:55PM +0200, Michael S. Tsirkin wrote:
  So the issue is that wrong block size (0x) was passed
  to guest.  Would it make sense to add some sanity checking in virtio-blk
  to make it not crash but fail in probe? Which block size values
  are sane?
 
 Yes, I'll cook up a patch.  Basically powers of two up from 512 bytes
 are theoretically sane.  In practice I doubt we'll ever see anything
 other than 512 or 4095 bytes.

I also noticed that 0x8000 causes a crash. You can play with other
sizes and see what happens.

-- 
MST

[Qemu-devel] Re: [PATCH-RFC 03/13] virtio: add iofd/irqfd support

On Tue, Jan 12, 2010 at 04:36:45PM -0600, Anthony Liguori wrote:
 On 01/11/2010 11:17 AM, Michael S. Tsirkin wrote:
 Add binding API to set iofd/irqfd support.
 Will be used by vhost.

 Signed-off-by: Michael S. Tsirkinm...@redhat.com
 ---
   hw/virtio.c |   13 ++---
   hw/virtio.h |4 
   2 files changed, 14 insertions(+), 3 deletions(-)

 diff --git a/hw/virtio.c b/hw/virtio.c
 index 8e3c9ad..b9ec863 100644
 --- a/hw/virtio.c
 +++ b/hw/virtio.c
 @@ -572,6 +572,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
 queue_size,
   returnvdev-vq[i];
   }

 +void virtio_irq(VirtIODevice *vdev, VirtQueue *vq)
 +{
 +vdev-isr |= 0x01;
 +virtio_notify_vector(vdev, vq-vector);
 +}
 +
   void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
   {
   /* Make sure used ring is updated before we check NO_INTERRUPT. */
 @@ -582,8 +588,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
(vq-inuse || vring_avail_idx(vq) != vq-last_avail_idx)))
   return;

 -vdev-isr |= 0x01;
 -virtio_notify_vector(vdev, vq-vector);
 +virtio_irq(vdev, vq);
   }

   void virtio_notify_config(VirtIODevice *vdev)
 @@ -696,8 +701,10 @@ VirtIODevice *virtio_common_init(const char *name, 
 uint16_t device_id,
   vdev-queue_sel = 0;
   vdev-config_vector = VIRTIO_NO_VECTOR;
   vdev-vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
 -for(i = 0; i  VIRTIO_PCI_QUEUE_MAX; i++)
 +for(i = 0; i  VIRTIO_PCI_QUEUE_MAX; i++) {
   vdev-vq[i].vector = VIRTIO_NO_VECTOR;
 +vdev-vq[i].vdev = vdev;
 +}

   vdev-name = name;
   vdev-config_len = config_size;
 diff --git a/hw/virtio.h b/hw/virtio.h
 index ca840e1..193b3f9 100644
 --- a/hw/virtio.h
 +++ b/hw/virtio.h
 @@ -88,6 +88,8 @@ typedef struct {
   int (*load_config)(void * opaque, QEMUFile *f);
   int (*load_queue)(void * opaque, int n, QEMUFile *f);
   unsigned (*get_features)(void * opaque);
 +int (*set_irqfd)(void * opaque, int n, int fd, bool assigned);
 +int (*set_queuefd)(void * opaque, int n, int fd, bool assigned);


 I'd like to see us abstract things like irqfd a bit more.  It should be  
 a first class object with an method to raise/lower.  In fact, you can  
 probably just use qemu_irq.

 Regards,

 Anthony Liguori

With a single provider, I fail to see the point of extra abstraction
layers, and I have no idea how a good abstraction would look. Let's see
when we have multiple implementations, then we can abstract them as
appropriate.  Also, irqfd does not have methods to raise/lower so
qemu_irq is far from a good fit.

-- 
MST

[Qemu-devel] [PATCH 0/6] Remove some dead assignments from clang analyzer report

Running the static checker clang-analyzer on the qemu sources, I found
a few dead assignments.

This patchset removes a few of those.

Amit Shah (6):
  vl.c: Remove dead assignment
  virtio: net: remove dead assignment
  x86: translate.c: remove dead assignment
  hw/vga.c: remove dead assignment
  qcow2-refcount: remove dead assignment
  json-parser: remove dead increment

 block/qcow2-refcount.c  |1 -
 hw/vga.c|1 -
 hw/virtio-net.c |2 +-
 json-parser.c   |2 --
 target-i386/translate.c |2 --
 vl.c|6 +-
 6 files changed, 2 insertions(+), 12 deletions(-)

[Qemu-devel] [PATCH 1/6] vl.c: Remove dead assignment

clang-analyzer pointed out the value of 'sockets' is never reused.

Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Andre Przywara andre.przyw...@amd.com
---
 vl.c |6 +-
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index b048e89..e49e7bd 100644
--- a/vl.c
+++ b/vl.c
@@ -2613,17 +2613,13 @@ static void smp_parse(const char *optarg)
 threads = threads  0 ? threads : 1;
 if (smp == 0) {
 smp = cores * threads * sockets;
-} else {
-sockets = smp / (cores * threads);
 }
 } else {
 if (cores == 0) {
 threads = threads  0 ? threads : 1;
 cores = smp / (sockets * threads);
 } else {
-if (sockets == 0) {
-sockets = smp / (cores * threads);
-} else {
+if (sockets) {
 threads = smp / (cores * sockets);
 }
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/6] virtio: net: remove dead assignment

clang-analyzer points out value assigned to 'len' is not used.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-net.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 02d9180..6e48997 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -532,7 +532,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 int len, total;
 struct iovec sg[VIRTQUEUE_MAX_SIZE];
 
-len = total = 0;
+total = 0;
 
 if ((i != 0  !n-mergeable_rx_bufs) ||
 virtqueue_pop(n-rx_vq, elem) == 0) {
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/6] x86: translate.c: remove dead assignment

clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 target-i386/translate.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 511a4ea..8078112 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2207,8 +2207,6 @@ static void gen_add_A0_ds_seg(DisasContext *s)
 if (s-override = 0) {
 override = s-override;
 must_add_seg = 1;
-} else {
-override = R_DS;
 }
 if (must_add_seg) {
 #ifdef TARGET_X86_64
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/6] hw/vga.c: remove dead assignment

clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/vga.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index d05f1f9..6a1a059 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1303,7 +1303,6 @@ static void vga_draw_text(VGACommonState *s, int 
full_update)
 line_offset = s-line_offset;
 
 vga_get_text_resolution(s, width, height, cw, cheight);
-x_incr = cw * ((ds_get_bits_per_pixel(s-ds) + 7)  3);
 if ((height * width)  CH_ATTR_SIZE) {
 /* better than nothing: exit if transient size is too big */
 return;
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/6] qcow2-refcount: remove dead assignment

clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 block/qcow2-refcount.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 54b19f8..3a2d44a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -511,7 +511,6 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 l2_table = NULL;
 l1_table = NULL;
 l1_size2 = l1_size * sizeof(uint64_t);
-l1_allocated = 0;
 if (l1_table_offset != s-l1_table_offset) {
 if (l1_size2 != 0) {
 l1_table = qemu_mallocz(align_offset(l1_size2, 512));
-- 
1.6.2.5

[Qemu-devel] [PATCH 6/6] json-parser: remove dead increment

clang-analyzer points out a redundant increment.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 json-parser.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/json-parser.c b/json-parser.c
index 7624c0f..e04932f 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -247,8 +247,6 @@ static QString *qstring_from_escaped_str(JSONParserContext 
*ctxt, QObject *token
 }
 }
 
-ptr++;
-
 return str;
 
 out:
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH-RFC 09/13] tap: add vhost/vhostfd options

On Tue, Jan 12, 2010 at 04:39:52PM -0600, Anthony Liguori wrote:
 On 01/11/2010 11:22 AM, Michael S. Tsirkin wrote:
 Looks like order got mixed up: vhost_net header
 is added by a follow-up patch. Will be fixed
 in the next revision.

 Signed-off-by: Michael S. Tsirkinm...@redhat.com
 ---
   net.c   |8 
   net/tap.c   |   29 +
   qemu-options.hx |4 +++-
   3 files changed, 40 insertions(+), 1 deletions(-)

 diff --git a/net.c b/net.c
 index 6ef93e6..b942d03 100644
 --- a/net.c
 +++ b/net.c
 @@ -976,6 +976,14 @@ static struct {
   .name = vnet_hdr,
   .type = QEMU_OPT_BOOL,
   .help = enable the IFF_VNET_HDR flag on the tap interface
 +}, {
 +.name = vhost,
 +.type = QEMU_OPT_BOOL,
 +.help = enable vhost-net network accelerator,
 +}, {
 +.name = vhostfd,
 +.type = QEMU_OPT_STRING,
 +.help = file descriptor of an already opened vhost net 
 device,
   },


 Semantically, I think making vhost it's own backend makes more sense  
 from a user perspective.


It doesn't. Users mostly do not care that vhost is used: they just get
fast virtio and that's all.  Users do care about e.g. tap because they
need setup scripts, understand bridging etc.


Do you propose -net tap be replaced with -net vhost?  This means vhost will
need to get tap flags if it is attached to tap and raw flags if attached
to raw, etc.

A separate backend that only works with virtio frontend is also
ugly. OTOH an option that has effect only with virtio frontend
is pretty usual, vnet_hdr is one such example.



 I don't think it's a significant code change.

 Regards,

 Anthony Liguori

-- 
MST

[Qemu-devel] Re: [PATCH 5/6] qcow2-refcount: remove dead assignment


On 01/13/2010 11:54 AM, Amit Shah wrote:

clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shahamit.s...@redhat.com
---
  block/qcow2-refcount.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 54b19f8..3a2d44a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -511,7 +511,6 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
  l2_table = NULL;
  l1_table = NULL;
  l1_size2 = l1_size * sizeof(uint64_t);
-l1_allocated = 0;
  if (l1_table_offset != s-l1_table_offset) {
  if (l1_size2 != 0) {
  l1_table = qemu_mallocz(align_offset(l1_size2, 512));


I'd remove the l1_allocated = 0 assignment in the else branch instead 
(the idea is, assign l1_allocated = 1 right after any qemu_malloc.


Paolo

[Qemu-devel] Re: [PATCH 5/6] qcow2-refcount: remove dead assignment

On (Wed) Jan 13 2010 [12:46:22], Paolo Bonzini wrote:
 On 01/13/2010 11:54 AM, Amit Shah wrote:
 clang-analyzer points out a redundant assignment.

 Signed-off-by: Amit Shahamit.s...@redhat.com
 ---
   block/qcow2-refcount.c |1 -
   1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
 index 54b19f8..3a2d44a 100644
 --- a/block/qcow2-refcount.c
 +++ b/block/qcow2-refcount.c
 @@ -511,7 +511,6 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
   l2_table = NULL;
   l1_table = NULL;
   l1_size2 = l1_size * sizeof(uint64_t);
 -l1_allocated = 0;
   if (l1_table_offset != s-l1_table_offset) {
   if (l1_size2 != 0) {
   l1_table = qemu_mallocz(align_offset(l1_size2, 512));

 I'd remove the l1_allocated = 0 assignment in the else branch instead  
 (the idea is, assign l1_allocated = 1 right after any qemu_malloc.

I thought about that too, but since the else{} part has some other code,
I decided to go that way.

(Also that if such assignments aren't placed in common code, they'll be
pointed out when someone tries to use them with the default undefined
value.)

Amit

Re: [Qemu-devel] [PATCH 6/6] pci host: make pci_data_{write, read}() get PCIConfigAddress.

2010-01-13 Thread Alexander Graf


On 12.01.2010, at 19:33, Michael S. Tsirkin wrote:

 Guys, I was wondering whether the following helper function will be
 helpful, as a lot of common code seems to be around identical b/w/l
 callbacks.  Comments?

I like the idea. That could potentially clean up quite a bit of code in qemu.

Alex

[Qemu-devel] Re: [PATCH] eepro100: Update ROM file support

2010-01-13 Thread Gerd Hoffmann


On 01/11/10 19:37, Michael S. Tsirkin wrote:

On Thu, Jan 07, 2010 at 05:13:30PM +0100, Stefan Weil wrote:

Use new way to associate ROM files to devices.


Patch looks good to me.


Maybe it is even possible to create a single
pxe-i8255x.bin which supports all eepro100 devices
(not supported with current etherboot).


Would be nice indeed.  Via .romfile we can easily handle any device - 
rom filename mapping though, so if this doesn't work out it isn't a big 
issue too.




Signed-off-by: Stefan Weilw...@mail.berlios.de



Gerd, could you ack this patch please?


Acked-by: Gerd Hoffmann kra...@redhat.com

cheers,
  Gerd

[Qemu-devel] [PATCH] virtio-blk: remove dead variable in virtio_blk_handle_scsi

2010-01-13 Thread Christoph Hellwig

As pointed out by clang size is only ever written to, but never actually
used. 

Signed-off-by: Christoph Hellwig h...@lst.de

Index: qemu/hw/virtio-blk.c
===
--- qemu.orig/hw/virtio-blk.c   2010-01-13 13:25:00.911004071 +0100
+++ qemu/hw/virtio-blk.c2010-01-13 13:25:17.014006323 +0100
@@ -165,7 +165,7 @@ static VirtIOBlockReq *virtio_blk_get_re
 static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 {
 struct sg_io_hdr hdr;
-int ret, size = 0;
+int ret;
 int status;
 int i;
 
@@ -194,7 +194,6 @@ static void virtio_blk_handle_scsi(VirtI
  * before the regular inhdr.
  */
 req-scsi = (void *)req-elem.in_sg[req-elem.in_num - 2].iov_base;
-size = sizeof(*req-in) + sizeof(*req-scsi);
 
 memset(hdr, 0, sizeof(struct sg_io_hdr));
 hdr.interface_id = 'S';
@@ -226,7 +225,6 @@ static void virtio_blk_handle_scsi(VirtI
 hdr.dxfer_len += req-elem.in_sg[i].iov_len;
 
 hdr.dxferp = req-elem.in_sg;
-size += hdr.dxfer_len;
 } else {
 /*
  * Some SCSI commands don't actually transfer any data.
@@ -236,7 +234,6 @@ static void virtio_blk_handle_scsi(VirtI
 
 hdr.sbp = req-elem.in_sg[req-elem.in_num - 3].iov_base;
 hdr.mx_sb_len = req-elem.in_sg[req-elem.in_num - 3].iov_len;
-size += hdr.mx_sb_len;
 
 ret = bdrv_ioctl(req-dev-bs, SG_IO, hdr);
 if (ret) {

Re: Making QMP self-documenting (was: [Qemu-devel] [PATCH 11/11] Change the monitor to use the new do_info_qtree.)

On Tue, 12 Jan 2010 19:57:29 +0100
Markus Armbruster arm...@redhat.com wrote:

 Here's my stab at self-documenting commands.  We need to describe the
 request, the reply, and possible errors.  First the request part.  Its
 format according to qemu-spec.txt is:
 
 { execute: json-string, arguments: json-object, id: json-value }
 
 The bits to document are:
 
 * Name.  This is the value of member execute in request objects.
 
   Aside: qmp-spec.txt permits an arbitrary string there.  I think we
   better restrict ourselves to something more tasteful.

 For example?

 * Description (arbitrary text).
 
   This is for human readers.

 Would be good to to use the command's help or the manual's description
from qemu-monitor.hx, so that the help command (and even the monitor's
documentation) could be generated from that data.

 The only problem are commands like balloon, which may behave
differently.

 * Request arguments.  The value of member arguments in request
   objects.  It's an object, so we just document the members.  For each
   member:
 
   - Name
 
   - Description
 
   - Type (more on that below)
 
   - Whether it is optional or required
 
   If we need more expressiveness than that, we might be making things
   too complicated.
 
 JSON Schema note: a natural way to describe all the possible request
 objects is as a union type of the individual request object types.  To
 document a request, you create a schema for its object type.
 
 Example:
 
 {
 title: A balloon request,
 description: Ask the guest to change its memory allocation.
 type: object,
 properties: {
 execute: {
 type: string,
 enum: [ balloon ]
 },
 arguments: {
 type: object,
 properties: {
 value: {
 type: integer,
 description: How much memory to use, in bytes.
 }
 }
 },
 id: {
 type: object
 }
 }
 }

 Looks good to me.

 Something for the future and not completely related to this, is that
today we use the args_type to do input validation (in both the user
and protocol Monitor).

 It would be a good step forward if we could move it to use this instead,
the only problem is how to translate some types.

 Now, that looks like a workable way to describe the balloon request to a
 client, but it's too much boilerplate to be suitable as source format
 for request documentation.  Even if we omit unneeded schema attributes
 like type: object.  I'd rather write the documentation in a more
 concise form, then encode it as JSON schema by substituting it into a
 template.
 
 Say we put it in the source, right next to the handler function:
 
 mon_cmd_doc balloon_doc = {
 .name = balloon,
 .description = Ask the guest to change its memory allocation.
 .arguments = { // this is an array
 {
 .name = value,
 .type = integer,
   // ^^^ this is a JSON schema type definition
 .description = How much memory to use, in bytes.
 }
 }
 };
 
 Or put it into qemu-monitor.hx.  I prefer next to the code, because that
 maximizes the chance that it gets updated when the code changes.

 What's the advantage of having it as C code (besides being
next to the code)?

 And what about generating user docs from that, for both user Monitor
and the protocol?

 My initial idea was to have it in pure JSON format somewhere, say
qemu-monitor.json.

 This way this file can be read by the Monitor (through the parser's API)
and also by an external script to generate the user docs.

 The disadvantages are:

1. Won't be next to the code

2. We may want to add more text to the user docs, like usage
examples

3. We'll have to write documentation in json format (not too bad,
as today it's a mix of C and some other format in qemu-monitor.hx)

 We could also get fancy and invent some text markup, which we then
 process into C code with a script, but I doubt it's worth it.

 I also don't think it's needed.

 On to the successful response part.  Its format according to
 qemu-spec.txt is:
 
 { return: json-object, id: json-value }
 
 Actually, we also return arrays of objects, so 'json-object' is a bug in
 the specification.
 
 To keep this growing memo under control, let's ignore returning arrays
 for now.
 
 The part to document is the return object(s).  This is similar to
 documenting the request's arguments object.  However, while many
 requests yield a single kind of response object, possibly with some
 optional parts, some requests yield one of several kinds of responses.
 
 Example: query-migrate has three kinds of responses: completed,
 active/not-block, active/block.  Here's its current documentation:
 
   - status: migration status
   - ram: only present if status is active, it is a QDict with the

[Qemu-devel] [PULL] misc fixes and cleanups

Please pull the following changes: eepro100 changes have been
out for a week without comments, and pci one seems
obvious.

The following changes since commit 72ff25e4e98d6dba9286d032b9ff5432553bbad5:
  Juergen Lock (1):
Workaround for broken OSS_GETVERSION on FreeBSD, part two

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony

Stefan Weil (3):
  eepro100: Fix initial value for PCI_STATUS
  eepro100: Update ROM file support
  pci: Add missing 'const' in argument to pci_get_xxx

 hw/eepro100.c |   15 ++-
 hw/pci.h  |   14 +++---
 2 files changed, 9 insertions(+), 20 deletions(-)

Re: [Qemu-devel] [PATCH 3/6] versatile_pci: user PCIHostState instead of PCIBus

2010-01-13 Thread Paul Brook

On Tuesday 12 January 2010, Isaku Yamahata wrote:
 To use pci host framework, use PCIHostState instead of PCIBus in
  PCIVPBState.

No.

pci_host.[ch] provides very specific functionality, it is not a generic PCI 
host device. Specifically it provides indirect access to PCI config space via 
a memory mapped {address,data} pair. The versatile PCI host exposes PCI config 
space directly, so should not be using this code.

If you want a generic framework for PCI hosts then you need to use something 
else. If nothing else, assuming that a PCI host bridge is always is SysBus 
device is wrong.

Paul

[Qemu-devel] [PATCH] move kbd/mouse handling to input.c

Move 200 lines out of vl.c already into common code that only needs to
be compiled once.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 Makefile.objs |2 +-
 input.c   |  238 +
 vl.c  |  214 +--
 3 files changed, 241 insertions(+), 213 deletions(-)
 create mode 100644 input.c

diff --git a/Makefile.objs b/Makefile.objs
index e8a44d7..5802d39 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -47,7 +47,7 @@ common-obj-y += $(qobject-obj-y)
 common-obj-y += readline.o console.o
 
 common-obj-y += tcg-runtime.o host-utils.o
-common-obj-y += irq.o ioport.o
+common-obj-y += irq.o ioport.o input.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_WM8750) += wm8750.o
diff --git a/input.c b/input.c
new file mode 100644
index 000..955b9ab
--- /dev/null
+++ b/input.c
@@ -0,0 +1,238 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include sysemu.h
+#include net.h
+#include monitor.h
+#include console.h
+#include qjson.h
+
+
+static QEMUPutKBDEvent *qemu_put_kbd_event;
+static void *qemu_put_kbd_event_opaque;
+static QEMUPutMouseEntry *qemu_put_mouse_event_head;
+static QEMUPutMouseEntry *qemu_put_mouse_event_current;
+
+void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
+{
+qemu_put_kbd_event_opaque = opaque;
+qemu_put_kbd_event = func;
+}
+
+QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
+void *opaque, int absolute,
+const char *name)
+{
+QEMUPutMouseEntry *s, *cursor;
+
+s = qemu_mallocz(sizeof(QEMUPutMouseEntry));
+
+s-qemu_put_mouse_event = func;
+s-qemu_put_mouse_event_opaque = opaque;
+s-qemu_put_mouse_event_absolute = absolute;
+s-qemu_put_mouse_event_name = qemu_strdup(name);
+s-next = NULL;
+
+if (!qemu_put_mouse_event_head) {
+qemu_put_mouse_event_head = qemu_put_mouse_event_current = s;
+return s;
+}
+
+cursor = qemu_put_mouse_event_head;
+while (cursor-next != NULL)
+cursor = cursor-next;
+
+cursor-next = s;
+qemu_put_mouse_event_current = s;
+
+return s;
+}
+
+void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry)
+{
+QEMUPutMouseEntry *prev = NULL, *cursor;
+
+if (!qemu_put_mouse_event_head || entry == NULL)
+return;
+
+cursor = qemu_put_mouse_event_head;
+while (cursor != NULL  cursor != entry) {
+prev = cursor;
+cursor = cursor-next;
+}
+
+if (cursor == NULL) // does not exist or list empty
+return;
+else if (prev == NULL) { // entry is head
+qemu_put_mouse_event_head = cursor-next;
+if (qemu_put_mouse_event_current == entry)
+qemu_put_mouse_event_current = cursor-next;
+qemu_free(entry-qemu_put_mouse_event_name);
+qemu_free(entry);
+return;
+}
+
+prev-next = entry-next;
+
+if (qemu_put_mouse_event_current == entry)
+qemu_put_mouse_event_current = prev;
+
+qemu_free(entry-qemu_put_mouse_event_name);
+qemu_free(entry);
+}
+
+void kbd_put_keycode(int keycode)
+{
+if (qemu_put_kbd_event) {
+qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
+}
+}
+
+void kbd_mouse_event(int dx, int dy, int dz, int buttons_state)
+{
+QEMUPutMouseEvent *mouse_event;
+void *mouse_event_opaque;
+int width;
+
+if (!qemu_put_mouse_event_current) {
+return;
+}
+
+mouse_event =
+qemu_put_mouse_event_current-qemu_put_mouse_event;
+mouse_event_opaque =
+qemu_put_mouse_event_current-qemu_put_mouse_event_opaque;
+
+if (mouse_event) {
+if (graphic_rotate) {
+if

Re: [Qemu-devel] [PATCH 3/6] versatile_pci: user PCIHostState instead of PCIBus

On Wed, Jan 13, 2010 at 01:02:50PM +, Paul Brook wrote:
 On Tuesday 12 January 2010, Isaku Yamahata wrote:
  To use pci host framework, use PCIHostState instead of PCIBus in
   PCIVPBState.
 
 No.
 
 pci_host.[ch] provides very specific functionality, it is not a generic PCI 
 host device. Specifically it provides indirect access to PCI config space via 
 a memory mapped {address,data} pair. The versatile PCI host exposes PCI 
 config 
 space directly, so should not be using this code.
 
 If you want a generic framework for PCI hosts then you need to use something 
 else. If nothing else, assuming that a PCI host bridge is always is SysBus 
 device is wrong.
 
 Paul

What most people seem to want is callback that will get length is a
parameter instead of supplying 3 functions. pci_host does it
but we do not need pci_host for this.

-- 
MST

[Qemu-devel] Re: [PATCH 6/6] pci host: make pci_data_{write, read}() get PCIConfigAddress.

2010-01-13 Thread Paul Brook

 I thought we will get rid of vpb_pci_config_addr, and fill in
 fields in PCIConfigAddress directly.  If we don't, and still
 recode into PC format, this is not making code any prettier
 so I don't really see what this buys us.

I agree. This patch seems to be introducing churn for no benefit.

See also comments about direct v.s. indirect access to PCI config space.
I suspect you're trying to use a common implementation for two fundamentally 
different things.

Paul

[Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf

2010-01-13 Thread Gleb Natapov

Initialize KVM paravirt cpuid leaf and allow user to control guest
visible PV features through -cpu flag.
  
Signed-off-by: Gleb Natapov g...@redhat.com
---
 v1-v2
  fix indentation
  remove unneeded ifdefs
 v2-v3
  added needed ifdefs (CONFIG_KVM_PARA) 

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f3834b3..216b00e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
 uint8_t nmi_pending;
 uint8_t has_error_code;
 uint32_t sipi_vector;
-
+uint32_t cpuid_kvm_features;
+
 /* in order to simplify APIC support, we leave this pointer to the
user */
 struct APICState *apic_state;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 049fccf..70762bb 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+static const char *kvm_feature_name[] = {
+kvmclock, kvm_nopiodelay, kvm_mmu, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
 static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
 uint32_t *ext_features,
 uint32_t *ext2_features,
-uint32_t *ext3_features)
+uint32_t *ext3_features,
+uint32_t *kvm_features)
 {
 int i;
 int found = 0;
@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, 
uint32_t *features,
 *ext3_features |= 1  i;
 found = 1;
 }
+for ( i = 0 ; i  32 ; i++ )
+if (kvm_feature_name[i]  !strcmp (flagname, kvm_feature_name[i])) {
+*kvm_features |= 1  i;
+found = 1;
+}
+
 if (!found) {
 fprintf(stderr, CPU feature %s not found\n, flagname);
 }
@@ -98,7 +112,7 @@ typedef struct x86_def_t {
 int family;
 int model;
 int stepping;
-uint32_t features, ext_features, ext2_features, ext3_features;
+uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
 uint32_t xlevel;
 char model_id[48];
 int vendor_override;
@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 
 char *s = strdup(cpu_model);
 char *featurestr, *name = strtok(s, ,);
-uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0;
-uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0;
+uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0, plus_kvm_features = 0;
+uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0, minus_kvm_features = 0;
 uint32_t numvalue;
 
 def = NULL;
@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 memcpy(x86_cpu_def, def, sizeof(*def));
 }
 
+plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
 add_flagname_to_bitmaps(hypervisor, plus_features,
-plus_ext_features, plus_ext2_features, plus_ext3_features);
+plus_ext_features, plus_ext2_features, plus_ext3_features,
+plus_kvm_features);
 
 featurestr = strtok(NULL, ,);
 
 while (featurestr) {
 char *val;
 if (featurestr[0] == '+') {
-add_flagname_to_bitmaps(featurestr + 1, plus_features, 
plus_ext_features, plus_ext2_features, plus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 1, plus_features, 
plus_ext_features, plus_ext2_features, plus_ext3_features, 
plus_kvm_features);
 } else if (featurestr[0] == '-') {
-add_flagname_to_bitmaps(featurestr + 1, minus_features, 
minus_ext_features, minus_ext2_features, minus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 1, minus_features, 
minus_ext_features, minus_ext2_features, minus_ext3_features, 
minus_kvm_features);
 } else if ((val = strchr(featurestr, '='))) {
 *val = 0; val++;
 if (!strcmp(featurestr, family)) {
@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 x86_cpu_def-ext_features |= plus_ext_features;
 x86_cpu_def-ext2_features |= plus_ext2_features;
 x86_cpu_def-ext3_features |= plus_ext3_features;
+x86_cpu_def-kvm_features |= plus_kvm_features;
 x86_cpu_def-features = ~minus_features;
 x86_cpu_def-ext_features = ~minus_ext_features;
 x86_cpu_def-ext2_features = ~minus_ext2_features;
 x86_cpu_def-ext3_features = ~minus_ext3_features;
+x86_cpu_def-kvm_features =

[Qemu-devel] [PATCH] osdep.c: Fix accept4 fallback

2010-01-13 Thread Kevin Wolf

Commit 3a03bfa5 added a fallback in case the Linux kernel running qemu is older
than the kernel of the build system. Unfortunately, v1 was committed instead of
v2, so the code has a bug that was revealed in the review (checking for the
wrong error code).

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 osdep.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/osdep.c b/osdep.c
index e4836e7..1310684 100644
--- a/osdep.c
+++ b/osdep.c
@@ -297,7 +297,7 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
 
 #ifdef CONFIG_ACCEPT4
 ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-if (ret != -1 || errno != EINVAL) {
+if (ret != -1 || errno != ENOSYS) {
 return ret;
 }
 #endif
-- 
1.6.2.5

Re: [Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf


On 01/13/2010 07:25 AM, Gleb Natapov wrote:

Initialize KVM paravirt cpuid leaf and allow user to control guest
visible PV features through -cpu flag.

Signed-off-by: Gleb Natapovg...@redhat.com
---
  v1-v2
   fix indentation
   remove unneeded ifdefs
  v2-v3
   added needed ifdefs (CONFIG_KVM_PARA)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f3834b3..216b00e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
  uint8_t nmi_pending;
  uint8_t has_error_code;
  uint32_t sipi_vector;
-
+uint32_t cpuid_kvm_features;
+
  /* in order to simplify APIC support, we leave this pointer to the
 user */
  struct APICState *apic_state;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 049fccf..70762bb 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
  NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };

+static const char *kvm_feature_name[] = {
+kvmclock, kvm_nopiodelay, kvm_mmu, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
  uint32_t *ext_features,
  uint32_t *ext2_features,
-uint32_t *ext3_features)
+uint32_t *ext3_features,
+uint32_t *kvm_features)
  {
  int i;
  int found = 0;
@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, 
uint32_t *features,
  *ext3_features |= 1  i;
  found = 1;
  }
+for ( i = 0 ; i  32 ; i++ )
+if (kvm_feature_name[i]  !strcmp (flagname, kvm_feature_name[i])) {
+*kvm_features |= 1  i;
+found = 1;
+}
+
  if (!found) {
  fprintf(stderr, CPU feature %s not found\n, flagname);
  }
@@ -98,7 +112,7 @@ typedef struct x86_def_t {
  int family;
  int model;
  int stepping;
-uint32_t features, ext_features, ext2_features, ext3_features;
+uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
  uint32_t xlevel;
  char model_id[48];
  int vendor_override;
@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)

  char *s = strdup(cpu_model);
  char *featurestr, *name = strtok(s, ,);
-uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0;
-uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0;
+uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0, plus_kvm_features = 0;
+uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0, minus_kvm_features = 0;
  uint32_t numvalue;

  def = NULL;
@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
  memcpy(x86_cpu_def, def, sizeof(*def));
  }

+plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
  add_flagname_to_bitmaps(hypervisor,plus_features,
-plus_ext_features,plus_ext2_features,plus_ext3_features);
+plus_ext_features,plus_ext2_features,plus_ext3_features,
+plus_kvm_features);

  featurestr = strtok(NULL, ,);

  while (featurestr) {
  char *val;
  if (featurestr[0] == '+') {
-add_flagname_to_bitmaps(featurestr + 
1,plus_features,plus_ext_features,plus_ext2_features,plus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 
1,plus_features,plus_ext_features,plus_ext2_features,plus_ext3_features,plus_kvm_features);
  } else if (featurestr[0] == '-') {
-add_flagname_to_bitmaps(featurestr + 
1,minus_features,minus_ext_features,minus_ext2_features,minus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 
1,minus_features,minus_ext_features,minus_ext2_features,minus_ext3_features,minus_kvm_features);
  } else if ((val = strchr(featurestr, '='))) {
  *val = 0; val++;
  if (!strcmp(featurestr, family)) {
@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
  x86_cpu_def-ext_features |= plus_ext_features;
  x86_cpu_def-ext2_features |= plus_ext2_features;
  x86_cpu_def-ext3_features |= plus_ext3_features;
+x86_cpu_def-kvm_features |= plus_kvm_features;
  x86_cpu_def-features= ~minus_features;
  x86_cpu_def-ext_features= ~minus_ext_features;
  x86_cpu_def-ext2_features= ~minus_ext2_features;
  x86_cpu_def-ext3_features= ~minus_ext3_features;
+

Re: [Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf

2010-01-13 Thread Gleb Natapov

On Wed, Jan 13, 2010 at 10:05:33AM -0600, Anthony Liguori wrote:
 +#ifdef CONFIG_KVM_PARA
 +/* Paravirtualization CPUIDs */
 +memcpy(signature, KVMKVMKVM\0\0\0, 12);
 +c =cpuid_data.entries[cpuid_i++];
 +memset(c, 0, sizeof(*c));
 +c-function = KVM_CPUID_SIGNATURE;
 +c-eax = 0;
 +c-ebx = signature[0];
 +c-ecx = signature[1];
 +c-edx = signature[2];
 +
 +c =cpuid_data.entries[cpuid_i++];
 +memset(c, 0, sizeof(*c));
 +c-function = KVM_CPUID_FEATURES;
 +c-eax = env-cpuid_kvm_features  get_para_features(env);
 +#endif
 +
   cpu_x86_cpuid(env, 0, 0,limit,unused,unused,unused);
 
 Instead of hooking here, would it make more sense to tie into the
 generic cpuid function in helper.c?
 
What do you mean? This patch ties into generic cpuid function in
helper.c and pars kvm cpu flags there. Here we just configure
kernel.

--
Gleb.

Re: [Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf

2010-01-13 Thread Gleb Natapov

On Wed, Jan 13, 2010 at 06:14:54PM +0200, Gleb Natapov wrote:
 On Wed, Jan 13, 2010 at 10:05:33AM -0600, Anthony Liguori wrote:
  +#ifdef CONFIG_KVM_PARA
  +/* Paravirtualization CPUIDs */
  +memcpy(signature, KVMKVMKVM\0\0\0, 12);
  +c =cpuid_data.entries[cpuid_i++];
  +memset(c, 0, sizeof(*c));
  +c-function = KVM_CPUID_SIGNATURE;
  +c-eax = 0;
  +c-ebx = signature[0];
  +c-ecx = signature[1];
  +c-edx = signature[2];
  +
  +c =cpuid_data.entries[cpuid_i++];
  +memset(c, 0, sizeof(*c));
  +c-function = KVM_CPUID_FEATURES;
  +c-eax = env-cpuid_kvm_features  get_para_features(env);
  +#endif
  +
cpu_x86_cpuid(env, 0, 0,limit,unused,unused,unused);
  
  Instead of hooking here, would it make more sense to tie into the
  generic cpuid function in helper.c?
  
 What do you mean? This patch ties into generic cpuid function in
 helper.c and pars kvm cpu flags there. Here we just configure
 kernel.
 
Or do you mean making so that PV leaf will be available via
cpu_x86_cpuid()? This make sense, but lets do it after merging this
code path with qemu-kvm and the proposed patch brings qemu and
qemu-kvm close together.
 
--
Gleb.

Re: [Qemu-devel] QMP forward compatibility support

2010-01-13 Thread Markus Armbruster

Luiz Capitulino lcapitul...@redhat.com writes:

 On Mon, 11 Jan 2010 18:24:24 -0600
 Anthony Liguori anth...@codemonkey.ws wrote:

 On 01/11/2010 06:04 PM, Luiz Capitulino wrote:
 
As async messages were one of the reasons for having QMP, I thought
  that there was a consensus that making it part of the original
  protocol was ok, meaning that they would be always available.
 
That's the only reason.
 
 
 Right, but then it's not a capability, it's a core feature.  I just 
 think it would be odd to advertise something as a capability and have it 
 not behave like other ones.

  Ok, so options are: call it a core feature and don't change anything
 or call it a capability and make it behave like any other capability.

  What's the better? Should we vote? :) Daniel seems to prefer the
 later.

If it's optional, leave it off by default because the consensus seems to
be to leave all optional features off by default.

It should be optional if we want to support clients that don't want it.
I don't think coping with it would be a terrible burden on clients, but
neither is having to ask for it.  Personally, I'd make it optional.

  3. We should add command(s) to enable/disable protocol features
 
  4. Proper feature negotiation is done in pause mode. That's, clients
  interested in enabling new protocol features should start QEMU in
  pause mode and enable the features they are interested in using
 
 
  Why does this matter?
 
  We should be careful to support connecting to a VM long after it's been
  started so any requirement like this is likely to cause trouble.
   
If I understood Markus's concerns correctly, he thinks that feature
  negotiation should happen before the protocol is running, ie. make
  it part of the initial handshake.
 
 
 I think forcing the negotiation before executing any commands is a good 
 idea.  But I don't think requiring the guest not to be running is 
 necessary or even useful.
 
 You don't want to have to support disabling and enabling features in the 
 middle of a protocol session because then you have to deal with weird 
 semantics.

  That's true, but I thought that doing that with pause mode was
 going to be better because it didn't require any change on QMP side.

  If this is a bad approach, then I was wrong.

  Now, making this part of the initial handshake brings some more
 design decisions and by making async messages a capability helps
 to test them.

  I'm thinking in something like this:

 1. Connection is made, the greeting message is sent and QMP is
 in 'handshake mode'

 2. In this mode only commands to enable/disable protocol
 capabilities are allowed

 3. When the client is done with the setup, it issues the
 command 'enable-qmp', which puts the protocol into 'running mode',
 where any command is accepted

Really any command?  What about commands to enable/disable protocol
capabilities?

Re: [Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf


On 01/13/2010 10:23 AM, Gleb Natapov wrote:

Or do you mean making so that PV leaf will be available via
cpu_x86_cpuid()? This make sense, but lets do it after merging this
code path with qemu-kvm and the proposed patch brings qemu and
qemu-kvm close together.
   


Yes, that's what I meant, and yes, I think that's a fine approach.

Regards,

Anthony Liguori

Re: [Qemu-devel] QMP forward compatibility support

On Wed, 13 Jan 2010 17:53:38 +0100
Markus Armbruster arm...@redhat.com wrote:

 Luiz Capitulino lcapitul...@redhat.com writes:
 
  On Mon, 11 Jan 2010 18:24:24 -0600
  Anthony Liguori anth...@codemonkey.ws wrote:
 
  On 01/11/2010 06:04 PM, Luiz Capitulino wrote:
  
 As async messages were one of the reasons for having QMP, I thought
   that there was a consensus that making it part of the original
   protocol was ok, meaning that they would be always available.
  
 That's the only reason.
  
  
  Right, but then it's not a capability, it's a core feature.  I just 
  think it would be odd to advertise something as a capability and have it 
  not behave like other ones.
 
   Ok, so options are: call it a core feature and don't change anything
  or call it a capability and make it behave like any other capability.
 
   What's the better? Should we vote? :) Daniel seems to prefer the
  later.
 
 If it's optional, leave it off by default because the consensus seems to
 be to leave all optional features off by default.
 
 It should be optional if we want to support clients that don't want it.
 I don't think coping with it would be a terrible burden on clients, but
 neither is having to ask for it.  Personally, I'd make it optional.

 Ok, will do.

   3. We should add command(s) to enable/disable protocol features
  
   4. Proper feature negotiation is done in pause mode. That's, clients
   interested in enabling new protocol features should start QEMU in
   pause mode and enable the features they are interested in using
  
  
   Why does this matter?
  
   We should be careful to support connecting to a VM long after it's been
   started so any requirement like this is likely to cause trouble.

 If I understood Markus's concerns correctly, he thinks that feature
   negotiation should happen before the protocol is running, ie. make
   it part of the initial handshake.
  
  
  I think forcing the negotiation before executing any commands is a good 
  idea.  But I don't think requiring the guest not to be running is 
  necessary or even useful.
  
  You don't want to have to support disabling and enabling features in the 
  middle of a protocol session because then you have to deal with weird 
  semantics.
 
   That's true, but I thought that doing that with pause mode was
  going to be better because it didn't require any change on QMP side.
 
   If this is a bad approach, then I was wrong.
 
   Now, making this part of the initial handshake brings some more
  design decisions and by making async messages a capability helps
  to test them.
 
   I'm thinking in something like this:
 
  1. Connection is made, the greeting message is sent and QMP is
  in 'handshake mode'
 
  2. In this mode only commands to enable/disable protocol
  capabilities are allowed
 
  3. When the client is done with the setup, it issues the
  command 'enable-qmp', which puts the protocol into 'running mode',
  where any command is accepted
 
 Really any command?  What about commands to enable/disable protocol
 capabilities?

 I think that playing with some protocol bits might be safe, like
enabling async messages.

 I'm not saying this is a good practice, but forbidding it seems a bit
extreme at first.

Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests

2010-01-13 Thread Markus Armbruster

Anthony Liguori anth...@codemonkey.ws writes:

 On 01/12/2010 01:16 AM, Amit Shah wrote:
 BTW I don't really want this too, I can get rid of it if everyone agrees
 we won't support clipboard writes  4k over vnc or if there's a better
 idea.


 Why bother trying to preserve message boundaries?   I think that's the
 fundamental question.

Yes.  Either it's a datagram or a stream pipe.  I always thought it
would be a stream pipe, as the name serial suggests.

As to the clipboard use case: same problem exists with any old stream
pipe, including TCP, same solutions apply.  If you told the peer I'm
going to send you 12345 bytes now, and your stream pipe chokes after
7890 bytes, you retry until everything got through.  If you want to be
able to abort a partial transfer and start a new one, you layer a
protocol suitable for that on top of your stream pipe.

Re: [Qemu-devel] QMP forward compatibility support

2010-01-13 Thread Markus Armbruster

Luiz Capitulino lcapitul...@redhat.com writes:

 On Wed, 13 Jan 2010 17:53:38 +0100
 Markus Armbruster arm...@redhat.com wrote:

 Luiz Capitulino lcapitul...@redhat.com writes:
[...]
   I'm thinking in something like this:
 
  1. Connection is made, the greeting message is sent and QMP is
  in 'handshake mode'
 
  2. In this mode only commands to enable/disable protocol
  capabilities are allowed
 
  3. When the client is done with the setup, it issues the
  command 'enable-qmp', which puts the protocol into 'running mode',
  where any command is accepted
 
 Really any command?  What about commands to enable/disable protocol
 capabilities?

  I think that playing with some protocol bits might be safe, like
 enabling async messages.

  I'm not saying this is a good practice, but forbidding it seems a bit
 extreme at first.

Allowing stuff when it turns out to be needed is less painful than
outlawing stuff when it turns out to be problematic.

Re: [Qemu-devel] QMP forward compatibility support

On Wed, 13 Jan 2010 18:38:57 +0100
Markus Armbruster arm...@redhat.com wrote:

 Luiz Capitulino lcapitul...@redhat.com writes:
 
  On Wed, 13 Jan 2010 17:53:38 +0100
  Markus Armbruster arm...@redhat.com wrote:
 
  Luiz Capitulino lcapitul...@redhat.com writes:
 [...]
I'm thinking in something like this:
  
   1. Connection is made, the greeting message is sent and QMP is
   in 'handshake mode'
  
   2. In this mode only commands to enable/disable protocol
   capabilities are allowed
  
   3. When the client is done with the setup, it issues the
   command 'enable-qmp', which puts the protocol into 'running mode',
   where any command is accepted
  
  Really any command?  What about commands to enable/disable protocol
  capabilities?
 
   I think that playing with some protocol bits might be safe, like
  enabling async messages.
 
   I'm not saying this is a good practice, but forbidding it seems a bit
  extreme at first.
 
 Allowing stuff when it turns out to be needed is less painful than
 outlawing stuff when it turns out to be problematic.

 I forgot to mention we can block them, that's fine. :)

 So, do we agree with the general design? I'll cook a RFC series.

[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V6)

On Mon, 11 Jan 2010 13:09:35 -0600
Adam Litke a...@us.ibm.com wrote:

 After some good discussion, V6 of this patch integrates well with the new QMP
 support.  When the monitor is in QMP mode, the query-balloon command triggers 
 a
 stats refresh request to the guest.  This request is asynchronous.  If the
 guest does not respond then nothing further happens.  When stats are updated, 
 a
 BALLOON monitor event is raised and the data element will contain the memory
 statistics.
 
 For the user monitor, a timer has been added to prevent monitor hangs with
 unresponsive guests.  When the timer fires, the most recently collected stats
 are returned along with an additional entry 'age' which indicates the number 
 of
 host_clock milliseconds that have passed since the stats were collected.
 
 This method for dealing with asynchronous commands may prove useful for other
 existing or future commands.  In that case, we may want to consider
 incorporating this into the actual monitor API.

 Yeah.

 Sorry for the delay in reviewing.

 I've tried to apply this patch to play with it, but turns out it conflicts
with recent changes in hw/virtio-balloon.

 Some comments on the QMP side of the patch follows.

 diff --git a/balloon.h b/balloon.h
 index 60b4a5d..7e29028 100644
 --- a/balloon.h
 +++ b/balloon.h
 @@ -16,12 +16,22 @@
  
  #include cpu-defs.h
  
 -typedef ram_addr_t (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
 +/* Timeout for synchronous stats requests (in seconds) */
 +#define QEMU_BALLOON_SYNC_TIMEOUT 5
 +
 +typedef enum {
 +QEMU_BALLOON_MODE_NONE = 0,  /* No stats request is active */
 +QEMU_BALLOON_MODE_SYNC = 1,  /* Synchronous stats request */
 +QEMU_BALLOON_MODE_ASYNC = 2, /* Asynchronous stats request */
 +} balloon_mode_t;
 +
 +typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target,
 +balloon_mode_t mode);
  
  void qemu_add_balloon_handler(QEMUBalloonEvent *func, void *opaque);
  
 -void qemu_balloon(ram_addr_t target);
 +int qemu_balloon(ram_addr_t target);
  
 -ram_addr_t qemu_balloon_status(void);
 +int qemu_balloon_status(balloon_mode_t mode);
  
  #endif
 diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
 index cfd3b41..bf67f4d 100644
 --- a/hw/virtio-balloon.c
 +++ b/hw/virtio-balloon.c
 @@ -19,6 +19,10 @@
  #include balloon.h
  #include virtio-balloon.h
  #include kvm.h
 +#include monitor.h
 +#include qlist.h
 +#include qint.h
 +#include qstring.h
  
  #if defined(__linux__)
  #include sys/mman.h
 @@ -27,9 +31,15 @@
  typedef struct VirtIOBalloon
  {
  VirtIODevice vdev;
 -VirtQueue *ivq, *dvq;
 +VirtQueue *ivq, *dvq, *svq;
  uint32_t num_pages;
  uint32_t actual;
 +uint64_t stats[VIRTIO_BALLOON_S_NR];
 +VirtQueueElement stats_vq_elem;
 +size_t stats_vq_offset;
 +balloon_mode_t stats_request_mode;
 +QEMUTimer *stats_timer;
 +uint64_t stats_updated;
  } VirtIOBalloon;
  
  static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
 @@ -46,6 +56,50 @@ static void balloon_page(void *addr, int deflate)
  #endif
  }
  
 +/*
 + * reset_stats - Mark all items in the stats array as unset
 + *
 + * This function needs to be called at device intialization and before
 + * before updating to a set of newly-generated stats.  This will ensure that 
 no
 + * stale values stick around in case the guest reports a subset of the 
 supported
 + * statistics.
 + */
 +static inline void reset_stats(VirtIOBalloon *dev)
 +{
 +int i;
 +for (i = 0; i  VIRTIO_BALLOON_S_NR; dev-stats[i++] = -1);
 +dev-stats_updated = qemu_get_clock(host_clock);
 +}
 +
 +static void stat_put(QDict *dict, const char *label, uint64_t val)
 +{
 +if (val != -1)
 +qdict_put(dict, label, qint_from_int(val));
 +}
 +
 +static QObject *get_stats_qobject(VirtIOBalloon *dev)
 +{
 +QDict *dict = qdict_new();
 +uint32_t actual = ram_size - (dev-actual  VIRTIO_BALLOON_PFN_SHIFT);
 +uint64_t age;
 +
 +stat_put(dict, actual, actual);
 +stat_put(dict, mem_swapped_in, dev-stats[VIRTIO_BALLOON_S_SWAP_IN]);
 +stat_put(dict, mem_swapped_out, dev-stats[VIRTIO_BALLOON_S_SWAP_OUT]);
 +stat_put(dict, major_page_faults, dev-stats[VIRTIO_BALLOON_S_MAJFLT]);
 +stat_put(dict, minor_page_faults, dev-stats[VIRTIO_BALLOON_S_MINFLT]);
 +stat_put(dict, free_mem, dev-stats[VIRTIO_BALLOON_S_MEMFREE]);
 +stat_put(dict, total_mem, dev-stats[VIRTIO_BALLOON_S_MEMTOT]);
 +
 +/* If age is over the timeout threshold, report it */
 +age = (qemu_get_clock(host_clock) - dev-stats_updated) /
 +  (get_ticks_per_sec() / 1000);
 +if (age = QEMU_BALLOON_SYNC_TIMEOUT * 1000)
 +stat_put(dict, age, age);
 +
 +return QOBJECT(dict);
 +}
 +
  /* FIXME: once we do a virtio refactoring, this will get subsumed into common
   * code */
  static size_t memcpy_from_iovector(void *data, size_t offset, size_t size,
 @@ -104,6 +158,65 @@ static void virtio_balloon_handle_output(VirtIODevice 
 *vdev,

Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests


On 01/13/2010 11:14 AM, Markus Armbruster wrote:

Anthony Liguorianth...@codemonkey.ws  writes:

   

On 01/12/2010 01:16 AM, Amit Shah wrote:
 

BTW I don't really want this too, I can get rid of it if everyone agrees
we won't support clipboard writes   4k over vnc or if there's a better
idea.

   

Why bother trying to preserve message boundaries?   I think that's the
fundamental question.
 

Yes.  Either it's a datagram or a stream pipe.  I always thought it
would be a stream pipe, as the name serial suggests.
   


And if it's a datagram, then we should accept that there will be a fixed 
max message size which is pretty common in all datagram protocols.  That 
fixed size should be no larger than what the transport supports so in 
this case, it would be 4k.


If a guest wants to send larger messages, it must build a continuation 
protocol on top of the datagram protocol.


Regards,

Anthony Liguori

[Qemu-devel] Re: [PULL] misc fixes and cleanups


On 01/13/2010 06:49 AM, Michael S. Tsirkin wrote:

Please pull the following changes: eepro100 changes have been
out for a week without comments, and pci one seems
obvious.
   


Applied.  Thanks.

Regards,

Anthony Liguori


The following changes since commit 72ff25e4e98d6dba9286d032b9ff5432553bbad5:
   Juergen Lock (1):
 Workaround for broken OSS_GETVERSION on FreeBSD, part two

are available in the git repository at:

   git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony

Stefan Weil (3):
   eepro100: Fix initial value for PCI_STATUS
   eepro100: Update ROM file support
   pci: Add missing 'const' in argument to pci_get_xxx

  hw/eepro100.c |   15 ++-
  hw/pci.h  |   14 +++---
  2 files changed, 9 insertions(+), 20 deletions(-)

Re: [Qemu-devel] [PATCH 0/9] PPC NewWorld fixery v3

2010-01-13 Thread Blue Swirl

On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf ag...@suse.de wrote:

On 12.01.2010, at 21:52, Blue Swirl wrote:

On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf ag...@suse.de wrote:

On 12.01.2010, at 20:45, Blue Swirl wrote:

On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf ag...@suse.de wrote:
I'm trying to get the PPC64 system emulation target working finally.
While doing so, I ran into several issues, all related to PCI this time.

This patchset fixes all the PCI config space access and PCI interrupt
mapping issues I've found on PPC64. Using this and a patched OpenBIOS
version, I can successfully access IDE devices and was booting a guest
into the shell from IDE using serial console.

To leverage this patch, you also need a few patches to OpenBIOS. I'll
present them to the OpenBIOS list, but in general getting patches into
Qemu is harder than getting them into OpenBIOS. So I want to wait for
the review process here first.

Find the OpenBIOS patch at: http://alex.csgraf.de/openbios-ppc-u3.patch

About the OpenBIOS patch, could you move the PCI_INT_MAP defines to a
PPC-specific header and make pci_host_set_interrupt_map() contents
surrounded by #ifdef CONFIG_PPC (to make it empty function for other
arches)?

Well, other archs should be able to use the same code. If OpenBIOS knows
how interrupts work for a particular device, it really should tell the OS
about it too IMHO.

I'm not so sure. Here's an example of a Sparc64 interrupt-map:

Node 0xf005f9d4
bus-range: 0001.0001
scsi-initiator-id: 0007
compatible: 70636931.3038652c.35303030.00706369
66mhz-capable:
fast-back-to-back:
devsel-speed: 0001
class-code: 00060400
revision-id: 0011
device-id: 5000
vendor-id: 108e
interrupt-map:
00010800...0001.f005f1e0.0021.00011000...0001.f005f1e0.000f.00011800...0001.f005f1e0.0020
interrupt-map-mask: 00fff800...0007

This translates to:

Interrupt PIN A on dev 00010800 - INT 0x21
Interrupt PIN A on dev 00011000 - INT 0x0f
Interrupt PIN A on dev 00011800 - INT 0x20

What does the corresponding code in OpenBIOS do to figure out which IRQ is
routed where?

Currently there isn't anything, so something may be better than
nothing. Would your code produce correct interrupt-map then also for
Sparc64?

The UniNorth case is really simple, because it doesn't take any mangling into
account. Usually PCI bridges also assign pins differently depending on the
port the card is plugged into, so an all devices on PIN A configuration
still ends up being distributed over all 4 interrupts.

I'm certainly open to more clever ideas. We could of course forget about all
the interrupt mapping and simply put PIC targeted interrupt properties into
each device's node. But I somehow like the map approach better, especially
because the normal (defined in the interrupt map draft) way of doing PCI
interrupts is to have an interrupt property of size=1 which defines the pin.

I should probably read the draft before trying to comment further.

[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V6)

2010-01-13 Thread Adam Litke

On Wed, 2010-01-13 at 16:04 -0200, Luiz Capitulino wrote:
  I've tried to apply this patch to play with it, but turns out it conflicts
 with recent changes in hw/virtio-balloon.

Ahh, I will continue my never-ending quest to stay current :)

  Some comments on the QMP side of the patch follows.

Thanks for your review!

snip

  +/*
  + * complete_stats_request - Clean up and report statistics.
  + */
  +static void complete_stats_request(VirtIOBalloon *vb)
  +{
  +QObject *stats = get_stats_qobject(vb);
  +
  +if (vb-stats_request_mode == QEMU_BALLOON_MODE_SYNC) {
  +qemu_del_timer(vb-stats_timer);
  +monitor_print_balloon(cur_mon, stats);
  +monitor_resume(cur_mon);
  +} else if (vb-stats_request_mode == QEMU_BALLOON_MODE_ASYNC) {
  +monitor_protocol_event(QEVENT_BALLOON, stats);
  +}
  +
  +vb-stats_request_mode = QEMU_BALLOON_MODE_NONE;
  +}
 
  In the previous thread Anthony raised some issues about the 'cur_mon'
 usage that made me concerned, because some important code rely on
 it (read async events).
 
  As far as I could check, 'cur_mon' is guaranteed to be the default
 Monitor which is fine if you're aware of it when putting QEMU to run,
 but I'm afraid that testing your patch with two Monitors (user and qmp)
 is not going to work.
 
  Maybe not a big deal, but would be good to be aware of potential
 issues.

I talked to Anthony and will try to fix this up.  I just need to start
passing the monitor pointer around as well.

snip
  * 
  +if (monitor_ctrl_mode(mon))
  +mode = QEMU_BALLOON_MODE_ASYNC;
  +else
  +
  +mode = monitor_ctrl_mode(mon) ?
  +QEMU_BALLOON_MODE_ASYNC : 
  QEMU_BALLOON_MODE_SYNC;
 
  I think what you want is:
 
 } else {
  mode = QEMU_BALLOON_MODE_SYNC;
 }

Bah... Left over gunk. 

snip

  -void qemu_balloon(ram_addr_t target)
  +int qemu_balloon(ram_addr_t target)
   {
  -if (qemu_balloon_event)
  -qemu_balloon_event(qemu_balloon_event_opaque, target);
  +if (qemu_balloon_event) {
  +qemu_balloon_event(qemu_balloon_event_opaque, target,
  +   QEMU_BALLOON_MODE_SYNC);
  +return 1;
  +} else {
  +return 0;
  +}
   }
 
  This is used by do_balloon() right? Which is also used by QMP,
 shouldn't it also handle async vs. sync?

qemu_balloon always acts synchronously.  It does not wait on the guest
to do anything and it does not return data.

-- 
Thanks,
Adam

[Qemu-devel] Re: virtio: Add memory statistics reporting to the balloon driver (V6)

On Wed, 13 Jan 2010 12:59:25 -0600
Adam Litke a...@us.ibm.com wrote:

   +/*
   + * complete_stats_request - Clean up and report statistics.
   + */
   +static void complete_stats_request(VirtIOBalloon *vb)
   +{
   +QObject *stats = get_stats_qobject(vb);
   +
   +if (vb-stats_request_mode == QEMU_BALLOON_MODE_SYNC) {
   +qemu_del_timer(vb-stats_timer);
   +monitor_print_balloon(cur_mon, stats);
   +monitor_resume(cur_mon);
   +} else if (vb-stats_request_mode == QEMU_BALLOON_MODE_ASYNC) {
   +monitor_protocol_event(QEVENT_BALLOON, stats);
   +}
   +
   +vb-stats_request_mode = QEMU_BALLOON_MODE_NONE;
   +}
  
   In the previous thread Anthony raised some issues about the 'cur_mon'
  usage that made me concerned, because some important code rely on
  it (read async events).
  
   As far as I could check, 'cur_mon' is guaranteed to be the default
  Monitor which is fine if you're aware of it when putting QEMU to run,
  but I'm afraid that testing your patch with two Monitors (user and qmp)
  is not going to work.
  
   Maybe not a big deal, but would be good to be aware of potential
  issues.
 
 I talked to Anthony and will try to fix this up.  I just need to start
 passing the monitor pointer around as well.

 After doing that you could help us improving the Monitor and check
the other cur_mon usages too :)

   -void qemu_balloon(ram_addr_t target)
   +int qemu_balloon(ram_addr_t target)
{
   -if (qemu_balloon_event)
   -qemu_balloon_event(qemu_balloon_event_opaque, target);
   +if (qemu_balloon_event) {
   +qemu_balloon_event(qemu_balloon_event_opaque, target,
   +   QEMU_BALLOON_MODE_SYNC);
   +return 1;
   +} else {
   +return 0;
   +}
}
  
   This is used by do_balloon() right? Which is also used by QMP,
  shouldn't it also handle async vs. sync?
 
 qemu_balloon always acts synchronously.  It does not wait on the guest
 to do anything and it does not return data.

 Fine then.

Re: [Qemu-devel] Static analysis using clang on the x86_64 target

2010-01-13 Thread Blue Swirl

On Wed, Jan 13, 2010 at 7:02 AM, Amit Shah amit.s...@redhat.com wrote:
 On (Tue) Jan 12 2010 [19:35:08], Blue Swirl wrote:
 On Tue, Jan 12, 2010 at 6:13 PM, Amit Shah amit.s...@redhat.com wrote:
  Hello,
 
  Here's a run of the clang analyzer on qemu sources for the x86_64
  target.
 
  See
 
  http://amitshah.fedorapeople.org/clang-output/2010-01-12-9/
 
  for the results.
 
  There are a few results there which look dubious but a lot of the output
  can be useful to fix the bugs.
 
  What's nice about the tool is that the output is the source code
  annotated with the branch decisions that were taken to point out to the
  case where a bug would be triggered.
 
  Doing this for all the targets takes a really long time plus lots of
  disk space (I stopped the compile at 400M of clang output).
 
  If there's interest in this kind of result, I can post a link to the
  list every week or so. However, some bugs reported make it slightly less
  appealing as real bugs could get lost in the noise.

 I'd be very interested in the results of Sparc32 and Sparc64 analyses.

 OK, I added the two targets to the run and got the following result:

 http://amitshah.fedorapeople.org/clang-output/2010-01-13-1/

 The bug count went up from 95 for just x86-64 to 131.

 However, a lot of these are dups as files get recompiled for each
 target.

Thanks. I fixed the warnings related to Sparc32. Were there really no
new warnings for Sparc64?

Re: [Qemu-devel] [PATCH 0/9] PPC NewWorld fixery v3

2010-01-13 Thread Alexander Graf

On 13.01.2010, at 19:47, Blue Swirl wrote:

On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf ag...@suse.de wrote:

On 12.01.2010, at 21:52, Blue Swirl wrote:

On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf ag...@suse.de wrote:

On 12.01.2010, at 20:45, Blue Swirl wrote:

Find the OpenBIOS patch at: http://alex.csgraf.de/openbios-ppc-u3.patch

Well, other archs should be able to use the same code. If OpenBIOS knows
how interrupts work for a particular device, it really should tell the OS
about it too IMHO.

I'm not so sure. Here's an example of a Sparc64 interrupt-map:

This translates to:

Interrupt PIN A on dev 00010800 - INT 0x21
Interrupt PIN A on dev 00011000 - INT 0x0f
Interrupt PIN A on dev 00011800 - INT 0x20

What does the corresponding code in OpenBIOS do to figure out which IRQ is
routed where?

Currently there isn't anything, so something may be better than
nothing. Would your code produce correct interrupt-map then also for
Sparc64?

Depends on how your PCI bridge maps interrupts. What does qemu's pci interrupt
map function for your pci bridge look like?

The UniNorth case is really simple, because it doesn't take any mangling
into account. Usually PCI bridges also assign pins differently depending on
the port the card is plugged into, so an all devices on PIN A
configuration still ends up being distributed over all 4 interrupts.

I should probably read the draft before trying to comment further.

http://playground.sun.com/1275/practice/imap/imap0_9d.pdf

Alex

Re: [Qemu-devel] [PATCH 0/9] PPC NewWorld fixery v3

2010-01-13 Thread Blue Swirl

On Wed, Jan 13, 2010 at 7:17 PM, Alexander Graf ag...@suse.de wrote:

 On 13.01.2010, at 19:47, Blue Swirl wrote:

 On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf ag...@suse.de wrote:

 On 12.01.2010, at 21:52, Blue Swirl wrote:

 On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf ag...@suse.de wrote:

 On 12.01.2010, at 20:45, Blue Swirl wrote:

 On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf ag...@suse.de wrote:
 I'm trying to get the PPC64 system emulation target working finally.
 While doing so, I ran into several issues, all related to PCI this time.

 This patchset fixes all the PCI config space access and PCI interrupt
 mapping issues I've found on PPC64. Using this and a patched OpenBIOS
 version, I can successfully access IDE devices and was booting a guest
 into the shell from IDE using serial console.

 To leverage this patch, you also need a few patches to OpenBIOS. I'll
 present them to the OpenBIOS list, but in general getting patches into
 Qemu is harder than getting them into OpenBIOS. So I want to wait for
 the review process here first.

 Find the OpenBIOS patch at: http://alex.csgraf.de/openbios-ppc-u3.patch

 About the OpenBIOS patch, could you move the PCI_INT_MAP defines to a
 PPC-specific header and make pci_host_set_interrupt_map() contents
 surrounded by #ifdef CONFIG_PPC (to make it empty function for other
 arches)?

 Well, other archs should be able to use the same code. If OpenBIOS knows 
 how interrupts work for a particular device, it really should tell the OS 
 about it too IMHO.

 I'm not so sure. Here's an example of a Sparc64 interrupt-map:

        Node 0xf005f9d4
            bus-range:  0001.0001
            scsi-initiator-id:  0007
            compatible:  70636931.3038652c.35303030.00706369
            66mhz-capable:
            fast-back-to-back:
            devsel-speed:  0001
            class-code:  00060400
            revision-id:  0011
            device-id:  5000
            vendor-id:  108e
            interrupt-map:
 00010800...0001.f005f1e0.0021.00011000...0001.f005f1e0.000f.00011800...0001.f005f1e0.0020
            interrupt-map-mask:  00fff800...0007


 This translates to:

 Interrupt PIN A on dev 00010800 - INT 0x21
 Interrupt PIN A on dev 00011000 - INT 0x0f
 Interrupt PIN A on dev 00011800 - INT 0x20

 What does the corresponding code in OpenBIOS do to figure out which IRQ is 
 routed where?

 Currently there isn't anything, so something may be better than
 nothing. Would your code produce correct interrupt-map then also for
 Sparc64?

 Depends on how your PCI bridge maps interrupts. What does qemu's pci 
 interrupt map function for your pci bridge look like?

/* The APB host has an IRQ line for each IRQ line of each slot.  */
static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
{
return ((pci_dev-devfn  0x18)  1) + irq_num;
}

This may be bogus though.



 The UniNorth case is really simple, because it doesn't take any mangling 
 into account. Usually PCI bridges also assign pins differently depending on 
 the port the card is plugged into, so an all devices on PIN A 
 configuration still ends up being distributed over all 4 interrupts.

 I'm certainly open to more clever ideas. We could of course forget about 
 all the interrupt mapping and simply put PIC targeted interrupt properties 
 into each device's node. But I somehow like the map approach better, 
 especially because the normal (defined in the interrupt map draft) way of 
 doing PCI interrupts is to have an interrupt property of size=1 which 
 defines the pin.

 I should probably read the draft before trying to comment further.

 http://playground.sun.com/1275/practice/imap/imap0_9d.pdf

Thanks.

Re: [Qemu-devel] [PATCH 0/9] PPC NewWorld fixery v3

2010-01-13 Thread Alexander Graf


On 13.01.2010, at 20:37, Blue Swirl wrote:

 On Wed, Jan 13, 2010 at 7:17 PM, Alexander Graf ag...@suse.de wrote:
 
 On 13.01.2010, at 19:47, Blue Swirl wrote:
 
 On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf ag...@suse.de wrote:
 
 On 12.01.2010, at 21:52, Blue Swirl wrote:
 
 On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf ag...@suse.de wrote:
 
 On 12.01.2010, at 20:45, Blue Swirl wrote:
 
 On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf ag...@suse.de wrote:
 I'm trying to get the PPC64 system emulation target working finally.
 While doing so, I ran into several issues, all related to PCI this 
 time.
 
 This patchset fixes all the PCI config space access and PCI interrupt
 mapping issues I've found on PPC64. Using this and a patched OpenBIOS
 version, I can successfully access IDE devices and was booting a guest
 into the shell from IDE using serial console.
 
 To leverage this patch, you also need a few patches to OpenBIOS. I'll
 present them to the OpenBIOS list, but in general getting patches into
 Qemu is harder than getting them into OpenBIOS. So I want to wait for
 the review process here first.
 
 Find the OpenBIOS patch at: http://alex.csgraf.de/openbios-ppc-u3.patch
 
 About the OpenBIOS patch, could you move the PCI_INT_MAP defines to a
 PPC-specific header and make pci_host_set_interrupt_map() contents
 surrounded by #ifdef CONFIG_PPC (to make it empty function for other
 arches)?
 
 Well, other archs should be able to use the same code. If OpenBIOS knows 
 how interrupts work for a particular device, it really should tell the 
 OS about it too IMHO.
 
 I'm not so sure. Here's an example of a Sparc64 interrupt-map:
 
Node 0xf005f9d4
bus-range:  0001.0001
scsi-initiator-id:  0007
compatible:  70636931.3038652c.35303030.00706369
66mhz-capable:
fast-back-to-back:
devsel-speed:  0001
class-code:  00060400
revision-id:  0011
device-id:  5000
vendor-id:  108e
interrupt-map:
 00010800...0001.f005f1e0.0021.00011000...0001.f005f1e0.000f.00011800...0001.f005f1e0.0020
interrupt-map-mask:  00fff800...0007
 
 
 This translates to:
 
 Interrupt PIN A on dev 00010800 - INT 0x21
 Interrupt PIN A on dev 00011000 - INT 0x0f
 Interrupt PIN A on dev 00011800 - INT 0x20
 
 What does the corresponding code in OpenBIOS do to figure out which IRQ is 
 routed where?
 
 Currently there isn't anything, so something may be better than
 nothing. Would your code produce correct interrupt-map then also for
 Sparc64?
 
 Depends on how your PCI bridge maps interrupts. What does qemu's pci 
 interrupt map function for your pci bridge look like?
 
 /* The APB host has an IRQ line for each IRQ line of each slot.  */
 static int pci_apb_map_irq(PCIDevice *pci_dev, int irq_num)
 {
return ((pci_dev-devfn  0x18)  1) + irq_num;
 }
 
 This may be bogus though.

Don't know, it might be correct :-). Either way, you'd have to do a map similar 
to the one in the dump you gave above. That should be fairly easy to generate 
programatically.

If you need help to understand interrupt-map properties, feel free to ask. I 
only finally managed to understand them recently myself.


Alex

[Qemu-devel] Re: [PATCH 2/6] virtio: net: remove dead assignment

On Wed, Jan 13, 2010 at 04:24:43PM +0530, Amit Shah wrote:
 clang-analyzer points out value assigned to 'len' is not used.
 
 Signed-off-by: Amit Shah amit.s...@redhat.com

Acked-by: Michael S. Tsirkin m...@redhat.com

 ---
  hw/virtio-net.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/hw/virtio-net.c b/hw/virtio-net.c
 index 02d9180..6e48997 100644
 --- a/hw/virtio-net.c
 +++ b/hw/virtio-net.c
 @@ -532,7 +532,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
 const uint8_t *buf, size_
  int len, total;
  struct iovec sg[VIRTQUEUE_MAX_SIZE];
  
 -len = total = 0;
 +total = 0;
  
  if ((i != 0  !n-mergeable_rx_bufs) ||
  virtqueue_pop(n-rx_vq, elem) == 0) {
 -- 
 1.6.2.5

[Qemu-devel] Re: [PATCH 1/2] kvm: Use kvm-kmod headers if available

2010-01-13 Thread Doug Goldstein

On Tue, Jan 12, 2010 at 12:53 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 Since kvm-kmod-2.6.32.2 we have an alternative source for recent KVM
 kernel headers. Use it when available and not overruled by --kerneldir.


Would it be possible to turn this into a configure option? Such that I
could say ./configure --with-kvm-kmod{=optional/path/to} because
otherwise this adds a potential automagical dependency onto kvm-kmod
(i.e. if the user had kvm-kmod installed at the time of build but then
removed them and went back with their kernel provided version)

Thanks.

-- 
Doug Goldstein

[Qemu-devel] [PATCH] rtl8139: fix clang reporting unused assignment of VLAN tagging data

2010-01-13 Thread Igor V. Kovalenko

From: Igor V. Kovalenko igor.v.kovale...@gmail.com

Currently we do not implement VLAN tagging for rtl8139(C+),
still data is read from ring buffer headers.

- augment unused assignment with TODO item
- cast txdw1 to void for now

Signed-off-by: Igor V. Kovalenko igor.v.kovale...@gmail.com
---
 hw/rtl8139.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 1f4f585..f04dd54 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1909,6 +1909,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
 cpu_physical_memory_read(cplus_tx_ring_desc,(uint8_t *)val, 4);
 txdw0 = le32_to_cpu(val);
+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
 cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)val, 4);
 txdw1 = le32_to_cpu(val);
 cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)val, 4);
@@ -1920,6 +1921,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
descriptor,
txdw0, txdw1, txbufLO, txbufHI));
 
+/* TODO: the following discard cast should clean clang analyzer output */
+(void)txdw1;
+
 /* w0 ownership flag */
 #define CP_TX_OWN (131)
 /* w0 end of ring flag */
@@ -2045,6 +2049,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 /* update ring data */
 val = cpu_to_le32(txdw0);
 cpu_physical_memory_write(cplus_tx_ring_desc,(uint8_t *)val, 4);
+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
 //val = cpu_to_le32(txdw1);
 //cpu_physical_memory_write(cplus_tx_ring_desc+4,  val, 4);

Re: [Qemu-devel] [PATCH v2] raw-posix: Detect legacy floppy via ioctl


On 01/12/2010 09:29 AM, Cole Robinson wrote:

Current legacy floppy detection is hardcoded based on source file
name. Make this smarter by attempting a floppy specific ioctl.

v2: Give ioctl check higher priority than filename check,
 s/IDE/legacy/

Signed-off-by: Cole Robinsoncrobi...@redhat.com
---
  block/raw-posix.c |   20 ++--
  1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index b7254d8..d67280e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1055,9 +1055,25 @@ static int floppy_open(BlockDriverState *bs, const char 
*filename, int flags)

  static int floppy_probe_device(const char *filename)
  {
+int fd, ret, prio;
+struct floppy_struct fdparam;
+
  if (strstart(filename, /dev/fd, NULL))
-return 100;
-return 0;
+prio = 50;
+
+fd = open(filename, O_RDONLY | O_NONBLOCK);
+if (fd  0) {
+goto out;
+}
+
+/* Attempt to detect via a floppy specific ioctl */
+ret = ioctl(fd, FDGETPRM,fdparam);
+if (!(ret  0  errno == EINVAL))
   


These two patches break boot from an image file.  My suspicious is that 
it's failing because the errno is ENOSYS.


You probably want to do the opposite and check for a positive return 
result instead of checking for the absence of a positive result.


Regards,

Anthony Liguori

Re: [Qemu-devel] Qemu's internal TFTP server breaks lock-step-iness of TFTP


On 01/07/2010 06:39 AM, Milan Plzik wrote:

   According to RFC 1350 and RFC 2347, TFTP server should answer RRQ by
either OACK or DATA packet. Qemu's internal TFTP server answers RRQ with
additional options by sending both OACK and DATA packet, thus breaking
the lock-step feature of the protocol, and also confuses client.

   Proposed solution would be to, in case of OACK packet, wait for ACK
from client and just then start sending data. Attached patch implements
this.

Signed-off-by: Thomas Horstentho...@horsten.com
Signed-off-by: Milan Plzikmilan.pl...@gmail.com

   


Applied.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v4 1/3] block: Introduce BDRV_O_NO_BACKING


On 01/12/2010 05:55 AM, Kevin Wolf wrote:

If an image references a backing file that doesn't exist, qemu-img info fails
to open this image. Exactly in this case the info would be valuable, though:
the user might want to find out which file is missing.

This patch introduces a BDRV_O_NO_BACKING flag to ignore the backing file when
opening the image. qemu-img info is the first user and provides info now even
if the backing file is invalid.

Signed-off-by: Kevin Wolfkw...@redhat.com
   


Applied all.  Thanks.

Regards,

Anthony Liguori

---
  block.c|4 ++--
  block.h|1 +
  qemu-img.c |2 +-
  3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 30ae2b1..994109e 100644
--- a/block.c
+++ b/block.c
@@ -477,7 +477,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
  unlink(filename);
  }
  #endif
-if (bs-backing_file[0] != '\0') {
+if ((flags  BDRV_O_NO_BACKING) == 0  bs-backing_file[0] != '\0') {
  /* if there is a backing file, use it */
  BlockDriver *back_drv = NULL;
  bs-backing_hd = bdrv_new();
@@ -1352,7 +1352,7 @@ const char *bdrv_get_encrypted_filename(BlockDriverState 
*bs)
  void bdrv_get_backing_filename(BlockDriverState *bs,
 char *filename, int filename_size)
  {
-if (!bs-backing_hd) {
+if (!bs-backing_file) {
  pstrcpy(filename, filename_size, );
  } else {
  pstrcpy(filename, filename_size, bs-backing_file);
diff --git a/block.h b/block.h
index fa51ddf..f660d5f 100644
--- a/block.h
+++ b/block.h
@@ -39,6 +39,7 @@ typedef struct QEMUSnapshotInfo {
  #define BDRV_O_NOCACHE 0x0020 /* do not use the host page cache */
  #define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
  #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread 
pool */
+#define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */

  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)

diff --git a/qemu-img.c b/qemu-img.c
index 1d97f2e..5ad88bf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -884,7 +884,7 @@ static int img_info(int argc, char **argv)
  } else {
  drv = NULL;
  }
-if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv)  0) {
+if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_NO_BACKING, drv)  0) {
  error(Could not open '%s', filename);
  }
  bdrv_get_format(bs, fmt_name, sizeof(fmt_name));

Re: [Qemu-devel] [PATCH 1/2] block: flush backing_hd in the right place


On 01/12/2010 06:49 AM, Christoph Hellwig wrote:

The backing device is only modified from bdrv_commit.  So instead of
flushing it every time bdrv_flush is called for the front-end device
only flush it after we're written data to it in bdrv_commit.

Signed-off-by: Christoph Hellwigh...@lst.de
   


Applied.  Thanks.

Regards,

Anthony Liguori


Index: qemu/block.c
===
--- qemu.orig/block.c   2010-01-12 11:34:35.549024986 +0100
+++ qemu/block.c2010-01-12 11:43:28.965006129 +0100
@@ -623,6 +623,12 @@ int bdrv_commit(BlockDriverState *bs)
  if (drv-bdrv_make_empty)
return drv-bdrv_make_empty(bs);

+/*
+ * Make sure all data we wrote to the backing device is actually
+ * stable on disk.
+ */
+if (bs-backing_hd)
+bdrv_flush(bs-backing_hd);
  return 0;
  }

@@ -1124,12 +1130,8 @@ const char *bdrv_get_device_name(BlockDr

  void bdrv_flush(BlockDriverState *bs)
  {
-if (!bs-drv)
-return;
-if (bs-drv-bdrv_flush)
+if (bs-drv  bs-drv-bdrv_flush)
  bs-drv-bdrv_flush(bs);
-if (bs-backing_hd)
-bdrv_flush(bs-backing_hd);
  }

  void bdrv_flush_all(void)
@@ -1806,11 +1808,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDr

  if (!drv)
  return NULL;
-
-/*
- * Note that unlike bdrv_flush the driver is reponsible for flushing a
- * backing image if it exists.
- */
  return drv-bdrv_aio_flush(bs, cb, opaque);
  }

Re: [Qemu-devel] [PATCH 1/6] vl.c: Remove dead assignment


On 01/13/2010 04:54 AM, Amit Shah wrote:

clang-analyzer pointed out the value of 'sockets' is never reused.

Signed-off-by: Amit Shahamit.s...@redhat.com
CC: Andre Przywaraandre.przyw...@amd.com
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  vl.c |6 +-
  1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index b048e89..e49e7bd 100644
--- a/vl.c
+++ b/vl.c
@@ -2613,17 +2613,13 @@ static void smp_parse(const char *optarg)
  threads = threads  0 ? threads : 1;
  if (smp == 0) {
  smp = cores * threads * sockets;
-} else {
-sockets = smp / (cores * threads);
  }
  } else {
  if (cores == 0) {
  threads = threads  0 ? threads : 1;
  cores = smp / (sockets * threads);
  } else {
-if (sockets == 0) {
-sockets = smp / (cores * threads);
-} else {
+if (sockets) {
  threads = smp / (cores * sockets);
  }
  }

Re: [Qemu-devel] [PATCH] virtio-blk: remove dead variable in virtio_blk_handle_scsi


On 01/13/2010 06:30 AM, Christoph Hellwig wrote:

As pointed out by clang size is only ever written to, but never actually
used.

Signed-off-by: Christoph Hellwigh...@lst.de
   


Applied.  Thanks.

Regards,

Anthony Liguori

Index: qemu/hw/virtio-blk.c
===
--- qemu.orig/hw/virtio-blk.c   2010-01-13 13:25:00.911004071 +0100
+++ qemu/hw/virtio-blk.c2010-01-13 13:25:17.014006323 +0100
@@ -165,7 +165,7 @@ static VirtIOBlockReq *virtio_blk_get_re
  static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
  {
  struct sg_io_hdr hdr;
-int ret, size = 0;
+int ret;
  int status;
  int i;

@@ -194,7 +194,6 @@ static void virtio_blk_handle_scsi(VirtI
   * before the regular inhdr.
   */
  req-scsi = (void *)req-elem.in_sg[req-elem.in_num - 2].iov_base;
-size = sizeof(*req-in) + sizeof(*req-scsi);

  memset(hdr, 0, sizeof(struct sg_io_hdr));
  hdr.interface_id = 'S';
@@ -226,7 +225,6 @@ static void virtio_blk_handle_scsi(VirtI
  hdr.dxfer_len += req-elem.in_sg[i].iov_len;

  hdr.dxferp = req-elem.in_sg;
-size += hdr.dxfer_len;
  } else {
  /*
   * Some SCSI commands don't actually transfer any data.
@@ -236,7 +234,6 @@ static void virtio_blk_handle_scsi(VirtI

  hdr.sbp = req-elem.in_sg[req-elem.in_num - 3].iov_base;
  hdr.mx_sb_len = req-elem.in_sg[req-elem.in_num - 3].iov_len;
-size += hdr.mx_sb_len;

  ret = bdrv_ioctl(req-dev-bs, SG_IO,hdr);
  if (ret) {

Re: [Qemu-devel] [PATCH] move kbd/mouse handling to input.c


On 01/13/2010 07:05 AM, Paolo Bonzini wrote:

Move 200 lines out of vl.c already into common code that only needs to
be compiled once.

Signed-off-by: Paolo Bonzinipbonz...@redhat.com
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  Makefile.objs |2 +-
  input.c   |  238 +
  vl.c  |  214 +--
  3 files changed, 241 insertions(+), 213 deletions(-)
  create mode 100644 input.c

diff --git a/Makefile.objs b/Makefile.objs
index e8a44d7..5802d39 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -47,7 +47,7 @@ common-obj-y += $(qobject-obj-y)
  common-obj-y += readline.o console.o

  common-obj-y += tcg-runtime.o host-utils.o
-common-obj-y += irq.o ioport.o
+common-obj-y += irq.o ioport.o input.o
  common-obj-$(CONFIG_PTIMER) += ptimer.o
  common-obj-$(CONFIG_MAX7310) += max7310.o
  common-obj-$(CONFIG_WM8750) += wm8750.o
diff --git a/input.c b/input.c
new file mode 100644
index 000..955b9ab
--- /dev/null
+++ b/input.c
@@ -0,0 +1,238 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include sysemu.h
+#include net.h
+#include monitor.h
+#include console.h
+#include qjson.h
+
+
+static QEMUPutKBDEvent *qemu_put_kbd_event;
+static void *qemu_put_kbd_event_opaque;
+static QEMUPutMouseEntry *qemu_put_mouse_event_head;
+static QEMUPutMouseEntry *qemu_put_mouse_event_current;
+
+void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
+{
+qemu_put_kbd_event_opaque = opaque;
+qemu_put_kbd_event = func;
+}
+
+QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
+void *opaque, int absolute,
+const char *name)
+{
+QEMUPutMouseEntry *s, *cursor;
+
+s = qemu_mallocz(sizeof(QEMUPutMouseEntry));
+
+s-qemu_put_mouse_event = func;
+s-qemu_put_mouse_event_opaque = opaque;
+s-qemu_put_mouse_event_absolute = absolute;
+s-qemu_put_mouse_event_name = qemu_strdup(name);
+s-next = NULL;
+
+if (!qemu_put_mouse_event_head) {
+qemu_put_mouse_event_head = qemu_put_mouse_event_current = s;
+return s;
+}
+
+cursor = qemu_put_mouse_event_head;
+while (cursor-next != NULL)
+cursor = cursor-next;
+
+cursor-next = s;
+qemu_put_mouse_event_current = s;
+
+return s;
+}
+
+void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry)
+{
+QEMUPutMouseEntry *prev = NULL, *cursor;
+
+if (!qemu_put_mouse_event_head || entry == NULL)
+return;
+
+cursor = qemu_put_mouse_event_head;
+while (cursor != NULL  cursor != entry) {
+prev = cursor;
+cursor = cursor-next;
+}
+
+if (cursor == NULL) // does not exist or list empty
+return;
+else if (prev == NULL) { // entry is head
+qemu_put_mouse_event_head = cursor-next;
+if (qemu_put_mouse_event_current == entry)
+qemu_put_mouse_event_current = cursor-next;
+qemu_free(entry-qemu_put_mouse_event_name);
+qemu_free(entry);
+return;
+}
+
+prev-next = entry-next;
+
+if (qemu_put_mouse_event_current == entry)
+qemu_put_mouse_event_current = prev;
+
+qemu_free(entry-qemu_put_mouse_event_name);
+qemu_free(entry);
+}
+
+void kbd_put_keycode(int keycode)
+{
+if (qemu_put_kbd_event) {
+qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
+}
+}
+
+void kbd_mouse_event(int dx, int dy, int dz, int buttons_state)
+{
+QEMUPutMouseEvent *mouse_event;
+void *mouse_event_opaque;
+int width;
+
+if (!qemu_put_mouse_event_current) {
+return;
+}
+
+mouse_event =
+qemu_put_mouse_event_current-qemu_put_mouse_event;
+mouse_event_opaque =
+

Re: [Qemu-devel] [PATCHv3] Add KVM paravirt cpuid leaf


On 01/13/2010 07:25 AM, Gleb Natapov wrote:

Initialize KVM paravirt cpuid leaf and allow user to control guest
visible PV features through -cpu flag.

Signed-off-by: Gleb Natapovg...@redhat.com
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  v1-v2
   fix indentation
   remove unneeded ifdefs
  v2-v3
   added needed ifdefs (CONFIG_KVM_PARA)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f3834b3..216b00e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
  uint8_t nmi_pending;
  uint8_t has_error_code;
  uint32_t sipi_vector;
-
+uint32_t cpuid_kvm_features;
+
  /* in order to simplify APIC support, we leave this pointer to the
 user */
  struct APICState *apic_state;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 049fccf..70762bb 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
  NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };

+static const char *kvm_feature_name[] = {
+kvmclock, kvm_nopiodelay, kvm_mmu, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
  uint32_t *ext_features,
  uint32_t *ext2_features,
-uint32_t *ext3_features)
+uint32_t *ext3_features,
+uint32_t *kvm_features)
  {
  int i;
  int found = 0;
@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, 
uint32_t *features,
  *ext3_features |= 1  i;
  found = 1;
  }
+for ( i = 0 ; i  32 ; i++ )
+if (kvm_feature_name[i]  !strcmp (flagname, kvm_feature_name[i])) {
+*kvm_features |= 1  i;
+found = 1;
+}
+
  if (!found) {
  fprintf(stderr, CPU feature %s not found\n, flagname);
  }
@@ -98,7 +112,7 @@ typedef struct x86_def_t {
  int family;
  int model;
  int stepping;
-uint32_t features, ext_features, ext2_features, ext3_features;
+uint32_t features, ext_features, ext2_features, ext3_features, 
kvm_features;
  uint32_t xlevel;
  char model_id[48];
  int vendor_override;
@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)

  char *s = strdup(cpu_model);
  char *featurestr, *name = strtok(s, ,);
-uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0;
-uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0;
+uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, 
plus_ext3_features = 0, plus_kvm_features = 0;
+uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 
0, minus_ext3_features = 0, minus_kvm_features = 0;
  uint32_t numvalue;

  def = NULL;
@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
  memcpy(x86_cpu_def, def, sizeof(*def));
  }

+plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
  add_flagname_to_bitmaps(hypervisor,plus_features,
-plus_ext_features,plus_ext2_features,plus_ext3_features);
+plus_ext_features,plus_ext2_features,plus_ext3_features,
+plus_kvm_features);

  featurestr = strtok(NULL, ,);

  while (featurestr) {
  char *val;
  if (featurestr[0] == '+') {
-add_flagname_to_bitmaps(featurestr + 
1,plus_features,plus_ext_features,plus_ext2_features,plus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 
1,plus_features,plus_ext_features,plus_ext2_features,plus_ext3_features,plus_kvm_features);
  } else if (featurestr[0] == '-') {
-add_flagname_to_bitmaps(featurestr + 
1,minus_features,minus_ext_features,minus_ext2_features,minus_ext3_features);
+add_flagname_to_bitmaps(featurestr + 
1,minus_features,minus_ext_features,minus_ext2_features,minus_ext3_features,minus_kvm_features);
  } else if ((val = strchr(featurestr, '='))) {
  *val = 0; val++;
  if (!strcmp(featurestr, family)) {
@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
  x86_cpu_def-ext_features |= plus_ext_features;
  x86_cpu_def-ext2_features |= plus_ext2_features;
  x86_cpu_def-ext3_features |= plus_ext3_features;
+x86_cpu_def-kvm_features |= plus_kvm_features;
  x86_cpu_def-features= ~minus_features;
  x86_cpu_def-ext_features= ~minus_ext_features;
  x86_cpu_def-ext2_features= ~minus_ext2_features;

Re: [Qemu-devel] [PATCH] osdep.c: Fix accept4 fallback


On 01/13/2010 09:20 AM, Kevin Wolf wrote:

Commit 3a03bfa5 added a fallback in case the Linux kernel running qemu is older
than the kernel of the build system. Unfortunately, v1 was committed instead of
v2, so the code has a bug that was revealed in the review (checking for the
wrong error code).

Signed-off-by: Kevin Wolfkw...@redhat.com
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  osdep.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/osdep.c b/osdep.c
index e4836e7..1310684 100644
--- a/osdep.c
+++ b/osdep.c
@@ -297,7 +297,7 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)

  #ifdef CONFIG_ACCEPT4
  ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-if (ret != -1 || errno != EINVAL) {
+if (ret != -1 || errno != ENOSYS) {
  return ret;
  }
  #endif

Re: [Qemu-devel] [PATCH] docs: New qdev-device-use.txt


On 12/17/2009 10:19 AM, Markus Armbruster wrote:

Signed-off-by: Markus Armbrusterarm...@redhat.com
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
I took the liberty to create docs/.  Existing documentation should move
there, but I left that for another day, because I want to get this file
out.

  docs/qdev-device-use.txt |  353 ++
  1 files changed, 353 insertions(+), 0 deletions(-)
  create mode 100644 docs/qdev-device-use.txt

diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
new file mode 100644
index 000..f252c8e
--- /dev/null
+++ b/docs/qdev-device-use.txt
@@ -0,0 +1,353 @@
+= How to convert to -device  friends =
+
+=== Specifying Bus and Address on Bus ===
+
+In qdev, each device has a parent bus.  Some devices provide one or
+more buses for children.  You can specify a device's parent bus with
+-device parameter bus.
+
+A device typically has a device address on its parent bus.  For buses
+where this address can be configured, devices provide a bus-specific
+property.  These are
+
+bus property name   value format
+PCI addr%x.%x (dev.fn, .fn optional)
+I2C address %u
+SCSIscsi-id %u
+
+Example: device i440FX-pcihost is on the root bus, and provides a PCI
+bus named pci.0.  To put a FOO device into its slot 4, use -device
+FOO,bus=/i440FX-pcihost/pci.0,addr=4.  The abbreviated form bus=pci.0
+also works as long as the bus name is unique.
+
+Note: the USB device address can't be controlled at this time.
+
+=== Block Devices ===
+
+A QEMU block device (drive) has a host and a guest part.
+
+In the general case, the guest device is connected to a controller
+device.  For instance, the IDE controller provides two IDE buses, each
+of which can have up to two ide-drive devices, and each ide-drive
+device is a guest part, and is connected to a host part.
+
+Except we sometimes lump controller, bus(es) and drive device(s) all
+together into a single device.  For instance, the ISA floppy
+controller is connected to up to two host drives.
+
+The old ways to define block devices define host and guest part
+together.  Sometimes, they can even define a controller device in
+addition to the block device.
+
+The new way keeps the parts separate: you create the host part with
+-drive, and guest device(s) with -device.
+
+The various old ways to define drives all boil down to the common form
+
+-drive if=TYPE,index=IDX,bus=BUS,unit=UNIT,HOST-OPTS...
+
+TYPE, BUS and UNIT identify the controller device, which of its buses
+to use, and the drive's address on that bus.  Details depend on TYPE.
+IDX is an alternative way to specify BUS and UNIT.
+
+In the new way, this becomes something like
+
+   -drive if=none,id=DRIVE-ID,HOST-OPTS...
+   -device DEVNAME,drive=DRIVE-ID,DEV-OPTS...
+
+The -device argument differs in detail for each kind of drive:
+
+* if=ide
+
+  -device ide-drive,drive=DRIVE-ID,bus=IDE-BUS,unit=UNIT
+
+  where IDE-BUS identifies an IDE bus, normally either ide.0 or ide.1,
+  and UNIT is either 0 or 1.
+
+  Bug: new way does not work for ide.1 unit 0 (in old terms: index=2)
+  unless you disable the default CD-ROM with -nodefaults.
+
+* if=scsi
+
+  The old way implicitly creates SCSI controllers as needed.  The new
+  way makes that explicit:
+
+  -device lsi53c895a,id=ID
+
+  As for all PCI devices, you can add bus=PCI-BUS,addr=DEVFN to
+  control the PCI device address.
+
+  This SCSI controller a single SCSI bus, named ID.0.  Put a disk on
+  it:
+
+  -device scsi-disk,drive=DRIVE-ID,bus=ID.0,scsi-id=SCSI-ID
+
+* if=floppy
+
+  -global isa-fdc,driveA=DRIVE-ID,driveB=DRIVE-ID
+
+  This is -global instead of -device, because the floppy controller is
+  created automatically, and we want to configure that one, not create
+  a second one (which isn't possible anyway).
+
+  Omitting a drive parameter makes that drive empty.
+
+  Bug: driveA works only if you disable the default floppy drive with
+  -nodefaults.
+
+* if=virtio
+
+  -device virtio-blk-pci,drive=DRIVE-ID,class=C,vectors=V
+
+  This lets you control PCI device class and MSI-X vectors.
+
+  As for all PCI devices, you can add bus=PCI-BUS,addr=DEVFN to
+  control the PCI device address.
+
+* if=pflash, if=mtd, if=sd, if=xen are not yet available with -device
+
+For USB devices, the old way is actually different:
+
+-usbdevice disk:format=FMT:FILENAME
+
+Provides much less control than -drive's HOST-OPTS...  The new way
+fixes that:
+
+-device usb-storage,drive=DRIVE-ID
+
+=== Character Devices ===
+
+A QEMU character device has a host and a guest part.
+
+The old ways to define character devices define host and guest part
+together.
+
+The new way keeps the parts separate: you create the host part with
+-chardev, and the guest device with -device.
+
+The various old ways to define a character device are all of the
+general form
+
+-FOO FOO-OPTS...,LEGACY-CHARDEV
+
+where

Re: [Qemu-devel] [PATCH v2] raw-posix: Detect CDROM via ioctl

2010-01-13 Thread Jamie Lokier

Christoph Hellwig wrote:
 On Tue, Jan 12, 2010 at 10:29:11AM -0500, Cole Robinson wrote:
   static int cdrom_probe_device(const char *filename)
   {
  +int fd, ret, prio;
  +
   if (strstart(filename, /dev/cd, NULL))
  -return 100;
  -return 0;
  +prio = 50;
  +
  +fd = open(filename, O_RDONLY | O_NONBLOCK);
  +if (fd  0) {
  +goto out;
  +}
  +
  +/* Attempt to detect via a CDROM specific ioctl */
  +ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
  +if (!(ret  0  errno == EINVAL))
  +prio = 100;
  +
  +close(fd);
  +out:
  +return prio;
   }
 
 Looks good.  We'll now get an open each from the cdrom and floppy
 drivers for each image we're trying to open, but I guess that should be
 fine.

A theoretical problem, that applies to all devices detected in this way:

A few ioctl values (the integers which the names expand to) overlap
between different devices.  It's not policy but it has happened.  This
shows up in strace sometimes, for example on Linux the isatty() C
library function uses TCGETS, but in strace it shows as:

SNDCTL_TMR_TIMEBASE or TCGETS

I tried isatty() on Linux sound devices and it seemed to be reliable anyway :-)

But, in theory, on some host or other, the CD-ROM and floppy checks
could do accidentally do something else on the wrong sort of device.
I don't know how assiduous hosts are about keeping the number spaces separate.

-- Jamie

Re: [Qemu-devel] [PATCH v2] raw-posix: Detect legacy floppy via ioctl

On 01/13/2010 06:07 PM, Anthony Liguori wrote:
 On 01/12/2010 09:29 AM, Cole Robinson wrote:
 Current legacy floppy detection is hardcoded based on source file
 name. Make this smarter by attempting a floppy specific ioctl.

 v2: Give ioctl check higher priority than filename check,
  s/IDE/legacy/

 Signed-off-by: Cole Robinsoncrobi...@redhat.com
 ---
   block/raw-posix.c |   20 ++--
   1 files changed, 18 insertions(+), 2 deletions(-)

 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index b7254d8..d67280e 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -1055,9 +1055,25 @@ static int floppy_open(BlockDriverState *bs,
 const char *filename, int flags)

   static int floppy_probe_device(const char *filename)
   {
 +int fd, ret, prio;
 +struct floppy_struct fdparam;
 +
   if (strstart(filename, /dev/fd, NULL))
 -return 100;
 -return 0;
 +prio = 50;
 +
 +fd = open(filename, O_RDONLY | O_NONBLOCK);
 +if (fd  0) {
 +goto out;
 +}
 +
 +/* Attempt to detect via a floppy specific ioctl */
 +ret = ioctl(fd, FDGETPRM,fdparam);
 +if (!(ret  0  errno == EINVAL))

 
 These two patches break boot from an image file.  My suspicious is that
 it's failing because the errno is ENOSYS.
 

Ugh, sorry about that.

 You probably want to do the opposite and check for a positive return
 result instead of checking for the absence of a positive result.
 

Sounds good.

Thanks,
Cole

[Qemu-devel] [PATCH v3] raw-posix: Detect CDROM via ioctl

Current CDROM detection is hardcoded based on source file name.
Make this smarter by attempting a CDROM specific ioctl.

This makes '-cdrom /dev/sr0' succeed with no media present.

v2:
Give ioctl check higher priority than filename check,

v3:
Actually initialize 'prio' variable
Check for ioctl success rather than absence of specific failure

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 block/raw-posix.c |   20 ++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 5a6a22b..a2c7508 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1140,9 +1140,25 @@ static int cdrom_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 static int cdrom_probe_device(const char *filename)
 {
+int fd, ret;
+int prio = 0;
+
 if (strstart(filename, /dev/cd, NULL))
-return 100;
-return 0;
+prio = 50;
+
+fd = open(filename, O_RDONLY | O_NONBLOCK);
+if (fd  0) {
+goto out;
+}
+
+/* Attempt to detect via a CDROM specific ioctl */
+ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
+if (ret = 0)
+prio = 100;
+
+close(fd);
+out:
+return prio;
 }
 
 static int cdrom_is_inserted(BlockDriverState *bs)
-- 
1.6.5.2

[Qemu-devel] [PATCH v3] raw-posix: Detect legacy floppy via ioctl

Current legacy floppy detection is hardcoded based on source file
name. Make this smarter by attempting a floppy specific ioctl.

v2:
Give ioctl check higher priority than filename check
s/IDE/legacy/

v3:
Actually initialize 'prio' variable
Check for ioctl success rather than absence of specific failure

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 block/raw-posix.c |   21 +++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index a2c7508..eea7e56 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1055,9 +1055,26 @@ static int floppy_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 static int floppy_probe_device(const char *filename)
 {
+int fd, ret;
+int prio = 0;
+struct floppy_struct fdparam;
+
 if (strstart(filename, /dev/fd, NULL))
-return 100;
-return 0;
+prio = 50;
+
+fd = open(filename, O_RDONLY | O_NONBLOCK);
+if (fd  0) {
+goto out;
+}
+
+/* Attempt to detect via a floppy specific ioctl */
+ret = ioctl(fd, FDGETPRM, fdparam);
+if (ret = 0)
+prio = 100;
+
+close(fd);
+out:
+return prio;
 }
 
 
-- 
1.6.5.2

[Qemu-devel] Re: Advise on updating SeaBIOS in stable


On 01/12/2010 10:51 PM, Kevin O'Connor wrote:

On Tue, Jan 12, 2010 at 01:43:47PM -0600, Anthony Liguori wrote:
   

Hi,

I'm ready to cut another qemu stable release and I'm contemplating
whether to update to 0.5.1 in stable.  Generally speaking, we try to
limit stable to bug fixes and changes that aren't user visible.

0.5.1 looks like a point on the master branch as opposed to a
separate branch.  I wonder what the thinking is within SeaBIOS about
what sort of changes will be in the 0.5.x series vs. what would
result in 0.6.0.
 

Hi Anthony,

I didn't have a particular release numbering scheme in mind when I
tagged 0.5.1.  I'd probably lean towards making a v0.5.0.x branch if
we want an update with just critical bug fixes.

However, there have only been a few bug fixes (mostly workarounds for
compiler oddities), though the yield fix (fb214dc7) and ram over 4gig
fix (669c991d) should go in.
   


I actually need the compiler fix to build on my laptop (F12) so I've 
included that too.  Care to take a look at 
git://git.qemu.org/seabios.git stable-0.5.0?  It survives some light 
testing and I'll be doing more thorough testing overnight.


If you want to add some more and/or tag a release, I'll resync again 
before cutting 0.12.2.



If you're looking to pull in 32bit pcibios support, then I don't think
it would be worthwhile to rebase to a stable branch, as the 32bit
pcibios support is easily the biggest user visible change in v0.5.1
(in the sense that Linux will call 32bit pcibios if it's available).
   


Unless there's a strong demand for it, I'd like to hold off on 32bit 
pcibios support.


Thanks,

Anthony Liguori


A couple of other changes could be user visible (eg, mptable), but I
think the risk here is pretty small (assuming we haven't introduced a
regression).

So, I'm okay with a stable branch (eg, v0.5.0.x), but I'm not sure
what you would like to see in that branch.

-Kevin

Re: [Qemu-devel] QMP forward compatibility support

2010-01-13 Thread Jamie Lokier

Markus Armbruster wrote:
 It should be optional if we want to support clients that don't want it.
 I don't think coping with it would be a terrible burden on clients, but
 neither is having to ask for it.  Personally, I'd make it optional.

It wouldn't be a terrible burden, but it'll be easier to write quick
and dirty synchronous clients if they know they don't have to
continuously consume output, to make sure the pipe doesn't fill up,
except immediately after they send a command they wait for the response.

So I'd make async responses optional too.

By quick and dirty, I am talking about one-liner Perl invocations in
shell scripts which control a qemu instance.  Something which is easy
with multi-monitor support :-)

-- Jamie

Re: [Qemu-devel] [PATCH v3] raw-posix: Detect CDROM via ioctl

On 01/13/2010 07:11 PM, malc wrote:
 On Wed, 13 Jan 2010, Cole Robinson wrote:
 
 Current CDROM detection is hardcoded based on source file name.
 Make this smarter by attempting a CDROM specific ioctl.

 This makes '-cdrom /dev/sr0' succeed with no media present.

 v2:
 Give ioctl check higher priority than filename check,

 v3:
 Actually initialize 'prio' variable
 Check for ioctl success rather than absence of specific failure
 
 Does it even compile on BSDs, Darwin etc?
 

The changed functions are all under #ifdef __linux__, so I assume its fine.
Haven't tested though.

- Cole

Re: [Qemu-devel] Static analysis using clang on the x86_64 target