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

2010-01-13 Thread Paolo Bonzini
Since pkgconfig can give different output for different targets,
it should be tried with the cross-compilation prefix first.

Signed-off-by: Paolo Bonzini 
---
 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 
 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 
 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 
 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

2010-01-13 Thread Paolo Bonzini
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

2010-01-13 Thread Paolo Bonzini
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 
---
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

2010-01-13 Thread Paolo Bonzini
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 
---
 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 2>&1; 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 
 #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

2010-01-13 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 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 2>&1; then
+  curlconfig='$pkgconfig libcurl'
+else
+  curlconfig=curl-config
+fi
+
 if test "$curl" != "no" ; then
   cat > $TMPC << EOF
 #include 
 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

2010-01-13 Thread Paolo Bonzini
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

2010-01-13 Thread Paolo Bonzini
Since pkgconfig can give different output for different targets,
it should be tried with the cross-compilation prefix first.

Signed-off-by: Paolo Bonzini 
---
 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 
 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 
 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 
 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

2010-01-13 Thread Paolo Bonzini
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 
---
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

2010-01-13 Thread Paolo Bonzini
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 
---
 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 2>&1; 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 
 #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

2010-01-13 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 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 2>&1; then
+  curlconfig="$pkgconfig libcurl"
+else
+  curlconfig=curl-config
+fi
+
 if test "$curl" != "no" ; then
   cat > $TMPC << EOF
 #include 
 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. Berrange"  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 
> 
> 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

2010-01-13 Thread Michael S. Tsirkin
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

2010-01-13 Thread Michael S. Tsirkin
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. Tsirkin
>> ---
>>   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,
>>   return&vdev->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

2010-01-13 Thread Amit Shah
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

2010-01-13 Thread Amit Shah
clang-analyzer pointed out the value of 'sockets' is never reused.

Signed-off-by: Amit Shah 
CC: Andre Przywara 
---
 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

2010-01-13 Thread Amit Shah
clang-analyzer points out value assigned to 'len' is not used.

Signed-off-by: Amit Shah 
---
 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

2010-01-13 Thread Amit Shah
clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah 
---
 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

2010-01-13 Thread Amit Shah
clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah 
---
 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

2010-01-13 Thread Amit Shah
clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah 
---
 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

2010-01-13 Thread Amit Shah
clang-analyzer points out a redundant increment.

Signed-off-by: Amit Shah 
---
 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

2010-01-13 Thread Michael S. Tsirkin
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. Tsirkin
>> ---
>>   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

2010-01-13 Thread Paolo Bonzini

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

clang-analyzer points out a redundant assignment.

Signed-off-by: Amit Shah
---
  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

2010-01-13 Thread Amit Shah
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 Shah
>> ---
>>   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 Weil

>

Gerd, could you ack this patch please?


Acked-by: Gerd Hoffmann 

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 

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

2010-01-13 Thread Luiz Capitulino
On Tue, 12 Jan 2010 19:57:29 +0100
Markus Armbruster  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/n

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

2010-01-13 Thread Michael S. Tsirkin
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

2010-01-13 Thread Paolo Bonzini
Move 200 lines out of vl.c already into common code that only needs to
be compiled once.

Signed-off-by: Paolo Bonzini 
---
 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 (qem

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

2010-01-13 Thread Michael S. Tsirkin
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 
---
 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_fea

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

2010-01-13 Thread Anthony Liguori

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

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

> On Mon, 11 Jan 2010 18:24:24 -0600
> Anthony Liguori  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

2010-01-13 Thread Anthony Liguori

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

2010-01-13 Thread Luiz Capitulino
On Wed, 13 Jan 2010 17:53:38 +0100
Markus Armbruster  wrote:

> Luiz Capitulino  writes:
> 
> > On Mon, 11 Jan 2010 18:24:24 -0600
> > Anthony Liguori  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  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  writes:

> On Wed, 13 Jan 2010 17:53:38 +0100
> Markus Armbruster  wrote:
>
>> Luiz Capitulino  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

2010-01-13 Thread Luiz Capitulino
On Wed, 13 Jan 2010 18:38:57 +0100
Markus Armbruster  wrote:

> Luiz Capitulino  writes:
> 
> > On Wed, 13 Jan 2010 17:53:38 +0100
> > Markus Armbruster  wrote:
> >
> >> Luiz Capitulino  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)

2010-01-13 Thread Luiz Capitulino
On Mon, 11 Jan 2010 13:09:35 -0600
Adam Litke  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 
> @@ -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

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

2010-01-13 Thread Anthony Liguori

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

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

2010-01-13 Thread Anthony Liguori

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  wrote:
>
> On 12.01.2010, at 21:52, Blue Swirl wrote:
>
>> On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf  wrote:
>>>
>>> On 12.01.2010, at 20:45, Blue Swirl wrote:
>>>
 On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf  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!



> > +/*
> > + * 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.


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



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

2010-01-13 Thread Luiz Capitulino
On Wed, 13 Jan 2010 12:59:25 -0600
Adam Litke  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  wrote:
> On (Tue) Jan 12 2010 [19:35:08], Blue Swirl wrote:
>> On Tue, Jan 12, 2010 at 6:13 PM, Amit Shah  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  wrote:
>> 
>> On 12.01.2010, at 21:52, Blue Swirl wrote:
>> 
>>> On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf  wrote:
 
 On 12.01.2010, at 20:45, Blue Swirl wrote:
 
> On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf  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 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


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  wrote:
>
> On 13.01.2010, at 19:47, Blue Swirl wrote:
>
>> On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf  wrote:
>>>
>>> On 12.01.2010, at 21:52, Blue Swirl wrote:
>>>
 On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf  wrote:
>
> On 12.01.2010, at 20:45, Blue Swirl wrote:
>
>> On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf  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  wrote:
>> 
>> On 13.01.2010, at 19:47, Blue Swirl wrote:
>> 
>>> On Tue, Jan 12, 2010 at 10:11 PM, Alexander Graf  wrote:
 
 On 12.01.2010, at 21:52, Blue Swirl wrote:
 
> On Tue, Jan 12, 2010 at 8:34 PM, Alexander Graf  wrote:
>> 
>> On 12.01.2010, at 20:45, Blue Swirl wrote:
>> 
>>> On Tue, Jan 12, 2010 at 11:58 AM, Alexander Graf  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

2010-01-13 Thread Michael S. Tsirkin
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 

Acked-by: Michael S. Tsirkin 

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

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 
---
 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 (1<<31)
 /* 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

2010-01-13 Thread Anthony Liguori

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

2010-01-13 Thread Anthony Liguori

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 Horsten
Signed-off-by: Milan Plzik

   


Applied.  Thanks.

Regards,

Anthony Liguori




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

2010-01-13 Thread Anthony Liguori

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 Wolf
   


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

2010-01-13 Thread Anthony Liguori

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 Hellwig
   


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 v3 1/4] use cross-prefix for pkgconfig

2010-01-13 Thread Anthony Liguori

On 01/13/2010 02:52 AM, Paolo Bonzini wrote:

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

Signed-off-by: Paolo Bonzini
   


Applied all.  Thanks.

Regards,

Anthony Liguori


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






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

2010-01-13 Thread Anthony Liguori

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 Shah
CC: Andre Przywara
   


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

2010-01-13 Thread Anthony Liguori

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 Hellwig
   


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

2010-01-13 Thread Anthony Liguori

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 Bonzini
   


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 =
+qemu_put_mouse_event_current-

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

2010-01-13 Thread Anthony Liguori

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 Natapov
   


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

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

2010-01-13 Thread Anthony Liguori

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 Wolf
   


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

2010-01-13 Thread Anthony Liguori

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

Signed-off-by: Markus Armbruster
   


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 FOO-OPTS... is specific t

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

2010-01-13 Thread Cole Robinson
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 Robinson
>> ---
>>   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

2010-01-13 Thread Cole Robinson
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 
---
 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

2010-01-13 Thread Cole Robinson
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 
---
 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

2010-01-13 Thread Anthony Liguori

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

2010-01-13 Thread malc
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?

[..snip..]

-- 
mailto:av1...@comtv.ru




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

2010-01-13 Thread Cole Robinson
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

2010-01-13 Thread Amit Shah
On (Wed) Jan 13 2010 [19:08:11], Blue Swirl wrote:
> 
> Thanks. I fixed the warnings related to Sparc32. Were there really no
> new warnings for Sparc64?

Looks like it; vl.c gets reported three times at the same locations so 3
arches have been compiled.

My test machine is down ATM, I can confirm later when it's up.

BTW for the patch

commit 884a0c7677cf8431d2a632673914994c2e01673d

pcnet: remove dead nested assignment, spotted by clang

diff --git a/hw/pcnet.c b/hw/pcnet.c
index 91d106d..44b5b31 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1608,7 +1608,7 @@ static void pcnet_aprom_writeb(void *opaque,
uint32_t addr, uint32_t val)
 static uint32_t pcnet_aprom_readb(void *opaque, uint32_t addr)
 {
 PCNetState *s = opaque;
-uint32_t val = s->prom[addr &= 15];
+uint32_t val = s->prom[addr & 15];
 #ifdef PCNET_DEBUG
 printf("pcnet_aprom_readb addr=0x%08x val=0x%02x\n", addr, val);
 #endif


if debugging is enabled, addr will now print a different value than
earlier.

Amit




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

2010-01-13 Thread Aurelien Jarno
On Wed, Jan 13, 2010 at 05:58:35PM -0600, Anthony Liguori wrote:
> 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.
>

I would really like to see either that, or support for bochsbios again.
Hurd is not able to boot correctly without 32bit pcibios support, and I 
fear it will be the case of other OSes.

Also 085debd93f52d36381ea13ef27e7f72e87fe62f5 could be interesting in a
new stable release, this fix comes from a problem detected on an image 
that was working with 0.11.x.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net