[Qemu-devel] [PATCH 14/16] vnc: threaded VNC server

2010-06-15 Thread Corentin Chary
Implement a threaded VNC server using the producer-consumer model.
The main thread will push encoding jobs (a list a rectangles to update)
in a queue, and the VNC worker thread will consume that queue and send
framebuffer updates to the output buffer.

The threaded VNC server can be enabled with ./configure --enable-vnc-thread.

If you don't want it, just use ./configure --disable-vnc-thread and a 
syncrhonous
queue of job will be used (which as exactly the same behavior as the old queue).
If you disable the VNC thread, all thread related code will not be built and 
there will
be no overhead.

Signed-off-by: Corentin Chary 
---
 Makefile.objs   |7 +-
 configure   |   13 ++
 ui/vnc-jobs-async.c |  331 +++
 ui/vnc-jobs-sync.c  |   73 +++
 ui/vnc-jobs.h   |   87 ++
 ui/vnc.c|  144 +++
 ui/vnc.h|   53 -
 7 files changed, 682 insertions(+), 26 deletions(-)
 create mode 100644 ui/vnc-jobs-async.c
 create mode 100644 ui/vnc-jobs-sync.c
 create mode 100644 ui/vnc-jobs.h

diff --git a/Makefile.objs b/Makefile.objs
index 0ee9346..866ddf1 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -111,10 +111,15 @@ ui-obj-y += vnc-enc-tight.o vnc-palette.o
 ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
 ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
 ui-obj-$(CONFIG_COCOA) += cocoa.o
+ifdef CONFIG_VNC_THREAD
+ui-obj-y += vnc-jobs-async.o
+else
+ui-obj-y += vnc-jobs-sync.o
+endif
 common-obj-y += $(addprefix ui/, $(ui-obj-y))
 
 common-obj-y += iov.o acl.o
-common-obj-$(CONFIG_IOTHREAD) += qemu-thread.o
+common-obj-$(CONFIG_THREAD) += qemu-thread.o
 common-obj-y += notify.o event_notifier.o
 common-obj-y += qemu-timer.o
 
diff --git a/configure b/configure
index d4c34f6..f5fdb8f 100755
--- a/configure
+++ b/configure
@@ -270,6 +270,7 @@ vnc_tls=""
 vnc_sasl=""
 vnc_jpeg=""
 vnc_png=""
+vnc_thread=""
 xen=""
 linux_aio=""
 vhost_net=""
@@ -584,6 +585,10 @@ for opt do
   ;;
   --enable-vnc-png) vnc_png="yes"
   ;;
+  --disable-vnc-thread) vnc_thread="no"
+  ;;
+  --enable-vnc-thread) vnc_thread="yes"
+  ;;
   --disable-slirp) slirp="no"
   ;;
   --disable-uuid) uuid="no"
@@ -832,6 +837,8 @@ echo "  --disable-vnc-jpeg   disable JPEG lossy 
compression for VNC server"
 echo "  --enable-vnc-jpegenable JPEG lossy compression for VNC server"
 echo "  --disable-vnc-pngdisable PNG compression for VNC server"
 echo "  --enable-vnc-png enable PNG compression for VNC server"
+echo "  --disable-vnc-thread disable threaded VNC server"
+echo "  --enable-vnc-thread  enable threaded VNC server"
 echo "  --disable-curses disable curses output"
 echo "  --enable-curses  enable curses output"
 echo "  --disable-curl   disable curl connectivity"
@@ -2126,6 +2133,7 @@ echo "VNC TLS support   $vnc_tls"
 echo "VNC SASL support  $vnc_sasl"
 echo "VNC JPEG support  $vnc_jpeg"
 echo "VNC PNG support   $vnc_png"
+echo "VNC thread$vnc_thread"
 if test -n "$sparc_cpu"; then
 echo "Target Sparc Arch $sparc_cpu"
 fi
@@ -2270,6 +2278,10 @@ if test "$vnc_png" = "yes" ; then
   echo "CONFIG_VNC_PNG=y" >> $config_host_mak
   echo "VNC_PNG_CFLAGS=$vnc_png_cflags" >> $config_host_mak
 fi
+if test "$vnc_thread" = "yes" ; then
+  echo "CONFIG_VNC_THREAD=y" >> $config_host_mak
+  echo "CONFIG_THREAD=y" >> $config_host_mak
+fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
 fi
@@ -2346,6 +2358,7 @@ if test "$xen" = "yes" ; then
 fi
 if test "$io_thread" = "yes" ; then
   echo "CONFIG_IOTHREAD=y" >> $config_host_mak
+  echo "CONFIG_THREAD=y" >> $config_host_mak
 fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
new file mode 100644
index 000..6e9cf08
--- /dev/null
+++ b/ui/vnc-jobs-async.c
@@ -0,0 +1,331 @@
+/*
+ * QEMU VNC display driver
+ *
+ * Copyright (C) 2006 Anthony Liguori 
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2009 Red Hat, Inc
+ * Copyright (C) 2010 Corentin Chary 
+ *
+ * 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
+ * TH

[Qemu-devel] [PATCH 16/16] vnc: tight: don't limit png rect size

2010-06-15 Thread Corentin Chary
PNG was introduced because some vnc HTML5 clients like noVNC
have slow zlib decoding, but really fast PNG rendering. This
means that if PNG is enabled we should send only PNG (and JPEG, fill),
and never something compressed directly with zlib.

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-tight.c |4 
 ui/vnc-enc-tight.h |1 -
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 1d926c9..e171074 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -97,10 +97,6 @@ static bool tight_can_send_png_rect(VncState *vs, int w, int 
h)
 vs->clientds.pf.bytes_per_pixel == 1) {
 return false;
 }
-
-if (w * h < VNC_TIGHT_PNG_MIN_RECT_SIZE) {
-return false;
-}
 return true;
 }
 #endif
diff --git a/ui/vnc-enc-tight.h b/ui/vnc-enc-tight.h
index dc7150a..a3add78 100644
--- a/ui/vnc-enc-tight.h
+++ b/ui/vnc-enc-tight.h
@@ -176,7 +176,6 @@
 #define VNC_TIGHT_MAX_SPLIT_TILE_SIZE   16
 
 #define VNC_TIGHT_JPEG_MIN_RECT_SIZE  4096
-#define VNC_TIGHT_PNG_MIN_RECT_SIZE   4096
 #define VNC_TIGHT_DETECT_SUBROW_WIDTH7
 #define VNC_TIGHT_DETECT_MIN_WIDTH   8
 #define VNC_TIGHT_DETECT_MIN_HEIGHT  8
-- 
1.7.1




[Qemu-devel] [PATCH 12/16] vnc: fix tight png memory leak

2010-06-15 Thread Corentin Chary
The tight.png buffer was never released.

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-tight.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index dbde08d..1d926c9 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1659,4 +1659,7 @@ void vnc_tight_clear(VncState *vs)
 #ifdef CONFIG_VNC_JPEG
 buffer_free(&vs->tight.jpeg);
 #endif
+#ifdef CONFIG_VNC_PNG
+buffer_free(&vs->tight.png);
+#endif
 }
-- 
1.7.1




[Qemu-devel] [Bug 594944] [NEW] --enable-debug error

2010-06-15 Thread htmldevelo...@gmail.com
Public bug reported:


This bug was already reported in 0.12.3, and in the present 0.12.4 is still 
present:

  CCsparc-softmmu/disas.o
  CCsparc64-softmmu/ide/qdev.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function 
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error: 
incompatible type for argument 3 of ‘tcg_gen_add_i32’
  CCsparc64-softmmu/ide/pci.o
  CCsparc-softmmu/i386-dis.o
make[1]: *** [translate.o] Error 1
make: *** [subdir-sparc64-linux-user] Error 2
make: *** Waiting for unfinished jobs
  CCsparc64-softmmu/ide/cmd646.o
  CCsparc64-softmmu/vga.o
  CCsparc64-softmmu/vga-pci.o
  CCsparc64-softmmu/fdc.o
  CCsparc64-softmmu/mc146818rtc.o
  CCsparc-softmmu/sparc-dis.o
  CCsparc64-softmmu/serial.o
  CCsparc64-softmmu/cirrus_vga.o
  CCsparc64-softmmu/parallel.o
  CCsparc64-softmmu/exec.o
  CCsparc64-softmmu/translate-all.o
  ARsparc-softmmu/libqemu.a
  LINK  sparc-softmmu/qemu-system-sparc
  CCsparc64-softmmu/cpu-exec.o
  CCsparc64-softmmu/translate.o
  CCsparc64-softmmu/tcg/tcg.o
  CCsparc64-softmmu/fpu/softfloat.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function 
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error: 
incompatible type for argument 3 of ‘tcg_gen_add_i32’
make[1]: *** [translate.o] Error 1
make[1]: *** Waiting for unfinished jobs
make: *** [subdir-sparc64-softmmu] Error 2

The following patch seemed to work:

diff -Nurp target-sparc/translate.c.orig  target-sparc/translate.c
--- target-sparc/translate.c.orig   2010-06-16 13:58:26.395527708 +0800
+++ target-sparc/translate.c2010-06-16 14:09:18.683573175 +0800
@@ -1663,27 +1663,28 @@ static inline TCGv get_src2(unsigned int
 #ifdef TARGET_SPARC64
 static inline void gen_load_trap_state_at_tl(TCGv_ptr r_tsptr, TCGv_ptr 
cpu_env)
 {
-TCGv r_tl = tcg_temp_new();
+TCGv_i32 r_tl = tcg_temp_new_i32();
 
 /* load env->tl into r_tl */
-{
-TCGv_i32 r_tl_tmp = tcg_temp_new_i32();
-tcg_gen_ld_i32(r_tl_tmp, cpu_env, offsetof(CPUSPARCState, tl));
-tcg_gen_ext_i32_tl(r_tl, r_tl_tmp);
-tcg_temp_free_i32(r_tl_tmp);
-}
+
+tcg_gen_ld_i32(r_tl, cpu_env, offsetof(CPUSPARCState, tl));
 
 /* tl = [0 ... MAXTL_MASK] where MAXTL_MASK must be power of 2 */
-tcg_gen_andi_tl(r_tl, r_tl, MAXTL_MASK);
+tcg_gen_andi_i32(r_tl, r_tl, MAXTL_MASK);
 
 /* calculate offset to current trap state from env->ts, reuse r_tl */
-tcg_gen_muli_tl(r_tl, r_tl, sizeof (trap_state));
+tcg_gen_muli_i32(r_tl, r_tl, sizeof (trap_state));
 tcg_gen_addi_ptr(r_tsptr, cpu_env, offsetof(CPUState, ts));
 
 /* tsptr = env->ts[env->tl & MAXTL_MASK] */
-tcg_gen_add_ptr(r_tsptr, r_tsptr, r_tl);
+{
+TCGv_ptr r_tl_tmp = tcg_temp_new_ptr();
+tcg_gen_ext_i32_ptr(r_tl_tmp, r_tl);
+tcg_gen_add_ptr(r_tsptr, r_tsptr, r_tl_tmp);
+tcg_temp_free_i32(r_tl_tmp);
+}
 
-tcg_temp_free(r_tl);
+tcg_temp_free_i32(r_tl);
 }
 #endif
 
It is following the previous bug fixs by Jay Foad:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg25585.html

Not sure if it is correct?

** Affects: qemu
 Importance: Undecided
 Status: New

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

Status in QEMU: New

Bug description:

This bug was already reported in 0.12.3, and in the present 0.12.4 is still 
present:

  CCsparc-softmmu/disas.o
  CCsparc64-softmmu/ide/qdev.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function 
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error: 
incompatible type for argument 3 of ‘tcg_gen_add_i32’
  CCsparc64-softmmu/ide/pci.o
  CCsparc-softmmu/i386-dis.o
make[1]: *** [translate.o] Error 1
make: *** [subdir-sparc64-linux-user] Error 2
make: *** Waiting for unfinished jobs
  CCsparc64-softmmu/ide/cmd646.o
  CCsparc64-softmmu/vga.o
  CCsparc64-softmmu/vga-pci.o
  CCsparc64-softmmu/fdc.o
  CCsparc64-softmmu/mc146818rtc.o
  CCsparc-softmmu/sparc-dis.o
  CCsparc64-softmmu/serial.o
  CCsparc64-softmmu/cirrus_vga.o
  CCsparc64-softmmu/parallel.o
  CCsparc64-softmmu/exec.o
  CCsparc64-softmmu/translate-all.o
  ARsparc-softmmu/libqemu.a
  LINK  sparc-softmmu/qemu-system-sparc
  CCsparc64-softmmu/cpu-exec.o
  CCsparc64-softmmu/translate.o
  CCsparc64-softmmu/tcg/tcg.o
  CCsparc64-softmmu/fpu/softfloat.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function 
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error: 
incompatible type for argument 3 of ‘tcg_gen_add_i32’
make[1]: *** [translate.o] Error 1
make[1]: ***

[Qemu-devel] [PATCH 11/16] vnc: encapsulate encoding members

2010-06-15 Thread Corentin Chary
This will allow to implement the threaded VNC server in a
more cleaner way.

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-hextile.c |   14 ++--
 ui/vnc-enc-tight.c   |  194 +-
 ui/vnc-enc-zlib.c|   34 +-
 ui/vnc.c |8 +-
 ui/vnc.h |   58 ---
 5 files changed, 157 insertions(+), 151 deletions(-)

diff --git a/ui/vnc-enc-hextile.c b/ui/vnc-enc-hextile.c
index fa4b264..364a491 100644
--- a/ui/vnc-enc-hextile.c
+++ b/ui/vnc-enc-hextile.c
@@ -75,7 +75,7 @@ int vnc_hextile_send_framebuffer_update(VncState *vs, int x,
 has_fg = has_bg = 0;
 for (j = y; j < (y + h); j += 16) {
 for (i = x; i < (x + w); i += 16) {
-vs->send_hextile_tile(vs, i, j,
+vs->hextile.send_tile(vs, i, j,
   MIN(16, x + w - i), MIN(16, y + h - j),
   last_bg, last_fg, &has_bg, &has_fg);
 }
@@ -91,25 +91,25 @@ void vnc_hextile_set_pixel_conversion(VncState *vs, int 
generic)
 if (!generic) {
 switch (vs->ds->surface->pf.bits_per_pixel) {
 case 8:
-vs->send_hextile_tile = send_hextile_tile_8;
+vs->hextile.send_tile = send_hextile_tile_8;
 break;
 case 16:
-vs->send_hextile_tile = send_hextile_tile_16;
+vs->hextile.send_tile = send_hextile_tile_16;
 break;
 case 32:
-vs->send_hextile_tile = send_hextile_tile_32;
+vs->hextile.send_tile = send_hextile_tile_32;
 break;
 }
 } else {
 switch (vs->ds->surface->pf.bits_per_pixel) {
 case 8:
-vs->send_hextile_tile = send_hextile_tile_generic_8;
+vs->hextile.send_tile = send_hextile_tile_generic_8;
 break;
 case 16:
-vs->send_hextile_tile = send_hextile_tile_generic_16;
+vs->hextile.send_tile = send_hextile_tile_generic_16;
 break;
 case 32:
-vs->send_hextile_tile = send_hextile_tile_generic_32;
+vs->hextile.send_tile = send_hextile_tile_generic_32;
 break;
 }
 }
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 870e215..dbde08d 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -120,7 +120,7 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
 int pixels = 0;
 int pix, left[3];
 uint errors;
-unsigned char *buf = vs->tight.buffer;
+unsigned char *buf = vs->tight.tight.buffer;
 
 /*
  * If client is big-endian, color samples begin from the second
@@ -187,7 +187,7 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
 int pixels = 0; \
 int sample, sum, left[3];   \
 uint errors;\
-unsigned char *buf = vs->tight.buffer;  \
+unsigned char *buf = vs->tight.tight.buffer;\
 \
 endian = ((vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG) !=\
   (vs->ds->surface->flags & QEMU_BIG_ENDIAN_FLAG)); \
@@ -267,8 +267,8 @@ static int
 tight_detect_smooth_image(VncState *vs, int w, int h)
 {
 uint errors;
-int compression = vs->tight_compression;
-int quality = vs->tight_quality;
+int compression = vs->tight.compression;
+int quality = vs->tight.quality;
 
 if (!vs->vd->lossy) {
 return 0;
@@ -280,7 +280,7 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
 return 0;
 }
 
-if (vs->tight_quality != -1) {
+if (vs->tight.quality != -1) {
 if (w * h < VNC_TIGHT_JPEG_MIN_RECT_SIZE) {
 return 0;
 }
@@ -291,9 +291,9 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
 }
 
 if (vs->clientds.pf.bytes_per_pixel == 4) {
-if (vs->tight_pixel24) {
+if (vs->tight.pixel24) {
 errors = tight_detect_smooth_image24(vs, w, h);
-if (vs->tight_quality != -1) {
+if (vs->tight.quality != -1) {
 return (errors < tight_conf[quality].jpeg_threshold24);
 }
 return (errors < tight_conf[compression].gradient_threshold24);
@@ -323,7 +323,7 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
 uint##bpp##_t c0, c1, ci;   \
 int i, n0, n1;  \
 \
-data = (uint##bpp##_t *)vs->tight.buffer;   \
+data = (uint##bpp##_t *)vs->tight.tight.buffer; \

[Qemu-devel] [PATCH 10/16] vnc: tight: stop using qdict for palette stuff

2010-06-15 Thread Corentin Chary
Profiling with callgrind seems to show that a lot of time is spent
in the palette code (mostly due to memory allocation and qdict to int
conversion).

This patch adds a VncPalette implementation. The palette is stored
in a hash table, like qdict, but which does way less memory allocations,
and doesn't suffer from the QObject overhead.

Signed-off-by: Corentin Chary 
---
 Makefile.objs  |2 +-
 ui/vnc-enc-tight.c |  163 +++
 ui/vnc-palette.c   |  136 +++
 ui/vnc-palette.h   |   63 
 4 files changed, 235 insertions(+), 129 deletions(-)
 create mode 100644 ui/vnc-palette.c
 create mode 100644 ui/vnc-palette.h

diff --git a/Makefile.objs b/Makefile.objs
index e63c5eb..0ee9346 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -107,7 +107,7 @@ ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
 ui-obj-$(CONFIG_CURSES) += curses.o
 ui-obj-y += vnc.o d3des.o
 ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o
-ui-obj-y += vnc-enc-tight.o
+ui-obj-y += vnc-enc-tight.o vnc-palette.o
 ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
 ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
 ui-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9e1f214..870e215 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -39,10 +39,10 @@
 #include "qemu-common.h"
 
 #include "bswap.h"
-#include "qdict.h"
 #include "qint.h"
 #include "vnc.h"
 #include "vnc-enc-tight.h"
+#include "vnc-palette.h"
 
 /* Compression level stuff. The following array contains various
encoder parameters for each of 10 compression levels (0..9).
@@ -85,7 +85,7 @@ static const struct {
 };
 
 static int send_png_rect(VncState *vs, int x, int y, int w, int h,
- QDict *palette);
+ VncPalette *palette);
 
 static bool tight_can_send_png_rect(VncState *vs, int w, int h)
 {
@@ -312,74 +312,13 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
 /*
  * Code to determine how many different colors used in rectangle.
  */
-
-static void tight_palette_rgb2buf(uint32_t rgb, int bpp, uint8_t buf[6])
-{
-memset(buf, 0, 6);
-
-if (bpp == 32) {
-buf[0] = ((rgb >> 24) & 0xFF);
-buf[1] = ((rgb >> 16) & 0xFF);
-buf[2] = ((rgb >>  8) & 0xFF);
-buf[3] = ((rgb >>  0) & 0xFF);
-buf[4] = ((buf[0] & 1) == 0) << 3 | ((buf[1] & 1) == 0) << 2;
-buf[4]|= ((buf[2] & 1) == 0) << 1 | ((buf[3] & 1) == 0) << 0;
-buf[0] |= 1;
-buf[1] |= 1;
-buf[2] |= 1;
-buf[3] |= 1;
-}
-if (bpp == 16) {
-buf[0] = ((rgb >> 8) & 0xFF);
-buf[1] = ((rgb >> 0) & 0xFF);
-buf[2] = ((buf[0] & 1) == 0) << 1 | ((buf[1] & 1) == 0) << 0;
-buf[0] |= 1;
-buf[1] |= 1;
-}
-}
-
-static uint32_t tight_palette_buf2rgb(int bpp, const uint8_t *buf)
-{
-uint32_t rgb = 0;
-
-if (bpp == 32) {
-rgb |= ((buf[0] & ~1) | !((buf[4] >> 3) & 1)) << 24;
-rgb |= ((buf[1] & ~1) | !((buf[4] >> 2) & 1)) << 16;
-rgb |= ((buf[2] & ~1) | !((buf[4] >> 1) & 1)) <<  8;
-rgb |= ((buf[3] & ~1) | !((buf[4] >> 0) & 1)) <<  0;
-}
-if (bpp == 16) {
-rgb |= ((buf[0] & ~1) | !((buf[2] >> 1) & 1)) << 8;
-rgb |= ((buf[1] & ~1) | !((buf[2] >> 0) & 1)) << 0;
-}
-return rgb;
-}
-
-
-static int tight_palette_insert(QDict *palette, uint32_t rgb, int bpp, int max)
-{
-uint8_t key[6];
-int idx = qdict_size(palette);
-bool present;
-
-tight_palette_rgb2buf(rgb, bpp, key);
-present = qdict_haskey(palette, (char *)key);
-if (idx >= max && !present) {
-return 0;
-}
-if (!present) {
-qdict_put(palette, (char *)key, qint_from_int(idx));
-}
-return qdict_size(palette);
-}
-
 #define DEFINE_FILL_PALETTE_FUNCTION(bpp)   \
 \
 static int  \
 tight_fill_palette##bpp(VncState *vs, int x, int y, \
 int max, size_t count,  \
 uint32_t *bg, uint32_t *fg, \
-struct QDict **palette) {   \
+VncPalette **palette) { \
 uint##bpp##_t *data;\
 uint##bpp##_t c0, c1, ci;   \
 int i, n0, n1;  \
@@ -426,24 +365,23 @@ static int tight_palette_insert(QDict *palette, uint32_t 
rgb, int bpp, int max)
 return 0;   \
 }   \

[Qemu-devel] [PATCH 15/16] vnc: add missing lock for vnc_cursor_define()

2010-06-15 Thread Corentin Chary
All vnc_write() calls must be locked (except the ones present before
the protocol initialization).

Signed-off-by: Corentin Chary 
---
 ui/vnc.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index cffe238..61f9a05 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -794,6 +794,7 @@ static int vnc_cursor_define(VncState *vs)
 int isize;
 
 if (vnc_has_feature(vs, VNC_FEATURE_RICH_CURSOR)) {
+vnc_lock_output(vs);
 vnc_write_u8(vs,  VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
 vnc_write_u8(vs,  0);  /*  padding */
 vnc_write_u16(vs, 1);  /*  # of rects  */
@@ -802,6 +803,7 @@ static int vnc_cursor_define(VncState *vs)
 isize = c->width * c->height * vs->clientds.pf.bytes_per_pixel;
 vnc_write_pixels_generic(vs, &pf, c->data, isize);
 vnc_write(vs, vs->vd->cursor_mask, vs->vd->cursor_msize);
+vnc_unlock_output(vs);
 return 0;
 }
 return -1;
-- 
1.7.1




[Qemu-devel] [PATCH 08/16] vnc: tight add PNG encoding

2010-06-15 Thread Corentin Chary
Introduce a new encoding: VNC_ENCODING_TIGHT_PNG (0x17) and a new
tight filter VNC_TIGHT_PNG (0x0A). When the client tells it supports the 0x17
encoding, the server will use tight, but will always send encoding pixels using
PNG instead of zlib. If the client also told it support JPEG, then the server 
can
send JPEG, because PNG will only be used in the cases zlib was used in normal 
tight.

This encoding was introduced to speed up HTML5 based VNC clients like noVNC 
[1], but
can also be used on devices like iPhone where PNG can be rendered in hardware.

[1] http://github.com/kanaka/noVNC/

Signed-off-by: Corentin Chary 
---
 Makefile.target|1 +
 configure  |   37 +++
 ui/vnc-enc-tight.c |  277 ++--
 ui/vnc-enc-tight.h |   17 ++--
 ui/vnc.c   |9 +-
 ui/vnc.h   |   11 ++
 6 files changed, 311 insertions(+), 41 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index d9e888a..a3ea025 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -178,6 +178,7 @@ LIBS+=-lz
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
 QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
+QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 
 # xen backend driver support
 obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
diff --git a/configure b/configure
index 304b4b0..d4c34f6 100755
--- a/configure
+++ b/configure
@@ -269,6 +269,7 @@ vde=""
 vnc_tls=""
 vnc_sasl=""
 vnc_jpeg=""
+vnc_png=""
 xen=""
 linux_aio=""
 vhost_net=""
@@ -579,6 +580,10 @@ for opt do
   ;;
   --enable-vnc-jpeg) vnc_jpeg="yes"
   ;;
+  --disable-vnc-png) vnc_png="no"
+  ;;
+  --enable-vnc-png) vnc_png="yes"
+  ;;
   --disable-slirp) slirp="no"
   ;;
   --disable-uuid) uuid="no"
@@ -825,6 +830,8 @@ echo "  --disable-vnc-sasl   disable SASL encryption 
for VNC server"
 echo "  --enable-vnc-saslenable SASL encryption for VNC server"
 echo "  --disable-vnc-jpeg   disable JPEG lossy compression for VNC server"
 echo "  --enable-vnc-jpegenable JPEG lossy compression for VNC server"
+echo "  --disable-vnc-pngdisable PNG compression for VNC server"
+echo "  --enable-vnc-png enable PNG compression for VNC server"
 echo "  --disable-curses disable curses output"
 echo "  --enable-curses  enable curses output"
 echo "  --disable-curl   disable curl connectivity"
@@ -1265,6 +1272,31 @@ EOF
 fi
 
 ##
+# VNC PNG detection
+if test "$vnc_png" = "yes" ; then
+cat > $TMPC <
+#include 
+int main(void) {
+png_structp png_ptr;
+png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
+return 0;
+}
+EOF
+vnc_png_cflags=""
+vnc_png_libs="-lpng"
+  if compile_prog "$vnc_png_cflags" "$vnc_png_libs" ; then
+vnc_png=yes
+libs_softmmu="$vnc_png_libs $libs_softmmu"
+  else
+if test "$vnc_png" = "yes" ; then
+  feature_not_found "vnc-png"
+fi
+vnc_png=no
+  fi
+fi
+
+##
 # fnmatch() probe, used for ACL routines
 fnmatch="no"
 cat > $TMPC << EOF
@@ -2093,6 +2125,7 @@ echo "Mixer emulation   $mixemu"
 echo "VNC TLS support   $vnc_tls"
 echo "VNC SASL support  $vnc_sasl"
 echo "VNC JPEG support  $vnc_jpeg"
+echo "VNC PNG support   $vnc_png"
 if test -n "$sparc_cpu"; then
 echo "Target Sparc Arch $sparc_cpu"
 fi
@@ -2233,6 +2266,10 @@ if test "$vnc_jpeg" = "yes" ; then
   echo "CONFIG_VNC_JPEG=y" >> $config_host_mak
   echo "VNC_JPEG_CFLAGS=$vnc_jpeg_cflags" >> $config_host_mak
 fi
+if test "$vnc_png" = "yes" ; then
+  echo "CONFIG_VNC_PNG=y" >> $config_host_mak
+  echo "VNC_PNG_CFLAGS=$vnc_png_cflags" >> $config_host_mak
+fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
 fi
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 4ff88a8..007d88f 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -26,13 +26,18 @@
  * THE SOFTWARE.
  */
 
-#include "qemu-common.h"
+#include "config-host.h"
 
+#ifdef CONFIG_VNC_PNG
+#include 
+#endif
 #ifdef CONFIG_VNC_JPEG
 #include 
 #include 
 #endif
 
+#include "qemu-common.h"
+
 #include "bswap.h"
 #include "qdict.h"
 #include "qint.h"
@@ -63,6 +68,28 @@ static const struct {
 { 65536, 2048,  32,  8192, 9, 9, 9, 6, 200, 500,  96, 80,   200,   500 }
 };
 
+#ifdef CONFIG_VNC_PNG
+static int send_png_rect(VncState *vs, int x, int y, int w, int h,
+ QDict *palette);
+
+static bool tight_can_send_png_rect(VncState *vs, int w, int h)
+{
+if (!vnc_has_feature(vs, VNC_FEATURE_TIGHT_PNG)) {
+return false;
+}
+
+if (ds_get_bytes_per_pixel(vs->ds) == 1 ||
+vs->clientds.pf.bytes_per_pixel == 1) {
+return false;
+}
+
+if (w * h < VNC_TIGHT_PNG_MIN_RECT_SIZE) {
+return false;
+}
+return true;
+}
+#endif
+
 /*
  * Code to guess if given rectangle is suitable for smooth image
  * compression (by applying "gradient" filter or JPEG coder).
@@ -466,

[Qemu-devel] [PATCH 13/16] qemu-thread: add qemu_mutex/cond_destroy and qemu_mutex_exit

2010-06-15 Thread Corentin Chary
Add some missing functions in qemu-thread. Currently qemu-thread
is only used for io-thread but it will used by the vnc server soon
and we need those functions instead of calling pthread directly.

Signed-off-by: Corentin Chary 
---
 qemu-thread.c |   22 ++
 qemu-thread.h |4 
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/qemu-thread.c b/qemu-thread.c
index faf4061..fbc78fe 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -34,6 +34,15 @@ void qemu_mutex_init(QemuMutex *mutex)
 error_exit(err, __func__);
 }
 
+void qemu_mutex_destroy(QemuMutex *mutex)
+{
+int err;
+
+err = pthread_mutex_destroy(&mutex->lock);
+if (err)
+error_exit(err, __func__);
+}
+
 void qemu_mutex_lock(QemuMutex *mutex)
 {
 int err;
@@ -90,6 +99,15 @@ void qemu_cond_init(QemuCond *cond)
 error_exit(err, __func__);
 }
 
+void qemu_cond_destroy(QemuCond *cond)
+{
+int err;
+
+err = pthread_cond_destroy(&cond->cond);
+if (err)
+error_exit(err, __func__);
+}
+
 void qemu_cond_signal(QemuCond *cond)
 {
 int err;
@@ -168,3 +186,7 @@ int qemu_thread_equal(QemuThread *thread1, QemuThread 
*thread2)
return pthread_equal(thread1->thread, thread2->thread);
 }
 
+void qemu_thread_exit(void *retval)
+{
+pthread_exit(retval);
+}
diff --git a/qemu-thread.h b/qemu-thread.h
index 5ef4a3a..19bb30c 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -20,12 +20,14 @@ typedef struct QemuCond QemuCond;
 typedef struct QemuThread QemuThread;
 
 void qemu_mutex_init(QemuMutex *mutex);
+void qemu_mutex_destroy(QemuMutex *mutex);
 void qemu_mutex_lock(QemuMutex *mutex);
 int qemu_mutex_trylock(QemuMutex *mutex);
 int qemu_mutex_timedlock(QemuMutex *mutex, uint64_t msecs);
 void qemu_mutex_unlock(QemuMutex *mutex);
 
 void qemu_cond_init(QemuCond *cond);
+void qemu_cond_destroy(QemuCond *cond);
 void qemu_cond_signal(QemuCond *cond);
 void qemu_cond_broadcast(QemuCond *cond);
 void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
@@ -37,4 +39,6 @@ void qemu_thread_create(QemuThread *thread,
 void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
 int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+void qemu_thread_exit(void *retval);
+
 #endif
-- 
1.7.1




[Qemu-devel] [PATCH 00/16] VNC updates for 0.13

2010-06-15 Thread Corentin Chary
Tight JPEG and Move to ui
=
This set starts by adding JPEG and gradient to tight, then move all ui code
in the ui/ subdirectory.
Thanks,

Since v1:
* Format patch with rename detection
* Add "lossy" parameter instead of "lossless"
* Disable lossy encodings by default
* Add a small tight fix (for indexed colors)

Since v2:
* Rebased on current master
* Removed a leak in send_jpeg_rect()


Misc
===
* Add the missing last color while filling palette
* Rewrite the palette code without using qdict. I did some profiling using 
`perf`
  and a lot of cpu time was spent in qdict, mainly due to memory allocation, 
hash, and
  qobject conversion. The new code is faster and uses less memory.

Tight PNG
==
This set introduce a new encoding: VNC_ENCODING_TIGHT_PNG (0x17) and a new
tight filter VNC_TIGHT_PNG (0x0A). When the client tells it supports the 0x17
encoding, the server will use tight, but will always send encoding pixels using
PNG instead of zlib. If the client also told it support JPEG, then the server 
can
send JPEG, because PNG will only be used in the cases zlib was used in normal 
tight.

This encoding was introduced to speed up HTML5 based VNC clients like noVNC 
[1], but
can also be used on devices like iPhone where PNG can be rendered in hardware.

I also made a quick patch to add support for PNG in gtk-vnc [2] and noVNC 
already support
PNG encoding. Note: There is a bug in gtk-vnc when using pixbuf on a 16bit 
display,
which also happens with JPEG.

[1] http://github.com/kanaka/noVNC/
[2] http://xf.iksaif.net/dev/vnc/gtk-vnc/0001-add-png-support.patch

Threaded Server
===

Since v1:
* Moved locks from VncState to VncDisplay because it's only used in vnc_refresh
* Use trylock in vnc_refresh. If there is an encoding task still running, 
reschedule the refresh.
 This really boost performances and make the vnc server truly asynchroneous. 
The only blocking
 lock is the output_mutex which is only held during a simple memcpy().
* Fixed issues found by Paolo, except the exit condition, mainly because we can 
only have
 one queue per VncState (due to zstreams), so this is not really an issue.
* Rebased on top of jpeg and ui/ patchs

Since v2:
* renamed vnc-jobs.c vnc-jobs-async.c
* added vnc-jobs.h, refactor functions declarations, export 
vnc_[un]lock_display()
 and vnc_[un]lock_output() and use them in vnc-jobs-async.c (reported by Avi)
* rework exit condition for vnc_worker_thread_loop (Paolo)
* abord -> abort (Paolo)
* call qemu_thread_self() (Paolo)
* Coding style issues (Alexander)
* Move from empty macros to empty statis inline (Alexander)

Alexander also suggested me to use stw_be_p() defined in cpu-all.h,
but when I tried to include cpu-all.h, it broke every thing. Anyway it can
be done later since this code is already present in vnc.c.

Also vnc_async_encoding_start() could be cleaner if encoding members where
in a specific structure, but this is a lot of changes, and as I'm also working
on encodings, I want this patch to be easy to rebase. So I'll do as soon as
the VNC server is merged.

Since v3:
* Encoding are data is now in specific structures, that makes
  vnc_async_encoding_start a lot cleaner.
* Added a missing vnc_output_lock(vs)

Corentin Chary (16):
  vnc: tight: add JPEG and gradient subencoding with smooth image
detection
  vnc: JPEG should be disabled if the client don't set tight quality
  vnc: add lossy option
  ui: move all ui components in ui/
  vnc: rename vnc-encoding-* vnc-enc-*
  vnc: tight: don't forget do at the last color
  vnc: tight: remove a memleak in send_jpeg_rect()
  vnc: tight add PNG encoding
  vnc: tight: specific zlib level and filters for each compression
level
  vnc: tight: stop using qdict for palette stuff
  vnc: encapsulate encoding members
  vnc: fix tight png memory leak
  qemu-thread: add qemu_mutex/cond_destroy and qemu_mutex_exit
  vnc: threaded VNC server
  vnc: add missing lock for vnc_cursor_define()
  vnc: tight: don't limit png rect size

 Makefile   |   38 +-
 Makefile.objs  |   29 +-
 Makefile.target|2 +
 configure  |   83 ++
 qemu-options.hx|7 +
 qemu-thread.c  |   22 +
 qemu-thread.h  |4 +
 cocoa.m => ui/cocoa.m  |0
 curses.c => ui/curses.c|0
 curses_keys.h => ui/curses_keys.h  |0
 d3des.c => ui/d3des.c  |0
 d3des.h => ui/d3des.h  |0
 keymaps.c => ui/keymaps.c  |0
 keymaps.h => ui/keymaps.h  |0
 sdl.c => ui/sdl.c  |0
 sdl_keysym.h => ui/sdl_keysym.h|0
 sdl_zoom.c => ui/sdl_zoom.c|0
 sdl_zoom.h => ui/sdl_z

[Qemu-devel] [PATCH 01/16] vnc: tight: add JPEG and gradient subencoding with smooth image detection

2010-06-15 Thread Corentin Chary
Add gradient filter and JPEG compression with an heuristic to detect how
lossy the comppression will be. This code has been adapted from
libvncserver/tight.c.

JPEG support can be enabled/disabled at compile time with --enable-vnc-jpeg
and --disable-vnc-jpeg.

Signed-off-by: Corentin Chary 
---
 Makefile.target  |1 +
 configure|   33 +++
 vnc-encoding-tight.c |  559 +-
 vnc-encoding-tight.h |5 +
 vnc.h|4 +
 5 files changed, 601 insertions(+), 1 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 478b89d..d9e888a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -177,6 +177,7 @@ LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
+QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
 
 # xen backend driver support
 obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
diff --git a/configure b/configure
index c0d8aa5..304b4b0 100755
--- a/configure
+++ b/configure
@@ -268,6 +268,7 @@ uuid=""
 vde=""
 vnc_tls=""
 vnc_sasl=""
+vnc_jpeg=""
 xen=""
 linux_aio=""
 vhost_net=""
@@ -574,6 +575,10 @@ for opt do
   ;;
   --enable-vnc-sasl) vnc_sasl="yes"
   ;;
+  --disable-vnc-jpeg) vnc_jpeg="no"
+  ;;
+  --enable-vnc-jpeg) vnc_jpeg="yes"
+  ;;
   --disable-slirp) slirp="no"
   ;;
   --disable-uuid) uuid="no"
@@ -818,6 +823,8 @@ echo "  --disable-vnc-tlsdisable TLS encryption for 
VNC server"
 echo "  --enable-vnc-tls enable TLS encryption for VNC server"
 echo "  --disable-vnc-sasl   disable SASL encryption for VNC server"
 echo "  --enable-vnc-saslenable SASL encryption for VNC server"
+echo "  --disable-vnc-jpeg   disable JPEG lossy compression for VNC server"
+echo "  --enable-vnc-jpegenable JPEG lossy compression for VNC server"
 echo "  --disable-curses disable curses output"
 echo "  --enable-curses  enable curses output"
 echo "  --disable-curl   disable curl connectivity"
@@ -1237,6 +1244,27 @@ EOF
 fi
 
 ##
+# VNC JPEG detection
+if test "$vnc_jpeg" = "yes" ; then
+cat > $TMPC <
+#include 
+int main(void) { struct jpeg_compress_struct s; jpeg_create_compress(&s); 
return 0; }
+EOF
+vnc_jpeg_cflags=""
+vnc_jpeg_libs="-ljpeg"
+  if compile_prog "$vnc_jpeg_cflags" "$vnc_jpeg_libs" ; then
+vnc_jpeg=yes
+libs_softmmu="$vnc_jpeg_libs $libs_softmmu"
+  else
+if test "$vnc_jpeg" = "yes" ; then
+  feature_not_found "vnc-jpeg"
+fi
+vnc_jpeg=no
+  fi
+fi
+
+##
 # fnmatch() probe, used for ACL routines
 fnmatch="no"
 cat > $TMPC << EOF
@@ -2064,6 +2092,7 @@ echo "Block whitelist   $block_drv_whitelist"
 echo "Mixer emulation   $mixemu"
 echo "VNC TLS support   $vnc_tls"
 echo "VNC SASL support  $vnc_sasl"
+echo "VNC JPEG support  $vnc_jpeg"
 if test -n "$sparc_cpu"; then
 echo "Target Sparc Arch $sparc_cpu"
 fi
@@ -2200,6 +2229,10 @@ if test "$vnc_sasl" = "yes" ; then
   echo "CONFIG_VNC_SASL=y" >> $config_host_mak
   echo "VNC_SASL_CFLAGS=$vnc_sasl_cflags" >> $config_host_mak
 fi
+if test "$vnc_jpeg" = "yes" ; then
+  echo "CONFIG_VNC_JPEG=y" >> $config_host_mak
+  echo "VNC_JPEG_CFLAGS=$vnc_jpeg_cflags" >> $config_host_mak
+fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
 fi
diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index faba483..5b69ff0 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -26,6 +26,14 @@
  * THE SOFTWARE.
  */
 
+#include "qemu-common.h"
+
+#ifdef CONFIG_VNC_JPEG
+#include 
+#include 
+#endif
+
+#include "bswap.h"
 #include "qdict.h"
 #include "qint.h"
 #include "vnc.h"
@@ -56,6 +64,206 @@ static const struct {
 };
 
 /*
+ * Code to guess if given rectangle is suitable for smooth image
+ * compression (by applying "gradient" filter or JPEG coder).
+ */
+
+static uint
+tight_detect_smooth_image24(VncState *vs, int w, int h)
+{
+int off;
+int x, y, d, dx;
+uint c;
+uint stats[256];
+int pixels = 0;
+int pix, left[3];
+uint errors;
+unsigned char *buf = vs->tight.buffer;
+
+/*
+ * If client is big-endian, color samples begin from the second
+ * byte (offset 1) of a 32-bit pixel value.
+ */
+off = !!(vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG);
+
+memset(stats, 0, sizeof (stats));
+
+for (y = 0, x = 0; y < h && x < w;) {
+for (d = 0; d < h - y && d < w - x - VNC_TIGHT_DETECT_SUBROW_WIDTH;
+ d++) {
+for (c = 0; c < 3; c++) {
+left[c] = buf[((y+d)*w+x+d)*4+off+c] & 0xFF;
+}
+for (dx = 1; dx <= VNC_TIGHT_DETECT_SUBROW_WIDTH; dx++) {
+for (c = 0; c < 3; c++) {
+pix = buf[((y+d)*w+x+d+dx)*4+off+c] & 0xFF;
+stats[abs(pix - left[c])]++;
+left[c] = pix;
+}
+pixels++;
+}
+}
+if (w > h) {
+ 

[Qemu-devel] [PATCH 09/16] vnc: tight: specific zlib level and filters for each compression level

2010-06-15 Thread Corentin Chary
Disable png filters for lower compression levels. This should lower
the CPU consumption and reduce encoding time.

This isn't in tight_conf because:
* tight_conf structure must not change, because it's shared with other
  tight implementations (libvncserver, etc..).
* it'd exceed the 80 col limit.
* PNG_ macros are only defined if CONFIG_VNC_PNG is defined

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-tight.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 007d88f..9e1f214 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -69,6 +69,21 @@ static const struct {
 };
 
 #ifdef CONFIG_VNC_PNG
+static const struct {
+int png_zlib_level, png_filters;
+} tight_png_conf[] = {
+{ 0, PNG_NO_FILTERS },
+{ 1, PNG_NO_FILTERS },
+{ 2, PNG_NO_FILTERS },
+{ 3, PNG_NO_FILTERS },
+{ 4, PNG_NO_FILTERS },
+{ 5, PNG_ALL_FILTERS },
+{ 6, PNG_ALL_FILTERS },
+{ 7, PNG_ALL_FILTERS },
+{ 8, PNG_ALL_FILTERS },
+{ 9, PNG_ALL_FILTERS },
+};
+
 static int send_png_rect(VncState *vs, int x, int y, int w, int h,
  QDict *palette);
 
@@ -1424,7 +1439,8 @@ static int send_png_rect(VncState *vs, int x, int y, int 
w, int h,
 png_infop info_ptr;
 png_colorp png_palette = NULL;
 size_t offset;
-int level = tight_conf[vs->tight_compression].raw_zlib_level;
+int level = tight_png_conf[vs->tight_compression].png_zlib_level;
+int filters = tight_png_conf[vs->tight_compression].png_filters;
 uint8_t *buf;
 int dy;
 
@@ -1443,6 +1459,7 @@ static int send_png_rect(VncState *vs, int x, int y, int 
w, int h,
 
 png_set_write_fn(png_ptr, (void *) vs, png_write_data, png_flush_data);
 png_set_compression_level(png_ptr, level);
+png_set_filter(png_ptr, PNG_FILTER_TYPE_DEFAULT, filters);
 
 if (palette) {
 color_type = PNG_COLOR_TYPE_PALETTE;
-- 
1.7.1




[Qemu-devel] [PATCH 07/16] vnc: tight: remove a memleak in send_jpeg_rect()

2010-06-15 Thread Corentin Chary
buf was never freed.

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-tight.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index ade8e5f..4ff88a8 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1247,8 +1247,6 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int 
w, int h, int quality)
 if (ds_get_bytes_per_pixel(vs->ds) == 1)
 return send_full_color_rect(vs, w, h);
 
-buf = qemu_malloc(w * 3);
-row[0] = buf;
 buffer_reserve(&vs->tight_jpeg, 2048);
 
 cinfo.err = jpeg_std_error(&jerr);
@@ -1270,10 +1268,13 @@ static int send_jpeg_rect(VncState *vs, int x, int y, 
int w, int h, int quality)
 
 jpeg_start_compress(&cinfo, true);
 
+buf = qemu_malloc(w * 3);
+row[0] = buf;
 for (dy = 0; dy < h; dy++) {
 jpeg_prepare_row(vs, buf, x, y + dy, w);
 jpeg_write_scanlines(&cinfo, row, 1);
 }
+qemu_free(buf);
 
 jpeg_finish_compress(&cinfo);
 jpeg_destroy_compress(&cinfo);
-- 
1.7.1




[Qemu-devel] [PATCH 05/16] vnc: rename vnc-encoding-* vnc-enc-*

2010-06-15 Thread Corentin Chary
For the same reason that we don't use vnc-authentication-sasl.c but
vnc-auth-sals.c. Because it's to long.

Signed-off-by: Corentin Chary 
---
 Makefile.objs|4 ++--
 ui/{vnchextile.h => vnc-enc-hextile-template.h}  |0
 ui/{vnc-encoding-hextile.c => vnc-enc-hextile.c} |   12 ++--
 ui/{vnc-encoding-tight.c => vnc-enc-tight.c} |2 +-
 ui/{vnc-encoding-tight.h => vnc-enc-tight.h} |0
 ui/{vnc-encoding-zlib.c => vnc-enc-zlib.c}   |0
 6 files changed, 9 insertions(+), 9 deletions(-)
 rename ui/{vnchextile.h => vnc-enc-hextile-template.h} (100%)
 rename ui/{vnc-encoding-hextile.c => vnc-enc-hextile.c} (93%)
 rename ui/{vnc-encoding-tight.c => vnc-enc-tight.c} (99%)
 rename ui/{vnc-encoding-tight.h => vnc-enc-tight.h} (100%)
 rename ui/{vnc-encoding-zlib.c => vnc-enc-zlib.c} (100%)

diff --git a/Makefile.objs b/Makefile.objs
index bbb5518..e63c5eb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -106,8 +106,8 @@ ui-obj-y += keymaps.o
 ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
 ui-obj-$(CONFIG_CURSES) += curses.o
 ui-obj-y += vnc.o d3des.o
-ui-obj-y += vnc-encoding-zlib.o vnc-encoding-hextile.o
-ui-obj-y += vnc-encoding-tight.o
+ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o
+ui-obj-y += vnc-enc-tight.o
 ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
 ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
 ui-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/vnchextile.h b/ui/vnc-enc-hextile-template.h
similarity index 100%
rename from ui/vnchextile.h
rename to ui/vnc-enc-hextile-template.h
diff --git a/ui/vnc-encoding-hextile.c b/ui/vnc-enc-hextile.c
similarity index 93%
rename from ui/vnc-encoding-hextile.c
rename to ui/vnc-enc-hextile.c
index 728f25e..fa4b264 100644
--- a/ui/vnc-encoding-hextile.c
+++ b/ui/vnc-enc-hextile.c
@@ -33,32 +33,32 @@ static void hextile_enc_cord(uint8_t *ptr, int x, int y, 
int w, int h)
 }
 
 #define BPP 8
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 
 #define BPP 16
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 
 #define BPP 32
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 
 #define GENERIC
 #define BPP 8
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 #undef GENERIC
 
 #define GENERIC
 #define BPP 16
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 #undef GENERIC
 
 #define GENERIC
 #define BPP 32
-#include "vnchextile.h"
+#include "vnc-enc-hextile-template.h"
 #undef BPP
 #undef GENERIC
 
diff --git a/ui/vnc-encoding-tight.c b/ui/vnc-enc-tight.c
similarity index 99%
rename from ui/vnc-encoding-tight.c
rename to ui/vnc-enc-tight.c
index c1a292b..358221d 100644
--- a/ui/vnc-encoding-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -37,7 +37,7 @@
 #include "qdict.h"
 #include "qint.h"
 #include "vnc.h"
-#include "vnc-encoding-tight.h"
+#include "vnc-enc-tight.h"
 
 /* Compression level stuff. The following array contains various
encoder parameters for each of 10 compression levels (0..9).
diff --git a/ui/vnc-encoding-tight.h b/ui/vnc-enc-tight.h
similarity index 100%
rename from ui/vnc-encoding-tight.h
rename to ui/vnc-enc-tight.h
diff --git a/ui/vnc-encoding-zlib.c b/ui/vnc-enc-zlib.c
similarity index 100%
rename from ui/vnc-encoding-zlib.c
rename to ui/vnc-enc-zlib.c
-- 
1.7.1




[Qemu-devel] [PATCH 04/16] ui: move all ui components in ui/

2010-06-15 Thread Corentin Chary
Move sdl, vnc, curses and cocoa UI into ui/ to cleanup
the root directory. Also remove some unnecessary explicit
targets from Makefile.

Signed-off-by: Corentin Chary 
---
 Makefile   |   38 +++-
 Makefile.objs  |   22 ++-
 cocoa.m => ui/cocoa.m  |0
 curses.c => ui/curses.c|0
 curses_keys.h => ui/curses_keys.h  |0
 d3des.c => ui/d3des.c  |0
 d3des.h => ui/d3des.h  |0
 keymaps.c => ui/keymaps.c  |0
 keymaps.h => ui/keymaps.h  |0
 sdl.c => ui/sdl.c  |0
 sdl_keysym.h => ui/sdl_keysym.h|0
 sdl_zoom.c => ui/sdl_zoom.c|0
 sdl_zoom.h => ui/sdl_zoom.h|0
 sdl_zoom_template.h => ui/sdl_zoom_template.h  |0
 vnc-auth-sasl.c => ui/vnc-auth-sasl.c  |0
 vnc-auth-sasl.h => ui/vnc-auth-sasl.h  |0
 vnc-auth-vencrypt.c => ui/vnc-auth-vencrypt.c  |0
 vnc-auth-vencrypt.h => ui/vnc-auth-vencrypt.h  |0
 .../vnc-encoding-hextile.c |0
 vnc-encoding-tight.c => ui/vnc-encoding-tight.c|0
 vnc-encoding-tight.h => ui/vnc-encoding-tight.h|0
 vnc-encoding-zlib.c => ui/vnc-encoding-zlib.c  |0
 vnc-tls.c => ui/vnc-tls.c  |0
 vnc-tls.h => ui/vnc-tls.h  |0
 vnc.c => ui/vnc.c  |0
 vnc.h => ui/vnc.h  |0
 vnc_keysym.h => ui/vnc_keysym.h|0
 vnchextile.h => ui/vnchextile.h|0
 x_keymap.c => ui/x_keymap.c|0
 x_keymap.h => ui/x_keymap.h|0
 30 files changed, 17 insertions(+), 43 deletions(-)
 rename cocoa.m => ui/cocoa.m (100%)
 rename curses.c => ui/curses.c (100%)
 rename curses_keys.h => ui/curses_keys.h (100%)
 rename d3des.c => ui/d3des.c (100%)
 rename d3des.h => ui/d3des.h (100%)
 rename keymaps.c => ui/keymaps.c (100%)
 rename keymaps.h => ui/keymaps.h (100%)
 rename sdl.c => ui/sdl.c (100%)
 rename sdl_keysym.h => ui/sdl_keysym.h (100%)
 rename sdl_zoom.c => ui/sdl_zoom.c (100%)
 rename sdl_zoom.h => ui/sdl_zoom.h (100%)
 rename sdl_zoom_template.h => ui/sdl_zoom_template.h (100%)
 rename vnc-auth-sasl.c => ui/vnc-auth-sasl.c (100%)
 rename vnc-auth-sasl.h => ui/vnc-auth-sasl.h (100%)
 rename vnc-auth-vencrypt.c => ui/vnc-auth-vencrypt.c (100%)
 rename vnc-auth-vencrypt.h => ui/vnc-auth-vencrypt.h (100%)
 rename vnc-encoding-hextile.c => ui/vnc-encoding-hextile.c (100%)
 rename vnc-encoding-tight.c => ui/vnc-encoding-tight.c (100%)
 rename vnc-encoding-tight.h => ui/vnc-encoding-tight.h (100%)
 rename vnc-encoding-zlib.c => ui/vnc-encoding-zlib.c (100%)
 rename vnc-tls.c => ui/vnc-tls.c (100%)
 rename vnc-tls.h => ui/vnc-tls.h (100%)
 rename vnc.c => ui/vnc.c (100%)
 rename vnc.h => ui/vnc.h (100%)
 rename vnc_keysym.h => ui/vnc_keysym.h (100%)
 rename vnchextile.h => ui/vnchextile.h (100%)
 rename x_keymap.c => ui/x_keymap.c (100%)
 rename x_keymap.h => ui/x_keymap.h (100%)

diff --git a/Makefile b/Makefile
index 221fbd8..f0295da 100644
--- a/Makefile
+++ b/Makefile
@@ -96,42 +96,14 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += 
$(FMOD_CFLAGS)
 
 QEMU_CFLAGS+=$(CURL_CFLAGS)
 
-cocoa.o: cocoa.m
+ui/cocoa.o: ui/cocoa.m
 
-keymaps.o: keymaps.c keymaps.h
+ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS)
 
-sdl_zoom.o: sdl_zoom.c sdl_zoom.h sdl_zoom_template.h
-
-sdl.o: sdl.c keymaps.h sdl_keysym.h sdl_zoom.h
-
-sdl.o audio/sdlaudio.o sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS)
-
-acl.o: acl.h acl.c
-
-vnc.h: vnc-tls.h vnc-auth-vencrypt.h vnc-auth-sasl.h keymaps.h
-
-vnc.o: vnc.c vnc.h vnc_keysym.h vnchextile.h d3des.c d3des.h acl.h
-
-vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
-
-vnc-tls.o: vnc-tls.c vnc.h
-
-vnc-auth-vencrypt.o: vnc-auth-vencrypt.c vnc.h
-
-vnc-auth-sasl.o: vnc-auth-sasl.c vnc.h
-
-vnc-encoding-zlib.o: vnc-encoding-zlib.c vnc.h
-
-vnc-encoding-hextile.o: vnc-encoding-hextile.c vnc.h
-
-vnc-encoding-tight.o: vnc-encoding-tight.c vnc.h vnc-encoding-tight.h
-
-curses.o: curses.c keymaps.h curses_keys.h
+ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
-iov.o: iov.c iov.h
-
 ##
 
 qemu-img.o: qemu-img-cmds.h
@@ -159,7 +131,7 @@ clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~
-   rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
net/*.o net/*

[Qemu-devel] [PATCH 03/16] vnc: add lossy option

2010-06-15 Thread Corentin Chary
The lossy option can be used to enable lossy compression
methods like gradient or jpeg. This patch disable them by
default.

Signed-off-by: Corentin Chary 
---
 qemu-options.hx  |7 +++
 vnc-encoding-tight.c |4 
 vnc.c|2 ++
 vnc.h|2 ++
 4 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index a6928b7..cf33a54 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -828,6 +828,13 @@ empty, with a @code{deny} policy. Thus no one will be 
allowed to
 use the VNC server until the ACLs have been loaded. This can be
 achieved using the @code{acl} monitor command.
 
+...@item lossy
+
+Enable lossy compression methods (gradient, JPEG, ...). If this
+option is set, VNC client may receive lossy framebuffer updates
+depending on its encoding settings. Enabling this option can save
+a lot of bandwidth at the expense of quality.
+
 @end table
 ETEXI
 
diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index 5b69ff0..c1a292b 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -228,6 +228,10 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
 int compression = vs->tight_compression;
 int quality = vs->tight_quality;
 
+if (!vs->vd->lossy) {
+return 0;
+}
+
 if (ds_get_bytes_per_pixel(vs->ds) == 1 ||
 vs->clientds.pf.bytes_per_pixel == 1 ||
 w < VNC_TIGHT_DETECT_MIN_WIDTH || h < VNC_TIGHT_DETECT_MIN_HEIGHT) {
diff --git a/vnc.c b/vnc.c
index 9cf38d1..ccd7aad 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2482,6 +2482,8 @@ int vnc_display_open(DisplayState *ds, const char 
*display)
 #endif
 } else if (strncmp(options, "acl", 3) == 0) {
 acl = 1;
+} else if (strncmp(options, "lossy", 5) == 0) {
+vs->lossy = true;
 }
 }
 
diff --git a/vnc.h b/vnc.h
index 2a9024d..ec90cd3 100644
--- a/vnc.h
+++ b/vnc.h
@@ -33,6 +33,7 @@
 #include "monitor.h"
 #include "audio/audio.h"
 #include 
+#include 
 
 #include "keymaps.h"
 
@@ -111,6 +112,7 @@ struct VncDisplay
 char *display;
 char *password;
 int auth;
+bool lossy;
 #ifdef CONFIG_VNC_TLS
 int subauth; /* Used by VeNCrypt */
 VncDisplayTLS tls;
-- 
1.7.1




[Qemu-devel] [PATCH 06/16] vnc: tight: don't forget do at the last color

2010-06-15 Thread Corentin Chary
While using indexed colors, the last color was never added to the palette.
Triggered with ubuntu livecd.

Signed-off-by: Corentin Chary 
---
 ui/vnc-enc-tight.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 358221d..ade8e5f 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -393,11 +393,11 @@ static int tight_palette_insert(QDict *palette, uint32_t 
rgb, int bpp, int max)
 if (data[i] == ci) {\
 continue;   \
 } else {\
+ci = data[i];   \
 if (!tight_palette_insert(*palette, (uint32_t)ci,   \
   bpp, max)) {  \
 return 0;   \
 }   \
-ci = data[i];   \
 }   \
 }   \
 \
-- 
1.7.1




[Qemu-devel] [PATCH 02/16] vnc: JPEG should be disabled if the client don't set tight quality

2010-06-15 Thread Corentin Chary
Disable JPEG compression by default and only enable it if the
VNC client has sent the requested quality.

Signed-off-by: Corentin Chary 
---
 vnc.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vnc.c b/vnc.c
index ed0e096..9cf38d1 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1644,7 +1644,7 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
 vs->features = 0;
 vs->vnc_encoding = 0;
 vs->tight_compression = 9;
-vs->tight_quality = 9;
+vs->tight_quality = -1; /* Lossless by default */
 vs->absolute = -1;
 
 /*
-- 
1.7.1




Re: [Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable

2010-06-15 Thread Cam Macdonell
On Tue, Jun 15, 2010 at 4:33 PM, Anthony Liguori  wrote:
> On 06/15/2010 05:26 PM, Cam Macdonell wrote:
>>
>> On Tue, Jun 15, 2010 at 10:32 AM, Anthony Liguori
>>  wrote:
>>
>>>
>>> On 06/15/2010 11:16 AM, Cam Macdonell wrote:
>>>

 How does this look for marking the device as non-migratable?  It adds a
 field
 'no_migrate' to the SaveStateEntry and tests for it in vmstate_save.
  This
 would
 replace anything that touches memory.

 Cam

 ---
  hw/hw.h  |    1 +
  savevm.c |   32 +---
  2 files changed, 30 insertions(+), 3 deletions(-)

 diff --git a/hw/hw.h b/hw/hw.h
 index d78d814..7c93f08 100644
 --- a/hw/hw.h
 +++ b/hw/hw.h
 @@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
                           void *opaque);

  void unregister_savevm(const char *idstr, void *opaque);
 +void mark_no_migrate(const char *idstr, void *opaque);


>>>
>>> I'm not thrilled with the name but the functionality is spot on.  I lack
>>> the
>>> creativity to offer a better name suggestion :-)
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>
>> Hmmm, in working on this it seems that the memory (from
>> qemu_ram_map()) is still attached even when the device is removed
>> (which causes migration to fail because there is an unexpected
>> memory).
>>
>> Is something like cpu_unregister_physical_memory()/qemu_ram_free() needed?
>>
>
> Yes.  You need to unregister any memory that you have registered upon device
> removal.

Is there an established way to achieve this?  I can't seem find
another device that unregisters memory registered with
cpu_register_physical_memory().  Is something like
cpu_unregister_physical_memory() needed?

Thanks,
Cam

>
> Regards,
>
> Anthony Liguori
>
>> Cam
>>
>>
>
>



[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter

2010-06-15 Thread Gleb Natapov
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> There is no need starting with the special value for hpet_cfg.count.
> Either Seabios is aware of the new firmware interface and properly
> interprets the counter or it simply ignores it anyway.
> 
I want seabios to be able to distinguish between old qemu and new one.
Hence special value. I used it incorrectly in may v2 seabios patch. Will
resend asap. Will teach me to not change logic at the last minute :( I
removed "valid" field between v1 and v2 of the patches and introduces
special value for count instead. As a result I made one bug in qemu and
one is seabios. Heh.

> Signed-off-by: Jan Kiszka 
> ---
>  hw/hpet.c |7 +--
>  1 files changed, 1 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/hpet.c b/hw/hpet.c
> index d5c406c..ed4e995 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -74,7 +74,7 @@ typedef struct HPETState {
>  uint8_t  hpet_id;   /* instance id */
>  } HPETState;
>  
> -struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> +struct hpet_fw_config hpet_cfg;
>  
>  static uint32_t hpet_in_legacy_mode(HPETState *s)
>  {
> @@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
>  int i, iomemtype;
>  HPETTimer *timer;
>  
> -if (hpet_cfg.count == UINT8_MAX) {
> -/* first instance */
> -hpet_cfg.count = 0;
> -}
> -
>  if (hpet_cfg.count == 8) {
>  fprintf(stderr, "Only 8 instances of HPET is allowed\n");
>  return -1;
> -- 
> 1.6.0.2

--
Gleb.



[Qemu-devel] Re: [PATCH] Fix comparison which always returned false

2010-06-15 Thread Gleb Natapov
On Tue, Jun 15, 2010 at 11:28:42PM +0200, Jan Kiszka wrote:
> Stefan Weil wrote:
> > Comparing an 8 bit value with ~0 does not work as expected.
> > Replace ~0 by UINT8_MAX in comparison and also in assignment
> > (and fix coding style, too).
> > 
> > Cc: Gleb Natapov 
> > Cc: Anthony Liguori 
> > Signed-off-by: Stefan Weil 
> > ---
> >  hw/hpet.c |6 --
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/hpet.c b/hw/hpet.c
> > index 0c80ee5..d5c406c 100644
> > --- a/hw/hpet.c
> > +++ b/hw/hpet.c
> > @@ -74,7 +74,7 @@ typedef struct HPETState {
> >  uint8_t  hpet_id;   /* instance id */
> >  } HPETState;
> >  
> > -struct hpet_fw_config hpet_cfg = {.count = ~0};
> > +struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> >  
> >  static uint32_t hpet_in_legacy_mode(HPETState *s)
> >  {
> > @@ -682,8 +682,10 @@ static int hpet_init(SysBusDevice *dev)
> >  int i, iomemtype;
> >  HPETTimer *timer;
> >  
> > -if (hpet_cfg.count == ~0) /* first instance */
> > +if (hpet_cfg.count == UINT8_MAX) {
> > +/* first instance */
> >  hpet_cfg.count = 0;
> > +}
> >  
> >  if (hpet_cfg.count == 8) {
> >  fprintf(stderr, "Only 8 instances of HPET is allowed\n");
> 
> That makes me wonder why we need to signal this special value of
> hpet_cfg.count to seabios at all. Why isn't plain 0 for no hpets
> sufficient, Gleb?
> 
> Jan
> 
I want bios to be able to distinguish between qemu that does not support
HPET cfg_fw and qemu that does. On the former bios should always create
HPET table for backwards compatibility.

--
Gleb.



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Alex Williamson
On Wed, 2010-06-16 at 02:30 +0100, Paul Brook wrote:
> Transferring the machine description on migration is a separate problem.
> 
> Lets say we associate each RAM block with a device. Each ram block also has a 
> name.  These names distinguish between blocks attached to a given device, but 
> need not be globally unique.  i.e. devices A and B can both have block named 
> "foo".  RAM block migration happens before device state migration (including 
> device properties).
> 
> There are three relevant migration failure modes:
> 
> (1) The same device is present, but has a different size property.
>   If the incoming block is larger than the allocated block then you loose.
> (2) A different device is present, but does not have a ram block of the same 
> name.
>   This safely fails migration because of the block name mismatch.
> (3) A different device is present, that happens to have a ram block of the 
> same name.
>   If the blocks are the same size then transferring the contents is harmless.
>   If they are different sizes then this will be caught by (1). Either way, the
>   migration will be failed once we get to the vmstate check.
> 
> Note how adding the device type to the canonical address does not effect the 
> outcome.
> 
> Going back to the original problem, (1) is the most interesting.
> 
> I suggest that the initial migration phase transfers a list of ram blocks. 
> Each entry in that list should be {canonical device path, name, size}. You 
> should lookup all these ram blocks, and fail migration if they are not 
> present 
> with the correct size[1].  This list also gives you a convenient numeric 
> index 
> to identify the block during RAM migration.
> 
> [1] In the future we may be able to resize blocks. However this is not safe 
> with the current API.

I think for the most part, you've just described the RAMBlock series of
patches I sent out last week.  I'll note that that series creates ram
blocks on the target if they aren't present because of the technicality
that we currently do not have a qemu_ram_free() to cleanup the list when
things go away.  Once we have that and cleanup drivers to use it, I
agree we should fail the migration if it occurs, or at least print out a
big warning so we can go fix the driver.  If I'm missing where else it's
significantly different please let me know.

Yes, case (3) would fail in the vmstate code without driver name in the
canonical path... or at least we hope it would.  But with the driver
name in the canonical path, we can avoid doing a useless operation, fail
earlier, and provide the vmstate with a key piece of information it can
use to help ensure that the incoming state information belongs to the
driver it thinks it does.  Seems like a win to me.  Thanks,

Alex




Re: [Qemu-devel] Q35 qemu repository?

2010-06-15 Thread Isaku Yamahata
Hi, thank you for interested in q35 stuff.
I set up the repository. It contains the snapshot of my local repo.

git clone http://people.valinux.co.jp/~yamahata/qemu/q35/qemu
git clone http://people.valinux.co.jp/~yamahata/qemu/q35/seabios
git clone http://people.valinux.co.jp/~yamahata/qemu/q35/vgabios

Note that modified seabios and vgabios should be used, and
ACPI DSDT for q35 must be used.

Example:
qemu-system-x86_64 ... -M pc_q35 -acpitable 
'load_header,data=roms/seabios/src/q35-acpi-dsdt.aml

My interest is mainly pcie aer and native hot plug, so power management
is out of my current scope. So I'm not sure my repo is useful
for your purpose or not.

thanks,

On Mon, Jun 14, 2010 at 02:56:19PM +0100, Matthew Garrett wrote:
> Hi there,
> 
> I'm currently doing some work on tidying up Linux's PCIe ASPM support, 
> and one thing that would be useful would be to be able to instrument 
> reads and writes made by Windows to PCIe space. I noticed that you've 
> been working on Q35 support for qemu - is there a public repository 
> which contains your patch sets?
> 
> Thank you,
> -- 
> Matthew Garrett | mj...@srcf.ucam.org
> 

-- 
yamahata



[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.

2010-06-15 Thread Isaku Yamahata
On Tue, Jun 15, 2010 at 12:12:07PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 15, 2010 at 02:06:46PM +0900, Isaku Yamahata wrote:
> > Don't overwrite pci header type.
> > Otherwise, multi function bit which pci_init_header_type() sets
> > appropriately is lost.
> > Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
> > which is already zero cleared.
> > 
> > Signed-off-by: Isaku Yamahata 
> 
> ...
> 
> > diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> > index 31c8d70..cdf3bc2 100644
> > --- a/hw/apb_pci.c
> > +++ b/hw/apb_pci.c
> > @@ -428,7 +428,8 @@ static int pbm_pci_host_init(PCIDevice *d)
> >   PCI_STATUS_DEVSEL_MEDIUM);
> >  pci_config_set_class(d->config, PCI_CLASS_BRIDGE_HOST);
> >  pci_set_byte(d->config + PCI_HEADER_TYPE,
> > - PCI_HEADER_TYPE_NORMAL);
> > + (pci_get_byte(d->config + PCI_HEADER_TYPE) &
> > +  PCI_HEADER_TYPE_MULTI_FUNCTION) | 
> > PCI_HEADER_TYPE_NORMAL);
> 
> what is this doing?

It changes the header type to normal device(bit 1-7) without overwriting
multi function bit(bit 8).

Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
on the other hand pbc_pci_host_init() sets the register
to PCI_HEADER_TYPE_NORMAL.
To be honest I don't know why it does so, but that is what Blue wants.
So I touch only multi function bit(bit 8) and leave other bit (bit 1-7)
unchanged.

If you don't like this hunk, I'll drop this hunk and leave it to Blue.
What do you think?


static PCIDeviceInfo pbm_pci_host_info = {
.qdev.name = "pbm",
.qdev.size = sizeof(PCIDevice),
.init  = pbm_pci_host_init,
.header_type  = PCI_HEADER_TYPE_BRIDGE, < Here
};

-- 
yamahata



[Qemu-devel] [PATCH] monitor: Really show snapshot information about all devices

2010-06-15 Thread Miguel Di Ciurcio Filho
The 'info snapshots' monitor command does not show snapshot information from all
available block devices.

Usage example:
$ qemu -hda disk1.qcow2 -hdb disk2.qcow2

(qemu) info snapshots
Snapshot devices: ide0-hd0
Snapshot list (from ide0-hd0):
IDTAG VM SIZEDATE   VM CLOCK
11.5M 2010-05-26 21:51:02   00:00:03.263
21.5M 2010-05-26 21:51:09   00:00:08.844
31.5M 2010-05-26 21:51:24   00:00:23.274
41.5M 2010-05-26 21:53:17   00:00:03.595

In the above case, disk2.qcow2 has snapshot information, but it is not being
shown. Only the first device is always shown.

This patch updates the do_info_snapshots() function do correctly show snapshot
information about all available block devices.

New output:
(qemu) info snapshots
Snapshot list from ide0-hd0:
IDTAG VM SIZEDATE   VM CLOCK
11.5M 2010-05-26 21:51:02   00:00:03.263
21.5M 2010-05-26 21:51:09   00:00:08.844
31.5M 2010-05-26 21:51:24   00:00:23.274
41.5M 2010-05-26 21:53:17   00:00:03.595

Snapshot list from ide0-hd1:
IDTAG VM SIZEDATE   VM CLOCK
1   0 2010-05-26 21:51:02   00:00:03.263
2   0 2010-05-26 21:51:09   00:00:08.844
3   0 2010-05-26 21:51:24   00:00:23.274
4   0 2010-05-26 21:53:17   00:00:03.595

Signed-off-by: Miguel Di Ciurcio Filho 
---
 savevm.c |   46 +++---
 1 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/savevm.c b/savevm.c
index 20354a8..0eacb9f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1858,8 +1858,8 @@ void do_delvm(Monitor *mon, const QDict *qdict)
 
 void do_info_snapshots(Monitor *mon)
 {
-BlockDriverState *bs, *bs1;
-QEMUSnapshotInfo *sn_tab, *sn;
+BlockDriverState *bs;
+QEMUSnapshotInfo *sn_tab;
 int nb_sns, i;
 char buf[256];
 
@@ -1868,27 +1868,27 @@ void do_info_snapshots(Monitor *mon)
 monitor_printf(mon, "No available block device supports snapshots\n");
 return;
 }
-monitor_printf(mon, "Snapshot devices:");
-bs1 = NULL;
-while ((bs1 = bdrv_next(bs1))) {
-if (bdrv_can_snapshot(bs1)) {
-if (bs == bs1)
-monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
-}
-}
-monitor_printf(mon, "\n");
 
-nb_sns = bdrv_snapshot_list(bs, &sn_tab);
-if (nb_sns < 0) {
-monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
-return;
-}
-monitor_printf(mon, "Snapshot list (from %s):\n",
-   bdrv_get_device_name(bs));
-monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
-for(i = 0; i < nb_sns; i++) {
-sn = &sn_tab[i];
-monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+bs = NULL;
+while ((bs = bdrv_next(bs))) {
+if (bdrv_can_snapshot(bs)) {
+monitor_printf(mon, "Snapshot list from %s:\n",
+   bdrv_get_device_name(bs));
+monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), 
NULL));
+
+nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+if (nb_sns < 0) {
+monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
+continue;
+}
+
+for (i = 0; i < nb_sns; i++) {
+monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, 
sizeof(buf),
+&sn_tab[i]));
+}
+
+qemu_free(sn_tab);
+monitor_printf(mon, "\n");
+}
 }
-qemu_free(sn_tab);
 }
-- 
1.7.1




Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Paul Brook
> > > > > See my comment above, I'm not seeing a sufficient argument about
> > > > > why driver name matching is a false sense of security.
> > > > 
> > > > I still say it should be the migration code that checks that both
> > > > vmstate structures are for the same type of device. i.e. if
> > > > necessary the device name should be embedded in the device state,
> > > > not the device path.
> > > 
> > > I not sure how that fixes the ramblock issue that started this whole
> > > discussion.  It's not part of device's vmstate, it goes w/ ram.  I
> > > think I'm missing a piece of the puzzle here?
> > 
> > My main point there was that ram blocks should be associated with a
> > device state.  Once you do this it should just work as we already know
> > how to match device states.
> > 
> > It you're trying to transfer ram blocks before matching up the rest of
> > the state then you're likely to loose in all kinds of different ways.

... or not. See below.

> Yes, I see, thanks for clarifying.  I agree, ideally we'd create the
> entire target image dynamically.  It'd still need to know how to connect
> all the guest devices to the host, but it makes more sense to me than
> half building the system from cmdline on target, then rest filled in.

Transferring the machine description on migration is a separate problem.

Lets say we associate each RAM block with a device. Each ram block also has a 
name.  These names distinguish between blocks attached to a given device, but 
need not be globally unique.  i.e. devices A and B can both have block named 
"foo".  RAM block migration happens before device state migration (including 
device properties).

There are three relevant migration failure modes:

(1) The same device is present, but has a different size property.
  If the incoming block is larger than the allocated block then you loose.
(2) A different device is present, but does not have a ram block of the same 
name.
  This safely fails migration because of the block name mismatch.
(3) A different device is present, that happens to have a ram block of the 
same name.
  If the blocks are the same size then transferring the contents is harmless.
  If they are different sizes then this will be caught by (1). Either way, the
  migration will be failed once we get to the vmstate check.

Note how adding the device type to the canonical address does not effect the 
outcome.

Going back to the original problem, (1) is the most interesting.

I suggest that the initial migration phase transfers a list of ram blocks. 
Each entry in that list should be {canonical device path, name, size}. You 
should lookup all these ram blocks, and fail migration if they are not present 
with the correct size[1].  This list also gives you a convenient numeric index 
to identify the block during RAM migration.

[1] In the future we may be able to resize blocks. However this is not safe 
with the current API.

Paul



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Chris Wright
* Paul Brook (p...@codesourcery.com) wrote:
> > * Paul Brook (p...@codesourcery.com) wrote:
> > > > > I find this argument contradictory. The migration code already needs
> > > > > to check whether a device is compatible before it allows migration. 
> > > > > The driver name is not sufficient to ensure compatibility, so I see
> > > > > no benefit in including it in the device address.
> > > > 
> > > > See my comment above, I'm not seeing a sufficient argument about why
> > > > driver name matching is a false sense of security.  If on an incoming
> > > > migration I'm able to match the source provided e1000.03.0/vmstate
> > > > against the target registered e1000.03.0/vmstate and hand off to the
> > > > e1000 driver to check version ids, you bet I'm feeling a lot more
> > > > secure than if I'm handing off to whatever happened to register
> > > > 03.0/vmstate on the target.
> > > 
> > > I still say it should be the migration code that checks that both vmstate
> > > structures are for the same type of device. i.e. if necessary the device
> > > name should be embedded in the device state, not the device path.
> > 
> > I not sure how that fixes the ramblock issue that started this whole
> > discussion.  It's not part of device's vmstate, it goes w/ ram.  I think
> > I'm missing a piece of the puzzle here?
> 
> My main point there was that ram blocks should be associated with a device 
> state.  Once you do this it should just work as we already know how to match 
> device states.
> 
> It you're trying to transfer ram blocks before matching up the rest of the 
> state then you're likely to loose in all kinds of different ways.

Yes, I see, thanks for clarifying.  I agree, ideally we'd create the
entire target image dynamically.  It'd still need to know how to connect
all the guest devices to the host, but it makes more sense to me than
half building the system from cmdline on target, then rest filled in.

I think that level of work is a ways away though.

thanks,
-chris



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Paul Brook
> * Paul Brook (p...@codesourcery.com) wrote:
> > > > I find this argument contradictory. The migration code already needs
> > > > to check whether a device is compatible before it allows migration. 
> > > > The driver name is not sufficient to ensure compatibility, so I see
> > > > no benefit in including it in the device address.
> > > 
> > > See my comment above, I'm not seeing a sufficient argument about why
> > > driver name matching is a false sense of security.  If on an incoming
> > > migration I'm able to match the source provided e1000.03.0/vmstate
> > > against the target registered e1000.03.0/vmstate and hand off to the
> > > e1000 driver to check version ids, you bet I'm feeling a lot more
> > > secure than if I'm handing off to whatever happened to register
> > > 03.0/vmstate on the target.
> > 
> > I still say it should be the migration code that checks that both vmstate
> > structures are for the same type of device. i.e. if necessary the device
> > name should be embedded in the device state, not the device path.
> 
> I not sure how that fixes the ramblock issue that started this whole
> discussion.  It's not part of device's vmstate, it goes w/ ram.  I think
> I'm missing a piece of the puzzle here?

My main point there was that ram blocks should be associated with a device 
state.  Once you do this it should just work as we already know how to match 
device states.

It you're trying to transfer ram blocks before matching up the rest of the 
state then you're likely to loose in all kinds of different ways.

Paul



Re: [Qemu-devel] [PATCH] Fix comparison which always returned false

2010-06-15 Thread Richard Henderson
On 06/15/2010 02:28 PM, Anthony Liguori wrote:
> On 06/15/2010 04:03 PM, Stefan Weil wrote:
>> Comparing an 8 bit value with ~0 does not work as expected.
>> Replace ~0 by UINT8_MAX in comparison and also in assignment
>> (and fix coding style, too).
>>
> 
> Because when the uint8_t gets promoted, it doesn't get zero filled.  I'd
> rather something a bit more obvious like HPET_INVALID_COUNT.

Er, yes it does.  The problem is that it *did* get zero-extended,
but ~0 is 0x, so the comparison fails.

But I really agree with Jan Kiszka down-thread -- why do we need
to signal this as a special case at all?


r~



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Chris Wright
* Paul Brook (p...@codesourcery.com) wrote:
> > > I find this argument contradictory. The migration code already needs to
> > > check whether a device is compatible before it allows migration.  The
> > > driver name is not sufficient to ensure compatibility, so I see no
> > > benefit in including it in the device address.
> > 
> > See my comment above, I'm not seeing a sufficient argument about why
> > driver name matching is a false sense of security.  If on an incoming
> > migration I'm able to match the source provided e1000.03.0/vmstate
> > against the target registered e1000.03.0/vmstate and hand off to the
> > e1000 driver to check version ids, you bet I'm feeling a lot more secure
> > than if I'm handing off to whatever happened to register 03.0/vmstate on
> > the target.
> 
> I still say it should be the migration code that checks that both vmstate 
> structures are for the same type of device. i.e. if necessary the device name 
> should be embedded in the device state, not the device path.

I not sure how that fixes the ramblock issue that started this whole
discussion.  It's not part of device's vmstate, it goes w/ ram.  I think
I'm missing a piece of the puzzle here?

thanks,
-chris



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Paul Brook
> > I find this argument contradictory. The migration code already needs to
> > check whether a device is compatible before it allows migration.  The
> > driver name is not sufficient to ensure compatibility, so I see no
> > benefit in including it in the device address.
> 
> See my comment above, I'm not seeing a sufficient argument about why
> driver name matching is a false sense of security.  If on an incoming
> migration I'm able to match the source provided e1000.03.0/vmstate
> against the target registered e1000.03.0/vmstate and hand off to the
> e1000 driver to check version ids, you bet I'm feeling a lot more secure
> than if I'm handing off to whatever happened to register 03.0/vmstate on
> the target.

I still say it should be the migration code that checks that both vmstate 
structures are for the same type of device. i.e. if necessary the device name 
should be embedded in the device state, not the device path.

Paul



[Qemu-devel] [PATCH] hpet: Clean up initial hpet counter

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.

Signed-off-by: Jan Kiszka 
---
 hw/hpet.c |7 +--
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/hw/hpet.c b/hw/hpet.c
index d5c406c..ed4e995 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -74,7 +74,7 @@ typedef struct HPETState {
 uint8_t  hpet_id;   /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {
@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
 int i, iomemtype;
 HPETTimer *timer;
 
-if (hpet_cfg.count == UINT8_MAX) {
-/* first instance */
-hpet_cfg.count = 0;
-}
-
 if (hpet_cfg.count == 8) {
 fprintf(stderr, "Only 8 instances of HPET is allowed\n");
 return -1;
-- 
1.6.0.2



[Qemu-devel] VLIW?

2010-06-15 Thread Gibbons, Scott
Not sure if this got through since I had stuff in my signature.

Has anyone done a port of QEMU to a VLIW architecture?  I'm interested in 
seeing what was done.

Thanks,
--Scott




[Qemu-devel] [PATCH v4 22/23] QMP: Fix python helper /wrt long return strings

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Remove the arbitrary limitation of 1024 characters per return string and
read complete lines instead. Required for device_show.

Signed-off-by: Jan Kiszka 
---
 QMP/qmp.py |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index d9da603..4062f84 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -63,10 +63,14 @@ class QEMUMonitorProtocol:
 
 def __json_read(self):
 try:
-return json.loads(self.sock.recv(1024))
+while True:
+line = json.loads(self.sockfile.readline())
+if not 'event' in line:
+return line
 except ValueError:
 return
 
 def __init__(self, filename):
 self.filename = filename
 self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+self.sockfile = self.sock.makefile()
-- 
1.6.0.2




Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Alex Williamson
On Wed, 2010-06-16 at 00:01 +0100, Paul Brook wrote:
> > > I find this argument contradictory. The migration code already needs to
> > > check whether a device is compatible before it allows migration.  The
> > > driver name is not sufficient to ensure compatibility, so I see no
> > > benefit in including it in the device address.
> > 
> > See my comment above, I'm not seeing a sufficient argument about why
> > driver name matching is a false sense of security.  If on an incoming
> > migration I'm able to match the source provided e1000.03.0/vmstate
> > against the target registered e1000.03.0/vmstate and hand off to the
> > e1000 driver to check version ids, you bet I'm feeling a lot more secure
> > than if I'm handing off to whatever happened to register 03.0/vmstate on
> > the target.
> 
> I still say it should be the migration code that checks that both vmstate 
> structures are for the same type of device. i.e. if necessary the device name 
> should be embedded in the device state, not the device path.

The migration code would check that ("%s/%s", path, name) match.  So
embedding the driver name into path gives us a per path namespace.  Sure
the migration code could check ("%s/%s/%s, path, dev->info->name, name),
but should it be the migration code's responsibility to dig that out?
And if you think that i440FX-pcihost is a useful driver name, then we'd
have to figure out which driver names are useful.  I think it's more
consistent to simply put them all there.  Thanks,

Alex





[Qemu-devel] [PATCH v4 20/23] monitor: Add basic device state visualization

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This introduces device_show, a monitor command that saves the vmstate of
a qdev device and visualizes it. Buffers are cut after 16 byte by
default, but the full content can be requested via '-f'. To pretty-print
sub-arrays, vmstate is extended to store the start index name. A new
qerror is introduced to signal a missing vmstate. QMP is not supported
as we cannot provide a stable interface, at least at this point.

Signed-off-by: Jan Kiszka 
---
 hw/hw.h |2 +
 hw/qdev.c   |  248 +++
 hw/qdev.h   |2 +
 qemu-monitor.hx |   20 +
 qerror.c|4 +
 qerror.h|3 +
 6 files changed, 279 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index a49d866..11b52e3 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -298,6 +298,7 @@ enum VMStateFlags {
 
 typedef struct {
 const char *name;
+const char *start_index;
 size_t offset;
 size_t size;
 size_t start;
@@ -412,6 +413,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
 .size   = sizeof(_type), \
 .flags  = VMS_ARRAY, \
 .offset = vmstate_offset_sub_array(_state, _field, _type, _start), \
+.start_index = (stringify(_start)),  \
 }
 
 #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, 
_type) {\
diff --git a/hw/qdev.c b/hw/qdev.c
index dbc5b10..1a84ff5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,9 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qjson.h"
+#include "qint.h"
+#include "qbuffer.h"
 
 static int qdev_hotplug = 0;
 
@@ -887,3 +890,248 @@ int do_device_del(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 }
 return qdev_unplug(dev);
 }
+
+#define NAME_COLUMN_WIDTH 23
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent);
+
+static void print_elem(Monitor *mon, const QObject *qelem, size_t size,
+   int column_pos, int indent)
+{
+int64_t data_size;
+const void *data;
+int n;
+
+if (qobject_type(qelem) == QTYPE_QDICT) {
+if (column_pos >= 0) {
+monitor_printf(mon, ".\n");
+}
+} else {
+monitor_printf(mon, ":");
+column_pos++;
+if (column_pos < NAME_COLUMN_WIDTH) {
+monitor_printf(mon, "%*c", NAME_COLUMN_WIDTH - column_pos, ' ');
+}
+}
+
+switch (qobject_type(qelem)) {
+case QTYPE_QDICT:
+print_field(mon, qobject_to_qdict(qelem), indent + 2);
+break;
+case QTYPE_QBUFFER:
+data = qbuffer_get_data(qobject_to_qbuffer(qelem));
+data_size = qbuffer_get_size(qobject_to_qbuffer(qelem));
+for (n = 0; n < data_size; ) {
+monitor_printf(mon, " %02x", *((uint8_t *)data+n));
+if (++n < size) {
+if (n % 16 == 0) {
+monitor_printf(mon, "\n%*c", NAME_COLUMN_WIDTH, ' ');
+} else if (n % 8 == 0) {
+monitor_printf(mon, " -");
+}
+}
+}
+if (data_size < size) {
+monitor_printf(mon, " ...");
+}
+monitor_printf(mon, "\n");
+break;
+case QTYPE_QINT:
+monitor_printf(mon, " %0*" PRIx64 "\n", (int)size * 2,
+   qint_get_int(qobject_to_qint(qelem)));
+break;
+default:
+assert(0);
+}
+}
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent)
+{
+const char *name = qdict_get_str(qfield, "name");
+const char *start = qdict_get_try_str(qfield, "start");
+int64_t size = qdict_get_int(qfield, "size");
+QList *qlist = qdict_get_qlist(qfield, "elems");
+QListEntry *entry, *sub_entry;
+QList *sub_list;
+int elem_no = 0;
+
+QLIST_FOREACH_ENTRY(qlist, entry) {
+QObject *qelem = qlist_entry_obj(entry);
+int pos = indent + strlen(name);
+
+if (qobject_type(qelem) == QTYPE_QLIST) {
+monitor_printf(mon, "%*c%s", indent, ' ', name);
+if (start) {
+pos += monitor_printf(mon, "[%s+%02x]", start, elem_no);
+} else {
+pos += monitor_printf(mon, "[%02x]", elem_no);
+}
+sub_list = qobject_to_qlist(qelem);
+QLIST_FOREACH_ENTRY(sub_list, sub_entry) {
+print_elem(mon, qlist_entry_obj(sub_entry), size, pos,
+   indent + 2);
+pos = -1;
+}
+} else {
+if (elem_no == 0) {
+monitor_printf(mon, "%*c%s", indent, ' ', name);
+} else {
+pos = -1;
+}
+print_elem(mon, qelem, size, pos, indent);
+}
+elem_no++;
+}
+}
+
+void device_user_print(Monitor *mon, const QObject *data)
+{
+QDict *qdict = qobject_to_qdict(data);

[Qemu-devel] [PATCH v4 18/23] QMP: Reserve namespace for complex object classes

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This reserves JSON objects that contain the key '__class__' for QMP-specific
complex objects. First user will be the buffer class.

Signed-off-by: Jan Kiszka 
---
 QMP/qmp-spec.txt |   16 +---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 9d30a8c..fa1dd62 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -146,6 +146,15 @@ The format is:
 For a listing of supported asynchronous events, please, refer to the
 qmp-events.txt file.
 
+2.6 Complex object classes
+--
+
+JSON objects that contain the key-value pair '"__class__": json-string' are
+reserved for QMP-specific complex object classes that. QMP specifies which
+further keys each of these objects include and how they are encoded.
+
+So far, no complex object class is specified.
+
 3. QMP Examples
 ===
 
@@ -229,9 +238,10 @@ avoid modifying QMP.  Both upstream and downstream need to 
take care to
 preserve long-term compatibility and interoperability.
 
 To help with that, QMP reserves JSON object member names beginning with
-'__' (double underscore) for downstream use ("downstream names").  This
-means upstream will never use any downstream names for its commands,
-arguments, errors, asynchronous events, and so forth.
+'__' (double underscore) for downstream use ("downstream names").  Downstream
+names MUST NOT end with '__' as this pattern is reserved for QMP-defined JSON
+object classes.  Upstream will never use any downstream names for its
+commands, arguments, errors, asynchronous events, and so forth.
 
 Any new names downstream wishes to add must begin with '__'.  To
 ensure compatibility with other downstreams, it is strongly
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 23/23] QMP: Add support for buffer class to qmp python helper

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This demonstrates the conversion of QMP buffer objects and does some
minimalistic pretty-printing.

Signed-off-by: Jan Kiszka 
---
 QMP/qmp.py |   25 +++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index 4062f84..4650918 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -8,7 +8,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.  See
 # the COPYING file in the top-level directory.
 
-import socket, json
+import socket, json, binascii
 
 class QMPError(Exception):
 pass
@@ -16,6 +16,18 @@ class QMPError(Exception):
 class QMPConnectError(QMPError):
 pass
 
+class QMPBuffer:
+def __init__(self, data):
+self.data = binascii.a2b_base64(data)
+
+def __repr__(self):
+str = ''
+for i in range(0, len(self.data)):
+if i > 0:
+str += ' '
+str += binascii.b2a_hex(self.data[i])
+return str
+
 class QEMUMonitorProtocol:
 def connect(self):
 self.sock.connect(self.filename)
@@ -61,10 +73,19 @@ class QEMUMonitorProtocol:
 # the Server won't read our input
 self.sock.send(json.dumps(cmd) + ' ')
 
+def __json_obj_hook(self, dct):
+if '__class__' in dct:
+if dct['__class__'] == 'buffer':
+return QMPBuffer(dct['data'])
+else:
+return
+return dct
+
 def __json_read(self):
 try:
 while True:
-line = json.loads(self.sockfile.readline())
+line = json.loads(self.sockfile.readline(),
+  object_hook=self.__json_obj_hook)
 if not 'event' in line:
 return line
 except ValueError:
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 16/23] monitor: Allow to exclude commands from QMP

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Ported commands that are marked 'user_only' will not be considered for
QMP monitor sessions. This allows to implement new commands that do not
(yet) provide a sufficiently stable interface for QMP use (e.g.
device_show).

Signed-off-by: Jan Kiszka 
---
 monitor.c |   18 +++---
 monitor.h |1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 2d3d70d..9c0a91c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -343,6 +343,11 @@ static inline bool monitor_handler_is_async(const 
mon_cmd_t *cmd)
 return cmd->flags & MONITOR_CMD_ASYNC;
 }
 
+static inline bool monitor_cmd_user_only(const mon_cmd_t *cmd)
+{
+return (cmd->flags & MONITOR_CMD_USER_ONLY);
+}
+
 static inline int monitor_has_error(const Monitor *mon)
 {
 return mon->error != NULL;
@@ -625,6 +630,11 @@ static int do_info(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 goto help;
 }
 
+if (monitor_ctrl_mode(mon) && monitor_cmd_user_only(cmd)) {
+qerror_report(QERR_COMMAND_NOT_FOUND, item);
+return -1;
+}
+
 if (monitor_handler_is_async(cmd)) {
 if (monitor_ctrl_mode(mon)) {
 qmp_async_info_handler(mon, cmd);
@@ -722,13 +732,14 @@ static void do_info_commands(Monitor *mon, QObject 
**ret_data)
 cmd_list = qlist_new();
 
 for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
-if (monitor_handler_ported(cmd) && !compare_cmd(cmd->name, "info")) {
+if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
+!compare_cmd(cmd->name, "info")) {
 qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
 }
 }
 
 for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-if (monitor_handler_ported(cmd)) {
+if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd)) {
 char buf[128];
 snprintf(buf, sizeof(buf), "query-%s", cmd->name);
 qlist_append_obj(cmd_list, get_cmd_dict(buf));
@@ -4376,7 +4387,8 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
   qobject_from_jsonf("{ 'item': %s }", info_item));
 } else {
 cmd = monitor_find_command(cmd_name);
-if (!cmd || !monitor_handler_ported(cmd)) {
+if (!cmd || !monitor_handler_ported(cmd)
+|| monitor_cmd_user_only(cmd)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
 goto err_input;
 }
diff --git a/monitor.h b/monitor.h
index e3f0119..ec1fe42 100644
--- a/monitor.h
+++ b/monitor.h
@@ -17,6 +17,7 @@ extern Monitor *default_mon;
 
 /* flags for monitor commands */
 #define MONITOR_CMD_ASYNC   0x0001
+#define MONITOR_CMD_USER_ONLY   0x0002
 
 /* QMP events */
 typedef enum MonitorEvent {
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 13/23] monitor: Allow to specify HMP-specifc command arguments

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

As we may want to shrink or enhance the argument set used for monitor
command in HMP mode, add a separate, optional argument string for that
case. When an HMP request is parsed, this argument string, if available,
takes precedence over the standard string.

Signed-off-by: Jan Kiszka 
---
 monitor.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index f535c56..7139c4e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -108,6 +108,7 @@ typedef struct mon_cmd_t {
 const char *params;
 const char *help;
 void (*user_print)(Monitor *mon, const QObject *data);
+const char *user_args_type;
 union {
 void (*info)(Monitor *mon);
 void (*info_new)(Monitor *mon, QObject **ret_data);
@@ -3310,7 +3311,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 }
 
 /* parse the parameters */
-typestr = cmd->args_type;
+typestr = cmd->user_args_type ? : cmd->args_type;
 for(;;) {
 typestr = key_get_info(typestr, &key);
 if (!typestr)
@@ -4040,7 +4041,7 @@ static void monitor_find_completion(const char *cmdline)
 goto cleanup;
 }
 
-ptype = next_arg_type(cmd->args_type);
+ptype = next_arg_type(cmd->user_args_type ? : cmd->args_type);
 for(i = 0; i < nb_args - 2; i++) {
 if (*ptype != '\0') {
 ptype = next_arg_type(ptype);
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 10/23] monitor: Fix command completion vs. boolean switches

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

We now have to move forward to the next argument type via next_arg_type.
This patch fixes completion for 'eject' and maybe also other commands.

Signed-off-by: Jan Kiszka 
---
 monitor.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 242aee6..c1006b4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3913,7 +3913,7 @@ static void monitor_find_completion(const char *cmdline)
 }
 str = args[nb_args - 1];
 if (*ptype == '-' && ptype[1] != '\0') {
-ptype += 2;
+ptype = next_arg_type(ptype);
 }
 switch(*ptype) {
 case 'F':
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 17/23] Add base64 encoder/decoder

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Will be used by QBuffer.

Signed-off-by: Jan Kiszka 
---
 Makefile.objs |2 +-
 base64.c  |  202 +
 base64.h  |   19 ++
 3 files changed, 222 insertions(+), 1 deletions(-)
 create mode 100644 base64.c
 create mode 100644 base64.h

diff --git a/Makefile.objs b/Makefile.objs
index 2bfb6d1..670b8a8 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,7 @@
 # QObject
 qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
-qobject-obj-y += qerror.o
+qobject-obj-y += qerror.o base64.o
 
 ###
 # block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/base64.c b/base64.c
new file mode 100644
index 000..750d0fb
--- /dev/null
+++ b/base64.c
@@ -0,0 +1,202 @@
+/*
+ * Base64 encoder/decoder conforming to RFC 4648
+ * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka 
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include 
+#include "base64.h"
+
+static const char base[] =
+"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+static void encode3to4(const uint8_t *src, char *dest)
+{
+uint32_t b32 = 0;
+int i, j = 18;
+
+for (i = 0; i < 3; i++) {
+b32 <<= 8;
+b32 |= src[i];
+}
+for (i = 0; i < 4; i++) {
+dest[i] = base[(b32 >> j) & 0x3F];
+j -= 6;
+}
+}
+
+static void encode2to4(const uint8_t *src, char *dest)
+{
+dest[0] = base[(src[0] >> 2) & 0x3F];
+dest[1] = base[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0F)];
+dest[2] = base[(src[1] & 0x0F) << 2];
+dest[3] = '=';
+}
+
+static void encode1to4(const uint8_t *src, char *dest)
+{
+dest[0] = base[(src[0] >> 2) & 0x3F];
+dest[1] = base[(src[0] & 0x03) << 4];
+dest[2] = '=';
+dest[3] = '=';
+}
+
+/*
+ * Encode data in 'src' of length 'srclen' to a base64 string, saving the
+ * null-terminated result in 'dest'. The size of the destition buffer must be
+ * at least ((srclen + 2) / 3) * 4 + 1.
+ */
+void base64_encode(const uint8_t *src, size_t srclen, char *dest)
+{
+while (srclen >= 3) {
+encode3to4(src, dest);
+src += 3;
+dest += 4;
+srclen -= 3;
+}
+switch (srclen) {
+case 2:
+encode2to4(src, dest);
+dest += 4;
+break;
+case 1:
+encode1to4(src, dest);
+dest += 4;
+break;
+case 0:
+break;
+}
+dest[0] = 0;
+}
+
+static int32_t codetovalue(char c)
+{
+if (c >= 'A' && c <= 'Z') {
+return c - 'A';
+} else if (c >= 'a' && c <= 'z') {
+return c - 'a' + 26;
+} else if (c >= '0' && c <= '9') {
+return c - '0' + 52;
+} else if (c == '+') {
+return 62;
+} else if ( c == '/') {
+return 63;
+} else {
+return -1;
+}
+}
+
+static int decode4to3 (const char *src, uint8_t *dest)
+{
+uint32_t b32 = 0;
+int32_t bits;
+int i;
+
+for (i = 0; i < 4; i++) {
+bits = codetovalue(src[i]);
+if (bits < 0) {
+return bits;
+}
+b32 <<= 6;
+b32 |= bits;
+}
+dest[0] = (b32 >> 16) & 0xFF;
+dest[1] = (b32 >> 8) & 0xFF;
+dest[2] = b32 & 0xFF;
+
+return 0;
+}
+
+static int decode3to2(const char *src, uint8_t *dest)
+{
+uint32_t b32 = 0;
+int32_t bits;
+
+bits = codetovalue(src[0]);
+if (bits < 0) {
+return bits;
+}
+b32 = (uint32_t)bits;
+b32 <<= 6;
+
+bits = codetovalue(src[1]);
+if (bits < 0) {
+return bits;
+}
+b32 |= (uint32_t)bits;
+b32 <<= 4;
+
+bits = codetovalue(src[2]);
+if (bits < 0) {
+return bits;
+}
+b32 |= ((uint32_t)bits) >> 2;
+
+dest[0] = (b32 >> 8) & 0xFF;
+dest[1] = b32 & 0xFF;
+
+return 0;
+}
+
+static int decode2to1(const char *src, uint8_t *dest)
+{
+uint32_t b32;
+int32_t bits;
+
+bits = codetovalue(src[0]);
+if (bits < 0) {
+return bits;
+}
+b32 = (uint32_t)bits << 2;
+
+bits = codetovalue(src[1]);
+if (bits < 0) {
+return bits;
+}
+b32 |= ((uint32_t)bits) >> 4;
+
+dest[0] = b32;
+
+return 0;
+}
+
+/*
+ * Convert string 'src' of length 'srclen' from base64 to binary form,
+ * saving the result in 'dest'. The size of the destination buffer must be at
+ * least srclen * 3 / 4.
+ *
+ * Returns 0 on success, -1 on conversion error.
+ */
+int base64_decode(const char *src, size_t srclen, uint8_t *dest)
+{
+int ret;
+
+while (srclen >= 4) {
+ret = decode4to3(src, dest);
+if (ret < 0) {
+return ret;
+}
+src += 4;
+dest += 3;
+srcl

[Qemu-devel] [PATCH v4 00/23] qdev path reworks & basic device state visualization

2010-06-15 Thread Jan Kiszka
This is v4 of this series. Besides small fixes, it's main focus is on
the groundwork for the visualization command: qdev path usability.

The significant changes are:
 - drop many of the inconsistent or ambiguous qtree abbreviations
 - devices can now be address via unique ID (no '/' allowed here) or an
   absolute path (must start with '/')
 - buses remain addressable via their ambiguous local name (mostly to
   avoid libvirt breakages) or their absolute path
 - the per-bus instance numbers introduced in this series are now
   printed consistently
 - monitor command completion inside option lists is added
   (allows to expand "device_add ...,bus=" and "drive=")
 - HMP commands can now have their own argument set (not yet urgently
   needed for device_show but likely already useful for other commands)

Git url as usual:

git://git.kiszka.org/qemu.git queues/device-show

Thanks once again for comments. Hope we can soon agree on a mergable
version for 0.13.

Jan Kiszka (23):
  qdev: Rework qtree path abbreviations
  qdev: Restrict direct bus addressing via its name
  qdev: Drop ID matching from qtree paths
  qdev: Allow device addressing via 'driver.instance'
  qdev: Convert device and bus lists to QTAILQ
  qdev: Push QMP mode checks into qbus_list_bus/dev
  qdev: Allow device specification by qtree path for device_del
  qdev: Introduce qdev_iterate_recursive
  monitor: Fix leakage during completion processing
  monitor: Fix command completion vs. boolean switches
  monitor: Add completion support for option lists
  monitor: Add completion for qdev paths
  monitor: Allow to specify HMP-specifc command arguments
  monitor: return length of printed string via monitor_[v]printf
  monitor: Establish cmd flags and convert the async tag
  monitor: Allow to exclude commands from QMP
  Add base64 encoder/decoder
  QMP: Reserve namespace for complex object classes
  QMP: Add QBuffer
  monitor: Add basic device state visualization
  QMP: Teach basic capability negotiation to python example
  QMP: Fix python helper /wrt long return strings
  QMP: Add support for buffer class to qmp python helper

 Makefile |5 +-
 Makefile.objs|4 +-
 QMP/qmp-shell|1 +
 QMP/qmp-spec.txt |   24 ++-
 QMP/qmp.py   |   29 +++-
 QMP/vm-info  |1 +
 base64.c |  202 +++
 base64.h |   19 ++
 check-qbuffer.c  |  172 
 configure|2 +-
 docs/qdev-device-use.txt |   13 ++-
 hw/acpi_piix4.c  |2 +-
 hw/hw.h  |2 +
 hw/i2c.c |2 +-
 hw/pci-hotplug.c |2 +-
 hw/qdev.c|  488 ++
 hw/qdev.h|   16 ++-
 hw/ssi.c |6 +-
 monitor.c|  258 +
 monitor.h|8 +-
 qbuffer.c|  116 +++
 qbuffer.h|   33 +++
 qemu-monitor.hx  |   34 +++-
 qemu-tool.c  |6 +-
 qerror.c |8 +-
 qerror.h |6 +-
 qjson.c  |   15 ++
 qobject.h|1 +
 28 files changed, 1315 insertions(+), 160 deletions(-)
 create mode 100644 base64.c
 create mode 100644 base64.h
 create mode 100644 check-qbuffer.c
 create mode 100644 qbuffer.c
 create mode 100644 qbuffer.h




[Qemu-devel] [PATCH v4 14/23] monitor: return length of printed string via monitor_[v]printf

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This simply forwards the result of the internal vsnprintf to the callers
of monitor_printf and monitor_vprintf. When invoked over a QMP session
or in absence of an active monitor, -1 is returned.

Signed-off-by: Jan Kiszka 
---
 monitor.c   |   23 +++
 monitor.h   |4 ++--
 qemu-tool.c |6 --
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/monitor.c b/monitor.c
index 7139c4e..aa0bdd6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -263,29 +263,36 @@ static void monitor_puts(Monitor *mon, const char *str)
 }
 }
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
 char buf[4096];
+int ret;
 
-if (!mon)
-return;
-
+if (!mon) {
+return -1;
+}
 mon_print_count_inc(mon);
 
 if (monitor_ctrl_mode(mon)) {
-return;
+return -1;
 }
 
-vsnprintf(buf, sizeof(buf), fmt, ap);
+ret = vsnprintf(buf, sizeof(buf), fmt, ap);
 monitor_puts(mon, buf);
+
+return ret;
 }
 
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 {
 va_list ap;
+int ret;
+
 va_start(ap, fmt);
-monitor_vprintf(mon, fmt, ap);
+ret = monitor_vprintf(mon, fmt, ap);
 va_end(ap);
+
+return ret;
 }
 
 void monitor_print_filename(Monitor *mon, const char *filename)
diff --git a/monitor.h b/monitor.h
index ea15469..32c0170 100644
--- a/monitor.h
+++ b/monitor.h
@@ -45,8 +45,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
 
 int monitor_get_fd(Monitor *mon, const char *fdname);
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 __attribute__ ((__format__ (__printf__, 2, 3)));
 void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
diff --git a/qemu-tool.c b/qemu-tool.c
index b39af86..f6ce6cd 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -43,12 +43,14 @@ void monitor_set_error(Monitor *mon, QError *qerror)
 {
 }
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
+return -1;
 }
 
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 {
+return -1;
 }
 
 void monitor_print_filename(Monitor *mon, const char *filename)
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 19/23] QMP: Add QBuffer

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This introduces a buffer object for use with QMP. As a buffer is not
natively encodable in JSON, we encode it as a base64 string and
encapsulate the result in the new QMP object class "buffer".

The first use case for this is pushing the content of buffers that are
part of a device state into a qdict.

Signed-off-by: Jan Kiszka 
---
 Makefile |5 +-
 Makefile.objs|2 +-
 QMP/qmp-spec.txt |   10 +++-
 check-qbuffer.c  |  172 ++
 configure|2 +-
 qbuffer.c|  116 
 qbuffer.h|   33 ++
 qjson.c  |   15 +
 qobject.h|1 +
 9 files changed, 351 insertions(+), 5 deletions(-)
 create mode 100644 check-qbuffer.c
 create mode 100644 qbuffer.c
 create mode 100644 qbuffer.h

diff --git a/Makefile b/Makefile
index 221fbd8..2267261 100644
--- a/Makefile
+++ b/Makefile
@@ -146,14 +146,15 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o 
qemu-error.o $(block-obj-y) $(qobj
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
 
-check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o 
check-qjson.o: $(GENERATED_HEADERS)
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o 
check-qjson.o check-qbuffer: $(GENERATED_HEADERS)
 
 check-qint: check-qint.o qint.o qemu-malloc.o
 check-qstring: check-qstring.o qstring.o qemu-malloc.o
 check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o 
qemu-malloc.o qlist.o
 check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
 check-qfloat: check-qfloat.o qfloat.o qemu-malloc.o
-check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o 
qjson.o json-streamer.o json-lexer.o json-parser.o qemu-malloc.o
+check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o 
qbuffer.o base64.o qjson.o json-streamer.o json-lexer.o json-parser.o 
qemu-malloc.o
+check-qbuffer: check-qbuffer.o qbuffer.o base64.o qstring.o qemu-malloc.o
 
 clean:
 # avoid old build problems by removing potentially incorrect old files
diff --git a/Makefile.objs b/Makefile.objs
index 670b8a8..82bc63f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
 ###
 # QObject
-qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qbuffer.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
 qobject-obj-y += qerror.o base64.o
 
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index fa1dd62..820e39d 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -153,7 +153,15 @@ JSON objects that contain the key-value pair '"__class__": 
json-string' are
 reserved for QMP-specific complex object classes that. QMP specifies which
 further keys each of these objects include and how they are encoded.
 
-So far, no complex object class is specified.
+2.6.1 Buffer class
+--
+
+This QMP object class allows to transport binary data. A buffer object
+consists of the following keys:
+
+{ "__class__": "buffer", "data": json-string }
+
+The data string is base64 encoded according to RFC 4648.
 
 3. QMP Examples
 ===
diff --git a/check-qbuffer.c b/check-qbuffer.c
new file mode 100644
index 000..b490230
--- /dev/null
+++ b/check-qbuffer.c
@@ -0,0 +1,172 @@
+/*
+ * QBuffer unit-tests.
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka 
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include 
+
+#include "qbuffer.h"
+#include "qemu-common.h"
+
+const char data[] = "some data";
+
+START_TEST(qbuffer_from_data_test)
+{
+QBuffer *qbuffer;
+
+qbuffer = qbuffer_from_data(data, sizeof(data));
+fail_unless(qbuffer != NULL);
+fail_unless(qbuffer->base.refcnt == 1);
+fail_unless(memcmp(data, qbuffer->data, sizeof(data)) == 0);
+fail_unless(qbuffer->size == sizeof(data));
+fail_unless(qobject_type(QOBJECT(qbuffer)) == QTYPE_QBUFFER);
+
+/* destroy doesn't exit yet */
+qemu_free(qbuffer->data);
+qemu_free(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_destroy_test)
+{
+QBuffer *qbuffer = qbuffer_from_data(data, sizeof(data));
+
+QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_data_test)
+{
+QBuffer *qbuffer;
+const void *ret_data;
+
+qbuffer = qbuffer_from_data(data, sizeof(data));
+ret_data = qbuffer_get_data(qbuffer);
+fail_unless(memcmp(ret_data, data, sizeof(data)) == 0);
+
+QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_size_test)
+{
+QBuffer *qbuffer;
+
+qbuffer = qbuffer_from_data(data, sizeof(data));
+fail_unless(qbuffer_get_size(qbuffer) == sizeof(data));
+
+QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_from_qs

[Qemu-devel] [PATCH v4 09/23] monitor: Fix leakage during completion processing

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Given too many arguments or an invalid command, we were leaking the
duplicated argument strings.

Signed-off-by: Jan Kiszka 
---
 monitor.c |   23 +++
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index 05a7ed1..242aee6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3877,8 +3877,9 @@ static void monitor_find_completion(const char *cmdline)
next arg */
 len = strlen(cmdline);
 if (len > 0 && qemu_isspace(cmdline[len - 1])) {
-if (nb_args >= MAX_ARGS)
-return;
+if (nb_args >= MAX_ARGS) {
+goto cleanup;
+}
 args[nb_args++] = qemu_strdup("");
 }
 if (nb_args <= 1) {
@@ -3893,12 +3894,15 @@ static void monitor_find_completion(const char *cmdline)
 }
 } else {
 /* find the command */
-for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
-if (compare_cmd(args[0], cmd->name))
-goto found;
+for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
+if (compare_cmd(args[0], cmd->name)) {
+break;
+}
 }
-return;
-found:
+if (!cmd->name) {
+goto cleanup;
+}
+
 ptype = next_arg_type(cmd->args_type);
 for(i = 0; i < nb_args - 2; i++) {
 if (*ptype != '\0') {
@@ -3948,8 +3952,11 @@ static void monitor_find_completion(const char *cmdline)
 break;
 }
 }
-for(i = 0; i < nb_args; i++)
+
+cleanup:
+for (i = 0; i < nb_args; i++) {
 qemu_free(args[i]);
+}
 }
 
 static int monitor_can_read(void *opaque)
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 06/23] qdev: Push QMP mode checks into qbus_list_bus/dev

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Simplifies the usage.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 25f6e62..ac450cf 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -514,6 +514,9 @@ static void qbus_list_bus(DeviceState *dev)
 BusState *child;
 const char *sep = " ";
 
+if (monitor_cur_is_qmp()) {
+return;
+}
 error_printf("child busses at \"%s.%d\":",
  dev->info->name, qdev_instance_no(dev));
 QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
@@ -528,6 +531,9 @@ static void qbus_list_dev(BusState *bus)
 DeviceState *dev;
 const char *sep = " ";
 
+if (monitor_cur_is_qmp()) {
+return;
+}
 error_printf("devices at \"%s\":", bus->name);
 QTAILQ_FOREACH(dev, &bus->children, sibling) {
 error_printf("%s\"%s.%d\"", sep, dev->info->name,
@@ -618,9 +624,7 @@ static BusState *qbus_find(const char *path)
 dev = qbus_find_dev(bus, elem);
 if (!dev) {
 qerror_report(QERR_DEVICE_NOT_FOUND, elem);
-if (!monitor_cur_is_qmp()) {
-qbus_list_dev(bus);
-}
+qbus_list_dev(bus);
 return NULL;
 }
 if (dev->num_child_bus == 0) {
@@ -641,9 +645,7 @@ static BusState *qbus_find(const char *path)
 bus = qbus_find_bus(dev, elem);
 if (!bus) {
 qerror_report(QERR_BUS_NOT_FOUND, elem);
-if (!monitor_cur_is_qmp()) {
-qbus_list_bus(dev);
-}
+qbus_list_bus(dev);
 return NULL;
 }
 }
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

This enables command line completion inside option strings. A list of
expected key names and their completion type can be appended to the 'O'
inside parentheses ('O(key:type,...)'). The first use case is block
device completion for the 'drive' option of 'device_add'.

Signed-off-by: Jan Kiszka 
---
 monitor.c   |   68 ++-
 qemu-monitor.hx |2 +-
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/monitor.c b/monitor.c
index c1006b4..3e0d862 100644
--- a/monitor.c
+++ b/monitor.c
@@ -68,6 +68,9 @@
  * 'O'  option string of the form NAME=VALUE,...
  *  parsed according to QemuOptsList given by its name
  *  Example: 'device:O' uses qemu_device_opts.
+ *  Command completion for specific keys can be requested via
+ *  appending '(NAME:TYPE,...)' with 'F', 'B' as type.
+ *  Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path.
  *  Restriction: only lists with empty desc are supported
  *  TODO lift the restriction
  * 'i'  32 bit integer
@@ -3353,6 +3356,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 QemuOptsList *opts_list;
 QemuOpts *opts;
 
+if (*typestr == '(') {
+while (*typestr++ != ')') {
+assert(*typestr != '\0');
+}
+}
 opts_list = qemu_find_opts(key);
 if (!opts_list || opts_list->desc->name) {
 goto bad_type;
@@ -3857,12 +3865,30 @@ static const char *next_arg_type(const char *typestr)
 return (p != NULL ? ++p : typestr);
 }
 
+static bool process_completion_type(char type, const char *str)
+{
+switch(type) {
+case 'F':
+/* file completion */
+readline_set_completion_index(cur_mon->rs, strlen(str));
+file_completion(str);
+return true;
+case 'B':
+/* block device name completion */
+readline_set_completion_index(cur_mon->rs, strlen(str));
+bdrv_iterate(block_completion_it, (void *)str);
+return true;
+default:
+return false;
+}
+}
+
 static void monitor_find_completion(const char *cmdline)
 {
 const char *cmdname;
 char *args[MAX_ARGS];
 int nb_args, i, len;
-const char *ptype, *str;
+const char *ptype, *str, *opt, *sep;
 const mon_cmd_t *cmd;
 const KeyDef *key;
 
@@ -3915,16 +3941,31 @@ static void monitor_find_completion(const char *cmdline)
 if (*ptype == '-' && ptype[1] != '\0') {
 ptype = next_arg_type(ptype);
 }
+if (process_completion_type(*ptype, str)) {
+goto cleanup;
+}
 switch(*ptype) {
-case 'F':
-/* file completion */
-readline_set_completion_index(cur_mon->rs, strlen(str));
-file_completion(str);
-break;
-case 'B':
-/* block device name completion */
-readline_set_completion_index(cur_mon->rs, strlen(str));
-bdrv_iterate(block_completion_it, (void *)str);
+case 'O':
+sep = strrchr(str, ',');
+opt = sep ? sep + 1 : str;
+sep = strchr(opt, '=');
+if (!sep) {
+break;
+}
+len = sep - opt;
+str = sep + 1;
+ptype += 2;
+while (*ptype != ')') {
+if (strlen(ptype) > len+1 && ptype[len] == ':' &&
+strncmp(ptype, opt, len) == 0) {
+process_completion_type(ptype[len+1], str);
+}
+while (*ptype++ != ',') {
+if (*ptype == ')') {
+break;
+}
+}
+}
 break;
 case 's':
 /* XXX: more generic ? */
@@ -3934,7 +3975,7 @@ static void monitor_find_completion(const char *cmdline)
 cmd_completion(str, cmd->name);
 }
 } else if (!strcmp(cmd->name, "sendkey")) {
-char *sep = strrchr(str, '-');
+sep = strrchr(str, '-');
 if (sep)
 str = sep + 1;
 readline_set_completion_index(cur_mon->rs, strlen(str));
@@ -4114,6 +4155,11 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, 
QDict *args)
 cmd_args.flag = *p++;
 cmd_args.optional = 1;
 } else if (cmd_args.type == 'O') {
+if (*p == '(') {
+while (*p++ != ')') {
+assert(*p != '\0');
+}
+}
 opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
 assert(opts_list);
 } else if (*p == '?') {
diff --git a/qemu-monitor.hx b/qemu-monitor.h

[Qemu-devel] [PATCH v4 12/23] monitor: Add completion for qdev paths

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Implement monitor command line completion for device tree paths. The
first user are device_add ('bus' option) and device_del.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c   |   19 +
 hw/qdev.h   |3 +
 monitor.c   |  115 ++-
 qemu-monitor.hx |4 +-
 4 files changed, 129 insertions(+), 12 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 466d8d5..dbc5b10 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -39,7 +39,6 @@ DeviceInfo *device_info_list;
 
 static BusState *qbus_find_recursive(BusState *bus, const char *name,
  const BusInfo *info);
-static BusState *qbus_find(const char *path, bool report_errors);
 
 /* Register a new device type.  */
 void qdev_register(DeviceInfo *info)
@@ -514,7 +513,7 @@ static DeviceState *qdev_find_id_recursive(BusState *bus, 
const char *id)
 return qdev_iterate_recursive(bus, find_id_callback, (void *)id);
 }
 
-static int qdev_instance_no(DeviceState *dev)
+int qdev_instance_no(DeviceState *dev)
 {
 struct DeviceState *sibling;
 int instance = 0;
@@ -611,7 +610,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char 
*elem)
 return NULL;
 }
 
-static BusState *qbus_find(const char *path, bool report_errors)
+BusState *qbus_find(const char *path, bool report_errors)
 {
 DeviceState *dev;
 BusState *bus = main_system_bus;
@@ -678,7 +677,7 @@ static BusState *qbus_find(const char *path, bool 
report_errors)
 }
 }
 
-static DeviceState *qdev_find(const char *path)
+DeviceState *qdev_find(const char *path, bool report_errors)
 {
 const char *dev_name;
 DeviceState *dev;
@@ -688,7 +687,7 @@ static DeviceState *qdev_find(const char *path)
 /* search for unique ID recursively if path is not absolute */
 if (path[0] != '/') {
 dev = qdev_find_id_recursive(main_system_bus, path);
-if (!dev) {
+if (!dev && report_errors) {
 qerror_report(QERR_DEVICE_NOT_FOUND, path);
 }
 return dev;
@@ -702,8 +701,10 @@ static DeviceState *qdev_find(const char *path)
 bus = qbus_find(bus_path, false);
 qemu_free(bus_path);
 if (!bus) {
-/* retry with full path to generate correct error message */
-bus = qbus_find(path, true);
+if (report_errors) {
+/* retry with full path to generate correct error message */
+bus = qbus_find(path, report_errors);
+}
 if (!bus) {
 return NULL;
 }
@@ -711,7 +712,7 @@ static DeviceState *qdev_find(const char *path)
 }
 
 dev = qbus_find_dev(bus, dev_name);
-if (!dev) {
+if (!dev && report_errors) {
 qerror_report(QERR_DEVICE_NOT_FOUND, dev_name);
 qbus_list_dev(bus);
 }
@@ -880,7 +881,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 const char *path = qdict_get_str(qdict, "device");
 DeviceState *dev;
 
-dev = qdev_find(path);
+dev = qdev_find(path, true);
 if (!dev) {
 return -1;
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index 111c876..59159a0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -171,6 +171,8 @@ CharDriverState *qdev_init_chardev(DeviceState *dev);
 BusState *qdev_get_parent_bus(DeviceState *dev);
 void *qdev_iterate_recursive(BusState *bus, qdev_iteratefn callback,
  void *opaque);
+DeviceState *qdev_find(const char *path, bool report_errors);
+int qdev_instance_no(DeviceState *dev);
 
 /*** BUS API. ***/
 
@@ -178,6 +180,7 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
  DeviceState *parent, const char *name);
 BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
 void qbus_free(BusState *bus);
+BusState *qbus_find(const char *path, bool report_errors);
 
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
 
diff --git a/monitor.c b/monitor.c
index 3e0d862..f535c56 100644
--- a/monitor.c
+++ b/monitor.c
@@ -64,12 +64,14 @@
  *
  * 'F'  filename
  * 'B'  block device name
+ * 'q'  qdev bus path
+ * 'Q'  qdev device path
  * 's'  string (accept optional quote)
  * 'O'  option string of the form NAME=VALUE,...
  *  parsed according to QemuOptsList given by its name
  *  Example: 'device:O' uses qemu_device_opts.
  *  Command completion for specific keys can be requested via
- *  appending '(NAME:TYPE,...)' with 'F', 'B' as type.
+ *  appending '(NAME:TYPE,...)' with 'F', 'B', 'q', 'Q' as type.
  *  Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path.
  *  Restriction: only lists with empty desc are supported
  *  TODO lift the restriction
@@ -3318,6 +3320,8 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 switch(c) {
 case 'F':
 case 'B':
+case 'q':
+c

[Qemu-devel] [PATCH v4 07/23] qdev: Allow device specification by qtree path for device_del

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Allow to specify the device to be removed via device_del not only by ID
but also by its full or abbreviated qtree path. For this purpose,
qdev_find is introduced which combines walking the qtree with searching
for device IDs if required.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c   |   75 ---
 qemu-monitor.hx |   10 +++---
 2 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index ac450cf..2d1d171 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -39,7 +39,7 @@ DeviceInfo *device_info_list;
 
 static BusState *qbus_find_recursive(BusState *bus, const char *name,
  const BusInfo *info);
-static BusState *qbus_find(const char *path);
+static BusState *qbus_find(const char *path, bool report_errors);
 
 /* Register a new device type.  */
 void qdev_register(DeviceInfo *info)
@@ -217,7 +217,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 /* find bus */
 path = qemu_opt_get(opts, "bus");
 if (path != NULL) {
-bus = qbus_find(path);
+bus = qbus_find(path, true);
 if (!bus) {
 return NULL;
 }
@@ -475,7 +475,7 @@ static BusState *qbus_find_recursive(BusState *bus, const 
char *name,
 return NULL;
 }
 
-static DeviceState *qdev_find_recursive(BusState *bus, const char *id)
+static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)
 {
 DeviceState *dev, *ret;
 BusState *child;
@@ -484,7 +484,7 @@ static DeviceState *qdev_find_recursive(BusState *bus, 
const char *id)
 if (dev->id && strcmp(dev->id, id) == 0)
 return dev;
 QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
-ret = qdev_find_recursive(child, id);
+ret = qdev_find_id_recursive(child, id);
 if (ret) {
 return ret;
 }
@@ -590,7 +590,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char 
*elem)
 return NULL;
 }
 
-static BusState *qbus_find(const char *path)
+static BusState *qbus_find(const char *path, bool report_errors)
 {
 DeviceState *dev;
 BusState *bus = main_system_bus;
@@ -600,7 +600,7 @@ static BusState *qbus_find(const char *path)
 /* search for bus name recursively if path is not absolute */
 if (path[0] != '/') {
 bus = qbus_find_recursive(bus, path, NULL);
-if (!bus) {
+if (!bus && report_errors) {
 qerror_report(QERR_BUS_NOT_FOUND, path);
 }
 return bus;
@@ -623,12 +623,16 @@ static BusState *qbus_find(const char *path)
 pos += len;
 dev = qbus_find_dev(bus, elem);
 if (!dev) {
-qerror_report(QERR_DEVICE_NOT_FOUND, elem);
-qbus_list_dev(bus);
+if (report_errors) {
+qerror_report(QERR_DEVICE_NOT_FOUND, elem);
+qbus_list_dev(bus);
+}
 return NULL;
 }
 if (dev->num_child_bus == 0) {
-qerror_report(QERR_DEVICE_NO_BUS, elem);
+if (report_errors) {
+qerror_report(QERR_DEVICE_NO_BUS, elem);
+}
 return NULL;
 }
 
@@ -644,13 +648,55 @@ static BusState *qbus_find(const char *path)
 pos += len;
 bus = qbus_find_bus(dev, elem);
 if (!bus) {
-qerror_report(QERR_BUS_NOT_FOUND, elem);
-qbus_list_bus(dev);
+if (report_errors) {
+qerror_report(QERR_BUS_NOT_FOUND, elem);
+qbus_list_bus(dev);
+}
 return NULL;
 }
 }
 }
 
+static DeviceState *qdev_find(const char *path)
+{
+const char *dev_name;
+DeviceState *dev;
+char *bus_path;
+BusState *bus;
+
+/* search for unique ID recursively if path is not absolute */
+if (path[0] != '/') {
+dev = qdev_find_id_recursive(main_system_bus, path);
+if (!dev) {
+qerror_report(QERR_DEVICE_NOT_FOUND, path);
+}
+return dev;
+}
+
+dev_name = strrchr(path, '/') + 1;
+
+bus_path = qemu_strdup(path);
+bus_path[dev_name - path] = 0;
+
+bus = qbus_find(bus_path, false);
+qemu_free(bus_path);
+if (!bus) {
+/* retry with full path to generate correct error message */
+bus = qbus_find(path, true);
+if (!bus) {
+return NULL;
+}
+dev_name = "";
+}
+
+dev = qbus_find_dev(bus, dev_name);
+if (!dev) {
+qerror_report(QERR_DEVICE_NOT_FOUND, dev_name);
+qbus_list_dev(bus);
+}
+return dev;
+}
+
 void qbus_create_inplace(BusState *bus, BusInfo *info,
  DeviceState *parent, const char *name)
 {
@@ -810,12 +856,11 @@ int do_device_add(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-const char *id = qdict_get_str(qdict, "id");
+const c

[Qemu-devel] [PATCH v4 03/23] qdev: Drop ID matching from qtree paths

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Device IDs may conflict with device names or aliases. From now on we
only accept them outside qtree paths. This also makes dumping IDs in
qbus_list_dev/bus obsolete.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c |   16 
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index c272c51..aa25155 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -497,8 +497,7 @@ static void qbus_list_bus(DeviceState *dev)
 BusState *child;
 const char *sep = " ";
 
-error_printf("child busses at \"%s\":",
- dev->id ? dev->id : dev->info->name);
+error_printf("child busses at \"%s\":", dev->info->name);
 QLIST_FOREACH(child, &dev->child_bus, sibling) {
 error_printf("%s\"%s\"", sep, child->name);
 sep = ", ";
@@ -514,8 +513,6 @@ static void qbus_list_dev(BusState *bus)
 error_printf("devices at \"%s\":", bus->name);
 QLIST_FOREACH(dev, &bus->children, sibling) {
 error_printf("%s\"%s\"", sep, dev->info->name);
-if (dev->id)
-error_printf("/\"%s\"", dev->id);
 sep = ", ";
 }
 error_printf("\n");
@@ -539,15 +536,10 @@ static DeviceState *qbus_find_dev(BusState *bus, char 
*elem)
 
 /*
  * try to match in order:
- *   (1) instance id, if present
- *   (2) driver name
- *   (3) driver alias, if present
+ *   (1) driver name
+ *   (2) driver alias, if present
  */
-QLIST_FOREACH(dev, &bus->children, sibling) {
-if (dev->id  &&  strcmp(dev->id, elem) == 0) {
-return dev;
-}
-}
+
 QLIST_FOREACH(dev, &bus->children, sibling) {
 if (strcmp(dev->info->name, elem) == 0) {
 return dev;
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 21/23] QMP: Teach basic capability negotiation to python example

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

As sending "qmp_capabilities" on session start became mandatory, both
python examples were broken.

Signed-off-by: Jan Kiszka 
---
 QMP/qmp-shell |1 +
 QMP/vm-info   |1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-shell b/QMP/qmp-shell
index f89b9af..a5b72d1 100755
--- a/QMP/qmp-shell
+++ b/QMP/qmp-shell
@@ -42,6 +42,7 @@ def main():
 
 qemu = qmp.QEMUMonitorProtocol(argv[1])
 qemu.connect()
+qemu.send("qmp_capabilities")
 
 print 'Connected!'
 
diff --git a/QMP/vm-info b/QMP/vm-info
index 8ebaeb3..be5b038 100755
--- a/QMP/vm-info
+++ b/QMP/vm-info
@@ -24,6 +24,7 @@ def main():
 
 qemu = qmp.QEMUMonitorProtocol(argv[1])
 qemu.connect()
+qemu.send("qmp_capabilities")
 
 for cmd in [ 'version', 'kvm', 'status', 'uuid', 'balloon' ]:
 print cmd + ': ' + str(qemu.send('query-' + cmd))
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 05/23] qdev: Convert device and bus lists to QTAILQ

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Cosmetic change to align the instance number assignment with bus
ordering. The current ordering due to QLIST_INSERT_HEAD is a bit
annoying when you dump the qtree or address devices via
'driver.instance'.

Signed-off-by: Jan Kiszka 
---
 hw/acpi_piix4.c  |2 +-
 hw/i2c.c |2 +-
 hw/pci-hotplug.c |2 +-
 hw/qdev.c|   41 +
 hw/qdev.h|8 
 hw/ssi.c |6 +++---
 6 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 8d1a628..38e8289 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -564,7 +564,7 @@ static void pciej_write(void *opaque, uint32_t addr, 
uint32_t val)
 PCIDevice *dev;
 int slot = ffs(val) - 1;
 
-QLIST_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
 dev = DO_UPCAST(PCIDevice, qdev, qdev);
 if (PCI_SLOT(dev->devfn) == slot) {
 qdev_free(qdev);
diff --git a/hw/i2c.c b/hw/i2c.c
index bee8e88..61ab6fa 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -84,7 +84,7 @@ int i2c_start_transfer(i2c_bus *bus, uint8_t address, int 
recv)
 DeviceState *qdev;
 i2c_slave *slave = NULL;
 
-QLIST_FOREACH(qdev, &bus->qbus.children, sibling) {
+QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
 i2c_slave *candidate = I2C_SLAVE_FROM_QDEV(qdev);
 if (candidate->address == address) {
 slave = candidate;
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c39e640..2793269 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -74,7 +74,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
 SCSIBus *scsibus;
 SCSIDevice *scsidev;
 
-scsibus = DO_UPCAST(SCSIBus, qbus, QLIST_FIRST(&adapter->child_bus));
+scsibus = DO_UPCAST(SCSIBus, qbus, QTAILQ_FIRST(&adapter->child_bus));
 if (!scsibus || strcmp(scsibus->qbus.info->name, "SCSI") != 0) {
 error_report("Device is not a SCSI adapter");
 return -1;
diff --git a/hw/qdev.c b/hw/qdev.c
index f4ae4a6..25f6e62 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -85,10 +85,11 @@ static DeviceState *qdev_create_from_info(BusState *bus, 
DeviceInfo *info)
 dev = qemu_mallocz(info->size);
 dev->info = info;
 dev->parent_bus = bus;
+QTAILQ_INIT(&dev->child_bus);
 qdev_prop_set_defaults(dev, dev->info->props);
 qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
 qdev_prop_set_globals(dev);
-QLIST_INSERT_HEAD(&bus->children, dev, sibling);
+QTAILQ_INSERT_TAIL(&bus->children, dev, sibling);
 if (qdev_hotplug) {
 assert(bus->allow_hotplug);
 dev->hotplugged = 1;
@@ -338,7 +339,7 @@ void qdev_free(DeviceState *dev)
 
 if (dev->state == DEV_STATE_INITIALIZED) {
 while (dev->num_child_bus) {
-bus = QLIST_FIRST(&dev->child_bus);
+bus = QTAILQ_FIRST(&dev->child_bus);
 qbus_free(bus);
 }
 if (dev->info->vmsd)
@@ -349,7 +350,7 @@ void qdev_free(DeviceState *dev)
 qemu_opts_del(dev->opts);
 }
 qemu_unregister_reset(qdev_reset, dev);
-QLIST_REMOVE(dev, sibling);
+QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
 for (prop = dev->info->props; prop && prop->name; prop++) {
 if (prop->info->free) {
 prop->info->free(dev, prop);
@@ -438,7 +439,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char 
*name)
 {
 BusState *bus;
 
-QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+QTAILQ_FOREACH(bus, &dev->child_bus, sibling) {
 if (strcmp(name, bus->name) == 0) {
 return bus;
 }
@@ -463,8 +464,8 @@ static BusState *qbus_find_recursive(BusState *bus, const 
char *name,
 return bus;
 }
 
-QLIST_FOREACH(dev, &bus->children, sibling) {
-QLIST_FOREACH(child, &dev->child_bus, sibling) {
+QTAILQ_FOREACH(dev, &bus->children, sibling) {
+QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
 ret = qbus_find_recursive(child, name, info);
 if (ret) {
 return ret;
@@ -479,10 +480,10 @@ static DeviceState *qdev_find_recursive(BusState *bus, 
const char *id)
 DeviceState *dev, *ret;
 BusState *child;
 
-QLIST_FOREACH(dev, &bus->children, sibling) {
+QTAILQ_FOREACH(dev, &bus->children, sibling) {
 if (dev->id && strcmp(dev->id, id) == 0)
 return dev;
-QLIST_FOREACH(child, &dev->child_bus, sibling) {
+QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
 ret = qdev_find_recursive(child, id);
 if (ret) {
 return ret;
@@ -497,7 +498,7 @@ static int qdev_instance_no(DeviceState *dev)
 struct DeviceState *sibling;
 int instance = 0;
 
-QLIST_FOREACH(sibling, &dev->parent_bus->children, sibling) {
+QTAILQ_FOREACH(sibling, &dev->parent_bus->children, sibling) {
 if (si

[Qemu-devel] [PATCH v4 08/23] qdev: Introduce qdev_iterate_recursive

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Add qdev_iterate_recursive to walk the complete qtree invoking a
callback for each device. Use this service to implement
qdev_find_id_recursive.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c |   29 +
 hw/qdev.h |3 +++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 2d1d171..466d8d5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -475,16 +475,22 @@ static BusState *qbus_find_recursive(BusState *bus, const 
char *name,
 return NULL;
 }
 
-static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)
+void *qdev_iterate_recursive(BusState *bus, qdev_iteratefn callback,
+ void *opaque)
 {
 DeviceState *dev, *ret;
 BusState *child;
 
+if (!bus) {
+bus = main_system_bus;
+}
 QTAILQ_FOREACH(dev, &bus->children, sibling) {
-if (dev->id && strcmp(dev->id, id) == 0)
-return dev;
+ret = callback(dev, opaque);
+if (ret) {
+return ret;
+}
 QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
-ret = qdev_find_id_recursive(child, id);
+ret = qdev_iterate_recursive(child, callback, opaque);
 if (ret) {
 return ret;
 }
@@ -493,6 +499,21 @@ static DeviceState *qdev_find_id_recursive(BusState *bus, 
const char *id)
 return NULL;
 }
 
+static void *find_id_callback(DeviceState *dev, void *opaque)
+{
+const char *id = opaque;
+
+if (dev->id && strcmp(dev->id, id) == 0) {
+return dev;
+}
+return NULL;
+}
+
+static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)
+{
+return qdev_iterate_recursive(bus, find_id_callback, (void *)id);
+}
+
 static int qdev_instance_no(DeviceState *dev)
 {
 struct DeviceState *sibling;
diff --git a/hw/qdev.h b/hw/qdev.h
index 170a63a..111c876 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -134,6 +134,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char 
*name);
 typedef int (*qdev_initfn)(DeviceState *dev, DeviceInfo *info);
 typedef int (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
+typedef void *(*qdev_iteratefn)(DeviceState *dev, void *opaque);
 
 struct DeviceInfo {
 const char *name;
@@ -168,6 +169,8 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, 
int n);
 CharDriverState *qdev_init_chardev(DeviceState *dev);
 
 BusState *qdev_get_parent_bus(DeviceState *dev);
+void *qdev_iterate_recursive(BusState *bus, qdev_iteratefn callback,
+ void *opaque);
 
 /*** BUS API. ***/
 
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 02/23] qdev: Restrict direct bus addressing via its name

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

We allow to address a bus only using its local name. This is ambiguous
but unfortunately so handy that people (specifically libvirt) will
likely complain if bus=pci.0 needs to be replaced with
bus=/i440FX-pcihost/pci.0 all over the place. So keep this for now but
drop at least support for starting a qtree walks with an abbreviated bus
name.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c |   22 +++---
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 7c4f039..c272c51 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -564,25 +564,17 @@ static DeviceState *qbus_find_dev(BusState *bus, char 
*elem)
 static BusState *qbus_find(const char *path)
 {
 DeviceState *dev;
-BusState *bus;
+BusState *bus = main_system_bus;
 char elem[128];
-int pos, len;
+int len, pos = 0;
 
-/* find start element */
-if (path[0] == '/') {
-bus = main_system_bus;
-pos = 0;
-} else {
-if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
-assert(!path[0]);
-elem[0] = len = 0;
-}
-bus = qbus_find_recursive(main_system_bus, elem, NULL);
+/* search for bus name recursively if path is not absolute */
+if (path[0] != '/') {
+bus = qbus_find_recursive(bus, path, NULL);
 if (!bus) {
-qerror_report(QERR_BUS_NOT_FOUND, elem);
-return NULL;
+qerror_report(QERR_BUS_NOT_FOUND, path);
 }
-pos = len;
+return bus;
 }
 
 for (;;) {
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 15/23] monitor: Establish cmd flags and convert the async tag

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

As we want to add more flags to monitor commands, convert the only so
far existing one accordingly.

Signed-off-by: Jan Kiszka 
---
 monitor.c   |6 +++---
 monitor.h   |3 +++
 qemu-monitor.hx |2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index aa0bdd6..2d3d70d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -118,7 +118,7 @@ typedef struct mon_cmd_t {
 int  (*cmd_async)(Monitor *mon, const QDict *params,
   MonitorCompletion *cb, void *opaque);
 } mhandler;
-int async;
+int flags;
 } mon_cmd_t;
 
 /* file descriptors passed via SCM_RIGHTS */
@@ -340,7 +340,7 @@ static inline int monitor_handler_ported(const mon_cmd_t 
*cmd)
 
 static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
 {
-return cmd->async != 0;
+return cmd->flags & MONITOR_CMD_ASYNC;
 }
 
 static inline int monitor_has_error(const Monitor *mon)
@@ -2544,7 +2544,7 @@ static const mon_cmd_t info_cmds[] = {
 .help   = "show balloon information",
 .user_print = monitor_print_balloon,
 .mhandler.info_async = do_info_balloon,
-.async  = 1,
+.flags  = MONITOR_CMD_ASYNC,
 },
 {
 .name   = "qtree",
diff --git a/monitor.h b/monitor.h
index 32c0170..e3f0119 100644
--- a/monitor.h
+++ b/monitor.h
@@ -15,6 +15,9 @@ extern Monitor *default_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 
+/* flags for monitor commands */
+#define MONITOR_CMD_ASYNC   0x0001
+
 /* QMP events */
 typedef enum MonitorEvent {
 QEVENT_SHUTDOWN,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 0034fed..2fe5ae8 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1287,7 +1287,7 @@ ETEXI
 .help   = "request VM to change its memory allocation (in MB)",
 .user_print = monitor_user_noop,
 .mhandler.cmd_async = do_balloon,
-.async  = 1,
+.flags  = MONITOR_CMD_ASYNC,
 },
 
 STEXI
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 04/23] qdev: Allow device addressing via 'driver.instance'

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Extend qbus_find_dev to allow addressing of devices without an unique id
via an optional per-bus instance number. The new formats are
'driver.instance' and 'alias.instance'. Attach this name extension
whenever an instantiated device is printed.

Signed-off-by: Jan Kiszka 
---
 docs/qdev-device-use.txt |   13 +++-
 hw/qdev.c|   48 +
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index f252c8e..58f2630 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -1,6 +1,6 @@
 = How to convert to -device & friends =
 
-=== Specifying Bus and Address on Bus ===
+=== Specifying Bus, Address on Bus, and Devices ===
 
 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
@@ -20,6 +20,17 @@ 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.
 
+Existing devices can be addressed either via a unique ID if it was
+assigned during creation or via the device tree path:
+
+/full_bus_address/driver_name[.instance_number]
+
+The instance number counts devices managed by the same driver on a
+specifc bus. It is zero-based.
+
+Example: /i440FX-pcihost/pci.0/e1000.1 addresses the second e1000
+adapter on the bus 'pci.0'.
+
 Note: the USB device address can't be controlled at this time.
 
 === Block Devices ===
diff --git a/hw/qdev.c b/hw/qdev.c
index aa25155..f4ae4a6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -492,12 +492,29 @@ static DeviceState *qdev_find_recursive(BusState *bus, 
const char *id)
 return NULL;
 }
 
+static int qdev_instance_no(DeviceState *dev)
+{
+struct DeviceState *sibling;
+int instance = 0;
+
+QLIST_FOREACH(sibling, &dev->parent_bus->children, sibling) {
+if (sibling->info == dev->info) {
+if (sibling == dev) {
+break;
+}
+instance++;
+}
+}
+return instance;
+}
+
 static void qbus_list_bus(DeviceState *dev)
 {
 BusState *child;
 const char *sep = " ";
 
-error_printf("child busses at \"%s\":", dev->info->name);
+error_printf("child busses at \"%s.%d\":",
+ dev->info->name, qdev_instance_no(dev));
 QLIST_FOREACH(child, &dev->child_bus, sibling) {
 error_printf("%s\"%s\"", sep, child->name);
 sep = ", ";
@@ -512,7 +529,8 @@ static void qbus_list_dev(BusState *bus)
 
 error_printf("devices at \"%s\":", bus->name);
 QLIST_FOREACH(dev, &bus->children, sibling) {
-error_printf("%s\"%s\"", sep, dev->info->name);
+error_printf("%s\"%s.%d\"", sep, dev->info->name,
+ qdev_instance_no(dev));
 sep = ", ";
 }
 error_printf("\n");
@@ -530,23 +548,35 @@ static BusState *qbus_find_bus(DeviceState *dev, char 
*elem)
 return NULL;
 }
 
-static DeviceState *qbus_find_dev(BusState *bus, char *elem)
+static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
 {
 DeviceState *dev;
+int instance, n;
+char buf[128];
 
 /*
  * try to match in order:
- *   (1) driver name
- *   (2) driver alias, if present
+ *   (1) driver name [.instance]
+ *   (2) driver alias [.instance], if present
  */
 
+if (sscanf(elem, "%127[^.].%u", buf, &instance) == 2) {
+elem = buf;
+} else {
+instance = 0;
+}
+
+n = 0;
 QLIST_FOREACH(dev, &bus->children, sibling) {
-if (strcmp(dev->info->name, elem) == 0) {
+if (strcmp(dev->info->name, elem) == 0 && n++ == instance) {
 return dev;
 }
 }
+
+n = 0;
 QLIST_FOREACH(dev, &bus->children, sibling) {
-if (dev->info->alias && strcmp(dev->info->alias, elem) == 0) {
+if (dev->info->alias && strcmp(dev->info->alias, elem) == 0 &&
+n++ == instance) {
 return dev;
 }
 }
@@ -710,8 +740,8 @@ static void qdev_print_props(Monitor *mon, DeviceState 
*dev, Property *props,
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
 BusState *child;
-qdev_printf("dev: %s, id \"%s\"\n", dev->info->name,
-dev->id ? dev->id : "");
+qdev_printf("dev: %s.%d, id \"%s\"\n", dev->info->name,
+qdev_instance_no(dev), dev->id ? dev->id : "");
 indent += 2;
 if (dev->num_gpio_in) {
 qdev_printf("gpio-in %d\n", dev->num_gpio_in);
-- 
1.6.0.2




[Qemu-devel] [PATCH v4 01/23] qdev: Rework qtree path abbreviations

2010-06-15 Thread Jan Kiszka
From: Jan Kiszka 

Path abbreviations suffer from the inconsistency that bus names can only
be omitted at the end of a path. Drop this special rule, and also remove
the now obsolete QERR_DEVICE_MULTIPLE_BUSSES.

Signed-off-by: Jan Kiszka 
---
 hw/qdev.c |   22 --
 qerror.c  |4 
 qerror.h  |3 ---
 3 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 61f999c..7c4f039 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -608,32 +608,18 @@ static BusState *qbus_find(const char *path)
 }
 return NULL;
 }
+if (dev->num_child_bus == 0) {
+qerror_report(QERR_DEVICE_NO_BUS, elem);
+return NULL;
+}
 
 assert(path[pos] == '/' || !path[pos]);
 while (path[pos] == '/') {
 pos++;
 }
-if (path[pos] == '\0') {
-/* last specified element is a device.  If it has exactly
- * one child bus accept it nevertheless */
-switch (dev->num_child_bus) {
-case 0:
-qerror_report(QERR_DEVICE_NO_BUS, elem);
-return NULL;
-case 1:
-return QLIST_FIRST(&dev->child_bus);
-default:
-qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
-if (!monitor_cur_is_qmp()) {
-qbus_list_bus(dev);
-}
-return NULL;
-}
-}
 
 /* find bus */
 if (sscanf(path+pos, "%127[^/]%n", elem, &len) != 1) {
-assert(0);
 elem[0] = len = 0;
 }
 pos += len;
diff --git a/qerror.c b/qerror.c
index 44d0bf8..786b5dc 100644
--- a/qerror.c
+++ b/qerror.c
@@ -77,10 +77,6 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Device '%(device)' is locked",
 },
 {
-.error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
-.desc  = "Device '%(device)' has multiple child busses",
-},
-{
 .error_fmt = QERR_DEVICE_NOT_ACTIVE,
 .desc  = "Device '%(device)' has not been activated by the guest",
 },
diff --git a/qerror.h b/qerror.h
index 77ae574..88474fb 100644
--- a/qerror.h
+++ b/qerror.h
@@ -73,9 +73,6 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_LOCKED \
 "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
 
-#define QERR_DEVICE_MULTIPLE_BUSSES \
-"{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
-
 #define QERR_DEVICE_NOT_ACTIVE \
 "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
 
-- 
1.6.0.2




Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Alex Williamson
On Tue, 2010-06-15 at 22:55 +0100, Paul Brook wrote:
> > On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote:
> > > > > Alex proposed to disambiguate by adding "identified properties of the
> > > > > immediate parent bus and device" to the path component.  For PCI,
> > > > > these are dev.fn.  Likewise for any other bus where devices have
> > > > > unambigous bus address.  The driver name carries no information!
> > > > 
> > > > From user POV, driver names are very handly to address a device
> > > > intuitively - except for the case you have tones of devices on the same
> > > > bus that are handled by the same driver. For that case we need to
> > > > augment the device name with a useful per-bus ID, derived from the bus
> > > > address where available, otherwise based on instance numbers.
> > > 
> > > This is where I think you're missing a trick. We don't need to augment
> > > the name, we just need to allow the bus id to be used instead.
> > 
> > For the case of a hot remove, I agree.  If the user specifies "pci_del
> > pci.0/03.0", that's completely sufficient because we don't care what's
> > in that slot, just remove it.  However, I still see some use cases for
> > device names in the path.  Take for example:
> > 
> > (A): /i440FX-pcihost/pci.0/e1000.05.0
> > 
> > vs
> > 
> > (B): /pci.0/05.0
> 
> > (removing both the root bridge driver name and the device driver name)
> 
> This is wrong. You still need an entry for the host pci bridge.

The host pci bridge has no identifying properties, so all we'd be
printing is the pci bridge driver name, which we claim adds no value.
Or are we saying that some driver names are useful while others aren't?

>  > If we attach a pci option rom to the device, create some ram to store
> > the option rom and name the ram block $(PATH)/rom, with (A) we know more
> > about the hierarchy to get to the actual devfn device, and we know the
> > type of device that's in the slot.  With (B), there's no robustness.  If
> > we migrated using (B), we could be stuffing a pc e1000 option rom into a
> > ppc lsi895, just because it happened to live that the same place and
> > have a ram block named rom.  Including driver names increases the
> > uniqueness of the path.
> 
> No it doesn't. It adds redundant information and a false sense of security. 
> Driver name is not sufficient to determine whether you've actually got a 
> compatible device. The type of the device doesn't reliably tell you anything 
> about how many ram blocks that device has, or how big they are.

Driver name is sufficient to tell me that driver "foo" created this
device on both the source and destination and hand off responsibility to
driver "foo" to make the determination about whether it's compatible.  I
think it's reasonably safe to say we're not going to have two completely
different drivers with name "foo"s within an instance of qemu and we're
not going to turn over the name to a different driver and expect
anything sane to happen wrt to migration.  I disagree that this is a
false sense of security, it gives me warm fuzzies.  Please convince me
otherwise.

> > > > > For other buses, we need to make something up.
> > > > > 
> > > > > Note that addressing by bus address rather than name is generally
> > > > > useful, not just in the context of savevm.  For instance, I'd
> > > > > appreciate being able to say something like "device_del pci.0/04.0".
> > > > 
> > > > And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump
> > > > the bus first before you can identify which device you want to remove.
> > > 
> > > We can allow both.
> > > A bus address is sufficient to uniquely identify a device.
> > 
> > A bus address (assuming it exists) is sufficient to uniquely identify a
> > device, on a given VM.  A bus address only identifies a location when
> > comparing two separate VMs.
> 
> I can't make any sense of this statement.

Makes sense to me ;)  A bus address is unique only within the context of
the VM.  Within that VM I can figure out that the device at that bus
address uses driver foo, so foo seems redundant in the device path.  Now
I try to apply that bus address to another VM, the migration target, and
all I have is a location.  I have to make a leap of faith that the
device at that location is the same as the device at the same location
on the migration source.  Where are those warm fuzzies?

> > >   I see no reason to
> > > require the driver name,  or to include it in the canonical device
> > > address.
> > 
> > Migration.  Including the driver name extends our ability to uniquely
> > identify a device across separate VMs.  It's then up to the vmstate code
> > to figure out whether the device are compatible for migrate state.
> 
> I find this argument contradictory. The migration code already needs to check 
> whether a device is compatible before it allows migration.  The driver name 
> is 
> not sufficient to ensure compatibility, so I see no benefit in including it 
> in 
> the device addre

Re: [Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable

2010-06-15 Thread Anthony Liguori

On 06/15/2010 05:26 PM, Cam Macdonell wrote:

On Tue, Jun 15, 2010 at 10:32 AM, Anthony Liguori  wrote:
   

On 06/15/2010 11:16 AM, Cam Macdonell wrote:
 

How does this look for marking the device as non-migratable?  It adds a
field
'no_migrate' to the SaveStateEntry and tests for it in vmstate_save.  This
would
replace anything that touches memory.

Cam

---
  hw/hw.h  |1 +
  savevm.c |   32 +---
  2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index d78d814..7c93f08 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
   void *opaque);

  void unregister_savevm(const char *idstr, void *opaque);
+void mark_no_migrate(const char *idstr, void *opaque);

   

I'm not thrilled with the name but the functionality is spot on.  I lack the
creativity to offer a better name suggestion :-)

Regards,

Anthony Liguori
 

Hmmm, in working on this it seems that the memory (from
qemu_ram_map()) is still attached even when the device is removed
(which causes migration to fail because there is an unexpected
memory).

Is something like cpu_unregister_physical_memory()/qemu_ram_free() needed?
   


Yes.  You need to unregister any memory that you have registered upon 
device removal.


Regards,

Anthony Liguori


Cam

   





[Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable

2010-06-15 Thread Cam Macdonell
On Tue, Jun 15, 2010 at 10:32 AM, Anthony Liguori  wrote:
> On 06/15/2010 11:16 AM, Cam Macdonell wrote:
>>
>> How does this look for marking the device as non-migratable?  It adds a
>> field
>> 'no_migrate' to the SaveStateEntry and tests for it in vmstate_save.  This
>> would
>> replace anything that touches memory.
>>
>> Cam
>>
>> ---
>>  hw/hw.h  |    1 +
>>  savevm.c |   32 +---
>>  2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/hw.h b/hw/hw.h
>> index d78d814..7c93f08 100644
>> --- a/hw/hw.h
>> +++ b/hw/hw.h
>> @@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
>>                           void *opaque);
>>
>>  void unregister_savevm(const char *idstr, void *opaque);
>> +void mark_no_migrate(const char *idstr, void *opaque);
>>
>
> I'm not thrilled with the name but the functionality is spot on.  I lack the
> creativity to offer a better name suggestion :-)
>
> Regards,
>
> Anthony Liguori

Hmmm, in working on this it seems that the memory (from
qemu_ram_map()) is still attached even when the device is removed
(which causes migration to fail because there is an unexpected
memory).

Is something like cpu_unregister_physical_memory()/qemu_ram_free() needed?

Cam



Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Paul Brook
> On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote:
> > > > Alex proposed to disambiguate by adding "identified properties of the
> > > > immediate parent bus and device" to the path component.  For PCI,
> > > > these are dev.fn.  Likewise for any other bus where devices have
> > > > unambigous bus address.  The driver name carries no information!
> > > 
> > > From user POV, driver names are very handly to address a device
> > > intuitively - except for the case you have tones of devices on the same
> > > bus that are handled by the same driver. For that case we need to
> > > augment the device name with a useful per-bus ID, derived from the bus
> > > address where available, otherwise based on instance numbers.
> > 
> > This is where I think you're missing a trick. We don't need to augment
> > the name, we just need to allow the bus id to be used instead.
> 
> For the case of a hot remove, I agree.  If the user specifies "pci_del
> pci.0/03.0", that's completely sufficient because we don't care what's
> in that slot, just remove it.  However, I still see some use cases for
> device names in the path.  Take for example:
> 
> (A): /i440FX-pcihost/pci.0/e1000.05.0
> 
> vs
> 
> (B): /pci.0/05.0

> (removing both the root bridge driver name and the device driver name)

This is wrong. You still need an entry for the host pci bridge.

 > If we attach a pci option rom to the device, create some ram to store
> the option rom and name the ram block $(PATH)/rom, with (A) we know more
> about the hierarchy to get to the actual devfn device, and we know the
> type of device that's in the slot.  With (B), there's no robustness.  If
> we migrated using (B), we could be stuffing a pc e1000 option rom into a
> ppc lsi895, just because it happened to live that the same place and
> have a ram block named rom.  Including driver names increases the
> uniqueness of the path.

No it doesn't. It adds redundant information and a false sense of security. 
Driver name is not sufficient to determine whether you've actually got a 
compatible device. The type of the device doesn't reliably tell you anything 
about how many ram blocks that device has, or how big they are.

> > > > For other buses, we need to make something up.
> > > > 
> > > > Note that addressing by bus address rather than name is generally
> > > > useful, not just in the context of savevm.  For instance, I'd
> > > > appreciate being able to say something like "device_del pci.0/04.0".
> > > 
> > > And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump
> > > the bus first before you can identify which device you want to remove.
> > 
> > We can allow both.
> > A bus address is sufficient to uniquely identify a device.
> 
> A bus address (assuming it exists) is sufficient to uniquely identify a
> device, on a given VM.  A bus address only identifies a location when
> comparing two separate VMs.

I can't make any sense of this statement.
 
> >   I see no reason to
> > require the driver name,  or to include it in the canonical device
> > address.
> 
> Migration.  Including the driver name extends our ability to uniquely
> identify a device across separate VMs.  It's then up to the vmstate code
> to figure out whether the device are compatible for migrate state.

I find this argument contradictory. The migration code already needs to check 
whether a device is compatible before it allows migration.  The driver name is 
not sufficient to ensure compatibility, so I see no benefit in including it in 
the device address. If we include the device name, why aren't we also 
including all other properties that would make the devices incompatible?

Paul



Re: [Qemu-devel] [PATCH] Fix comparison which always returned false

2010-06-15 Thread malc
On Wed, 16 Jun 2010, malc wrote:

> On Tue, 15 Jun 2010, Stefan Weil wrote:
> 
> This should go in asap, it breaks PPC in quite visible and ugly way...

Right... forgot something, DIY...

Thanks, applied.

[..snip..]

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



Re: [Qemu-devel] [PATCH] Fix comparison which always returned false

2010-06-15 Thread malc
On Tue, 15 Jun 2010, Stefan Weil wrote:

This should go in asap, it breaks PPC in quite visible and ugly way...

> Comparing an 8 bit value with ~0 does not work as expected.
> Replace ~0 by UINT8_MAX in comparison and also in assignment
> (and fix coding style, too).
> 
> Cc: Gleb Natapov 
> Cc: Anthony Liguori 
> Signed-off-by: Stefan Weil 
> ---
>  hw/hpet.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/hpet.c b/hw/hpet.c
> index 0c80ee5..d5c406c 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -74,7 +74,7 @@ typedef struct HPETState {
>  uint8_t  hpet_id;   /* instance id */
>  } HPETState;
>  
> -struct hpet_fw_config hpet_cfg = {.count = ~0};
> +struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>  
>  static uint32_t hpet_in_legacy_mode(HPETState *s)
>  {
> @@ -682,8 +682,10 @@ static int hpet_init(SysBusDevice *dev)
>  int i, iomemtype;
>  HPETTimer *timer;
>  
> -if (hpet_cfg.count == ~0) /* first instance */
> +if (hpet_cfg.count == UINT8_MAX) {
> +/* first instance */
>  hpet_cfg.count = 0;
> +}
>  
>  if (hpet_cfg.count == 8) {
>  fprintf(stderr, "Only 8 instances of HPET is allowed\n");
> 

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



[Qemu-devel] Re: [PATCH] Fix comparison which always returned false

2010-06-15 Thread Jan Kiszka
Stefan Weil wrote:
> Comparing an 8 bit value with ~0 does not work as expected.
> Replace ~0 by UINT8_MAX in comparison and also in assignment
> (and fix coding style, too).
> 
> Cc: Gleb Natapov 
> Cc: Anthony Liguori 
> Signed-off-by: Stefan Weil 
> ---
>  hw/hpet.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/hpet.c b/hw/hpet.c
> index 0c80ee5..d5c406c 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -74,7 +74,7 @@ typedef struct HPETState {
>  uint8_t  hpet_id;   /* instance id */
>  } HPETState;
>  
> -struct hpet_fw_config hpet_cfg = {.count = ~0};
> +struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>  
>  static uint32_t hpet_in_legacy_mode(HPETState *s)
>  {
> @@ -682,8 +682,10 @@ static int hpet_init(SysBusDevice *dev)
>  int i, iomemtype;
>  HPETTimer *timer;
>  
> -if (hpet_cfg.count == ~0) /* first instance */
> +if (hpet_cfg.count == UINT8_MAX) {
> +/* first instance */
>  hpet_cfg.count = 0;
> +}
>  
>  if (hpet_cfg.count == 8) {
>  fprintf(stderr, "Only 8 instances of HPET is allowed\n");

That makes me wonder why we need to signal this special value of
hpet_cfg.count to seabios at all. Why isn't plain 0 for no hpets
sufficient, Gleb?

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] Fix comparison which always returned false

2010-06-15 Thread Anthony Liguori

On 06/15/2010 04:03 PM, Stefan Weil wrote:

Comparing an 8 bit value with ~0 does not work as expected.
Replace ~0 by UINT8_MAX in comparison and also in assignment
(and fix coding style, too).
   


Because when the uint8_t gets promoted, it doesn't get zero filled.  I'd 
rather something a bit more obvious like HPET_INVALID_COUNT.


Regards,

Anthony Liguori


Cc: Gleb Natapov
Cc: Anthony Liguori
Signed-off-by: Stefan Weil
---
  hw/hpet.c |6 --
  1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/hpet.c b/hw/hpet.c
index 0c80ee5..d5c406c 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -74,7 +74,7 @@ typedef struct HPETState {
  uint8_t  hpet_id;   /* instance id */
  } HPETState;

-struct hpet_fw_config hpet_cfg = {.count = ~0};
+struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};

  static uint32_t hpet_in_legacy_mode(HPETState *s)
  {
@@ -682,8 +682,10 @@ static int hpet_init(SysBusDevice *dev)
  int i, iomemtype;
  HPETTimer *timer;

-if (hpet_cfg.count == ~0) /* first instance */
+if (hpet_cfg.count == UINT8_MAX) {
+/* first instance */
  hpet_cfg.count = 0;
+}

  if (hpet_cfg.count == 8) {
  fprintf(stderr, "Only 8 instances of HPET is allowed\n");
   





Re: [Qemu-devel] Re: [CFR 9/10] device_del command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 03:48 PM, Miguel Di Ciurcio Filho wrote:

On Tue, Jun 15, 2010 at 1:59 PM, Jan Kiszka  wrote:
   

Anthony Liguori wrote:
 

device_del
--

Remove a device.

Arguments:

- "id": the device's ID (json-string)
   

"id" should become "device" (I hope to send the corresponding patches
this night). The idea is to not only allow global device IDs but also
qtree paths here.

 

Now I'm confused. On a previous email when discussing about
query-netdev, I initially proposed this[1]:

->  { "execute": "query-netdev" }
<- {
   "return": [
  {
 "device": "tap.0",
 "vlan": 0,
 "info": {
"script": "/etc/qemu-ifup",
"downscript": "/etc/qemu-ifdown",
"ifname": "tap0",
"model": "tap"
 },
  },

And latter on Anthony suggested to replace "device" by "id" [2].

So my doubt is: we should identify guest/frontend devices in QMP using
the word "device" instead of "id" and identify backend devices by
using the word "id"?
   


id.

What Jan is referring to is the qdev device path verses the qdev id.  
netdev objects are not qdev devices, they are just QemuOpts.  In netdev 
parlance, the unique identifier is called the 'id'.


Regards,

Anthony Liguori


Regards,

Miguel

[1] http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00619.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00943.html
   





[Qemu-devel] [PATCH] Fix comparison which always returned false

2010-06-15 Thread Stefan Weil
Comparing an 8 bit value with ~0 does not work as expected.
Replace ~0 by UINT8_MAX in comparison and also in assignment
(and fix coding style, too).

Cc: Gleb Natapov 
Cc: Anthony Liguori 
Signed-off-by: Stefan Weil 
---
 hw/hpet.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/hpet.c b/hw/hpet.c
index 0c80ee5..d5c406c 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -74,7 +74,7 @@ typedef struct HPETState {
 uint8_t  hpet_id;   /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = ~0};
+struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {
@@ -682,8 +682,10 @@ static int hpet_init(SysBusDevice *dev)
 int i, iomemtype;
 HPETTimer *timer;
 
-if (hpet_cfg.count == ~0) /* first instance */
+if (hpet_cfg.count == UINT8_MAX) {
+/* first instance */
 hpet_cfg.count = 0;
+}
 
 if (hpet_cfg.count == 8) {
 fprintf(stderr, "Only 8 instances of HPET is allowed\n");
-- 
1.5.6.5




Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

2010-06-15 Thread Alex Williamson
On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote:
> > > Alex proposed to disambiguate by adding "identified properties of the
> > > immediate parent bus and device" to the path component.  For PCI, these
> > > are dev.fn.  Likewise for any other bus where devices have unambigous
> > > bus address.  The driver name carries no information!
> > 
> > From user POV, driver names are very handly to address a device
> > intuitively - except for the case you have tones of devices on the same
> > bus that are handled by the same driver. For that case we need to
> > augment the device name with a useful per-bus ID, derived from the bus
> > address where available, otherwise based on instance numbers.
> 
> This is where I think you're missing a trick. We don't need to augment the 
> name, we just need to allow the bus id to be used instead.

For the case of a hot remove, I agree.  If the user specifies "pci_del
pci.0/03.0", that's completely sufficient because we don't care what's
in that slot, just remove it.  However, I still see some use cases for
device names in the path.  Take for example:

(A): /i440FX-pcihost/pci.0/e1000.05.0

vs

(B): /pci.0/05.0

(removing both the root bridge driver name and the device driver name)

If we attach a pci option rom to the device, create some ram to store
the option rom and name the ram block $(PATH)/rom, with (A) we know more
about the hierarchy to get to the actual devfn device, and we know the
type of device that's in the slot.  With (B), there's no robustness.  If
we migrated using (B), we could be stuffing a pc e1000 option rom into a
ppc lsi895, just because it happened to live that the same place and
have a ram block named rom.  Including driver names increases the
uniqueness of the path.

Another example; if we have two drivers that create a vmstate with name
"foo", plug one driver into slot 03.0 on the migration source, the other
into slot 03.0 on the migration destination, what happens?  It seems
likely that the destination will try to load the vmstate from a
different driver and fail in wonderful and bizarre ways.  If we use (A),
each path automatically has it's own namespace.

ISA is also a good example even though it doesn't do hotplug.  Given
this set:

/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300

versus this set:

/pci.0.01.0/isa.0
/pci.0.01.0/isa.0/0x3f8
/pci.0.01.0/isa.0/0x378
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0/0x300

Which one has devices that are easier to uniquely identify?
 
> > > For other buses, we need to make something up.
> > > 
> > > Note that addressing by bus address rather than name is generally
> > > useful, not just in the context of savevm.  For instance, I'd appreciate
> > > being able to say something like "device_del pci.0/04.0".
> > 
> > And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump
> > the bus first before you can identify which device you want to remove.
> 
> We can allow both.
> 
> A bus address is sufficient to uniquely identify a device.

A bus address (assuming it exists) is sufficient to uniquely identify a
device, on a given VM.  A bus address only identifies a location when
comparing two separate VMs.

>   I see no reason to 
> require the driver name,  or to include it in the canonical device address.

Migration.  Including the driver name extends our ability to uniquely
identify a device across separate VMs.  It's then up to the vmstate code
to figure out whether the device are compatible for migrate state.

> > > An easy way to get that is to reserve part of the name space for bus
> > > addresses.  If the path component starts with a letter, it's an ID or
> > > driver name.  If it starts with say '@', it's a bus address in
> > > bus-specific syntax.  The bus provides a method to look it up.
> > 
> > I would prefer [@|.]. The former is
> > set for buses that implement some to-be-defined device addressing
> > service, the latter is the default on buses where that service is not
> > available.
> 
> If we have bus-address then I see no good reason to also add instance-no.
> For busses that no natural address, we can define the address to be an 
> instance number.

I agree, it should be a bug for any device with a bus address to have an
instance number other than zero.  Anything without a bus number can make
use of instance numbers, and hopefully will never be hotplugged.

> > > That way, we gain a useful feature, and avoid having an savevm-specific
> > > "device path" that isn't recognized anywhere else.
> > 
> > Agreed, we should find one solution for all use cases.
> 
> I wasn't aware that there was any suggestion of a separate savevm-specific 
> path.  The whole point of a device path is to uniquely identify a device 
> w

Re: [Qemu-devel] Re: [CFR 9/10] device_del command

2010-06-15 Thread Miguel Di Ciurcio Filho
On Tue, Jun 15, 2010 at 1:59 PM, Jan Kiszka  wrote:
> Anthony Liguori wrote:
>> device_del
>> --
>>
>> Remove a device.
>>
>> Arguments:
>>
>> - "id": the device's ID (json-string)
>
> "id" should become "device" (I hope to send the corresponding patches
> this night). The idea is to not only allow global device IDs but also
> qtree paths here.
>

Now I'm confused. On a previous email when discussing about
query-netdev, I initially proposed this[1]:

-> { "execute": "query-netdev" }
<- {
  "return": [
 {
"device": "tap.0",
"vlan": 0,
"info": {
   "script": "/etc/qemu-ifup",
   "downscript": "/etc/qemu-ifdown",
   "ifname": "tap0",
   "model": "tap"
},
 },

And latter on Anthony suggested to replace "device" by "id" [2].

So my doubt is: we should identify guest/frontend devices in QMP using
the word "device" instead of "id" and identify backend devices by
using the word "id"?

Regards,

Miguel

[1] http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00619.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00943.html



[Qemu-devel] [PATCH v7 1/4] Device specification for shared memory PCI device

2010-06-15 Thread Cam Macdonell

Signed-off-by: Cam Macdonell 
---
 docs/specs/ivshmem_device_spec.txt |   96 
 1 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 docs/specs/ivshmem_device_spec.txt

diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
new file mode 100644
index 000..23dd2ba
--- /dev/null
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -0,0 +1,96 @@
+
+Device Specification for Inter-VM shared memory device
+--
+
+The Inter-VM shared memory device is designed to share a region of memory to
+userspace in multiple virtual guests.  The memory region does not belong to any
+guest, but is a POSIX memory object on the host.  Optionally, the device may
+support sending interrupts to other guests sharing the same memory region.
+
+
+The Inter-VM PCI device
+---
+
+*BARs*
+
+The device supports three BARs.  BAR0 is a 1 Kbyte MMIO region to support
+registers.  BAR1 is used for MSI-X when it is enabled in the device.  BAR2 is
+used to map the shared memory object from the host.  The size of BAR2 is
+specified when the guest is started and must be a power of 2 in size.
+
+*Registers*
+
+The device currently supports 4 registers of 32-bits each.  Registers
+are used for synchronization between guests sharing the same memory object when
+interrupts are supported (this requires using the shared memory server).
+
+The server assigns each VM an ID number and sends this ID number to the Qemu
+process when the guest starts.
+
+enum ivshmem_registers {
+IntrMask = 0,
+IntrStatus = 4,
+IVPosition = 8,
+Doorbell = 12
+};
+
+The first two registers are the interrupt mask and status registers.  Mask and
+status are only used with pin-based interrupts.  They are unused with MSI
+interrupts.
+
+Status Register: The status register is set to 1 when an interrupt occurs.
+
+Mask Register: The mask register is bitwise ANDed with the interrupt status
+and the result will raise an interrupt if it is non-zero.  However, since 1 is
+the only value the status will be set to, it is only the first bit of the mask
+that has any effect.  Therefore interrupts can be masked by setting the first
+bit to 0 and unmasked by setting the first bit to 1.
+
+IVPosition Register: The IVPosition register is read-only and reports the
+guest's ID number.  The guest IDs are non-negative integers.  When using the
+server, since the server is a separate process, the VM ID will only be set when
+the device is ready (shared memory is received from the server and accessible 
via
+the device).  If the device is not ready, the IVPosition will return -1.
+Applications should ensure that they have a valid VM ID before accessing the
+shared memory.
+
+Doorbell Register:  To interrupt another guest, a guest must write to the
+Doorbell register.  The doorbell register is 32-bits, logically divided into
+two 16-bit fields.  The high 16-bits are the guest ID to interrupt and the low
+16-bits are the interrupt vector to trigger.  The semantics of the value
+written to the doorbell depends on whether the device is using MSI or a regular
+pin-based interrupt.  In short, MSI uses vectors while regular interrupts set 
the
+status register.
+
+Regular Interrupts
+
+If regular interrupts are used (due to either a guest not supporting MSI or the
+user specifying not to use them on startup) then the value written to the lower
+16-bits of the Doorbell register results is arbitrary and will trigger an
+interrupt in the destination guest.
+
+Message Signalled Interrupts
+
+A ivshmem device may support multiple MSI vectors.  If so, the lower 16-bits
+written to the Doorbell register must be between 0 and the maximum number of
+vectors the guest supports.  The lower 16 bits written to the doorbell is the
+MSI vector that will be raised in the destination guest.  The number of MSI
+vectors is configurable but it is set when the VM is started.
+
+The important thing to remember with MSI is that it is only a signal, no status
+is set (since MSI interrupts are not shared).  All information other than the
+interrupt itself should be communicated via the shared memory region.  Devices
+supporting multiple MSI vectors can use different vectors to indicate different
+events have occurred.  The semantics of interrupt vectors are left to the
+user's discretion.
+
+
+Usage in the Guest
+--
+
+The shared memory device is intended to be used with the provided UIO driver.
+Very little configuration is needed.  The guest should map BAR0 to access the
+registers (an array of 32-bit ints allows simple writing) and map BAR2 to
+access the shared memory region itself.  The size of the shared memory region
+is specified when the guest (or shared memory server) is started.  A guest may
+map the whole shared memory region or only part of it.
-- 
1.6.6.1




[Qemu-devel] [PATCH v7 4/4] Inter-VM shared memory PCI device

2010-06-15 Thread Cam Macdonell
Support an inter-vm shared memory device that maps a shared-memory object as a
PCI device in the guest.  This patch also supports interrupts between guest by
communicating over a unix domain socket.  This patch applies to the qemu-kvm
repository.

-device ivshmem,size=[,shm=]

Interrupts are supported between multiple VMs by using a shared memory server
by using a chardev socket.

-device ivshmem,size=[,shm=]
   [,chardev=][,msi=on][,irqfd=on][,vectors=n][,role=peer|master]
-chardev socket,path=,id=

The shared memory server, sample programs and init scripts are in a git repo 
here:

www.gitorious.org/nahanni

Signed-off-by: Cam Macdonell 
---
 Makefile.target |3 +
 hw/ivshmem.c|  823 +++
 qemu-char.c |6 +
 qemu-char.h |3 +
 qemu-doc.texi   |   43 +++
 5 files changed, 878 insertions(+), 0 deletions(-)
 create mode 100644 hw/ivshmem.c

diff --git a/Makefile.target b/Makefile.target
index a0e9747..1e99ec8 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -203,6 +203,9 @@ obj-$(CONFIG_USB_OHCI) += usb-ohci.o
 obj-y += rtl8139.o
 obj-y += e1000.o
 
+# Inter-VM PCI shared memory
+obj-y += ivshmem.o
+
 # Hardware support
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
new file mode 100644
index 000..af035c3
--- /dev/null
+++ b/hw/ivshmem.c
@@ -0,0 +1,823 @@
+/*
+ * Inter-VM Shared Memory PCI device.
+ *
+ * Author:
+ *  Cam Macdonell 
+ *
+ * Based On: cirrus_vga.c
+ *  Copyright (c) 2004 Fabrice Bellard
+ *  Copyright (c) 2004 Makoto Suzuki (suzu)
+ *
+ *  and rtl8139.c
+ *  Copyright (c) 2006 Igor Kovalenko
+ *
+ * This code is licensed under the GNU GPL v2.
+ */
+#include "hw.h"
+#include "pc.h"
+#include "pci.h"
+#include "msix.h"
+#include "kvm.h"
+
+#include 
+#include 
+
+#define IVSHMEM_IRQFD   0
+#define IVSHMEM_MSI 1
+
+#define IVSHMEM_PEER0
+#define IVSHMEM_MASTER  1
+
+#define IVSHMEM_REG_BAR_SIZE 0x100
+
+//#define DEBUG_IVSHMEM
+#ifdef DEBUG_IVSHMEM
+#define IVSHMEM_DPRINTF(fmt, ...)\
+do {printf("IVSHMEM: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define IVSHMEM_DPRINTF(fmt, ...)
+#endif
+
+typedef struct Peer {
+int nb_eventfds;
+int *eventfds;
+} Peer;
+
+typedef struct EventfdEntry {
+PCIDevice *pdev;
+int vector;
+} EventfdEntry;
+
+typedef struct IVShmemState {
+PCIDevice dev;
+uint32_t intrmask;
+uint32_t intrstatus;
+uint32_t doorbell;
+
+CharDriverState **eventfd_chr;
+CharDriverState *server_chr;
+int ivshmem_mmio_io_addr;
+
+pcibus_t mmio_addr;
+pcibus_t shm_pci_addr;
+uint64_t ivshmem_offset;
+uint64_t ivshmem_size; /* size of shared memory region */
+int shm_fd; /* shared memory file descriptor */
+
+Peer *peers;
+int nb_peers; /* how many guests we have space for */
+int max_peer; /* maximum numbered peer */
+
+int vm_id;
+uint32_t vectors;
+uint32_t features;
+EventfdEntry *eventfd_table;
+
+char * shmobj;
+char * sizearg;
+char * role;
+int role_val;   /* scalar to avoid multiple string comparisons */
+} IVShmemState;
+
+/* registers for the Inter-VM shared memory device */
+enum ivshmem_registers {
+INTRMASK = 0,
+INTRSTATUS = 4,
+IVPOSITION = 8,
+DOORBELL = 12,
+};
+
+static inline uint32_t ivshmem_has_feature(IVShmemState *ivs, unsigned int 
feature) {
+return (ivs->features & (1 << feature));
+}
+
+static inline bool is_power_of_two(uint64_t x) {
+return (x & (x - 1)) == 0;
+}
+
+static void ivshmem_map(PCIDevice *pci_dev, int region_num,
+pcibus_t addr, pcibus_t size, int type)
+{
+IVShmemState *s = DO_UPCAST(IVShmemState, dev, pci_dev);
+
+s->shm_pci_addr = addr;
+
+if (s->ivshmem_offset > 0) {
+cpu_register_physical_memory(s->shm_pci_addr, s->ivshmem_size,
+s->ivshmem_offset);
+}
+
+IVSHMEM_DPRINTF("guest pci addr = %" FMT_PCIBUS ",guest h/w addr = %" 
PRIu64
+"size = %" FMT_PCIBUS "\n", addr, s->ivshmem_offset, size);
+
+}
+
+/* accessing registers - based on rtl8139 */
+static void ivshmem_update_irq(IVShmemState *s, int val)
+{
+int isr;
+isr = (s->intrstatus & s->intrmask) & 0x;
+
+/* don't print ISR resets */
+if (isr) {
+IVSHMEM_DPRINTF("Set IRQ to %d (%04x %04x)\n",
+   isr ? 1 : 0, s->intrstatus, s->intrmask);
+}
+
+qemu_set_irq(s->dev.irq[0], (isr != 0));
+}
+
+static void ivshmem_IntrMask_write(IVShmemState *s, uint32_t val)
+{
+IVSHMEM_DPRINTF("IntrMask write(w) val = 0x%04x\n", val);
+
+s->intrmask = val;
+
+ivshmem_update_irq(s, val);
+}
+
+static uint32_t ivshmem_IntrMask_read(IVShmemState *s)
+{
+uint32_t ret = s->intrmask;
+
+IVSHMEM_DPRINTF("intrmask read(w) val = 0x%04x\n", ret);
+
+return ret;
+}
+
+static void ivshm

[Qemu-devel] [PATCH v7 3/4] Support marking a device as non-migratable

2010-06-15 Thread Cam Macdonell
A non-migratable device should be removed before migration and re-added after.

Signed-off-by: Cam Macdonell 
---
 hw/hw.h  |1 +
 savevm.c |   32 +---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index d78d814..894e6b0 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
  void *opaque);
 
 void unregister_savevm(const char *idstr, void *opaque);
+void register_device_unmigratable(const char *idstr, void *opaque);
 
 typedef void QEMUResetHandler(void *opaque);
 
diff --git a/savevm.c b/savevm.c
index ada5c63..ac1d70d 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1024,6 +1024,7 @@ typedef struct SaveStateEntry {
 LoadStateHandler *load_state;
 const VMStateDescription *vmsd;
 void *opaque;
+int no_migrate;
 } SaveStateEntry;
 
 
@@ -1070,6 +1071,7 @@ int register_savevm_live(const char *idstr,
 se->load_state = load_state;
 se->opaque = opaque;
 se->vmsd = NULL;
+se->no_migrate = 0;
 
 if (instance_id == -1) {
 se->instance_id = calculate_new_instance_id(idstr);
@@ -1104,6 +1106,19 @@ void unregister_savevm(const char *idstr, void *opaque)
 }
 }
 
+/* mark a device as not to be migrated, that is the device should be
+   unplugged before migration */
+void register_device_unmigratable(const char *idstr, void *opaque)
+{
+SaveStateEntry *se;
+
+QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+if (strcmp(se->idstr, idstr) == 0 && se->opaque == opaque) {
+se->no_migrate = 1;
+}
+}
+}
+
 int vmstate_register_with_alias_id(int instance_id,
const VMStateDescription *vmsd,
void *opaque, int alias_id,
@@ -1278,13 +1293,19 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry 
*se, int version_id)
 return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
 }
 
-static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
+static int vmstate_save(QEMUFile *f, SaveStateEntry *se)
 {
+if (se->no_migrate) {
+return -1;
+}
+
 if (!se->vmsd) { /* Old style */
 se->save_state(f, se->opaque);
-return;
+return 0;
 }
 vmstate_save_state(f,se->vmsd, se->opaque);
+
+return 0;
 }
 
 #define QEMU_VM_FILE_MAGIC   0x5145564d
@@ -1378,6 +1399,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
 int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
 {
 SaveStateEntry *se;
+int r;
 
 cpu_synchronize_all_states();
 
@@ -1410,7 +1432,11 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
 qemu_put_be32(f, se->instance_id);
 qemu_put_be32(f, se->version_id);
 
-vmstate_save(f, se);
+r = vmstate_save(f, se);
+if (r < 0) {
+monitor_printf(mon, "cannot migrate with device '%s'\n", 
se->idstr);
+return r;
+}
 }
 
 qemu_put_byte(f, QEMU_VM_EOF);
-- 
1.6.3.2.198.g6096d




[Qemu-devel] [PATCH v7 2/4] Add function to assign ioeventfd to MMIO.

2010-06-15 Thread Cam Macdonell

Signed-off-by: Cam Macdonell 
---
 kvm-all.c |   32 
 kvm.h |1 +
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 47f58a6..2982631 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1257,6 +1257,38 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t 
*sigset)
 return r;
 }
 
+int kvm_set_ioeventfd_mmio_long(int fd, uint32_t addr, uint32_t val, bool 
assign)
+{
+#ifdef KVM_IOEVENTFD
+int ret;
+struct kvm_ioeventfd iofd;
+
+iofd.datamatch = val;
+iofd.addr = addr;
+iofd.len = 4;
+iofd.flags = KVM_IOEVENTFD_FLAG_DATAMATCH;
+iofd.fd = fd;
+
+if (!kvm_enabled()) {
+return -ENOSYS;
+}
+
+if (!assign) {
+iofd.flags |= KVM_IOEVENTFD_FLAG_DEASSIGN;
+}
+
+ret = kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &iofd);
+
+if (ret < 0) {
+return -errno;
+}
+
+return 0;
+#else
+return -ENOSYS;
+#endif
+}
+
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool 
assign)
 {
 #ifdef KVM_IOEVENTFD
diff --git a/kvm.h b/kvm.h
index aab5118..52e3a7f 100644
--- a/kvm.h
+++ b/kvm.h
@@ -181,6 +181,7 @@ static inline void cpu_synchronize_post_init(CPUState *env)
 }
 
 #endif
+int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool 
assign);
 
 #if defined(KVM_IRQFD) && defined(CONFIG_KVM)
 int kvm_set_irqfd(int gsi, int fd, bool assigned);
-- 
1.6.6.1




[Qemu-devel] [PATCH v7 0/4] Inter-VM shared memory device

2010-06-15 Thread Cam Macdonell
Latest patch for PCI shared memory device that maps a host shared memory object
to be shared between guests

new in this series

- replace marking memory from v6 with marking device as unmigratable 
indicating
  that it should be unplugged before migration and re-added after.
- 'peer' case changed to require removal before migration, only 'master'
  devices can be migrated while attached.

v6
- migration support with 'master' and 'peer' roles for guest to determine
  who "owns" memory
- modified phys_ram_dirty array for marking memory as not to be migrated

v5:
- fixed segfault for non-server case
- code style fixes
- removed limit on the number of guests
- shared memory server is now in qemu.git/contrib
- made ioeventfd setup function generic
- removed interrupts when guest joined (let application handle it)

v4:
- moved to single Doorbell register and use datamatch to trigger different
  VMs rather than one register per eventfd
- remove writing arbitrary values to eventfds.  Only values of 1 are now
  written to ensure correct usage

Cam Macdonell (4):
  Device specification for shared memory PCI device
  Add function to assign ioeventfd to MMIO.
  Support marking a device as non-migratable
  Inter-VM shared memory PCI device

 Makefile.target|3 +
 docs/specs/ivshmem_device_spec.txt |   96 +
 hw/hw.h|1 +
 hw/ivshmem.c   |  823 
 kvm-all.c  |   32 ++
 kvm.h  |1 +
 qemu-char.c|6 +
 qemu-char.h|3 +
 qemu-doc.texi  |   43 ++
 savevm.c   |   32 ++-
 10 files changed, 1037 insertions(+), 3 deletions(-)
 create mode 100644 docs/specs/ivshmem_device_spec.txt
 create mode 100644 hw/ivshmem.c




[Qemu-devel] Re: [RFC PATCH 0/5] Introduce canonical device hierarchy string

2010-06-15 Thread Alex Williamson
On Tue, 2010-06-15 at 11:12 +0200, Gerd Hoffmann wrote:
> > ISA: serial/parallel = iobase, others??
> 
> ne2k_isa has iobase too.

Added

> I think all remaining isa devices (timer, kbd, vga, ...) have a fixed 
> i/o base and can be only once in the system.

Yep, isa doesn't support hotplug either, so there's usually only one of
the devices and they don't move.

> > ide-drive: unit
> > I2C: address
> >
> > virtio-serial doesn't seem to make a DeviceState per port, so I think it
> > can be skipped.  I'm sure I'm still missing some...
> 
> Hmm?  -device virtio-serial -device virtconsole,chardev=foo,port=23

Ok, there's a nr property on virtconsole, I'll add that.  But I still
don't see ports enumerated as qdev entries for generic virtio-serial.

Alex






Re: [Qemu-devel] [CFR 7/10] cpu command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

cpu
---

Set the default CPU.
   


We need to remove this from QMP IMHO.  All QMP commands should take a 
CPU index.


Regards,

Anthony Liguori


Arguments:

- "index": the CPU's index (json-int)

Example:

->  { "execute": "cpu", "arguments": { "index": 0 } }
<- { "return": {} }

Note: CPUs' indexes are obtained with the 'query-cpus' command.

   





[Qemu-devel] [CFR 2/10] qmp: block_passwd command

2010-06-15 Thread Anthony Liguori
block_passwd


Set the password of encrypted block devices.

Arguments:

- "device": device name (json-string)
- "password": password (json-string)

Example:

-> { "execute": "block_passwd", "arguments": { "device": "ide0-hd0",
   "password": "12345" } }
<- { "return": {} }




[Qemu-devel] [CFR 6/10] cont command

2010-06-15 Thread Anthony Liguori
cont


Resume emulation.

Arguments: None.

Example:

-> { "execute": "cont" }
<- { "return": {} }



[Qemu-devel] [PATCH 1/2] Return usb device to host on usb_del command

2010-06-15 Thread Shahar Havivi

Signed-off-by: Shahar Havivi 
---
 usb-linux.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 88273ff..22a85e3 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -991,6 +991,7 @@ static int usb_host_close(USBHostDevice *dev)
 async_complete(dev);
 dev->closing = 0;
 usb_device_detach(&dev->dev);
+ioctl(dev->fd, USBDEVFS_RESET);
 close(dev->fd);
 dev->fd = -1;
 return 0;
-- 
1.7.0.4




[Qemu-devel] [PATCH 0/2] qemu-io: fix aio_read/write problems

2010-06-15 Thread MORITA Kazutaka
Hi,

This patchset fixes the following qemu-io problems:

 - Qemu-io exits suddenly when we do aio_read/write to drivers which
   use posix-aio-compat.

 - We cannot get the results of aio_read/write if we don't do other
   operations.  This problem occurs when the block driver uses a fd
   handler to get I/O completion.

Thanks,

Kazutaka


MORITA Kazutaka (2):
  qemu-io: retry fgets() when errno is EINTR
  qemu-io: check registered fds in command_loop()

 cmd.c |   56 ++--
 1 files changed, 42 insertions(+), 14 deletions(-)




[Qemu-devel] [PATCH 0/2 v4] Release usb devices on shutdown and usb_del

2010-06-15 Thread Shahar Havivi
v4:
use exit notifier instead of atexit()


Shahar Havivi (2):
  Return usb device to host on usb_del command
  Return usb device to host on exit

 usb-linux.c |   20 
 1 files changed, 20 insertions(+), 0 deletions(-)




[Qemu-devel] [PATCH 2/2] Return usb device to host on exit

2010-06-15 Thread Shahar Havivi

Signed-off-by: Shahar Havivi 
---
 usb-linux.c |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 22a85e3..4b5aeb6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -33,6 +33,7 @@
 #include "qemu-common.h"
 #include "qemu-timer.h"
 #include "monitor.h"
+#include "sysemu.h"
 
 #include 
 #include 
@@ -89,6 +90,8 @@ static char *usb_host_device_path;
 #define USB_FS_SYS 3
 
 static int usb_fs_type;
+static int usb_notify_set;
+static Notifier usb_host_notifier;
 
 /* endpoint association data */
 struct endp_data {
@@ -286,6 +289,17 @@ static void async_cancel(USBPacket *unused, void *opaque)
 }
 }
 
+static void usb_host_cleanup(struct Notifier* n)
+{
+struct USBHostDevice *s;
+
+QTAILQ_FOREACH(s, &hostdevs, next) {
+if (s->fd != -1) {
+ioctl(s->fd, USBDEVFS_RESET);
+}
+}
+}
+
 static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration)
 {
 int dev_descr_len, config_descr_len;
@@ -1066,6 +1080,11 @@ USBDevice *usb_host_device_open(const char *devname)
 qdev_prop_set_uint32(&dev->qdev, "vendorid",  filter.vendor_id);
 qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id);
 qdev_init_nofail(&dev->qdev);
+if (!usb_notify_set) {
+usb_notify_set = 1;
+usb_host_notifier.notify = usb_host_cleanup;
+qemu_add_exit_notifier(&usb_host_notifier);
+}
 return dev;
 
 fail:
-- 
1.7.0.4




Re: [Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable

2010-06-15 Thread Markus Armbruster
Anthony Liguori  writes:

> On 06/15/2010 11:16 AM, Cam Macdonell wrote:
>> How does this look for marking the device as non-migratable?  It adds a field
>> 'no_migrate' to the SaveStateEntry and tests for it in vmstate_save.  This 
>> would
>> replace anything that touches memory.
>>
>> Cam
>>
>> ---
>>   hw/hw.h  |1 +
>>   savevm.c |   32 +---
>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/hw.h b/hw/hw.h
>> index d78d814..7c93f08 100644
>> --- a/hw/hw.h
>> +++ b/hw/hw.h
>> @@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
>>void *opaque);
>>
>>   void unregister_savevm(const char *idstr, void *opaque);
>> +void mark_no_migrate(const char *idstr, void *opaque);
>>
>
> I'm not thrilled with the name but the functionality is spot on.  I
> lack the creativity to offer a better name suggestion :-)

Tongue firmly in cheek: mark_sedentary()?



Re: [Qemu-devel] Re: [RFC PATCH 0/5] Introduce canonical device hierarchy string

2010-06-15 Thread Alex Williamson
On Tue, 2010-06-15 at 10:53 +0200, Markus Armbruster wrote:
> Alex Williamson  writes:
> 
> > On Mon, 2010-06-14 at 09:02 +0200, Gerd Hoffmann wrote:
> >> Hi,
> >> 
> >> > My premise with this attempt is that we walk the hierarchy and use the
> >> > names to create the base of the path.  As we get to the device,
> >> > particularly to the parent bus of the device, we need to start looking at
> >> > properties to ensure uniqueness.
> >> 
> >> You'll need that for every bus along the way down to the device.  Create 
> >> a virtual machine with two lsi scsi host adapters, then attach a disk 
> >> with scsi id 0 to each.  Just the scsi id isn't good enougth to identify 
> >> the device.  You'll need the lsi pci address too.
> >
> > Yep, see below.
> >
> >> > For now, the only properties I've tagged as path
> >> > properties are PCI bus addresses and MAC addresses.
> >> 
> >> mac address isn't needed here.  You need the property which specifies 
> >> the bus address.  For PCI this obviously is the PCI address.  For scsi 
> >> the scsi id.  For ISA you can use the I/O port base.  virtio-serial the 
> >> port number, ...
> >
> > PCI: addr
> > SCSI: scsi-id
> > ISA: serial/parallel = iobase, others??
> 
> If there's no iobase (pathological case), require ID.
> 
> > ide-drive: unit
> 
> Bus name is IDE, but it's clear enough what you mean :)

I put ide-drive here because the unit is a property of the device, not
the bus.

> > I2C: address
> >
> > virtio-serial doesn't seem to make a DeviceState per port, so I think it
> > can be skipped.
> 
> Really?
> 
> Anyway, its port number should do as bus address.

Maybe I'm not specifying it correctly.  I see a max_nr_ports property,
but I don't see that each port is a separate qdev.

> >  I'm sure I'm still missing some...
> 
> s390-virtio
> SSI
> System

I'll need some help coming up with useful properties to key on for
these.  I had hoped there's only one System bus.

> USB

usb-storage seems to have a useful drive property that lets me
distinguish these devices:

/i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0/scsi.0/scsi-disk.0
/i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb1/scsi.0/scsi-disk.0
 drive

But otherwise USB is disappointingly devoid of useful properties at the
bus level.

Alex




[Qemu-devel] [PATCH 1/2] qemu-io: retry fgets() when errno is EINTR

2010-06-15 Thread MORITA Kazutaka
posix-aio-compat sends a signal in aio operations, so we should
consider that fgets() could be interrupted here.

Signed-off-by: MORITA Kazutaka 
---
 cmd.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/cmd.c b/cmd.c
index 2336334..460df92 100644
--- a/cmd.c
+++ b/cmd.c
@@ -272,7 +272,10 @@ fetchline(void)
return NULL;
printf("%s", get_prompt());
fflush(stdout);
+again:
if (!fgets(line, MAXREADLINESZ, stdin)) {
+   if (errno == EINTR)
+   goto again;
free(line);
return NULL;
}
-- 
1.5.6.5




[Qemu-devel] [PATCH 2/2] qemu-io: check registered fds in command_loop()

2010-06-15 Thread MORITA Kazutaka
Some block drivers use an aio handler and do I/O completion routines
in it.  However, the handler is not invoked if we only do
aio_read/write, because registered fds are not checked at all.

This patch registers a command processing function as a fd handler to
STDIO, and calls qemu_aio_wait() in command_loop().  Any other
handlers can be invoked when user input is idle.

Signed-off-by: MORITA Kazutaka 
---
 cmd.c |   53 +++--
 1 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/cmd.c b/cmd.c
index 460df92..2b66e24 100644
--- a/cmd.c
+++ b/cmd.c
@@ -24,6 +24,7 @@
 #include 
 
 #include "cmd.h"
+#include "qemu-aio.h"
 
 #define _(x)   x   /* not gettext support yet */
 
@@ -149,6 +150,37 @@ add_args_command(
args_func = af;
 }
 
+static char *get_prompt(void);
+
+static void do_command(void *opaque)
+{
+   int c;
+   int *done = opaque;
+   char*input;
+   char**v;
+   const cmdinfo_t *ct;
+
+   if ((input = fetchline()) == NULL) {
+   *done = 1;
+   return;
+   }
+   v = breakline(input, &c);
+   if (c) {
+   ct = find_command(v[0]);
+   if (ct)
+   *done = command(ct, c, v);
+   else
+   fprintf(stderr, _("command \"%s\" not found\n"),
+   v[0]);
+   }
+   doneline(input, v);
+
+   if (*done == 0) {
+   printf("%s", get_prompt());
+   fflush(stdout);
+   }
+}
+
 void
 command_loop(void)
 {
@@ -186,20 +218,15 @@ command_loop(void)
free(cmdline);
return;
}
+
+   printf("%s", get_prompt());
+   fflush(stdout);
+
+   qemu_aio_set_fd_handler(STDIN_FILENO, do_command, NULL, NULL, NULL, 
&done);
while (!done) {
-   if ((input = fetchline()) == NULL)
-   break;
-   v = breakline(input, &c);
-   if (c) {
-   ct = find_command(v[0]);
-   if (ct)
-   done = command(ct, c, v);
-   else
-   fprintf(stderr, _("command \"%s\" not found\n"),
-   v[0]);
-   }
-   doneline(input, v);
+   qemu_aio_wait();
}
+   qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
 }
 
 /* from libxcmd/input.c */
@@ -270,8 +297,6 @@ fetchline(void)
 
if (!line)
return NULL;
-   printf("%s", get_prompt());
-   fflush(stdout);
 again:
if (!fgets(line, MAXREADLINESZ, stdin)) {
if (errno == EINTR)
-- 
1.5.6.5




[Qemu-devel] Re: [CFR 9/10] device_del command

2010-06-15 Thread Jan Kiszka
Anthony Liguori wrote:
> device_del
> --
> 
> Remove a device.
> 
> Arguments:
> 
> - "id": the device's ID (json-string)

"id" should become "device" (I hope to send the corresponding patches
this night). The idea is to not only allow global device IDs but also
qtree paths here.

> 
> Example:
> 
> -> { "execute": "device_del", "arguments": { "id": "net1" } }
> <- { "return": {} }
> 
> 
> 

A reference to docs/qdev-device-use.txt would be good here as well (as
done for device_add).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] Re: [CFR 7/10] cpu command

2010-06-15 Thread Jan Kiszka
Anthony Liguori wrote:
> cpu
> ---
> 
> Set the default CPU.
> 
> Arguments:
> 
> - "index": the CPU's index (json-int)
> 
> Example:
> 
> -> { "execute": "cpu", "arguments": { "index": 0 } }
> <- { "return": {} }
> 
> Note: CPUs' indexes are obtained with the 'query-cpus' command.
> 

To my understanding, this command is supposed to die. All QMP commands
addressing a specific CPU should do this explicitly.

(Will send a patch to enable this without having to modify the HMP
syntax. And another patch to allow masking commands from QMP.)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [CFR 4/10] command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

change
--

Change a removable medium or VNC configuration.
   


I think this command is awkward and should be split into a change-media 
and change-vnc-password command.



Arguments:

- "device": device name (json-string)
- "target": filename or item (json-string)
- "arg": additional argument (json-string, optional)
   


For change-media:

1) what happens if the drive does not support removable media?
2) what happens if the drive is locked?
3) what happens if media is not currently present?
4) what if I want to pass additional options to target like format=raw?
5) is the media change immediately present to the guest upon return of 
the command?


For change-vnc-password:

1) what if VNC is not in use?
2) what if I don't have vnc authentication enabled?
3) does changing the password have any affect on existing sessions?
4) is a new password required immediately after the command completes?

Regards,

Anthony Liguori


Examples:

1. Change a removable medium

->  { "execute": "change",
  "arguments": { "device": "ide1-cd0",
 "target": "/srv/images/Fedora-12-x86_64-DVD.iso" } 
}
<- { "return": {} }

2. Change VNC password

->  { "execute": "change",
  "arguments": { "device": "vnc", "target": "password",
 "arg": "foobar1" } }
<- { "return": {} }


   





Re: [Qemu-devel] [CFR 9/10] device_del command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

device_del
--

Remove a device.

Arguments:

- "id": the device's ID (json-string)
   


How does one discover a device's ID?

What are the failure conditions?

Is the device removed from the guest immediately upon receiving the return?

Regards,

Anthony Liguori


Example:

->  { "execute": "device_del", "arguments": { "id": "net1" } }
<- { "return": {} }


   





[Qemu-devel] [CFR 10/10] eject command

2010-06-15 Thread Anthony Liguori
eject
-

Eject a removable medium.

Arguments: 

- force: force ejection (json-bool, optional)
- device: device name (json-string)

Example:

-> { "execute": "eject", "arguments": { "device": "ide1-cd0" } }
<- { "return": {} }

Note: The "force" argument defaults to false.




Re: [Qemu-devel] [CFR 8/10] device_add command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

device_add
--

Add a device.

Arguments:

- "driver": the name of the new device's driver (json-string)
   


What class of name is this?  I believe this is a qdev name but the 
example is misleading because someone could reasonable do { "driver": 
"virtio", "id": "net1"}



- "bus": the device's parent bus (device tree path, json-string, optional)
- "id": the device's ID, must be unique (json-string)
- device properties
   


I think we need to document all of the supported devices and their 
properties as part of the spec.


What happens if we cannot add the device?  How does one use this for hot 
add?


Is the device available within the guest immediately after the 
device_add operation completes?


Regards,

Anthony Liguori


Example:

->  { "execute": "device_add", "arguments": { "driver": "e1000", "id": "net1" } 
}
<- { "return": {} }

Notes:

(1) For detailed information about this command, please refer to the
 'docs/qdev-device-use.txt' file.

(2) It's possible to list device properties by running QEMU with the
 "-device DEVICE,\?" command-line argument, where DEVICE is the device's 
name

   





Re: [Qemu-devel] [CFR 6/10] cont command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

cont

   


continue -> aliases cont.


Resume emulation.
   


What happens if execution is not stopped?

Regards,

Anthony Liguori


Arguments: None.

Example:

->  { "execute": "cont" }
<- { "return": {} }

   





Re: [Qemu-devel] [CFR 2/10] qmp: block_passwd command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

block_passwd

   


I dislike abbreviations.  I also think that we should make commands 
verbs.  So I'd like to change the name to set_block_password and then we 
can alias block_passwd to that command if we need to.



Set the password of encrypted block devices.

Arguments:

- "device": device name (json-string)
- "password": password (json-string)
   


What happens if:

1) device does not exist
2) media is not present
3) block format does not support encryption
4) password is invalid

Regards,

Anthony Liguori


Example:

->  { "execute": "block_passwd", "arguments": { "device": "ide0-hd0",
"password": "12345" } }
<- { "return": {} }


   





[Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:16 AM, Cam Macdonell wrote:

How does this look for marking the device as non-migratable?  It adds a field
'no_migrate' to the SaveStateEntry and tests for it in vmstate_save.  This would
replace anything that touches memory.

Cam

---
  hw/hw.h  |1 +
  savevm.c |   32 +---
  2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index d78d814..7c93f08 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr,
   void *opaque);

  void unregister_savevm(const char *idstr, void *opaque);
+void mark_no_migrate(const char *idstr, void *opaque);
   


I'm not thrilled with the name but the functionality is spot on.  I lack 
the creativity to offer a better name suggestion :-)


Regards,

Anthony Liguori


  typedef void QEMUResetHandler(void *opaque);

diff --git a/savevm.c b/savevm.c
index 017695b..2642a9c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1023,6 +1023,7 @@ typedef struct SaveStateEntry {
  LoadStateHandler *load_state;
  const VMStateDescription *vmsd;
  void *opaque;
+int no_migrate;
  } SaveStateEntry;


@@ -1069,6 +1070,7 @@ int register_savevm_live(const char *idstr,
  se->load_state = load_state;
  se->opaque = opaque;
  se->vmsd = NULL;
+se->no_migrate = 0;

  if (instance_id == -1) {
  se->instance_id = calculate_new_instance_id(idstr);
@@ -1103,6 +1105,19 @@ void unregister_savevm(const char *idstr, void *opaque)
  }
  }

+/* mark a device as not to be migrated, that is the device should be
+   unplugged before migration */
+void mark_no_migrate(const char *idstr, void *opaque)
+{
+SaveStateEntry *se;
+
+QTAILQ_FOREACH(se,&savevm_handlers, entry) {
+if (strcmp(se->idstr, idstr) == 0&&  se->opaque == opaque) {
+se->no_migrate = 1;
+}
+}
+}
+
  int vmstate_register_with_alias_id(int instance_id,
 const VMStateDescription *vmsd,
 void *opaque, int alias_id,
@@ -1277,13 +1292,19 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry 
*se, int version_id)
  return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
  }

-static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
+static int vmstate_save(QEMUFile *f, SaveStateEntry *se)
  {
+if (se->no_migrate) {
+return -1;
+}
+
  if (!se->vmsd) { /* Old style */
  se->save_state(f, se->opaque);
-return;
+return 0;
  }
  vmstate_save_state(f,se->vmsd, se->opaque);
+
+return 0;
  }

  #define QEMU_VM_FILE_MAGIC   0x5145564d
@@ -1377,6 +1398,7 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f)
  int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
  {
  SaveStateEntry *se;
+int r;

  cpu_synchronize_all_states();

@@ -1409,7 +1431,11 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
  qemu_put_be32(f, se->instance_id);
  qemu_put_be32(f, se->version_id);

-vmstate_save(f, se);
+r = vmstate_save(f, se);
+if (r<  0) {
+monitor_printf(mon, "cannot migrate with device '%s'\n", 
se->idstr);
+return r;
+}
  }

  qemu_put_byte(f, QEMU_VM_EOF);
   





Re: [Qemu-devel] [CFR 5/10] closefd command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

closefd
---

Close a file descriptor previously passed via SCM rights.

Arguments:

- "fdname": file descriptor name (json-string)
   


I wonder if setfd/closefd should be QMP functions or whether we should 
make it core features of the protocol?


Regards,

Anthony Liguori


Example:

->  { "execute": "closefd", "arguments": { "fdname": "fd1" } }
<- { "return": {} }


   





[Qemu-devel] [CFR 4/10] command

2010-06-15 Thread Anthony Liguori
change
--

Change a removable medium or VNC configuration.

Arguments:

- "device": device name (json-string)
- "target": filename or item (json-string)
- "arg": additional argument (json-string, optional)

Examples:

1. Change a removable medium

-> { "execute": "change",
 "arguments": { "device": "ide1-cd0",
"target": "/srv/images/Fedora-12-x86_64-DVD.iso" } }
<- { "return": {} }

2. Change VNC password

-> { "execute": "change",
 "arguments": { "device": "vnc", "target": "password",
"arg": "foobar1" } }
<- { "return": {} }




[Qemu-devel] [CFR 7/10] cpu command

2010-06-15 Thread Anthony Liguori
cpu
---

Set the default CPU.

Arguments:

- "index": the CPU's index (json-int)

Example:

-> { "execute": "cpu", "arguments": { "index": 0 } }
<- { "return": {} }

Note: CPUs' indexes are obtained with the 'query-cpus' command.



Re: [Qemu-devel] [CFR 1/10] qmp: balloon command

2010-06-15 Thread Anthony Liguori

On 06/15/2010 11:30 AM, Anthony Liguori wrote:

balloon
---

Request VM to change its memory allocation (in bytes).
   


We ought to clarify a few points:

1) What happens if there isn't a balloon device?
2) What happens upon reboot?
3) What is the valid range for value?
4) How does a user determine when the guest has actually responded to 
the balloon request?


This is a case where I think we ought to use async events because the 
guest can adjust it's balloon allotment without an explicit request from 
the host.  In fact, if we add a shrinker callback to the balloon driver 
(and we should) it will do exactly this.


Regards,

Anthony Liguori


Arguments:

- "value": New memory allocation (json-int)

Example:

->  { "execute": "balloon", "arguments": { "value": 536870912 } }
<- { "return": {} }


   





  1   2   3   >