* [Qemu-devel] [PATCH 0/9] initial spice support.
@ 2010-08-19 12:40 Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library Gerd Hoffmann
` (8 more replies)
0 siblings, 9 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Hi,
/me wants getting the spice support merging started, here is the first
batch of patches. It brings just the very basic bits:
* Detect spice in configure, Makefile windup.
* Support for keyboard, mouse and tablet.
* Support for simple display output (works as DisplayChangeListener,
plays with any gfx card, sends simple draw commands to update
dirty regions).
Note that this patch series does *not* yet contain the qxl paravirtual
gfx card. That will come as part of a additional patch series after
sorting the vgabios support.
The patches are also available in the git repository at:
git://anongit.freedesktop.org/spice/qemu submit.2
cheers,
Gerd
Gerd Hoffmann (9):
add pflib: PixelFormat conversion library.
configure: add logging
add spice into the configure file
configure: require spice 0.5.3
spice: core bits
spice: add keyboard
spice: add mouse
spice: simple display
spice: add tablet support
Makefile.objs | 3 +
configure | 42 ++++++-
pflib.c | 213 ++++++++++++++++++++++++++++++
pflib.h | 20 +++
qemu-config.c | 23 ++++
qemu-config.h | 1 +
qemu-options.hx | 8 +
qemu-spice.h | 24 ++++
spice-display.c | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
spice-display.h | 52 ++++++++
spice-input.c | 173 +++++++++++++++++++++++++
spice.c | 153 ++++++++++++++++++++++
vl.c | 22 +++-
13 files changed, 1118 insertions(+), 3 deletions(-)
create mode 100644 pflib.c
create mode 100644 pflib.h
create mode 100644 qemu-spice.h
create mode 100644 spice-display.c
create mode 100644 spice-display.h
create mode 100644 spice-input.c
create mode 100644 spice.c
^ permalink raw reply [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library.
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:04 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 2/9] configure: add logging Gerd Hoffmann
` (7 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.objs | 1 +
pflib.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
pflib.h | 20 ++++++
3 files changed, 234 insertions(+), 0 deletions(-)
create mode 100644 pflib.c
create mode 100644 pflib.h
diff --git a/Makefile.objs b/Makefile.objs
index 4a1eaa1..edfca87 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -83,6 +83,7 @@ common-obj-y += qemu-char.o savevm.o #aio.o
common-obj-y += msmouse.o ps2.o
common-obj-y += qdev.o qdev-properties.o
common-obj-y += block-migration.o
+common-obj-y += pflib.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/pflib.c b/pflib.c
new file mode 100644
index 0000000..1154d0c
--- /dev/null
+++ b/pflib.c
@@ -0,0 +1,213 @@
+/*
+ * PixelFormat conversion library.
+ *
+ * Author: Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu-common.h"
+#include "console.h"
+#include "pflib.h"
+
+typedef struct QemuPixel QemuPixel;
+
+typedef void (*pf_convert)(QemuPfConv *conv,
+ void *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_from)(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt);
+typedef void (*pf_convert_to)(PixelFormat *pf,
+ void *dst, QemuPixel *src, uint32_t cnt);
+
+struct QemuPfConv {
+ pf_convert convert;
+ PixelFormat src;
+ PixelFormat dst;
+
+ /* for copy_generic() */
+ pf_convert_from conv_from;
+ pf_convert_to conv_to;
+ QemuPixel *conv_buf;
+ uint32_t conv_cnt;
+};
+
+struct QemuPixel {
+ uint8_t red;
+ uint8_t green;
+ uint8_t blue;
+ uint8_t alpha;
+};
+
+/* ----------------------------------------------------------------------- */
+/* PixelFormat -> QemuPixel conversions */
+
+static void conv_16_to_pixel(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt)
+{
+ uint16_t *src16 = src;
+
+ while (cnt > 0) {
+ dst->red = ((*src16 & pf->rmask) >> pf->rshift) << (8 - pf->rbits);
+ dst->green = ((*src16 & pf->gmask) >> pf->gshift) << (8 - pf->gbits);
+ dst->blue = ((*src16 & pf->bmask) >> pf->bshift) << (8 - pf->bbits);
+ dst->alpha = ((*src16 & pf->amask) >> pf->ashift) << (8 - pf->abits);
+ dst++, src16++, cnt--;
+ }
+}
+
+/* assumes pf->{r,g,b,a}bits == 8 */
+static void conv_32_to_pixel_fast(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt)
+{
+ uint32_t *src32 = src;
+
+ while (cnt > 0) {
+ dst->red = (*src32 & pf->rmask) >> pf->rshift;
+ dst->green = (*src32 & pf->gmask) >> pf->gshift;
+ dst->blue = (*src32 & pf->bmask) >> pf->bshift;
+ dst->alpha = (*src32 & pf->amask) >> pf->ashift;
+ dst++, src32++, cnt--;
+ }
+}
+
+static void conv_32_to_pixel_generic(PixelFormat *pf,
+ QemuPixel *dst, void *src, uint32_t cnt)
+{
+ uint32_t *src32 = src;
+
+ while (cnt > 0) {
+ if (pf->rbits < 8) {
+ dst->red = ((*src32 & pf->rmask) >> pf->rshift) << (8 - pf->rbits);
+ } else {
+ dst->red = ((*src32 & pf->rmask) >> pf->rshift) >> (pf->rbits - 8);
+ }
+ if (pf->gbits < 8) {
+ dst->green = ((*src32 & pf->gmask) >> pf->gshift) << (8 - pf->gbits);
+ } else {
+ dst->green = ((*src32 & pf->gmask) >> pf->gshift) >> (pf->gbits - 8);
+ }
+ if (pf->bbits < 8) {
+ dst->blue = ((*src32 & pf->bmask) >> pf->bshift) << (8 - pf->bbits);
+ } else {
+ dst->blue = ((*src32 & pf->bmask) >> pf->bshift) >> (pf->bbits - 8);
+ }
+ if (pf->abits < 8) {
+ dst->alpha = ((*src32 & pf->amask) >> pf->ashift) << (8 - pf->abits);
+ } else {
+ dst->alpha = ((*src32 & pf->amask) >> pf->ashift) >> (pf->abits - 8);
+ }
+ dst++, src32++, cnt--;
+ }
+}
+
+/* ----------------------------------------------------------------------- */
+/* QemuPixel -> PixelFormat conversions */
+
+static void conv_pixel_to_16(PixelFormat *pf,
+ void *dst, QemuPixel *src, uint32_t cnt)
+{
+ uint16_t *dst16 = dst;
+
+ while (cnt > 0) {
+ *dst16 = ((uint16_t)src->red >> (8 - pf->rbits)) << pf->rshift;
+ *dst16 |= ((uint16_t)src->green >> (8 - pf->gbits)) << pf->gshift;
+ *dst16 |= ((uint16_t)src->blue >> (8 - pf->bbits)) << pf->bshift;
+ *dst16 |= ((uint16_t)src->alpha >> (8 - pf->abits)) << pf->ashift;
+ dst16++, src++, cnt--;
+ }
+}
+
+static void conv_pixel_to_32(PixelFormat *pf,
+ void *dst, QemuPixel *src, uint32_t cnt)
+{
+ uint32_t *dst32 = dst;
+
+ while (cnt > 0) {
+ *dst32 = ((uint32_t)src->red >> (8 - pf->rbits)) << pf->rshift;
+ *dst32 |= ((uint32_t)src->green >> (8 - pf->gbits)) << pf->gshift;
+ *dst32 |= ((uint32_t)src->blue >> (8 - pf->bbits)) << pf->bshift;
+ *dst32 |= ((uint32_t)src->alpha >> (8 - pf->abits)) << pf->ashift;
+ dst32++, src++, cnt--;
+ }
+}
+
+/* ----------------------------------------------------------------------- */
+/* PixelFormat -> PixelFormat conversions */
+
+static void convert_copy(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+ uint32_t bytes = cnt * conv->src.bytes_per_pixel;
+ memcpy(dst, src, bytes);
+}
+
+static void convert_generic(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+ if (conv->conv_cnt < cnt) {
+ conv->conv_cnt = cnt;
+ conv->conv_buf = qemu_realloc(conv->conv_buf, sizeof(QemuPixel) * conv->conv_cnt);
+ }
+ conv->conv_from(&conv->src, conv->conv_buf, src, cnt);
+ conv->conv_to(&conv->dst, dst, conv->conv_buf, cnt);
+}
+
+/* ----------------------------------------------------------------------- */
+/* public interface */
+
+QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src)
+{
+ QemuPfConv *conv = qemu_mallocz(sizeof(QemuPfConv));
+
+ conv->src = *src;
+ conv->dst = *dst;
+
+ if (memcmp(&conv->src, &conv->dst, sizeof(PixelFormat)) == 0) {
+ /* formats identical, can simply copy */
+ conv->convert = convert_copy;
+ } else {
+ /* generic two-step conversion: src -> QemuPixel -> dst */
+ switch (conv->src.bytes_per_pixel) {
+ case 2:
+ conv->conv_from = conv_16_to_pixel;
+ break;
+ case 4:
+ if (conv->src.rbits == 8 && conv->src.gbits == 8 && conv->src.bbits == 8) {
+ conv->conv_from = conv_32_to_pixel_fast;
+ } else {
+ conv->conv_from = conv_32_to_pixel_generic;
+ }
+ break;
+ default:
+ goto err;
+ }
+ switch (conv->dst.bytes_per_pixel) {
+ case 2:
+ conv->conv_to = conv_pixel_to_16;
+ break;
+ case 4:
+ conv->conv_to = conv_pixel_to_32;
+ break;
+ default:
+ goto err;
+ }
+ conv->convert = convert_generic;
+ }
+ return conv;
+
+err:
+ qemu_free(conv);
+ return NULL;
+}
+
+void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
+{
+ conv->convert(conv, dst, src, cnt);
+}
+
+void qemu_pf_conv_put(QemuPfConv *conv)
+{
+ if (conv) {
+ qemu_free(conv->conv_buf);
+ qemu_free(conv);
+ }
+}
diff --git a/pflib.h b/pflib.h
new file mode 100644
index 0000000..b70c313
--- /dev/null
+++ b/pflib.h
@@ -0,0 +1,20 @@
+#ifndef __QEMU_PFLIB_H
+#define __QEMU_PFLIB_H
+
+/*
+ * PixelFormat conversion library.
+ *
+ * Author: Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+typedef struct QemuPfConv QemuPfConv;
+
+QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src);
+void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt);
+void qemu_pf_conv_put(QemuPfConv *conv);
+
+#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 2/9] configure: add logging
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:05 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 3/9] add spice into the configure file Gerd Hoffmann
` (6 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Write compile commands and messages to config.log.
Useful for debugging configure.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index a20371c..13d8be0 100755
--- a/configure
+++ b/configure
@@ -16,15 +16,18 @@ TMPO="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.o"
TMPE="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.exe"
trap "rm -f $TMPC $TMPO $TMPE ; exit" EXIT INT QUIT TERM
+rm -f config.log
compile_object() {
- $cc $QEMU_CFLAGS -c -o $TMPO $TMPC > /dev/null 2> /dev/null
+ echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
+ $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
}
compile_prog() {
local_cflags="$1"
local_ldflags="$2"
- $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
+ echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
+ $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
}
# check whether a command is available to this shell (may be either an
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 3/9] add spice into the configure file
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 2/9] configure: add logging Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:06 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 4/9] configure: require spice 0.5.3 Gerd Hoffmann
` (5 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 13d8be0..56e7084 100755
--- a/configure
+++ b/configure
@@ -318,6 +318,7 @@ pkgversion=""
check_utests="no"
user_pie="no"
zero_malloc=""
+spice=""
# OS specific
if check_define __linux__ ; then
@@ -619,6 +620,10 @@ for opt do
;;
--enable-kvm) kvm="yes"
;;
+ --disable-spice) spice="no"
+ ;;
+ --enable-spice) spice="yes"
+ ;;
--enable-profiler) profiler="yes"
;;
--enable-cocoa)
@@ -898,6 +903,8 @@ echo " --enable-docs enable documentation build"
echo " --disable-docs disable documentation build"
echo " --disable-vhost-net disable vhost-net acceleration support"
echo " --enable-vhost-net enable vhost-net acceleration support"
+echo " --disable-spice disable spice"
+echo " --enable-spice enable spice"
echo ""
echo "NOTE: The object files are built at the place where configure is launched"
exit 1
@@ -2048,6 +2055,30 @@ if compile_prog "" ""; then
gcc_attribute_warn_unused_result=yes
fi
+# spice probe
+if test "$spice" != "no" ; then
+ cat > $TMPC << EOF
+#include <spice.h>
+int main(void) { spice_server_new(); return 0; }
+EOF
+ spice_proto_ver=$($pkgconfig --modversion spice-protocol 2>/dev/null)
+ spice_server_ver=$($pkgconfig --modversion spice-server 2>/dev/null)
+ spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2>/dev/null)
+ spice_libs=$($pkgconfig --libs spice-protocol spice-server 2>/dev/null)
+ if compile_prog "$spice_cflags" "$spice_libs" ; then
+ spice="yes"
+ libs_softmmu="$libs_softmmu $spice_libs"
+ QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
+ else
+ if test "$spice" = "yes" ; then
+ feature_not_found "spice"
+ fi
+ spice="no"
+ fi
+fi
+
+##########################################
+
##########################################
# check if we have fdatasync
@@ -2190,6 +2221,7 @@ echo "preadv support $preadv"
echo "fdatasync $fdatasync"
echo "uuid support $uuid"
echo "vhost-net support $vhost_net"
+echo "spice support $spice"
if test $sdl_too_old = "yes"; then
echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -2427,6 +2459,10 @@ if test "$fdatasync" = "yes" ; then
echo "CONFIG_FDATASYNC=y" >> $config_host_mak
fi
+if test "$spice" = "yes" ; then
+ echo "CONFIG_SPICE=y" >> $config_host_mak
+fi
+
# XXX: suppress that
if [ "$bsd" = "yes" ] ; then
echo "CONFIG_BSD=y" >> $config_host_mak
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 4/9] configure: require spice 0.5.3
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (2 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 3/9] add spice into the configure file Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 5/9] spice: core bits Gerd Hoffmann
` (4 subsequent siblings)
8 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 56e7084..0998e86 100755
--- a/configure
+++ b/configure
@@ -2061,11 +2061,10 @@ if test "$spice" != "no" ; then
#include <spice.h>
int main(void) { spice_server_new(); return 0; }
EOF
- spice_proto_ver=$($pkgconfig --modversion spice-protocol 2>/dev/null)
- spice_server_ver=$($pkgconfig --modversion spice-server 2>/dev/null)
spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2>/dev/null)
spice_libs=$($pkgconfig --libs spice-protocol spice-server 2>/dev/null)
- if compile_prog "$spice_cflags" "$spice_libs" ; then
+ if $pkgconfig --atleast-version=0.5.3 spice-server &&\
+ compile_prog "$spice_cflags" "$spice_libs" ; then
spice="yes"
libs_softmmu="$libs_softmmu $spice_libs"
QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 5/9] spice: core bits
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (3 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 4/9] configure: require spice 0.5.3 Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:19 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 6/9] spice: add keyboard Gerd Hoffmann
` (3 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Add -spice command line switch. Has support setting passwd and port for
now. With this patch applied the spice client can successfully connect
to qemu. You can't do anything useful yet though.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.objs | 2 +
qemu-config.c | 23 ++++++++
qemu-config.h | 1 +
qemu-options.hx | 8 +++
qemu-spice.h | 22 ++++++++
spice.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
vl.c | 15 ++++++
7 files changed, 222 insertions(+), 0 deletions(-)
create mode 100644 qemu-spice.h
create mode 100644 spice.c
diff --git a/Makefile.objs b/Makefile.objs
index edfca87..021067b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,6 +88,8 @@ common-obj-y += pflib.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
+common-obj-$(CONFIG_SPICE) += spice.o
+
audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
audio-obj-$(CONFIG_SDL) += sdlaudio.o
audio-obj-$(CONFIG_OSS) += ossaudio.o
diff --git a/qemu-config.c b/qemu-config.c
index 95abe61..4cf3b53 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -342,6 +342,26 @@ QemuOptsList qemu_cpudef_opts = {
},
};
+#ifdef CONFIG_SPICE
+QemuOptsList qemu_spice_opts = {
+ .name = "spice",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_spice_opts.head),
+ .desc = {
+ {
+ .name = "port",
+ .type = QEMU_OPT_NUMBER,
+ },{
+ .name = "password",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "disable-ticketing",
+ .type = QEMU_OPT_BOOL,
+ },
+ { /* end if list */ }
+ },
+};
+#endif
+
static QemuOptsList *vm_config_groups[] = {
&qemu_drive_opts,
&qemu_chardev_opts,
@@ -352,6 +372,9 @@ static QemuOptsList *vm_config_groups[] = {
&qemu_global_opts,
&qemu_mon_opts,
&qemu_cpudef_opts,
+#ifdef CONFIG_SPICE
+ &qemu_spice_opts,
+#endif
NULL,
};
diff --git a/qemu-config.h b/qemu-config.h
index dca69d4..3a90213 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -14,6 +14,7 @@ extern QemuOptsList qemu_rtc_opts;
extern QemuOptsList qemu_global_opts;
extern QemuOptsList qemu_mon_opts;
extern QemuOptsList qemu_cpudef_opts;
+extern QemuOptsList qemu_spice_opts;
QemuOptsList *qemu_find_opts(const char *group);
int qemu_set_option(const char *str);
diff --git a/qemu-options.hx b/qemu-options.hx
index db86feb..c05c219 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -674,6 +674,14 @@ STEXI
Enable SDL.
ETEXI
+#ifdef CONFIG_SPICE
+DEF("spice", HAS_ARG, QEMU_OPTION_spice,
+ "-spice <args> use spice\n", QEMU_ARCH_ALL)
+STEXI
+Use Spice.
+ETEXI
+#endif
+
DEF("portrait", 0, QEMU_OPTION_portrait,
"-portrait rotate graphical output 90 deg left (only PXA LCD)\n",
QEMU_ARCH_ALL)
diff --git a/qemu-spice.h b/qemu-spice.h
new file mode 100644
index 0000000..5597576
--- /dev/null
+++ b/qemu-spice.h
@@ -0,0 +1,22 @@
+#ifndef QEMU_SPICE_H
+#define QEMU_SPICE_H
+
+#ifdef CONFIG_SPICE
+
+#include <spice.h>
+
+#include "qemu-option.h"
+#include "qemu-config.h"
+
+extern SpiceServer *spice_server;
+extern int using_spice;
+
+void qemu_spice_init(void);
+
+#else /* CONFIG_SPICE */
+
+#define using_spice 0
+
+#endif /* CONFIG_SPICE */
+
+#endif /* QEMU_SPICE_H */
diff --git a/spice.c b/spice.c
new file mode 100644
index 0000000..50fa5ca
--- /dev/null
+++ b/spice.c
@@ -0,0 +1,151 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <spice.h>
+#include <spice-experimental.h>
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "qemu-timer.h"
+#include "qemu-queue.h"
+#include "monitor.h"
+
+/* core bits */
+
+SpiceServer *spice_server;
+int using_spice = 0;
+
+struct SpiceTimer {
+ QEMUTimer *timer;
+ QTAILQ_ENTRY(SpiceTimer) next;
+};
+static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
+
+static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
+{
+ SpiceTimer *timer;
+
+ timer = qemu_mallocz(sizeof(*timer));
+ timer->timer = qemu_new_timer(rt_clock, func, opaque);
+ QTAILQ_INSERT_TAIL(&timers, timer, next);
+ return timer;
+}
+
+static void timer_start(SpiceTimer *timer, uint32_t ms)
+{
+ qemu_mod_timer(timer->timer, qemu_get_clock(rt_clock) + ms);
+}
+
+static void timer_cancel(SpiceTimer *timer)
+{
+ qemu_del_timer(timer->timer);
+}
+
+static void timer_remove(SpiceTimer *timer)
+{
+ qemu_del_timer(timer->timer);
+ qemu_free_timer(timer->timer);
+ QTAILQ_REMOVE(&timers, timer, next);
+ free(timer);
+}
+
+struct SpiceWatch {
+ int fd;
+ int event_mask;
+ SpiceWatchFunc func;
+ void *opaque;
+ QTAILQ_ENTRY(SpiceWatch) next;
+};
+static QTAILQ_HEAD(, SpiceWatch) watches = QTAILQ_HEAD_INITIALIZER(watches);
+
+static void watch_read(void *opaque)
+{
+ SpiceWatch *watch = opaque;
+ watch->func(watch->fd, SPICE_WATCH_EVENT_READ, watch->opaque);
+}
+
+static void watch_write(void *opaque)
+{
+ SpiceWatch *watch = opaque;
+ watch->func(watch->fd, SPICE_WATCH_EVENT_WRITE, watch->opaque);
+}
+
+static void watch_update_mask(SpiceWatch *watch, int event_mask)
+{
+ IOHandler *on_read = NULL;
+ IOHandler *on_write = NULL;
+
+ watch->event_mask = event_mask;
+ if (watch->event_mask & SPICE_WATCH_EVENT_READ)
+ on_read = watch_read;
+ if (watch->event_mask & SPICE_WATCH_EVENT_WRITE)
+ on_read = watch_write;
+ qemu_set_fd_handler(watch->fd, on_read, on_write, watch);
+}
+
+static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void *opaque)
+{
+ SpiceWatch *watch;
+
+ watch = qemu_mallocz(sizeof(*watch));
+ watch->fd = fd;
+ watch->func = func;
+ watch->opaque = opaque;
+ QTAILQ_INSERT_TAIL(&watches, watch, next);
+
+ watch_update_mask(watch, event_mask);
+ return watch;
+}
+
+static void watch_remove(SpiceWatch *watch)
+{
+ watch_update_mask(watch, 0);
+ QTAILQ_REMOVE(&watches, watch, next);
+ qemu_free(watch);
+}
+
+static SpiceCoreInterface core_interface = {
+ .base.type = SPICE_INTERFACE_CORE,
+ .base.description = "qemu core services",
+ .base.major_version = SPICE_INTERFACE_CORE_MAJOR,
+ .base.minor_version = SPICE_INTERFACE_CORE_MINOR,
+
+ .timer_add = timer_add,
+ .timer_start = timer_start,
+ .timer_cancel = timer_cancel,
+ .timer_remove = timer_remove,
+
+ .watch_add = watch_add,
+ .watch_update_mask = watch_update_mask,
+ .watch_remove = watch_remove,
+};
+
+/* functions for the rest of qemu */
+
+void qemu_spice_init(void)
+{
+ QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
+ const char *password;
+ int port;
+
+ if (!opts)
+ return;
+ port = qemu_opt_get_number(opts, "port", 0);
+ if (!port)
+ return;
+ password = qemu_opt_get(opts, "password");
+
+ spice_server = spice_server_new();
+ spice_server_set_port(spice_server, port);
+ if (password)
+ spice_server_set_ticket(spice_server, password, 0, 0, 0);
+ if (qemu_opt_get_bool(opts, "disable-ticketing", 0))
+ spice_server_set_noauth(spice_server);
+
+ /* TODO: make configurable via cmdline */
+ spice_server_set_image_compression(spice_server, SPICE_IMAGE_COMPRESS_AUTO_GLZ);
+
+ spice_server_init(spice_server, &core_interface);
+ using_spice = 1;
+}
diff --git a/vl.c b/vl.c
index b3e3676..7f391c0 100644
--- a/vl.c
+++ b/vl.c
@@ -161,6 +161,8 @@ int main(int argc, char **argv)
#include "cpus.h"
#include "arch_init.h"
+#include "qemu-spice.h"
+
//#define DEBUG_NET
//#define DEBUG_SLIRP
@@ -2600,6 +2602,15 @@ int main(int argc, char **argv, char **envp)
}
break;
}
+#ifdef CONFIG_SPICE
+ case QEMU_OPTION_spice:
+ opts = qemu_opts_parse(&qemu_spice_opts, optarg, 0);
+ if (!opts) {
+ fprintf(stderr, "parse error: %s\n", optarg);
+ exit(1);
+ }
+ break;
+#endif
case QEMU_OPTION_writeconfig:
{
FILE *fp;
@@ -2868,6 +2879,10 @@ int main(int argc, char **argv, char **envp)
}
qemu_add_globals();
+#ifdef CONFIG_SPICE
+ qemu_spice_init();
+#endif
+
machine->init(ram_size, boot_devices,
kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (4 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 5/9] spice: core bits Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:24 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 7/9] spice: add mouse Gerd Hoffmann
` (2 subsequent siblings)
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Open keyboard channel. Now you can type into the spice client and the
keyboard events are sent to your guest. You'll need some other display
like vnc to actually see the guest responding to them though.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.objs | 2 +-
qemu-spice.h | 1 +
spice-input.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
spice.c | 2 ++
4 files changed, 61 insertions(+), 1 deletions(-)
create mode 100644 spice-input.c
diff --git a/Makefile.objs b/Makefile.objs
index 021067b..6ddb373 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,7 +88,7 @@ common-obj-y += pflib.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
-common-obj-$(CONFIG_SPICE) += spice.o
+common-obj-$(CONFIG_SPICE) += spice.o spice-input.o
audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
audio-obj-$(CONFIG_SDL) += sdlaudio.o
diff --git a/qemu-spice.h b/qemu-spice.h
index 5597576..ceb3db2 100644
--- a/qemu-spice.h
+++ b/qemu-spice.h
@@ -12,6 +12,7 @@ extern SpiceServer *spice_server;
extern int using_spice;
void qemu_spice_init(void);
+void qemu_spice_input_init(void);
#else /* CONFIG_SPICE */
diff --git a/spice-input.c b/spice-input.c
new file mode 100644
index 0000000..e1014d7
--- /dev/null
+++ b/spice-input.c
@@ -0,0 +1,57 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <spice.h>
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "console.h"
+
+/* keyboard bits */
+
+typedef struct QemuSpiceKbd {
+ SpiceKbdInstance sin;
+ int ledstate;
+} QemuSpiceKbd;
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag);
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin);
+static void kbd_leds(void *opaque, int l);
+
+static const SpiceKbdInterface kbd_interface = {
+ .base.type = SPICE_INTERFACE_KEYBOARD,
+ .base.description = "qemu keyboard",
+ .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
+ .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
+ .push_scan_freg = kbd_push_key,
+ .get_leds = kbd_get_leds,
+};
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
+{
+ kbd_put_keycode(frag);
+}
+
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin)
+{
+ QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
+ return kbd->ledstate;
+}
+
+static void kbd_leds(void *opaque, int ledstate)
+{
+ QemuSpiceKbd *kbd = opaque;
+ kbd->ledstate = ledstate;
+ spice_server_kbd_leds(&kbd->sin, ledstate);
+}
+
+void qemu_spice_input_init(void)
+{
+ QemuSpiceKbd *kbd;
+
+ kbd = qemu_mallocz(sizeof(*kbd));
+ kbd->sin.base.sif = &kbd_interface.base;
+ spice_server_add_interface(spice_server, &kbd->sin.base);
+ qemu_add_led_event_handler(kbd_leds, kbd);
+}
diff --git a/spice.c b/spice.c
index 50fa5ca..c763d52 100644
--- a/spice.c
+++ b/spice.c
@@ -148,4 +148,6 @@ void qemu_spice_init(void)
spice_server_init(spice_server, &core_interface);
using_spice = 1;
+
+ qemu_spice_input_init();
}
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (5 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 6/9] spice: add keyboard Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:25 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 8/9] spice: simple display Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 9/9] spice: add tablet support Gerd Hoffmann
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Open mouse channel. Now you can move the guests mouse pointer.
No tablet / absolute positioning (yet) though.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
spice-input.c | 31 +++++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/spice-input.c b/spice-input.c
index e1014d7..8f3deb4 100644
--- a/spice-input.c
+++ b/spice-input.c
@@ -46,12 +46,43 @@ static void kbd_leds(void *opaque, int ledstate)
spice_server_kbd_leds(&kbd->sin, ledstate);
}
+/* mouse bits */
+
+typedef struct QemuSpiceMouse {
+ SpiceMouseInstance sin;
+} QemuSpiceMouse;
+
+static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
+ uint32_t buttons_state)
+{
+ kbd_mouse_event(dx, dy, dz, buttons_state);
+}
+
+static void mouse_buttons(SpiceMouseInstance *sin, uint32_t buttons_state)
+{
+ kbd_mouse_event(0, 0, 0, buttons_state);
+}
+
+static const SpiceMouseInterface mouse_interface = {
+ .base.type = SPICE_INTERFACE_MOUSE,
+ .base.description = "mouse",
+ .base.major_version = SPICE_INTERFACE_MOUSE_MAJOR,
+ .base.minor_version = SPICE_INTERFACE_MOUSE_MINOR,
+ .motion = mouse_motion,
+ .buttons = mouse_buttons,
+};
+
void qemu_spice_input_init(void)
{
QemuSpiceKbd *kbd;
+ QemuSpiceMouse *mouse;
kbd = qemu_mallocz(sizeof(*kbd));
kbd->sin.base.sif = &kbd_interface.base;
spice_server_add_interface(spice_server, &kbd->sin.base);
qemu_add_led_event_handler(kbd_leds, kbd);
+
+ mouse = qemu_mallocz(sizeof(*mouse));
+ mouse->sin.base.sif = &mouse_interface.base;
+ spice_server_add_interface(spice_server, &mouse->sin.base);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (6 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 7/9] spice: add mouse Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:39 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 9/9] spice: add tablet support Gerd Hoffmann
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
With that patch applied you'll actually see the guests screen in the
spice client. This does *not* bring qxl and full spice support though.
This is basically the qxl vga mode made more generic, so it plays
together with any qemu-emulated gfx card. You can display stdvga or
cirrus via spice client. You can have both vnc and spice enabled and
clients connected at the same time.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Makefile.objs | 2 +-
qemu-spice.h | 1 +
spice-display.c | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
spice-display.h | 52 ++++++++
vl.c | 7 +-
5 files changed, 447 insertions(+), 2 deletions(-)
create mode 100644 spice-display.c
create mode 100644 spice-display.h
diff --git a/Makefile.objs b/Makefile.objs
index 6ddb373..2f40529 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,7 +88,7 @@ common-obj-y += pflib.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
-common-obj-$(CONFIG_SPICE) += spice.o spice-input.o
+common-obj-$(CONFIG_SPICE) += spice.o spice-input.o spice-display.o
audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
audio-obj-$(CONFIG_SDL) += sdlaudio.o
diff --git a/qemu-spice.h b/qemu-spice.h
index ceb3db2..f061004 100644
--- a/qemu-spice.h
+++ b/qemu-spice.h
@@ -13,6 +13,7 @@ extern int using_spice;
void qemu_spice_init(void);
void qemu_spice_input_init(void);
+void qemu_spice_display_init(DisplayState *ds);
#else /* CONFIG_SPICE */
diff --git a/spice-display.c b/spice-display.c
new file mode 100644
index 0000000..1e6d939
--- /dev/null
+++ b/spice-display.c
@@ -0,0 +1,387 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <string.h>
+#include <pthread.h>
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "qemu-timer.h"
+#include "qemu-queue.h"
+#include "monitor.h"
+#include "console.h"
+#include "sysemu.h"
+
+#include "spice-display.h"
+
+static int debug = 0;
+
+int qemu_spice_rect_is_empty(const QXLRect* r)
+{
+ return r->top == r->bottom || r->left == r->right;
+}
+
+void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
+{
+ if (qemu_spice_rect_is_empty(r)) {
+ return;
+ }
+
+ if (qemu_spice_rect_is_empty(dest)) {
+ *dest = *r;
+ return;
+ }
+
+ dest->top = MIN(dest->top, r->top);
+ dest->left = MIN(dest->left, r->left);
+ dest->bottom = MAX(dest->bottom, r->bottom);
+ dest->right = MAX(dest->right, r->right);
+}
+
+SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+{
+ SimpleSpiceUpdate *update;
+ QXLDrawable *drawable;
+ QXLImage *image;
+ QXLCommand *cmd;
+ uint8_t *src, *dst;
+ int by, bw, bh;
+
+ if (qemu_spice_rect_is_empty(&ssd->dirty)) {
+ return NULL;
+ };
+
+ pthread_mutex_lock(&ssd->lock);
+ if (debug > 1)
+ fprintf(stderr, "%s: lr %d -> %d, tb -> %d -> %d\n", __FUNCTION__,
+ ssd->dirty.left, ssd->dirty.right,
+ ssd->dirty.top, ssd->dirty.bottom);
+
+ update = qemu_mallocz(sizeof(*update));
+ drawable = &update->drawable;
+ image = &update->image;
+ cmd = &update->ext.cmd;
+
+ bw = ssd->dirty.right - ssd->dirty.left;
+ bh = ssd->dirty.bottom - ssd->dirty.top;
+ update->bitmap = qemu_malloc(bw * bh * 4);
+
+ drawable->bbox = ssd->dirty;
+ drawable->clip.type = SPICE_CLIP_TYPE_NONE;
+ drawable->effect = QXL_EFFECT_OPAQUE;
+ drawable->release_info.id = (intptr_t)update;
+ drawable->type = QXL_DRAW_COPY;
+
+ drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT;
+ drawable->u.copy.src_bitmap = (intptr_t)image;
+ drawable->u.copy.src_area.right = bw;
+ drawable->u.copy.src_area.bottom = bh;
+
+ QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_DEVICE, ssd->unique++);
+ image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
+ image->bitmap.flags = QXL_BITMAP_DIRECT | QXL_BITMAP_TOP_DOWN;
+ image->bitmap.stride = bw * 4;
+ image->descriptor.width = image->bitmap.x = bw;
+ image->descriptor.height = image->bitmap.y = bh;
+ image->bitmap.data = (intptr_t)(update->bitmap);
+ image->bitmap.palette = 0;
+ image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
+
+ if (ssd->conv == NULL) {
+ PixelFormat dst = qemu_default_pixelformat(32);
+ ssd->conv = qemu_pf_conv_get(&dst, &ssd->ds->surface->pf);
+ assert(ssd->conv);
+ }
+
+ src = ds_get_data(ssd->ds) +
+ ssd->dirty.top * ds_get_linesize(ssd->ds) +
+ ssd->dirty.left * ds_get_bytes_per_pixel(ssd->ds);
+ dst = update->bitmap;
+ for (by = 0; by < bh; by++) {
+ qemu_pf_conv_run(ssd->conv, dst, src, bw);
+ src += ds_get_linesize(ssd->ds);
+ dst += image->bitmap.stride;
+ }
+
+ cmd->type = QXL_CMD_DRAW;
+ cmd->data = (intptr_t)drawable;
+
+ memset(&ssd->dirty, 0, sizeof(ssd->dirty));
+ pthread_mutex_unlock(&ssd->lock);
+ return update;
+}
+
+void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate *update)
+{
+ qemu_free(update->bitmap);
+ qemu_free(update);
+}
+
+void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
+{
+ QXLDevMemSlot memslot;
+
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+
+ memset(&memslot, 0, sizeof(memslot));
+ memslot.slot_group_id = MEMSLOT_GROUP_HOST;
+ memslot.virt_end = ~0;
+ ssd->worker->add_memslot(ssd->worker, &memslot);
+}
+
+void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
+{
+ QXLDevSurfaceCreate surface;
+
+ if (debug)
+ fprintf(stderr, "%s: %dx%d\n", __FUNCTION__,
+ ds_get_width(ssd->ds), ds_get_height(ssd->ds));
+
+ surface.format = SPICE_SURFACE_FMT_32_xRGB;
+ surface.width = ds_get_width(ssd->ds);
+ surface.height = ds_get_height(ssd->ds);
+ surface.stride = -surface.width * 4;
+ surface.mouse_mode = 0;
+ surface.flags = 0;
+ surface.type = 0;
+ surface.mem = (intptr_t)ssd->buf;
+ surface.group_id = MEMSLOT_GROUP_HOST;
+ ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
+}
+
+void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
+{
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+
+ ssd->worker->destroy_primary_surface(ssd->worker, 0);
+}
+
+void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
+{
+ SimpleSpiceDisplay *ssd = opaque;
+
+ if (running) {
+ ssd->worker->start(ssd->worker);
+ } else {
+ ssd->worker->stop(ssd->worker);
+ }
+ ssd->running = running;
+}
+
+/* display listener callbacks */
+
+void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
+ int x, int y, int w, int h)
+{
+ QXLRect update_area;
+
+ if (debug > 1)
+ fprintf(stderr, "%s: x %d y %d w %d h %d\n", __FUNCTION__, x, y, w, h);
+ update_area.left = x,
+ update_area.right = x + w;
+ update_area.top = y;
+ update_area.bottom = y + h;
+
+ pthread_mutex_lock(&ssd->lock);
+ if (qemu_spice_rect_is_empty(&ssd->dirty)) {
+ ssd->notify++;
+ }
+ qemu_spice_rect_union(&ssd->dirty, &update_area);
+ pthread_mutex_unlock(&ssd->lock);
+}
+
+void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
+{
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+
+ pthread_mutex_lock(&ssd->lock);
+ memset(&ssd->dirty, 0, sizeof(ssd->dirty));
+ pthread_mutex_unlock(&ssd->lock);
+
+ qemu_spice_destroy_host_primary(ssd);
+ qemu_spice_create_host_primary(ssd);
+ qemu_pf_conv_put(ssd->conv);
+ ssd->conv = NULL;
+
+ pthread_mutex_lock(&ssd->lock);
+ memset(&ssd->dirty, 0, sizeof(ssd->dirty));
+ ssd->notify++;
+ pthread_mutex_unlock(&ssd->lock);
+}
+
+void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
+{
+ if (debug > 2)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ vga_hw_update();
+ if (ssd->notify) {
+ ssd->notify = 0;
+ ssd->worker->wakeup(ssd->worker);
+ if (debug > 1)
+ fprintf(stderr, "%s: notify\n", __FUNCTION__);
+ }
+}
+
+/* spice display interface callbacks */
+
+static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
+{
+ SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
+
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ ssd->worker = qxl_worker;
+}
+
+static void interface_set_compression_level(QXLInstance *sin, int level)
+{
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ /* nothing to do */
+}
+
+static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
+{
+ if (debug > 2)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ /* nothing to do */
+}
+
+static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
+{
+ SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
+
+ info->memslot_gen_bits = MEMSLOT_GENERATION_BITS;
+ info->memslot_id_bits = MEMSLOT_SLOT_BITS;
+ info->num_memslots = NUM_MEMSLOTS;
+ info->num_memslots_groups = NUM_MEMSLOTS_GROUPS;
+ info->internal_groupslot_id = 0;
+ info->qxl_ram_size = ssd->bufsize;
+ info->n_surfaces = NUM_SURFACES;
+}
+
+static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
+{
+ SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
+ SimpleSpiceUpdate *update;
+
+ if (debug > 2)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ update = qemu_spice_create_update(ssd);
+ if (update == NULL) {
+ return false;
+ }
+ *ext = update->ext;
+ return true;
+}
+
+static int interface_req_cmd_notification(QXLInstance *sin)
+{
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ return 1;
+}
+
+static void interface_release_resource(QXLInstance *sin,
+ struct QXLReleaseInfoExt ext)
+{
+ SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
+ uintptr_t id;
+
+ if (debug > 1)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ id = ext.info->id;
+ qemu_spice_destroy_update(ssd, (void*)id);
+}
+
+static int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt *ext)
+{
+ if (debug > 2)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ return false;
+}
+
+static int interface_req_cursor_notification(QXLInstance *sin)
+{
+ if (debug)
+ fprintf(stderr, "%s:\n", __FUNCTION__);
+ return 1;
+}
+
+static void interface_notify_update(QXLInstance *sin, uint32_t update_id)
+{
+ fprintf(stderr, "%s: abort()\n", __FUNCTION__);
+ abort();
+}
+
+static int interface_flush_resources(QXLInstance *sin)
+{
+ fprintf(stderr, "%s: abort()\n", __FUNCTION__);
+ abort();
+ return 0;
+}
+
+static const QXLInterface dpy_interface = {
+ .base.type = SPICE_INTERFACE_QXL,
+ .base.description = "qemu simple display",
+ .base.major_version = SPICE_INTERFACE_QXL_MAJOR,
+ .base.minor_version = SPICE_INTERFACE_QXL_MINOR,
+
+ .attache_worker = interface_attach_worker,
+ .set_compression_level = interface_set_compression_level,
+ .set_mm_time = interface_set_mm_time,
+
+ .get_init_info = interface_get_init_info,
+ .get_command = interface_get_command,
+ .req_cmd_notification = interface_req_cmd_notification,
+ .release_resource = interface_release_resource,
+ .get_cursor_command = interface_get_cursor_command,
+ .req_cursor_notification = interface_req_cursor_notification,
+ .notify_update = interface_notify_update,
+ .flush_resources = interface_flush_resources,
+};
+
+static SimpleSpiceDisplay sdpy;
+
+static void display_update(struct DisplayState *ds, int x, int y, int w, int h)
+{
+ qemu_spice_display_update(&sdpy, x, y, w, h);
+}
+
+static void display_resize(struct DisplayState *ds)
+{
+ qemu_spice_display_resize(&sdpy);
+}
+
+static void display_refresh(struct DisplayState *ds)
+{
+ qemu_spice_display_refresh(&sdpy);
+}
+
+static DisplayChangeListener display_listener = {
+ .dpy_update = display_update,
+ .dpy_resize = display_resize,
+ .dpy_refresh = display_refresh,
+};
+
+void qemu_spice_display_init(DisplayState *ds)
+{
+ assert(sdpy.ds == NULL);
+ sdpy.ds = ds;
+ sdpy.bufsize = (16 * 1024 * 1024);
+ sdpy.buf = qemu_malloc(sdpy.bufsize);
+ pthread_mutex_init(&sdpy.lock, NULL);
+ register_displaychangelistener(ds, &display_listener);
+
+ sdpy.qxl.base.sif = &dpy_interface.base;
+ spice_server_add_interface(spice_server, &sdpy.qxl.base);
+ assert(sdpy.worker);
+
+ qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler, &sdpy);
+ qemu_spice_create_host_memslot(&sdpy);
+ qemu_spice_create_host_primary(&sdpy);
+}
diff --git a/spice-display.h b/spice-display.h
new file mode 100644
index 0000000..b55e7ea
--- /dev/null
+++ b/spice-display.h
@@ -0,0 +1,52 @@
+#include <spice/ipc_ring.h>
+#include <spice/enums.h>
+#include <spice/qxl_dev.h>
+
+#include "pflib.h"
+
+#define NUM_MEMSLOTS 8
+#define MEMSLOT_GENERATION_BITS 8
+#define MEMSLOT_SLOT_BITS 8
+
+#define MEMSLOT_GROUP_HOST 0
+#define MEMSLOT_GROUP_GUEST 1
+#define NUM_MEMSLOTS_GROUPS 2
+
+#define NUM_SURFACES 1024
+
+typedef struct SimpleSpiceDisplay {
+ DisplayState *ds;
+ void *buf;
+ int bufsize;
+ QXLWorker *worker;
+ QXLInstance qxl;
+ uint32_t unique;
+ QemuPfConv *conv;
+
+ pthread_mutex_t lock;
+ QXLRect dirty;
+ int notify;
+ int running;
+} SimpleSpiceDisplay;
+
+typedef struct SimpleSpiceUpdate {
+ QXLDrawable drawable;
+ QXLImage image;
+ QXLCommandExt ext;
+ uint8_t *bitmap;
+} SimpleSpiceUpdate;
+
+int qemu_spice_rect_is_empty(const QXLRect* r);
+void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r);
+
+SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *sdpy);
+void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate *update);
+void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd);
+void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd);
+void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd);
+void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason);
+
+void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
+ int x, int y, int w, int h);
+void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
+void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
diff --git a/vl.c b/vl.c
index 7f391c0..f68f8e8 100644
--- a/vl.c
+++ b/vl.c
@@ -2910,7 +2910,7 @@ int main(int argc, char **argv, char **envp)
/* just use the first displaystate for the moment */
ds = get_displaystate();
- if (display_type == DT_DEFAULT) {
+ if (display_type == DT_DEFAULT && !using_spice) {
#if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
display_type = DT_SDL;
#else
@@ -2950,6 +2950,11 @@ int main(int argc, char **argv, char **envp)
default:
break;
}
+#ifdef CONFIG_SPICE
+ if (using_spice) {
+ qemu_spice_display_init(ds);
+ }
+#endif
dpy_resize(ds);
dcl = ds->listeners;
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [Qemu-devel] [PATCH 9/9] spice: add tablet support
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
` (7 preceding siblings ...)
2010-08-19 12:40 ` [Qemu-devel] [PATCH 8/9] spice: simple display Gerd Hoffmann
@ 2010-08-19 12:40 ` Gerd Hoffmann
2010-08-19 14:40 ` Anthony Liguori
8 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Add support for the spice tablet interface. The tablet interface will
be registered (and then used by the spice client) as soon as a absolute
pointing device is available and used by the guest, i.e. you'll have to
configure your guest with '-usbdevice tablet'.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
spice-display.c | 2 +-
spice-input.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 93 insertions(+), 8 deletions(-)
diff --git a/spice-display.c b/spice-display.c
index 1e6d939..fec2432 100644
--- a/spice-display.c
+++ b/spice-display.c
@@ -143,7 +143,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
surface.width = ds_get_width(ssd->ds);
surface.height = ds_get_height(ssd->ds);
surface.stride = -surface.width * 4;
- surface.mouse_mode = 0;
+ surface.mouse_mode = true;
surface.flags = 0;
surface.type = 0;
surface.mem = (intptr_t)ssd->buf;
diff --git a/spice-input.c b/spice-input.c
index 8f3deb4..5646ff9 100644
--- a/spice-input.c
+++ b/spice-input.c
@@ -1,5 +1,6 @@
#include <stdlib.h>
#include <stdio.h>
+#include <stdbool.h>
#include <string.h>
#include <spice.h>
@@ -48,9 +49,13 @@ static void kbd_leds(void *opaque, int ledstate)
/* mouse bits */
-typedef struct QemuSpiceMouse {
- SpiceMouseInstance sin;
-} QemuSpiceMouse;
+typedef struct QemuSpicePointer {
+ SpiceMouseInstance mouse;
+ SpiceTabletInstance tablet;
+ int width, height, x, y;
+ Notifier mouse_mode;
+ bool absolute;
+} QemuSpicePointer;
static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
uint32_t buttons_state)
@@ -72,17 +77,97 @@ static const SpiceMouseInterface mouse_interface = {
.buttons = mouse_buttons,
};
+static void tablet_set_logical_size(SpiceTabletInstance* sin, int width, int height)
+{
+ QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+ fprintf(stderr, "%s: %dx%d\n", __FUNCTION__, width, height);
+ if (height < 16)
+ height = 16;
+ if (width < 16)
+ width = 16;
+ pointer->width = width;
+ pointer->height = height;
+}
+
+static void tablet_position(SpiceTabletInstance* sin, int x, int y,
+ uint32_t buttons_state)
+{
+ QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+ pointer->x = x * 0x7FFF / (pointer->width - 1);
+ pointer->y = y * 0x7FFF / (pointer->height - 1);
+ kbd_mouse_event(pointer->x, pointer->y, 0, buttons_state);
+}
+
+
+static void tablet_wheel(SpiceTabletInstance* sin, int wheel,
+ uint32_t buttons_state)
+{
+ QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+ kbd_mouse_event(pointer->x, pointer->y, wheel, buttons_state);
+}
+
+static void tablet_buttons(SpiceTabletInstance *sin,
+ uint32_t buttons_state)
+{
+ QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
+
+ kbd_mouse_event(pointer->x, pointer->y, 0, buttons_state);
+}
+
+static const SpiceTabletInterface tablet_interface = {
+ .base.type = SPICE_INTERFACE_TABLET,
+ .base.description = "tablet",
+ .base.major_version = SPICE_INTERFACE_TABLET_MAJOR,
+ .base.minor_version = SPICE_INTERFACE_TABLET_MINOR,
+ .set_logical_size = tablet_set_logical_size,
+ .position = tablet_position,
+ .wheel = tablet_wheel,
+ .buttons = tablet_buttons,
+};
+
+static void mouse_mode_notifier(Notifier *notifier)
+{
+ QemuSpicePointer *pointer = container_of(notifier, QemuSpicePointer, mouse_mode);
+ bool is_absolute = kbd_mouse_is_absolute();
+ bool has_absolute = kbd_mouse_has_absolute();
+
+ fprintf(stderr, "%s: absolute pointer: %s%s\n", __FUNCTION__,
+ has_absolute ? "present" : "not available",
+ is_absolute ? "+active" : "");
+
+ if (pointer->absolute == is_absolute)
+ return;
+
+ if (is_absolute) {
+ fprintf(stderr, "%s: using absolute pointer (client mode)\n", __FUNCTION__);
+ spice_server_add_interface(spice_server, &pointer->tablet.base);
+ } else {
+ fprintf(stderr, "%s: using relative pointer (server mode)\n", __FUNCTION__);
+ spice_server_remove_interface(&pointer->tablet.base);
+ }
+ pointer->absolute = is_absolute;
+}
+
void qemu_spice_input_init(void)
{
QemuSpiceKbd *kbd;
- QemuSpiceMouse *mouse;
+ QemuSpicePointer *pointer;
kbd = qemu_mallocz(sizeof(*kbd));
kbd->sin.base.sif = &kbd_interface.base;
spice_server_add_interface(spice_server, &kbd->sin.base);
qemu_add_led_event_handler(kbd_leds, kbd);
- mouse = qemu_mallocz(sizeof(*mouse));
- mouse->sin.base.sif = &mouse_interface.base;
- spice_server_add_interface(spice_server, &mouse->sin.base);
+ pointer = qemu_mallocz(sizeof(*pointer));
+ pointer->mouse.base.sif = &mouse_interface.base;
+ pointer->tablet.base.sif = &tablet_interface.base;
+ spice_server_add_interface(spice_server, &pointer->mouse.base);
+
+ pointer->absolute = false;
+ pointer->mouse_mode.notify = mouse_mode_notifier;
+ qemu_add_mouse_mode_change_notifier(&pointer->mouse_mode);
+ mouse_mode_notifier(&pointer->mouse_mode);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library.
2010-08-19 12:40 ` [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library Gerd Hoffmann
@ 2010-08-19 14:04 ` Anthony Liguori
2010-08-19 15:34 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:04 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
This code is more or less stand alone so it's a good candidate for
having some in-tree unit tests.
I can provide some examples of how it should be done if you like, but
just something that lives in tests/ that exercises the functionality and
can be run under valgrind to verify that we aren't leaking memory.
Using libcheck is also reasonable (take a look at the various check-q*
in the top level).
Regards,
Anthony Liguori
> ---
> Makefile.objs | 1 +
> pflib.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> pflib.h | 20 ++++++
> 3 files changed, 234 insertions(+), 0 deletions(-)
> create mode 100644 pflib.c
> create mode 100644 pflib.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 4a1eaa1..edfca87 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -83,6 +83,7 @@ common-obj-y += qemu-char.o savevm.o #aio.o
> common-obj-y += msmouse.o ps2.o
> common-obj-y += qdev.o qdev-properties.o
> common-obj-y += block-migration.o
> +common-obj-y += pflib.o
>
> common-obj-$(CONFIG_BRLAPI) += baum.o
> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
> diff --git a/pflib.c b/pflib.c
> new file mode 100644
> index 0000000..1154d0c
> --- /dev/null
> +++ b/pflib.c
> @@ -0,0 +1,213 @@
> +/*
> + * PixelFormat conversion library.
> + *
> + * Author: Gerd Hoffmann<kraxel@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +#include "qemu-common.h"
> +#include "console.h"
> +#include "pflib.h"
> +
> +typedef struct QemuPixel QemuPixel;
> +
> +typedef void (*pf_convert)(QemuPfConv *conv,
> + void *dst, void *src, uint32_t cnt);
> +typedef void (*pf_convert_from)(PixelFormat *pf,
> + QemuPixel *dst, void *src, uint32_t cnt);
> +typedef void (*pf_convert_to)(PixelFormat *pf,
> + void *dst, QemuPixel *src, uint32_t cnt);
> +
> +struct QemuPfConv {
> + pf_convert convert;
> + PixelFormat src;
> + PixelFormat dst;
> +
> + /* for copy_generic() */
> + pf_convert_from conv_from;
> + pf_convert_to conv_to;
> + QemuPixel *conv_buf;
> + uint32_t conv_cnt;
> +};
> +
> +struct QemuPixel {
> + uint8_t red;
> + uint8_t green;
> + uint8_t blue;
> + uint8_t alpha;
> +};
> +
> +/* ----------------------------------------------------------------------- */
> +/* PixelFormat -> QemuPixel conversions */
> +
> +static void conv_16_to_pixel(PixelFormat *pf,
> + QemuPixel *dst, void *src, uint32_t cnt)
> +{
> + uint16_t *src16 = src;
> +
> + while (cnt> 0) {
> + dst->red = ((*src16& pf->rmask)>> pf->rshift)<< (8 - pf->rbits);
> + dst->green = ((*src16& pf->gmask)>> pf->gshift)<< (8 - pf->gbits);
> + dst->blue = ((*src16& pf->bmask)>> pf->bshift)<< (8 - pf->bbits);
> + dst->alpha = ((*src16& pf->amask)>> pf->ashift)<< (8 - pf->abits);
> + dst++, src16++, cnt--;
> + }
> +}
> +
> +/* assumes pf->{r,g,b,a}bits == 8 */
> +static void conv_32_to_pixel_fast(PixelFormat *pf,
> + QemuPixel *dst, void *src, uint32_t cnt)
> +{
> + uint32_t *src32 = src;
> +
> + while (cnt> 0) {
> + dst->red = (*src32& pf->rmask)>> pf->rshift;
> + dst->green = (*src32& pf->gmask)>> pf->gshift;
> + dst->blue = (*src32& pf->bmask)>> pf->bshift;
> + dst->alpha = (*src32& pf->amask)>> pf->ashift;
> + dst++, src32++, cnt--;
> + }
> +}
> +
> +static void conv_32_to_pixel_generic(PixelFormat *pf,
> + QemuPixel *dst, void *src, uint32_t cnt)
> +{
> + uint32_t *src32 = src;
> +
> + while (cnt> 0) {
> + if (pf->rbits< 8) {
> + dst->red = ((*src32& pf->rmask)>> pf->rshift)<< (8 - pf->rbits);
> + } else {
> + dst->red = ((*src32& pf->rmask)>> pf->rshift)>> (pf->rbits - 8);
> + }
> + if (pf->gbits< 8) {
> + dst->green = ((*src32& pf->gmask)>> pf->gshift)<< (8 - pf->gbits);
> + } else {
> + dst->green = ((*src32& pf->gmask)>> pf->gshift)>> (pf->gbits - 8);
> + }
> + if (pf->bbits< 8) {
> + dst->blue = ((*src32& pf->bmask)>> pf->bshift)<< (8 - pf->bbits);
> + } else {
> + dst->blue = ((*src32& pf->bmask)>> pf->bshift)>> (pf->bbits - 8);
> + }
> + if (pf->abits< 8) {
> + dst->alpha = ((*src32& pf->amask)>> pf->ashift)<< (8 - pf->abits);
> + } else {
> + dst->alpha = ((*src32& pf->amask)>> pf->ashift)>> (pf->abits - 8);
> + }
> + dst++, src32++, cnt--;
> + }
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* QemuPixel -> PixelFormat conversions */
> +
> +static void conv_pixel_to_16(PixelFormat *pf,
> + void *dst, QemuPixel *src, uint32_t cnt)
> +{
> + uint16_t *dst16 = dst;
> +
> + while (cnt> 0) {
> + *dst16 = ((uint16_t)src->red>> (8 - pf->rbits))<< pf->rshift;
> + *dst16 |= ((uint16_t)src->green>> (8 - pf->gbits))<< pf->gshift;
> + *dst16 |= ((uint16_t)src->blue>> (8 - pf->bbits))<< pf->bshift;
> + *dst16 |= ((uint16_t)src->alpha>> (8 - pf->abits))<< pf->ashift;
> + dst16++, src++, cnt--;
> + }
> +}
> +
> +static void conv_pixel_to_32(PixelFormat *pf,
> + void *dst, QemuPixel *src, uint32_t cnt)
> +{
> + uint32_t *dst32 = dst;
> +
> + while (cnt> 0) {
> + *dst32 = ((uint32_t)src->red>> (8 - pf->rbits))<< pf->rshift;
> + *dst32 |= ((uint32_t)src->green>> (8 - pf->gbits))<< pf->gshift;
> + *dst32 |= ((uint32_t)src->blue>> (8 - pf->bbits))<< pf->bshift;
> + *dst32 |= ((uint32_t)src->alpha>> (8 - pf->abits))<< pf->ashift;
> + dst32++, src++, cnt--;
> + }
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* PixelFormat -> PixelFormat conversions */
> +
> +static void convert_copy(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> + uint32_t bytes = cnt * conv->src.bytes_per_pixel;
> + memcpy(dst, src, bytes);
> +}
> +
> +static void convert_generic(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> + if (conv->conv_cnt< cnt) {
> + conv->conv_cnt = cnt;
> + conv->conv_buf = qemu_realloc(conv->conv_buf, sizeof(QemuPixel) * conv->conv_cnt);
> + }
> + conv->conv_from(&conv->src, conv->conv_buf, src, cnt);
> + conv->conv_to(&conv->dst, dst, conv->conv_buf, cnt);
> +}
> +
> +/* ----------------------------------------------------------------------- */
> +/* public interface */
> +
> +QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src)
> +{
> + QemuPfConv *conv = qemu_mallocz(sizeof(QemuPfConv));
> +
> + conv->src = *src;
> + conv->dst = *dst;
> +
> + if (memcmp(&conv->src,&conv->dst, sizeof(PixelFormat)) == 0) {
> + /* formats identical, can simply copy */
> + conv->convert = convert_copy;
> + } else {
> + /* generic two-step conversion: src -> QemuPixel -> dst */
> + switch (conv->src.bytes_per_pixel) {
> + case 2:
> + conv->conv_from = conv_16_to_pixel;
> + break;
> + case 4:
> + if (conv->src.rbits == 8&& conv->src.gbits == 8&& conv->src.bbits == 8) {
> + conv->conv_from = conv_32_to_pixel_fast;
> + } else {
> + conv->conv_from = conv_32_to_pixel_generic;
> + }
> + break;
> + default:
> + goto err;
> + }
> + switch (conv->dst.bytes_per_pixel) {
> + case 2:
> + conv->conv_to = conv_pixel_to_16;
> + break;
> + case 4:
> + conv->conv_to = conv_pixel_to_32;
> + break;
> + default:
> + goto err;
> + }
> + conv->convert = convert_generic;
> + }
> + return conv;
> +
> +err:
> + qemu_free(conv);
> + return NULL;
> +}
> +
> +void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt)
> +{
> + conv->convert(conv, dst, src, cnt);
> +}
> +
> +void qemu_pf_conv_put(QemuPfConv *conv)
> +{
> + if (conv) {
> + qemu_free(conv->conv_buf);
> + qemu_free(conv);
> + }
> +}
> diff --git a/pflib.h b/pflib.h
> new file mode 100644
> index 0000000..b70c313
> --- /dev/null
> +++ b/pflib.h
> @@ -0,0 +1,20 @@
> +#ifndef __QEMU_PFLIB_H
> +#define __QEMU_PFLIB_H
> +
> +/*
> + * PixelFormat conversion library.
> + *
> + * Author: Gerd Hoffmann<kraxel@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +typedef struct QemuPfConv QemuPfConv;
> +
> +QemuPfConv *qemu_pf_conv_get(PixelFormat *dst, PixelFormat *src);
> +void qemu_pf_conv_run(QemuPfConv *conv, void *dst, void *src, uint32_t cnt);
> +void qemu_pf_conv_put(QemuPfConv *conv);
> +
> +#endif
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 2/9] configure: add logging
2010-08-19 12:40 ` [Qemu-devel] [PATCH 2/9] configure: add logging Gerd Hoffmann
@ 2010-08-19 14:05 ` Anthony Liguori
2010-08-19 15:44 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:05 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Write compile commands and messages to config.log.
> Useful for debugging configure.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
Indeed.
Acked-by: Anthony Liguori <aliguori@us.ibm.com>
Given the large nature of spice, I suggest that you setup a subtree for
it and do a pull request when the first round is ready.
Regards,
Anthony Liguori
> ---
> configure | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index a20371c..13d8be0 100755
> --- a/configure
> +++ b/configure
> @@ -16,15 +16,18 @@ TMPO="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.o"
> TMPE="${TMPDIR1}/qemu-conf-${RANDOM}-$$-${RANDOM}.exe"
>
> trap "rm -f $TMPC $TMPO $TMPE ; exit" EXIT INT QUIT TERM
> +rm -f config.log
>
> compile_object() {
> - $cc $QEMU_CFLAGS -c -o $TMPO $TMPC> /dev/null 2> /dev/null
> + echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC>> config.log
> + $cc $QEMU_CFLAGS -c -o $TMPO $TMPC>> config.log 2>&1
> }
>
> compile_prog() {
> local_cflags="$1"
> local_ldflags="$2"
> - $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags> /dev/null 2> /dev/null
> + echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags>> config.log
> + $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags>> config.log 2>&1
> }
>
> # check whether a command is available to this shell (may be either an
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 3/9] add spice into the configure file
2010-08-19 12:40 ` [Qemu-devel] [PATCH 3/9] add spice into the configure file Gerd Hoffmann
@ 2010-08-19 14:06 ` Anthony Liguori
0 siblings, 0 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:06 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
Acked-by: Anthony Liguori <aliguori@us.ibm.com>
Regards,
Anthony Liguori
> ---
> configure | 36 ++++++++++++++++++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index 13d8be0..56e7084 100755
> --- a/configure
> +++ b/configure
> @@ -318,6 +318,7 @@ pkgversion=""
> check_utests="no"
> user_pie="no"
> zero_malloc=""
> +spice=""
>
> # OS specific
> if check_define __linux__ ; then
> @@ -619,6 +620,10 @@ for opt do
> ;;
> --enable-kvm) kvm="yes"
> ;;
> + --disable-spice) spice="no"
> + ;;
> + --enable-spice) spice="yes"
> + ;;
> --enable-profiler) profiler="yes"
> ;;
> --enable-cocoa)
> @@ -898,6 +903,8 @@ echo " --enable-docs enable documentation build"
> echo " --disable-docs disable documentation build"
> echo " --disable-vhost-net disable vhost-net acceleration support"
> echo " --enable-vhost-net enable vhost-net acceleration support"
> +echo " --disable-spice disable spice"
> +echo " --enable-spice enable spice"
> echo ""
> echo "NOTE: The object files are built at the place where configure is launched"
> exit 1
> @@ -2048,6 +2055,30 @@ if compile_prog "" ""; then
> gcc_attribute_warn_unused_result=yes
> fi
>
> +# spice probe
> +if test "$spice" != "no" ; then
> + cat> $TMPC<< EOF
> +#include<spice.h>
> +int main(void) { spice_server_new(); return 0; }
> +EOF
> + spice_proto_ver=$($pkgconfig --modversion spice-protocol 2>/dev/null)
> + spice_server_ver=$($pkgconfig --modversion spice-server 2>/dev/null)
> + spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2>/dev/null)
> + spice_libs=$($pkgconfig --libs spice-protocol spice-server 2>/dev/null)
> + if compile_prog "$spice_cflags" "$spice_libs" ; then
> + spice="yes"
> + libs_softmmu="$libs_softmmu $spice_libs"
> + QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
> + else
> + if test "$spice" = "yes" ; then
> + feature_not_found "spice"
> + fi
> + spice="no"
> + fi
> +fi
> +
> +##########################################
> +
> ##########################################
> # check if we have fdatasync
>
> @@ -2190,6 +2221,7 @@ echo "preadv support $preadv"
> echo "fdatasync $fdatasync"
> echo "uuid support $uuid"
> echo "vhost-net support $vhost_net"
> +echo "spice support $spice"
>
> if test $sdl_too_old = "yes"; then
> echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -2427,6 +2459,10 @@ if test "$fdatasync" = "yes" ; then
> echo "CONFIG_FDATASYNC=y">> $config_host_mak
> fi
>
> +if test "$spice" = "yes" ; then
> + echo "CONFIG_SPICE=y">> $config_host_mak
> +fi
> +
> # XXX: suppress that
> if [ "$bsd" = "yes" ] ; then
> echo "CONFIG_BSD=y">> $config_host_mak
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 5/9] spice: core bits
2010-08-19 12:40 ` [Qemu-devel] [PATCH 5/9] spice: core bits Gerd Hoffmann
@ 2010-08-19 14:19 ` Anthony Liguori
2010-08-20 11:54 ` Gerd Hoffmann
2010-08-25 12:37 ` Gerd Hoffmann
0 siblings, 2 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:19 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Add -spice command line switch. Has support setting passwd and port for
> now. With this patch applied the spice client can successfully connect
> to qemu. You can't do anything useful yet though.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> Makefile.objs | 2 +
> qemu-config.c | 23 ++++++++
> qemu-config.h | 1 +
> qemu-options.hx | 8 +++
> qemu-spice.h | 22 ++++++++
> spice.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> vl.c | 15 ++++++
> 7 files changed, 222 insertions(+), 0 deletions(-)
> create mode 100644 qemu-spice.h
> create mode 100644 spice.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index edfca87..021067b 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -88,6 +88,8 @@ common-obj-y += pflib.o
> common-obj-$(CONFIG_BRLAPI) += baum.o
> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>
> +common-obj-$(CONFIG_SPICE) += spice.o
> +
> audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
> audio-obj-$(CONFIG_SDL) += sdlaudio.o
> audio-obj-$(CONFIG_OSS) += ossaudio.o
> diff --git a/qemu-config.c b/qemu-config.c
> index 95abe61..4cf3b53 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -342,6 +342,26 @@ QemuOptsList qemu_cpudef_opts = {
> },
> };
>
> +#ifdef CONFIG_SPICE
>
Now's probably a good time to make vm_config_groups a list that can be
dynamically added to so that you can register the option group from
within spice.c.
This would require a new qemu_opts_parse() that took a name of an
QemuOptsList instead of the pointer to it but then you can return
failure if spice is unsupported.
The result would be that we wouldn't need an #ifdef here, -spice would
always be a command line option, but if we didn't support spice, we'd
return an error if we tried to use it.
> diff --git a/qemu-config.h b/qemu-config.h
> index dca69d4..3a90213 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -14,6 +14,7 @@ extern QemuOptsList qemu_rtc_opts;
> extern QemuOptsList qemu_global_opts;
> extern QemuOptsList qemu_mon_opts;
> extern QemuOptsList qemu_cpudef_opts;
> +extern QemuOptsList qemu_spice_opts;
>
> QemuOptsList *qemu_find_opts(const char *group);
> int qemu_set_option(const char *str);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index db86feb..c05c219 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -674,6 +674,14 @@ STEXI
> Enable SDL.
> ETEXI
>
> +#ifdef CONFIG_SPICE
> +DEF("spice", HAS_ARG, QEMU_OPTION_spice,
> + "-spice<args> use spice\n", QEMU_ARCH_ALL)
> +STEXI
> +Use Spice.
> +ETEXI
> +#endif
> +
>
As mentioned above, I prefer run time failure instead of build time
failure. One reason is that we build documentation based on this file
and it's awkward to have different documentation based on what's
available in your build.
For instance, if I don't have libspice installed and I regenerate the
doc on qemu.org, no one will be able to find out about spice from the help.
BTW, you could have better documentation than 'Use Spice.' :-)
> DEF("portrait", 0, QEMU_OPTION_portrait,
> "-portrait rotate graphical output 90 deg left (only PXA LCD)\n",
> QEMU_ARCH_ALL)
> diff --git a/qemu-spice.h b/qemu-spice.h
> new file mode 100644
> index 0000000..5597576
> --- /dev/null
> +++ b/qemu-spice.h
> @@ -0,0 +1,22 @@
> +#ifndef QEMU_SPICE_H
> +#define QEMU_SPICE_H
> +
> +#ifdef CONFIG_SPICE
> +
> +#include<spice.h>
> +
> +#include "qemu-option.h"
> +#include "qemu-config.h"
> +
> +extern SpiceServer *spice_server;
> +extern int using_spice;
> +
> +void qemu_spice_init(void);
>
Could you avoid this with a module_init()?
Please make sure to have copyright/licenses in all files too.
> +
> +#else /* CONFIG_SPICE */
> +
> +#define using_spice 0
>
Please make this at least a macro.
> +#endif /* CONFIG_SPICE */
> +
> +#endif /* QEMU_SPICE_H */
> diff --git a/spice.c b/spice.c
> new file mode 100644
> index 0000000..50fa5ca
> --- /dev/null
> +++ b/spice.c
> @@ -0,0 +1,151 @@
>
Copyrights.
> +#include<stdlib.h>
> +#include<stdio.h>
> +#include<string.h>
>
This all comes in qemu-common.h
> +#include<spice.h>
> +#include<spice-experimental.h>
> +
> +#include "qemu-common.h"
> +#include "qemu-spice.h"
> +#include "qemu-timer.h"
> +#include "qemu-queue.h"
> +#include "monitor.h"
> +
> +/* core bits */
> +
> +SpiceServer *spice_server;
> +int using_spice = 0;
> +
> +struct SpiceTimer {
> + QEMUTimer *timer;
> + QTAILQ_ENTRY(SpiceTimer) next;
> +};
> +static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
> +
> +static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
> +{
> + SpiceTimer *timer;
> +
> + timer = qemu_mallocz(sizeof(*timer));
> + timer->timer = qemu_new_timer(rt_clock, func, opaque);
> + QTAILQ_INSERT_TAIL(&timers, timer, next);
> + return timer;
> +}
> +
> +static void timer_start(SpiceTimer *timer, uint32_t ms)
> +{
> + qemu_mod_timer(timer->timer, qemu_get_clock(rt_clock) + ms);
> +}
> +
> +static void timer_cancel(SpiceTimer *timer)
> +{
> + qemu_del_timer(timer->timer);
> +}
> +
> +static void timer_remove(SpiceTimer *timer)
> +{
> + qemu_del_timer(timer->timer);
> + qemu_free_timer(timer->timer);
> + QTAILQ_REMOVE(&timers, timer, next);
> + free(timer);
> +}
>
qemu_malloc mixed with free().
This can probably be a non-Spice function too. I want to propose a new
Timer wrapper that behaves much like what you have above. There's a
couple things I'd suggest to do though. Let me post a quick patch as a
follow up.
> +struct SpiceWatch {
> + int fd;
> + int event_mask;
> + SpiceWatchFunc func;
> + void *opaque;
> + QTAILQ_ENTRY(SpiceWatch) next;
> +};
> +static QTAILQ_HEAD(, SpiceWatch) watches = QTAILQ_HEAD_INITIALIZER(watches);
> +
> +static void watch_read(void *opaque)
> +{
> + SpiceWatch *watch = opaque;
> + watch->func(watch->fd, SPICE_WATCH_EVENT_READ, watch->opaque);
> +}
> +
> +static void watch_write(void *opaque)
> +{
> + SpiceWatch *watch = opaque;
> + watch->func(watch->fd, SPICE_WATCH_EVENT_WRITE, watch->opaque);
> +}
> +
> +static void watch_update_mask(SpiceWatch *watch, int event_mask)
> +{
> + IOHandler *on_read = NULL;
> + IOHandler *on_write = NULL;
> +
> + watch->event_mask = event_mask;
> + if (watch->event_mask& SPICE_WATCH_EVENT_READ)
> + on_read = watch_read;
> + if (watch->event_mask& SPICE_WATCH_EVENT_WRITE)
> + on_read = watch_write;
> + qemu_set_fd_handler(watch->fd, on_read, on_write, watch);
> +}
> +
> +static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void *opaque)
> +{
> + SpiceWatch *watch;
> +
> + watch = qemu_mallocz(sizeof(*watch));
> + watch->fd = fd;
> + watch->func = func;
> + watch->opaque = opaque;
> + QTAILQ_INSERT_TAIL(&watches, watch, next);
> +
> + watch_update_mask(watch, event_mask);
> + return watch;
> +}
> +
> +static void watch_remove(SpiceWatch *watch)
> +{
> + watch_update_mask(watch, 0);
> + QTAILQ_REMOVE(&watches, watch, next);
> + qemu_free(watch);
> +}
>
I understand the value of these wrappers but it should be common code IMHO.
> +static SpiceCoreInterface core_interface = {
> + .base.type = SPICE_INTERFACE_CORE,
> + .base.description = "qemu core services",
> + .base.major_version = SPICE_INTERFACE_CORE_MAJOR,
> + .base.minor_version = SPICE_INTERFACE_CORE_MINOR,
> +
> + .timer_add = timer_add,
> + .timer_start = timer_start,
> + .timer_cancel = timer_cancel,
> + .timer_remove = timer_remove,
> +
> + .watch_add = watch_add,
> + .watch_update_mask = watch_update_mask,
> + .watch_remove = watch_remove,
> +};
>
Ah, the interface is dictated by libspice. Okay, ignore the bit about
common code.
> +/* functions for the rest of qemu */
> +
> +void qemu_spice_init(void)
> +{
> + QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
> + const char *password;
> + int port;
>
Perhaps qemu_spice_init() should be passed the option list instead of
relying on the global here.
> + if (!opts)
> + return;
> + port = qemu_opt_get_number(opts, "port", 0);
> + if (!port)
> + return;
> + password = qemu_opt_get(opts, "password");
> +
> + spice_server = spice_server_new();
> + spice_server_set_port(spice_server, port);
> + if (password)
> + spice_server_set_ticket(spice_server, password, 0, 0, 0);
> + if (qemu_opt_get_bool(opts, "disable-ticketing", 0))
> + spice_server_set_noauth(spice_server);
> +
> + /* TODO: make configurable via cmdline */
> + spice_server_set_image_compression(spice_server, SPICE_IMAGE_COMPRESS_AUTO_GLZ);
> +
> + spice_server_init(spice_server,&core_interface);
> + using_spice = 1;
> +}
> diff --git a/vl.c b/vl.c
> index b3e3676..7f391c0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -161,6 +161,8 @@ int main(int argc, char **argv)
> #include "cpus.h"
> #include "arch_init.h"
>
> +#include "qemu-spice.h"
> +
> //#define DEBUG_NET
> //#define DEBUG_SLIRP
>
> @@ -2600,6 +2602,15 @@ int main(int argc, char **argv, char **envp)
> }
> break;
> }
> +#ifdef CONFIG_SPICE
> + case QEMU_OPTION_spice:
> + opts = qemu_opts_parse(&qemu_spice_opts, optarg, 0);
> + if (!opts) {
> + fprintf(stderr, "parse error: %s\n", optarg);
> + exit(1);
> + }
> + break;
> +#endif
> case QEMU_OPTION_writeconfig:
> {
> FILE *fp;
> @@ -2868,6 +2879,10 @@ int main(int argc, char **argv, char **envp)
> }
> qemu_add_globals();
>
> +#ifdef CONFIG_SPICE
> + qemu_spice_init();
> +#endif
>
I think there's two paths we can take here. We can either use
module_init() with a new class for this location or we can be more
explicit and pass the option list here.
I think I'd be happy with either one. In any event, I'd like to avoid
an #ifdef here.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-19 12:40 ` [Qemu-devel] [PATCH 6/9] spice: add keyboard Gerd Hoffmann
@ 2010-08-19 14:24 ` Anthony Liguori
2010-08-20 12:34 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:24 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Open keyboard channel. Now you can type into the spice client and the
> keyboard events are sent to your guest. You'll need some other display
> like vnc to actually see the guest responding to them though.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> Makefile.objs | 2 +-
> qemu-spice.h | 1 +
> spice-input.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> spice.c | 2 ++
> 4 files changed, 61 insertions(+), 1 deletions(-)
> create mode 100644 spice-input.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 021067b..6ddb373 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -88,7 +88,7 @@ common-obj-y += pflib.o
> common-obj-$(CONFIG_BRLAPI) += baum.o
> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>
> -common-obj-$(CONFIG_SPICE) += spice.o
> +common-obj-$(CONFIG_SPICE) += spice.o spice-input.o
>
> audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
> audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/qemu-spice.h b/qemu-spice.h
> index 5597576..ceb3db2 100644
> --- a/qemu-spice.h
> +++ b/qemu-spice.h
> @@ -12,6 +12,7 @@ extern SpiceServer *spice_server;
> extern int using_spice;
>
> void qemu_spice_init(void);
> +void qemu_spice_input_init(void);
>
> #else /* CONFIG_SPICE */
>
> diff --git a/spice-input.c b/spice-input.c
> new file mode 100644
> index 0000000..e1014d7
> --- /dev/null
> +++ b/spice-input.c
> @@ -0,0 +1,57 @@
> +#include<stdlib.h>
> +#include<stdio.h>
> +#include<string.h>
>
copyright and extra headers.
> +
> +#include<spice.h>
> +
> +#include "qemu-common.h"
> +#include "qemu-spice.h"
> +#include "console.h"
> +
> +/* keyboard bits */
> +
> +typedef struct QemuSpiceKbd {
> + SpiceKbdInstance sin;
> + int ledstate;
> +} QemuSpiceKbd;
> +
> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag);
> +static uint8_t kbd_get_leds(SpiceKbdInstance *sin);
> +static void kbd_leds(void *opaque, int l);
> +
> +static const SpiceKbdInterface kbd_interface = {
> + .base.type = SPICE_INTERFACE_KEYBOARD,
> + .base.description = "qemu keyboard",
> + .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
> + .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
> + .push_scan_freg = kbd_push_key,
> + .get_leds = kbd_get_leds,
> +};
> +
> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
> +{
> + kbd_put_keycode(frag);
> +}
>
Instead of using this interface which includes multi-byte sequences, it
would probably make sense to use the same compressed encoding that VNC
uses and introduce a new function that takes this encoding type. The
advantage is that one call == one keycode so it's far less prone to
goofiness.
> +
> +static uint8_t kbd_get_leds(SpiceKbdInstance *sin)
> +{
> + QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
> + return kbd->ledstate;
> +}
> +
> +static void kbd_leds(void *opaque, int ledstate)
> +{
> + QemuSpiceKbd *kbd = opaque;
> + kbd->ledstate = ledstate;
> + spice_server_kbd_leds(&kbd->sin, ledstate);
> +}
>
It's probably more robust in the long term to explicitly convert the
ledstate to from the QEMU format to the libspice format even if they
both happen to be the same.
Regards,
Anthony Liguori
> +void qemu_spice_input_init(void)
> +{
> + QemuSpiceKbd *kbd;
> +
> + kbd = qemu_mallocz(sizeof(*kbd));
> + kbd->sin.base.sif =&kbd_interface.base;
> + spice_server_add_interface(spice_server,&kbd->sin.base);
> + qemu_add_led_event_handler(kbd_leds, kbd);
> +}
> diff --git a/spice.c b/spice.c
> index 50fa5ca..c763d52 100644
> --- a/spice.c
> +++ b/spice.c
> @@ -148,4 +148,6 @@ void qemu_spice_init(void)
>
> spice_server_init(spice_server,&core_interface);
> using_spice = 1;
> +
> + qemu_spice_input_init();
> }
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-19 12:40 ` [Qemu-devel] [PATCH 7/9] spice: add mouse Gerd Hoffmann
@ 2010-08-19 14:25 ` Anthony Liguori
2010-08-20 12:42 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:25 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Open mouse channel. Now you can move the guests mouse pointer.
> No tablet / absolute positioning (yet) though.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> spice-input.c | 31 +++++++++++++++++++++++++++++++
> 1 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/spice-input.c b/spice-input.c
> index e1014d7..8f3deb4 100644
> --- a/spice-input.c
> +++ b/spice-input.c
> @@ -46,12 +46,43 @@ static void kbd_leds(void *opaque, int ledstate)
> spice_server_kbd_leds(&kbd->sin, ledstate);
> }
>
> +/* mouse bits */
> +
> +typedef struct QemuSpiceMouse {
> + SpiceMouseInstance sin;
> +} QemuSpiceMouse;
> +
> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
> + uint32_t buttons_state)
> +{
> + kbd_mouse_event(dx, dy, dz, buttons_state);
>
dz is an odd interface. We use it to represent additional buttons which
really makes no sense. If you still can, I'd suggest moving dz into
buttons_state.
Previous comment still applies though, you should explicitly convert the
button_states from QEMU format to Spice format to future proof.
Regards,
Anthony Liguori
> +}
> +
> +static void mouse_buttons(SpiceMouseInstance *sin, uint32_t buttons_state)
> +{
> + kbd_mouse_event(0, 0, 0, buttons_state);
> +}
> +
> +static const SpiceMouseInterface mouse_interface = {
> + .base.type = SPICE_INTERFACE_MOUSE,
> + .base.description = "mouse",
> + .base.major_version = SPICE_INTERFACE_MOUSE_MAJOR,
> + .base.minor_version = SPICE_INTERFACE_MOUSE_MINOR,
> + .motion = mouse_motion,
> + .buttons = mouse_buttons,
> +};
> +
> void qemu_spice_input_init(void)
> {
> QemuSpiceKbd *kbd;
> + QemuSpiceMouse *mouse;
>
> kbd = qemu_mallocz(sizeof(*kbd));
> kbd->sin.base.sif =&kbd_interface.base;
> spice_server_add_interface(spice_server,&kbd->sin.base);
> qemu_add_led_event_handler(kbd_leds, kbd);
> +
> + mouse = qemu_mallocz(sizeof(*mouse));
> + mouse->sin.base.sif =&mouse_interface.base;
> + spice_server_add_interface(spice_server,&mouse->sin.base);
> }
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 12:40 ` [Qemu-devel] [PATCH 8/9] spice: simple display Gerd Hoffmann
@ 2010-08-19 14:39 ` Anthony Liguori
2010-08-19 15:23 ` malc
2010-08-19 16:05 ` Gerd Hoffmann
0 siblings, 2 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:39 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> With that patch applied you'll actually see the guests screen in the
> spice client. This does *not* bring qxl and full spice support though.
> This is basically the qxl vga mode made more generic, so it plays
> together with any qemu-emulated gfx card. You can display stdvga or
> cirrus via spice client. You can have both vnc and spice enabled and
> clients connected at the same time.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> Makefile.objs | 2 +-
> qemu-spice.h | 1 +
> spice-display.c | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> spice-display.h | 52 ++++++++
> vl.c | 7 +-
> 5 files changed, 447 insertions(+), 2 deletions(-)
> create mode 100644 spice-display.c
> create mode 100644 spice-display.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 6ddb373..2f40529 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -88,7 +88,7 @@ common-obj-y += pflib.o
> common-obj-$(CONFIG_BRLAPI) += baum.o
> common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>
> -common-obj-$(CONFIG_SPICE) += spice.o spice-input.o
> +common-obj-$(CONFIG_SPICE) += spice.o spice-input.o spice-display.o
>
> audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
> audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/qemu-spice.h b/qemu-spice.h
> index ceb3db2..f061004 100644
> --- a/qemu-spice.h
> +++ b/qemu-spice.h
> @@ -13,6 +13,7 @@ extern int using_spice;
>
> void qemu_spice_init(void);
> void qemu_spice_input_init(void);
> +void qemu_spice_display_init(DisplayState *ds);
>
> #else /* CONFIG_SPICE */
>
> diff --git a/spice-display.c b/spice-display.c
> new file mode 100644
> index 0000000..1e6d939
> --- /dev/null
> +++ b/spice-display.c
> @@ -0,0 +1,387 @@
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<stdbool.h>
> +#include<stdint.h>
> +#include<string.h>
>
Copyright + unnecessary headers.
> +#include<pthread.h>
>
Hopefully this isn't actually needed. If it is, this needs to be a bit
more clever.
> +
> +#include "qemu-common.h"
> +#include "qemu-spice.h"
> +#include "qemu-timer.h"
> +#include "qemu-queue.h"
> +#include "monitor.h"
> +#include "console.h"
> +#include "sysemu.h"
> +
> +#include "spice-display.h"
> +
> +static int debug = 0;
> +
> +int qemu_spice_rect_is_empty(const QXLRect* r)
> +{
> + return r->top == r->bottom || r->left == r->right;
> +}
> +
> +void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
> +{
> + if (qemu_spice_rect_is_empty(r)) {
> + return;
> + }
> +
> + if (qemu_spice_rect_is_empty(dest)) {
> + *dest = *r;
> + return;
> + }
> +
> + dest->top = MIN(dest->top, r->top);
> + dest->left = MIN(dest->left, r->left);
> + dest->bottom = MAX(dest->bottom, r->bottom);
> + dest->right = MAX(dest->right, r->right);
> +}
> +
> +SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
> +{
> + SimpleSpiceUpdate *update;
> + QXLDrawable *drawable;
> + QXLImage *image;
> + QXLCommand *cmd;
> + uint8_t *src, *dst;
> + int by, bw, bh;
> +
> + if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> + return NULL;
> + };
> +
> + pthread_mutex_lock(&ssd->lock);
>
The locking worries me here.
It only makes sense if the spice interface_* callbacks can be invoked
from threads independently of any of the QEMU threads.
If that's the case, that means that the interface_* code is potentially
running in parallel to another QEMU thread that's holding the qemu_mutex.
So just acquiring a private lock in the interface_* code and then
calling into common QEMU code could result in re-entrance in interfaces
that aren't re-entrant.
> + if (debug> 1)
> + fprintf(stderr, "%s: lr %d -> %d, tb -> %d -> %d\n", __FUNCTION__,
> + ssd->dirty.left, ssd->dirty.right,
> + ssd->dirty.top, ssd->dirty.bottom);
> +
> + update = qemu_mallocz(sizeof(*update));
> + drawable =&update->drawable;
> + image =&update->image;
> + cmd =&update->ext.cmd;
> +
> + bw = ssd->dirty.right - ssd->dirty.left;
> + bh = ssd->dirty.bottom - ssd->dirty.top;
> + update->bitmap = qemu_malloc(bw * bh * 4);
>
While not really unsafe, the qemu_malloc functions are not guaranteed to
be re-entrant by the interfaces today. It's also terribly subtle to
have to rely on implicit re-entrance safety.
> +
> + drawable->bbox = ssd->dirty;
> + drawable->clip.type = SPICE_CLIP_TYPE_NONE;
> + drawable->effect = QXL_EFFECT_OPAQUE;
> + drawable->release_info.id = (intptr_t)update;
> + drawable->type = QXL_DRAW_COPY;
> +
> + drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT;
> + drawable->u.copy.src_bitmap = (intptr_t)image;
> + drawable->u.copy.src_area.right = bw;
> + drawable->u.copy.src_area.bottom = bh;
> +
> + QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_DEVICE, ssd->unique++);
> + image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
> + image->bitmap.flags = QXL_BITMAP_DIRECT | QXL_BITMAP_TOP_DOWN;
> + image->bitmap.stride = bw * 4;
> + image->descriptor.width = image->bitmap.x = bw;
> + image->descriptor.height = image->bitmap.y = bh;
> + image->bitmap.data = (intptr_t)(update->bitmap);
> + image->bitmap.palette = 0;
> + image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
> +
> + if (ssd->conv == NULL) {
> + PixelFormat dst = qemu_default_pixelformat(32);
> + ssd->conv = qemu_pf_conv_get(&dst,&ssd->ds->surface->pf);
> + assert(ssd->conv);
> + }
> +
> + src = ds_get_data(ssd->ds) +
> + ssd->dirty.top * ds_get_linesize(ssd->ds) +
> + ssd->dirty.left * ds_get_bytes_per_pixel(ssd->ds);
> + dst = update->bitmap;
> + for (by = 0; by< bh; by++) {
> + qemu_pf_conv_run(ssd->conv, dst, src, bw);
> + src += ds_get_linesize(ssd->ds);
> + dst += image->bitmap.stride;
> + }
> +
> + cmd->type = QXL_CMD_DRAW;
> + cmd->data = (intptr_t)drawable;
> +
> + memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> + pthread_mutex_unlock(&ssd->lock);
> + return update;
> +}
>
As we move to better support for re-entrance, we have to be careful we
don't create a scenario where some code relies on certain functions to
be re-entrant without it clearly being part of the contract.
So in the very least, any function that gets touched by something not
carrying the qemu_mutex needs to have a comment in the definition and
declaration that the functions are required to be re-entrant. Also, the
spice-display code would benefit from clearly stating where you were
holding the qemu_mutex and where you weren't.
> +
> +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate *update)
> +{
> + qemu_free(update->bitmap);
> + qemu_free(update);
> +}
> +
> +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
> +{
> + QXLDevMemSlot memslot;
> +
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
>
a dprintf() would better fit qemu's style.
> + memset(&memslot, 0, sizeof(memslot));
> + memslot.slot_group_id = MEMSLOT_GROUP_HOST;
> + memslot.virt_end = ~0;
> + ssd->worker->add_memslot(ssd->worker,&memslot);
> +}
> +
> +void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
> +{
> + QXLDevSurfaceCreate surface;
> +
> + if (debug)
> + fprintf(stderr, "%s: %dx%d\n", __FUNCTION__,
> + ds_get_width(ssd->ds), ds_get_height(ssd->ds));
> +
> + surface.format = SPICE_SURFACE_FMT_32_xRGB;
> + surface.width = ds_get_width(ssd->ds);
> + surface.height = ds_get_height(ssd->ds);
> + surface.stride = -surface.width * 4;
> + surface.mouse_mode = 0;
> + surface.flags = 0;
> + surface.type = 0;
> + surface.mem = (intptr_t)ssd->buf;
> + surface.group_id = MEMSLOT_GROUP_HOST;
> + ssd->worker->create_primary_surface(ssd->worker, 0,&surface);
> +}
> +
> +void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
> +{
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> +
> + ssd->worker->destroy_primary_surface(ssd->worker, 0);
> +}
> +
> +void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
> +{
> + SimpleSpiceDisplay *ssd = opaque;
> +
> + if (running) {
> + ssd->worker->start(ssd->worker);
> + } else {
> + ssd->worker->stop(ssd->worker);
> + }
> + ssd->running = running;
> +}
> +
> +/* display listener callbacks */
> +
> +void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
> + int x, int y, int w, int h)
> +{
> + QXLRect update_area;
> +
> + if (debug> 1)
> + fprintf(stderr, "%s: x %d y %d w %d h %d\n", __FUNCTION__, x, y, w, h);
> + update_area.left = x,
> + update_area.right = x + w;
> + update_area.top = y;
> + update_area.bottom = y + h;
> +
> + pthread_mutex_lock(&ssd->lock);
> + if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> + ssd->notify++;
> + }
> + qemu_spice_rect_union(&ssd->dirty,&update_area);
> + pthread_mutex_unlock(&ssd->lock);
> +}
> +
> +void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
> +{
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> +
> + pthread_mutex_lock(&ssd->lock);
> + memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> + pthread_mutex_unlock(&ssd->lock);
> +
> + qemu_spice_destroy_host_primary(ssd);
> + qemu_spice_create_host_primary(ssd);
> + qemu_pf_conv_put(ssd->conv);
> + ssd->conv = NULL;
> +
> + pthread_mutex_lock(&ssd->lock);
> + memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> + ssd->notify++;
> + pthread_mutex_unlock(&ssd->lock);
> +}
> +
> +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
> +{
> + if (debug> 2)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + vga_hw_update();
> + if (ssd->notify) {
> + ssd->notify = 0;
> + ssd->worker->wakeup(ssd->worker);
> + if (debug> 1)
> + fprintf(stderr, "%s: notify\n", __FUNCTION__);
> + }
> +}
> +
> +/* spice display interface callbacks */
> +
> +static void interface_attach_worker(QXLInstance *sin, QXLWorker *qxl_worker)
> +{
> + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> +
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + ssd->worker = qxl_worker;
> +}
> +
> +static void interface_set_compression_level(QXLInstance *sin, int level)
> +{
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + /* nothing to do */
> +}
> +
> +static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
> +{
> + if (debug> 2)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + /* nothing to do */
> +}
> +
> +static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
> +{
> + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> +
> + info->memslot_gen_bits = MEMSLOT_GENERATION_BITS;
> + info->memslot_id_bits = MEMSLOT_SLOT_BITS;
> + info->num_memslots = NUM_MEMSLOTS;
> + info->num_memslots_groups = NUM_MEMSLOTS_GROUPS;
> + info->internal_groupslot_id = 0;
> + info->qxl_ram_size = ssd->bufsize;
> + info->n_surfaces = NUM_SURFACES;
> +}
> +
> +static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
> +{
> + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> + SimpleSpiceUpdate *update;
> +
> + if (debug> 2)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + update = qemu_spice_create_update(ssd);
> + if (update == NULL) {
> + return false;
> + }
> + *ext = update->ext;
> + return true;
> +}
> +
> +static int interface_req_cmd_notification(QXLInstance *sin)
> +{
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + return 1;
> +}
> +
> +static void interface_release_resource(QXLInstance *sin,
> + struct QXLReleaseInfoExt ext)
> +{
> + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> + uintptr_t id;
> +
> + if (debug> 1)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + id = ext.info->id;
> + qemu_spice_destroy_update(ssd, (void*)id);
> +}
> +
> +static int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt *ext)
> +{
> + if (debug> 2)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + return false;
> +}
> +
> +static int interface_req_cursor_notification(QXLInstance *sin)
> +{
> + if (debug)
> + fprintf(stderr, "%s:\n", __FUNCTION__);
> + return 1;
> +}
> +
> +static void interface_notify_update(QXLInstance *sin, uint32_t update_id)
> +{
> + fprintf(stderr, "%s: abort()\n", __FUNCTION__);
> + abort();
> +}
> +
> +static int interface_flush_resources(QXLInstance *sin)
> +{
> + fprintf(stderr, "%s: abort()\n", __FUNCTION__);
> + abort();
> + return 0;
> +}
> +
> +static const QXLInterface dpy_interface = {
> + .base.type = SPICE_INTERFACE_QXL,
> + .base.description = "qemu simple display",
> + .base.major_version = SPICE_INTERFACE_QXL_MAJOR,
> + .base.minor_version = SPICE_INTERFACE_QXL_MINOR,
> +
> + .attache_worker = interface_attach_worker,
> + .set_compression_level = interface_set_compression_level,
> + .set_mm_time = interface_set_mm_time,
> +
> + .get_init_info = interface_get_init_info,
> + .get_command = interface_get_command,
> + .req_cmd_notification = interface_req_cmd_notification,
> + .release_resource = interface_release_resource,
> + .get_cursor_command = interface_get_cursor_command,
> + .req_cursor_notification = interface_req_cursor_notification,
> + .notify_update = interface_notify_update,
> + .flush_resources = interface_flush_resources,
> +};
> +
> +static SimpleSpiceDisplay sdpy;
> +
> +static void display_update(struct DisplayState *ds, int x, int y, int w, int h)
> +{
> + qemu_spice_display_update(&sdpy, x, y, w, h);
> +}
> +
> +static void display_resize(struct DisplayState *ds)
> +{
> + qemu_spice_display_resize(&sdpy);
> +}
> +
> +static void display_refresh(struct DisplayState *ds)
> +{
> + qemu_spice_display_refresh(&sdpy);
> +}
> +
> +static DisplayChangeListener display_listener = {
> + .dpy_update = display_update,
> + .dpy_resize = display_resize,
> + .dpy_refresh = display_refresh,
> +};
> +
> +void qemu_spice_display_init(DisplayState *ds)
> +{
> + assert(sdpy.ds == NULL);
> + sdpy.ds = ds;
> + sdpy.bufsize = (16 * 1024 * 1024);
> + sdpy.buf = qemu_malloc(sdpy.bufsize);
> + pthread_mutex_init(&sdpy.lock, NULL);
> + register_displaychangelistener(ds,&display_listener);
> +
> + sdpy.qxl.base.sif =&dpy_interface.base;
> + spice_server_add_interface(spice_server,&sdpy.qxl.base);
> + assert(sdpy.worker);
> +
> + qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,&sdpy);
> + qemu_spice_create_host_memslot(&sdpy);
> + qemu_spice_create_host_primary(&sdpy);
> +}
> diff --git a/spice-display.h b/spice-display.h
> new file mode 100644
> index 0000000..b55e7ea
> --- /dev/null
> +++ b/spice-display.h
> @@ -0,0 +1,52 @@
> +#include<spice/ipc_ring.h>
> +#include<spice/enums.h>
> +#include<spice/qxl_dev.h>
> +
> +#include "pflib.h"
> +
> +#define NUM_MEMSLOTS 8
> +#define MEMSLOT_GENERATION_BITS 8
> +#define MEMSLOT_SLOT_BITS 8
> +
> +#define MEMSLOT_GROUP_HOST 0
> +#define MEMSLOT_GROUP_GUEST 1
> +#define NUM_MEMSLOTS_GROUPS 2
> +
> +#define NUM_SURFACES 1024
> +
> +typedef struct SimpleSpiceDisplay {
> + DisplayState *ds;
> + void *buf;
> + int bufsize;
> + QXLWorker *worker;
> + QXLInstance qxl;
> + uint32_t unique;
> + QemuPfConv *conv;
> +
> + pthread_mutex_t lock;
> + QXLRect dirty;
> + int notify;
> + int running;
> +} SimpleSpiceDisplay;
> +
> +typedef struct SimpleSpiceUpdate {
> + QXLDrawable drawable;
> + QXLImage image;
> + QXLCommandExt ext;
> + uint8_t *bitmap;
> +} SimpleSpiceUpdate;
> +
> +int qemu_spice_rect_is_empty(const QXLRect* r);
> +void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r);
> +
> +SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *sdpy);
> +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate *update);
> +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd);
> +void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd);
> +void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd);
> +void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason);
> +
> +void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
> + int x, int y, int w, int h);
> +void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
> +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
> diff --git a/vl.c b/vl.c
> index 7f391c0..f68f8e8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2910,7 +2910,7 @@ int main(int argc, char **argv, char **envp)
> /* just use the first displaystate for the moment */
> ds = get_displaystate();
>
> - if (display_type == DT_DEFAULT) {
> + if (display_type == DT_DEFAULT&& !using_spice) {
> #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
> display_type = DT_SDL;
> #else
> @@ -2950,6 +2950,11 @@ int main(int argc, char **argv, char **envp)
> default:
> break;
> }
> +#ifdef CONFIG_SPICE
> + if (using_spice) {
> + qemu_spice_display_init(ds);
> + }
> +#endif
>
Having spice as an independent interface to the current display_type
switching seems awkward to me.
Regards,
Anthony Liguori
> dpy_resize(ds);
>
> dcl = ds->listeners;
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 9/9] spice: add tablet support
2010-08-19 12:40 ` [Qemu-devel] [PATCH 9/9] spice: add tablet support Gerd Hoffmann
@ 2010-08-19 14:40 ` Anthony Liguori
0 siblings, 0 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 14:40 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> Add support for the spice tablet interface. The tablet interface will
> be registered (and then used by the spice client) as soon as a absolute
> pointing device is available and used by the guest, i.e. you'll have to
> configure your guest with '-usbdevice tablet'.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> spice-display.c | 2 +-
> spice-input.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 93 insertions(+), 8 deletions(-)
>
> diff --git a/spice-display.c b/spice-display.c
> index 1e6d939..fec2432 100644
> --- a/spice-display.c
> +++ b/spice-display.c
> @@ -143,7 +143,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
> surface.width = ds_get_width(ssd->ds);
> surface.height = ds_get_height(ssd->ds);
> surface.stride = -surface.width * 4;
> - surface.mouse_mode = 0;
> + surface.mouse_mode = true;
> surface.flags = 0;
> surface.type = 0;
> surface.mem = (intptr_t)ssd->buf;
> diff --git a/spice-input.c b/spice-input.c
> index 8f3deb4..5646ff9 100644
> --- a/spice-input.c
> +++ b/spice-input.c
> @@ -1,5 +1,6 @@
> #include<stdlib.h>
> #include<stdio.h>
> +#include<stdbool.h>
> #include<string.h>
>
> #include<spice.h>
> @@ -48,9 +49,13 @@ static void kbd_leds(void *opaque, int ledstate)
>
> /* mouse bits */
>
> -typedef struct QemuSpiceMouse {
> - SpiceMouseInstance sin;
> -} QemuSpiceMouse;
> +typedef struct QemuSpicePointer {
> + SpiceMouseInstance mouse;
> + SpiceTabletInstance tablet;
> + int width, height, x, y;
> + Notifier mouse_mode;
> + bool absolute;
> +} QemuSpicePointer;
>
> static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int dz,
> uint32_t buttons_state)
> @@ -72,17 +77,97 @@ static const SpiceMouseInterface mouse_interface = {
> .buttons = mouse_buttons,
> };
>
> +static void tablet_set_logical_size(SpiceTabletInstance* sin, int width, int height)
> +{
> + QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
> +
> + fprintf(stderr, "%s: %dx%d\n", __FUNCTION__, width, height);
> + if (height< 16)
> + height = 16;
> + if (width< 16)
> + width = 16;
> + pointer->width = width;
> + pointer->height = height;
> +}
> +
> +static void tablet_position(SpiceTabletInstance* sin, int x, int y,
> + uint32_t buttons_state)
> +{
> + QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
> +
> + pointer->x = x * 0x7FFF / (pointer->width - 1);
> + pointer->y = y * 0x7FFF / (pointer->height - 1);
> + kbd_mouse_event(pointer->x, pointer->y, 0, buttons_state);
> +}
> +
> +
> +static void tablet_wheel(SpiceTabletInstance* sin, int wheel,
> + uint32_t buttons_state)
> +{
> + QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
> +
> + kbd_mouse_event(pointer->x, pointer->y, wheel, buttons_state);
> +}
> +
> +static void tablet_buttons(SpiceTabletInstance *sin,
> + uint32_t buttons_state)
> +{
> + QemuSpicePointer *pointer = container_of(sin, QemuSpicePointer, tablet);
> +
> + kbd_mouse_event(pointer->x, pointer->y, 0, buttons_state);
> +}
> +
> +static const SpiceTabletInterface tablet_interface = {
> + .base.type = SPICE_INTERFACE_TABLET,
> + .base.description = "tablet",
> + .base.major_version = SPICE_INTERFACE_TABLET_MAJOR,
> + .base.minor_version = SPICE_INTERFACE_TABLET_MINOR,
> + .set_logical_size = tablet_set_logical_size,
> + .position = tablet_position,
> + .wheel = tablet_wheel,
> + .buttons = tablet_buttons,
> +};
> +
> +static void mouse_mode_notifier(Notifier *notifier)
> +{
> + QemuSpicePointer *pointer = container_of(notifier, QemuSpicePointer, mouse_mode);
> + bool is_absolute = kbd_mouse_is_absolute();
> + bool has_absolute = kbd_mouse_has_absolute();
> +
> + fprintf(stderr, "%s: absolute pointer: %s%s\n", __FUNCTION__,
> + has_absolute ? "present" : "not available",
> + is_absolute ? "+active" : "");
> +
> + if (pointer->absolute == is_absolute)
> + return;
> +
> + if (is_absolute) {
> + fprintf(stderr, "%s: using absolute pointer (client mode)\n", __FUNCTION__);
> + spice_server_add_interface(spice_server,&pointer->tablet.base);
> + } else {
> + fprintf(stderr, "%s: using relative pointer (server mode)\n", __FUNCTION__);
> + spice_server_remove_interface(&pointer->tablet.base);
> + }
> + pointer->absolute = is_absolute;
> +}
> +
>
Minus the fprintf()s and the lack of conversion of button formats, this
all looks pretty reasonable.
Regards,
Anthony Liguori
> void qemu_spice_input_init(void)
> {
> QemuSpiceKbd *kbd;
> - QemuSpiceMouse *mouse;
> + QemuSpicePointer *pointer;
>
> kbd = qemu_mallocz(sizeof(*kbd));
> kbd->sin.base.sif =&kbd_interface.base;
> spice_server_add_interface(spice_server,&kbd->sin.base);
> qemu_add_led_event_handler(kbd_leds, kbd);
>
> - mouse = qemu_mallocz(sizeof(*mouse));
> - mouse->sin.base.sif =&mouse_interface.base;
> - spice_server_add_interface(spice_server,&mouse->sin.base);
> + pointer = qemu_mallocz(sizeof(*pointer));
> + pointer->mouse.base.sif =&mouse_interface.base;
> + pointer->tablet.base.sif =&tablet_interface.base;
> + spice_server_add_interface(spice_server,&pointer->mouse.base);
> +
> + pointer->absolute = false;
> + pointer->mouse_mode.notify = mouse_mode_notifier;
> + qemu_add_mouse_mode_change_notifier(&pointer->mouse_mode);
> + mouse_mode_notifier(&pointer->mouse_mode);
> }
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 14:39 ` Anthony Liguori
@ 2010-08-19 15:23 ` malc
2010-08-19 15:34 ` Anthony Liguori
2010-08-19 16:05 ` Gerd Hoffmann
1 sibling, 1 reply; 43+ messages in thread
From: malc @ 2010-08-19 15:23 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Gerd Hoffmann, qemu-devel
On Thu, 19 Aug 2010, Anthony Liguori wrote:
> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
> > With that patch applied you'll actually see the guests screen in the
> > spice client. This does *not* bring qxl and full spice support though.
> > This is basically the qxl vga mode made more generic, so it plays
> > together with any qemu-emulated gfx card. You can display stdvga or
> > cirrus via spice client. You can have both vnc and spice enabled and
> > clients connected at the same time.
[..snip..]
> > +
> > +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate
> > *update)
> > +{
> > + qemu_free(update->bitmap);
> > + qemu_free(update);
> > +}
> > +
> > +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
> > +{
> > + QXLDevMemSlot memslot;
> > +
> > + if (debug)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> >
>
> a dprintf() would better fit qemu's style.
A dprintf is a POSIX function.
>
> > + memset(&memslot, 0, sizeof(memslot));
> > + memslot.slot_group_id = MEMSLOT_GROUP_HOST;
> > + memslot.virt_end = ~0;
> > + ssd->worker->add_memslot(ssd->worker,&memslot);
> > +}
> > +
> > +void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
> > +{
> > + QXLDevSurfaceCreate surface;
> > +
> > + if (debug)
> > + fprintf(stderr, "%s: %dx%d\n", __FUNCTION__,
> > + ds_get_width(ssd->ds), ds_get_height(ssd->ds));
> > +
> > + surface.format = SPICE_SURFACE_FMT_32_xRGB;
> > + surface.width = ds_get_width(ssd->ds);
> > + surface.height = ds_get_height(ssd->ds);
> > + surface.stride = -surface.width * 4;
> > + surface.mouse_mode = 0;
> > + surface.flags = 0;
> > + surface.type = 0;
> > + surface.mem = (intptr_t)ssd->buf;
> > + surface.group_id = MEMSLOT_GROUP_HOST;
> > + ssd->worker->create_primary_surface(ssd->worker, 0,&surface);
> > +}
> > +
> > +void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
> > +{
> > + if (debug)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > +
> > + ssd->worker->destroy_primary_surface(ssd->worker, 0);
> > +}
> > +
> > +void qemu_spice_vm_change_state_handler(void *opaque, int running, int
> > reason)
> > +{
> > + SimpleSpiceDisplay *ssd = opaque;
> > +
> > + if (running) {
> > + ssd->worker->start(ssd->worker);
> > + } else {
> > + ssd->worker->stop(ssd->worker);
> > + }
> > + ssd->running = running;
> > +}
> > +
> > +/* display listener callbacks */
> > +
> > +void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
> > + int x, int y, int w, int h)
> > +{
> > + QXLRect update_area;
> > +
> > + if (debug> 1)
> > + fprintf(stderr, "%s: x %d y %d w %d h %d\n", __FUNCTION__, x, y, w,
> > h);
> > + update_area.left = x,
> > + update_area.right = x + w;
> > + update_area.top = y;
> > + update_area.bottom = y + h;
> > +
> > + pthread_mutex_lock(&ssd->lock);
> > + if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> > + ssd->notify++;
> > + }
> > + qemu_spice_rect_union(&ssd->dirty,&update_area);
> > + pthread_mutex_unlock(&ssd->lock);
> > +}
> > +
> > +void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
> > +{
> > + if (debug)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > +
> > + pthread_mutex_lock(&ssd->lock);
> > + memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> > + pthread_mutex_unlock(&ssd->lock);
> > +
> > + qemu_spice_destroy_host_primary(ssd);
> > + qemu_spice_create_host_primary(ssd);
> > + qemu_pf_conv_put(ssd->conv);
> > + ssd->conv = NULL;
> > +
> > + pthread_mutex_lock(&ssd->lock);
> > + memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> > + ssd->notify++;
> > + pthread_mutex_unlock(&ssd->lock);
> > +}
> > +
> > +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
> > +{
> > + if (debug> 2)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > + vga_hw_update();
> > + if (ssd->notify) {
> > + ssd->notify = 0;
> > + ssd->worker->wakeup(ssd->worker);
> > + if (debug> 1)
> > + fprintf(stderr, "%s: notify\n", __FUNCTION__);
> > + }
> > +}
> > +
> > +/* spice display interface callbacks */
> > +
> > +static void interface_attach_worker(QXLInstance *sin, QXLWorker
> > *qxl_worker)
> > +{
> > + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> > +
> > + if (debug)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > + ssd->worker = qxl_worker;
> > +}
> > +
> > +static void interface_set_compression_level(QXLInstance *sin, int level)
> > +{
> > + if (debug)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > + /* nothing to do */
> > +}
> > +
> > +static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
> > +{
> > + if (debug> 2)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > + /* nothing to do */
> > +}
> > +
> > +static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
> > +{
> > + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> > +
> > + info->memslot_gen_bits = MEMSLOT_GENERATION_BITS;
> > + info->memslot_id_bits = MEMSLOT_SLOT_BITS;
> > + info->num_memslots = NUM_MEMSLOTS;
> > + info->num_memslots_groups = NUM_MEMSLOTS_GROUPS;
> > + info->internal_groupslot_id = 0;
> > + info->qxl_ram_size = ssd->bufsize;
> > + info->n_surfaces = NUM_SURFACES;
> > +}
> > +
> > +static int interface_get_command(QXLInstance *sin, struct QXLCommandExt
> > *ext)
> > +{
> > + SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
> > + SimpleSpiceUpdate *update;
> > +
> > + if (debug> 2)
> > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > + update = qemu_spice_create_update(ssd);
[..snip..]
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 15:23 ` malc
@ 2010-08-19 15:34 ` Anthony Liguori
2010-08-19 15:36 ` malc
2010-08-19 15:40 ` Alexander Graf
0 siblings, 2 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 15:34 UTC (permalink / raw)
To: malc; +Cc: Gerd Hoffmann, qemu-devel
On 08/19/2010 10:23 AM, malc wrote:
>
>>> +
>>> +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate
>>> *update)
>>> +{
>>> + qemu_free(update->bitmap);
>>> + qemu_free(update);
>>> +}
>>> +
>>> +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
>>> +{
>>> + QXLDevMemSlot memslot;
>>> +
>>> + if (debug)
>>> + fprintf(stderr, "%s:\n", __FUNCTION__);
>>>
>>>
>> a dprintf() would better fit qemu's style.
>>
> A dprintf is a POSIX function.
>
Apparently a recent POSIX function...
Preferred alternatives? I really dislike all caps macros.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library.
2010-08-19 14:04 ` Anthony Liguori
@ 2010-08-19 15:34 ` Gerd Hoffmann
2010-08-19 19:09 ` Anthony Liguori
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 15:34 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
On 08/19/10 16:04, Anthony Liguori wrote:
> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
> This code is more or less stand alone so it's a good candidate for
> having some in-tree unit tests.
>
> I can provide some examples of how it should be done if you like, but
> just something that lives in tests/ that exercises the functionality and
> can be run under valgrind to verify that we aren't leaking memory.
Yea, some example would be great. Commented tests/example.c maybe?
thanks,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 15:34 ` Anthony Liguori
@ 2010-08-19 15:36 ` malc
2010-08-19 19:03 ` Anthony Liguori
2010-08-19 15:40 ` Alexander Graf
1 sibling, 1 reply; 43+ messages in thread
From: malc @ 2010-08-19 15:36 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Gerd Hoffmann, qemu-devel
On Thu, 19 Aug 2010, Anthony Liguori wrote:
> On 08/19/2010 10:23 AM, malc wrote:
> >
> > > > +
> > > > +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy,
> > > > SimpleSpiceUpdate
> > > > *update)
> > > > +{
> > > > + qemu_free(update->bitmap);
> > > > + qemu_free(update);
> > > > +}
> > > > +
> > > > +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
> > > > +{
> > > > + QXLDevMemSlot memslot;
> > > > +
> > > > + if (debug)
> > > > + fprintf(stderr, "%s:\n", __FUNCTION__);
> > > >
> > > >
> > > a dprintf() would better fit qemu's style.
> > >
> > A dprintf is a POSIX function.
> >
>
> Apparently a recent POSIX function...
>
> Preferred alternatives? I really dislike all caps macros.
Macros should be all capps.
[..snip..]
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 15:34 ` Anthony Liguori
2010-08-19 15:36 ` malc
@ 2010-08-19 15:40 ` Alexander Graf
2010-08-25 11:09 ` Gerd Hoffmann
1 sibling, 1 reply; 43+ messages in thread
From: Alexander Graf @ 2010-08-19 15:40 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Gerd Hoffmann, qemu-devel
On 19.08.2010, at 17:34, Anthony Liguori wrote:
> On 08/19/2010 10:23 AM, malc wrote:
>>
>>>> +
>>>> +void qemu_spice_destroy_update(SimpleSpiceDisplay *sdpy, SimpleSpiceUpdate
>>>> *update)
>>>> +{
>>>> + qemu_free(update->bitmap);
>>>> + qemu_free(update);
>>>> +}
>>>> +
>>>> +void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
>>>> +{
>>>> + QXLDevMemSlot memslot;
>>>> +
>>>> + if (debug)
>>>> + fprintf(stderr, "%s:\n", __FUNCTION__);
>>>>
>>>>
>>> a dprintf() would better fit qemu's style.
>>>
>> A dprintf is a POSIX function.
>>
>
> Apparently a recent POSIX function...
In my man page it's declared as
#define _GNU_SOURCE
#include <stdio.h>
int dprintf(int fd, const char *format, ...);
but maybe it did become POSIX recently. Either way, it's used already :).
> Preferred alternatives? I really dislike all caps macros.
printd should be fine.
Alex
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 2/9] configure: add logging
2010-08-19 14:05 ` Anthony Liguori
@ 2010-08-19 15:44 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 15:44 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
On 08/19/10 16:05, Anthony Liguori wrote:
> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
>> Write compile commands and messages to config.log.
>> Useful for debugging configure.
>>
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
> Indeed.
>
> Acked-by: Anthony Liguori <aliguori@us.ibm.com>
>
> Given the large nature of spice, I suggest that you setup a subtree for
> it and do a pull request when the first round is ready.
There *is* a git tree (referenced in the cover letter) to pull from.
Will do that for the next version too ;)
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 14:39 ` Anthony Liguori
2010-08-19 15:23 ` malc
@ 2010-08-19 16:05 ` Gerd Hoffmann
2010-08-19 19:00 ` Anthony Liguori
1 sibling, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-19 16:05 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
>> + pthread_mutex_lock(&ssd->lock);
>
> The locking worries me here.
>
> It only makes sense if the spice interface_* callbacks can be invoked
> from threads independently of any of the QEMU threads.
>
> If that's the case, that means that the interface_* code is potentially
> running in parallel to another QEMU thread that's holding the qemu_mutex.
Yes, interface_* code can be called back from spice server thread context.
> So just acquiring a private lock in the interface_* code and then
> calling into common QEMU code could result in re-entrance in interfaces
> that aren't re-entrant.
I think there are no such calls.
> While not really unsafe, the qemu_malloc functions are not guaranteed to
> be re-entrant by the interfaces today. It's also terribly subtle to have
> to rely on implicit re-entrance safety.
The underlying malloc() is re-entrant, isn't it?
pflib (which is called too) is re-entrant too.
Otherwise only private data is accessed (under lock when needed).
> So in the very least, any function that gets touched by something not
> carrying the qemu_mutex needs to have a comment in the definition and
> declaration that the functions are required to be re-entrant. Also, the
> spice-display code would benefit from clearly stating where you were
> holding the qemu_mutex and where you weren't.
Yep.
>> +#ifdef CONFIG_SPICE
>> + if (using_spice) {
>> + qemu_spice_display_init(ds);
>> + }
>> +#endif
>
> Having spice as an independent interface to the current display_type
> switching seems awkward to me.
Having remote desktop protocols as DT_something seems awkward to me. It
makes sense for the local display (being none, curses, sdl, fbdev,
whatever). For remote display protocols I see no reason why we
shouldn't have multiple of them enabled at the same time, so the user
can connect with whatever he wants. And that even in parallel to a
local display if needed.
The state the patch introduces is a bit inconsistent though. But I'd
rather drop DT_VNC instead of adding DT_SPICE.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 16:05 ` Gerd Hoffmann
@ 2010-08-19 19:00 ` Anthony Liguori
0 siblings, 0 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 19:00 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 11:05 AM, Gerd Hoffmann wrote:
>> While not really unsafe, the qemu_malloc functions are not guaranteed to
>> be re-entrant by the interfaces today. It's also terribly subtle to have
>> to rely on implicit re-entrance safety.
>
> The underlying malloc() is re-entrant, isn't it?
> pflib (which is called too) is re-entrant too.
> Otherwise only private data is accessed (under lock when needed).
Yes, I looked too and agree that it's safe now. But we're sloppy as
hell in qemu about depending on global state and I can imagine someone
adding something to these functions that would create an issue.
I think documentation would be sufficient, at least for now.
>> Having spice as an independent interface to the current display_type
>> switching seems awkward to me.
>
> Having remote desktop protocols as DT_something seems awkward to me.
> It makes sense for the local display (being none, curses, sdl, fbdev,
> whatever). For remote display protocols I see no reason why we
> shouldn't have multiple of them enabled at the same time, so the user
> can connect with whatever he wants. And that even in parallel to a
> local display if needed.
>
> The state the patch introduces is a bit inconsistent though. But I'd
> rather drop DT_VNC instead of adding DT_SPICE.
Yes, I would think that would be reasonable.
Regards,
Anthony Liguori
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 15:36 ` malc
@ 2010-08-19 19:03 ` Anthony Liguori
2010-08-19 19:10 ` malc
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 19:03 UTC (permalink / raw)
To: malc; +Cc: Gerd Hoffmann, qemu-devel
On 08/19/2010 10:36 AM, malc wrote:
>> Apparently a recent POSIX function...
>>
>> Preferred alternatives? I really dislike all caps macros.
>>
> Macros should be all capps.
>
If you like angry code.
But honestly, just make printd() a static inline and then every one's
happy :-)
Regards,
Anthony Liguori
> [..snip..]
>
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library.
2010-08-19 15:34 ` Gerd Hoffmann
@ 2010-08-19 19:09 ` Anthony Liguori
2010-08-20 10:38 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-19 19:09 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/19/2010 10:34 AM, Gerd Hoffmann wrote:
> On 08/19/10 16:04, Anthony Liguori wrote:
>> On 08/19/2010 07:40 AM, Gerd Hoffmann wrote:
>>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>>
>> This code is more or less stand alone so it's a good candidate for
>> having some in-tree unit tests.
>>
>> I can provide some examples of how it should be done if you like, but
>> just something that lives in tests/ that exercises the functionality and
>> can be run under valgrind to verify that we aren't leaking memory.
>
> Yea, some example would be great. Commented tests/example.c maybe?
This is one of a few that I've written:
http://repo.or.cz/w/qemu/aliguori-queue.git/blob/refs/heads/qpi:/tests/test-buffer.c
You can see the rest of the plumbing in the tree. The libcheck tests
are also pretty reasonable although to be honest I dislike libcheck. I
don't find it to offer any real value compared to just something that
succeeds silently or fails upon error.
I'm working on isolating the serial device right now to write tests
against it but decoupling it from everything else is a fair bit of
work. I'd eventually like to get to the point where all new code had
corresponding in tree unit tests.
Regards,
Anthony Liguori
> thanks,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 19:03 ` Anthony Liguori
@ 2010-08-19 19:10 ` malc
0 siblings, 0 replies; 43+ messages in thread
From: malc @ 2010-08-19 19:10 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Gerd Hoffmann, qemu-devel
On Thu, 19 Aug 2010, Anthony Liguori wrote:
> On 08/19/2010 10:36 AM, malc wrote:
> > > Apparently a recent POSIX function...
> > >
> > > Preferred alternatives? I really dislike all caps macros.
> > >
> > Macros should be all capps.
> >
>
> If you like angry code.
I'm a self-flagelation kind of person.
>
> But honestly, just make printd() a static inline and then every one's happy
> :-)
That much is true.
>
> Regards,
>
> Anthony Liguori
>
> > [..snip..]
> >
> >
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library.
2010-08-19 19:09 ` Anthony Liguori
@ 2010-08-20 10:38 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 10:38 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
> You can see the rest of the plumbing in the tree.
Hmm, "make test" (and trying to build anything in tests/) fails with
separate source/build dirs. But it isn't obvious to me why it does.
The Makefile takes care to set vpath, so I think it should have picked
up the source files just fine ...
Ideas anyone?
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 5/9] spice: core bits
2010-08-19 14:19 ` Anthony Liguori
@ 2010-08-20 11:54 ` Gerd Hoffmann
2010-08-25 12:37 ` Gerd Hoffmann
1 sibling, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 11:54 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
>> +#ifdef CONFIG_SPICE
>
> Now's probably a good time to make vm_config_groups a list that can be
> dynamically added to so that you can register the option group from
> within spice.c.
>
> This would require a new qemu_opts_parse() that took a name of an
> QemuOptsList instead of the pointer to it but then you can return
> failure if spice is unsupported.
Ah, you want me fix virtfs too ;)
Patches are on the list. Well, at least a start. For further cleanups
we'll need some more *_init() magic for the non-devices stuff.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-19 14:24 ` Anthony Liguori
@ 2010-08-20 12:34 ` Gerd Hoffmann
2010-08-20 13:18 ` Anthony Liguori
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 12:34 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
>> +static const SpiceKbdInterface kbd_interface = {
>> + .base.type = SPICE_INTERFACE_KEYBOARD,
>> + .base.description = "qemu keyboard",
>> + .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
>> + .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
>> + .push_scan_freg = kbd_push_key,
>> + .get_leds = kbd_get_leds,
>> +};
>> +
>> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
>> +{
>> + kbd_put_keycode(frag);
>> +}
>
> Instead of using this interface which includes multi-byte sequences, it
> would probably make sense to use the same compressed encoding that VNC
> uses and introduce a new function that takes this encoding type. The
> advantage is that one call == one keycode so it's far less prone to
> goofiness.
Hmm? Not sure what you want here ...
kbd_push_key is called by libspice which feeds us with a ps/2 data
stream for the keyboard. Yes, one of those x86-isms in spice. Yes, the
ps/2 data stream also travels over the wire, i.e. the spice client has
to hop through loops to convert whatever it gets into a ps/2 data stream
for us.
Luckily ps/2 data is exactly what qemu expects to get via
kbd_put_keycode, so I can simply pass on what I get, and it is IMHO
pretty pointless to do something else ...
> It's probably more robust in the long term to explicitly convert the
> ledstate to from the QEMU format to the libspice format even if they
> both happen to be the same.
Ok.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-19 14:25 ` Anthony Liguori
@ 2010-08-20 12:42 ` Gerd Hoffmann
2010-08-20 13:19 ` Anthony Liguori
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 12:42 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
>> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int
>> dz,
>> + uint32_t buttons_state)
>> +{
>> + kbd_mouse_event(dx, dy, dz, buttons_state);
>
> dz is an odd interface. We use it to represent additional buttons which
> really makes no sense. If you still can, I'd suggest moving dz into
> buttons_state.
Again this is libspice interface (and I think also wire protocol), so I
can't change it. I can convert dz into some button_mask bits before
calling kbd_mouse_event, but looking at the vnc code it seems qemu
expects the mouse wheel events being passed via dz not button_state.
> Previous comment still applies though, you should explicitly convert the
> button_states from QEMU format to Spice format to future proof.
Ok.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-20 12:34 ` Gerd Hoffmann
@ 2010-08-20 13:18 ` Anthony Liguori
2010-08-20 13:56 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-20 13:18 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/20/2010 07:34 AM, Gerd Hoffmann wrote:
>
>>> +static const SpiceKbdInterface kbd_interface = {
>>> + .base.type = SPICE_INTERFACE_KEYBOARD,
>>> + .base.description = "qemu keyboard",
>>> + .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
>>> + .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
>>> + .push_scan_freg = kbd_push_key,
>>> + .get_leds = kbd_get_leds,
>>> +};
>>> +
>>> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
>>> +{
>>> + kbd_put_keycode(frag);
>>> +}
>>
>> Instead of using this interface which includes multi-byte sequences, it
>> would probably make sense to use the same compressed encoding that VNC
>> uses and introduce a new function that takes this encoding type. The
>> advantage is that one call == one keycode so it's far less prone to
>> goofiness.
>
> Hmm? Not sure what you want here ...
>
> kbd_push_key is called by libspice which feeds us with a ps/2 data
> stream for the keyboard. Yes, one of those x86-isms in spice. Yes,
> the ps/2 data stream also travels over the wire, i.e. the spice client
> has to hop through loops to convert whatever it gets into a ps/2 data
> stream for us.
>
> Luckily ps/2 data is exactly what qemu expects to get via
> kbd_put_keycode, so I can simply pass on what I get, and it is IMHO
> pretty pointless to do something else ...
It's not actually ps/2 data. It's AT scan codes plus an internal
encoding to indicate press vs. release using the high bit.
Additionally, some special keys are encoded with two calls to
kbd_put_keycode using the 0xe0 prefix (the grey code).
That gets converted to ps/2 data by decoding the high bit and generating
a special PS/2 keyboard sequence if necessary. The grey sequence just
happens to work because the logic in ps2_put_keycode is simple. IOW,
the ps/2 protocol looks like:
[0xe0,][0xf0,]raw_keycode
0xe0 indicates that the keycode is a "grey" code and 0xf0 indicates it's
a key release event. raw_keycode is a PS/2 raw keycode that has a 1-1
mapping from AT scan codes.
To generate the possible sequences, we would do:
// normal key press; PS/2: at2raw(at_keycode)
kbd_put_keycode(at_keycode);
// grey key press; PS/2 0xe0, at2raw(at_keycode)
kbd_put_keycode(0xe0);
kbd_put_keycode(at_keycode);
// normal key release; PS/2 0xf0, at2raw(at_keycode)
kbd_put_keycode(at_keycode | 0x80);
// grey key release; PS/2 0xe0, 0xf0, at2raw(at_keycode)
kbd_put_keycode(0xe0);
kbd_put_keycode(at_keycode | 0x80);
In the VNC encoding, instead of using the high bit to represent the key
down vs. key up event, we use separate messages for key down vs. key
up. We then use the high bit to encode whether the grey code is needed
which let's us send keycodes in a single message with a fixed keycode
size of 8 bits.
So what I'm proposing is that we modify kbd_put_keycode to also reflect
this:
// normal keys
kbd_keycode_press(at_keycode); // PS/2 at2raw(at_keycode)
kbd_keycode_release(at_keycode); // PS/2 0xf0, at2raw(at_keycode)
// grey keys; PS/2 0xe0, at2raw(at_keycode)
kbd_keycode_press(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
at2raw(at_keycode)
kbd_keycode_release(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
at2raw(at_keycode)
If it's not already too late, I'd suggest making this the Spice protocol
interface. The current kbd_put_keycode interface is just a QEMU
implementation detail and having different size byte sequences is a very
awkward interface.
Regards,
Anthony Liguori
>> It's probably more robust in the long term to explicitly convert the
>> ledstate to from the QEMU format to the libspice format even if they
>> both happen to be the same.
>
> Ok.
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-20 12:42 ` Gerd Hoffmann
@ 2010-08-20 13:19 ` Anthony Liguori
2010-08-20 14:03 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Anthony Liguori @ 2010-08-20 13:19 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/20/2010 07:42 AM, Gerd Hoffmann wrote:
> Hi,
>
>>> +static void mouse_motion(SpiceMouseInstance *sin, int dx, int dy, int
>>> dz,
>>> + uint32_t buttons_state)
>>> +{
>>> + kbd_mouse_event(dx, dy, dz, buttons_state);
>>
>> dz is an odd interface. We use it to represent additional buttons which
>> really makes no sense. If you still can, I'd suggest moving dz into
>> buttons_state.
>
> Again this is libspice interface (and I think also wire protocol), so
> I can't change it. I can convert dz into some button_mask bits before
> calling kbd_mouse_event, but looking at the vnc code it seems qemu
> expects the mouse wheel events being passed via dz not button_state.
That's unfortunate but understood. Is Spice considered a stable
API/wire protocol at this point?
Regards,
Anthony Liguori
>
>> Previous comment still applies though, you should explicitly convert the
>> button_states from QEMU format to Spice format to future proof.
>
> Ok.
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-20 13:18 ` Anthony Liguori
@ 2010-08-20 13:56 ` Gerd Hoffmann
2010-08-20 14:15 ` Daniel P. Berrange
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 13:56 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
> It's not actually ps/2 data. It's AT scan codes plus an internal
> encoding to indicate press vs. release using the high bit. Additionally,
> some special keys are encoded with two calls to kbd_put_keycode using
> the 0xe0 prefix (the grey code).
Wheee. From a brief look at the code it seems this *is* the spice wire
protocol. One more place where spice uses knowledge about qemu
internals. Unfortunaly this one escaped my attention until now, so it
didn't got fixed :-(
> So what I'm proposing is that we modify kbd_put_keycode to also reflect
> this:
>
> // normal keys
> kbd_keycode_press(at_keycode); // PS/2 at2raw(at_keycode)
> kbd_keycode_release(at_keycode); // PS/2 0xf0, at2raw(at_keycode)
>
> // grey keys; PS/2 0xe0, at2raw(at_keycode)
> kbd_keycode_press(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
> at2raw(at_keycode)
> kbd_keycode_release(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
> at2raw(at_keycode)
>
> If it's not already too late, I'd suggest making this the Spice protocol
> interface.
No. I think for now I have to deal with the mess in case qemu decides
to change the internal interface. And when ever touching the spice wire
protocol to fixup this mess I will *not* use AT keycodes. Handling
anything with extra internet / multimedia / whatever keys in a sane way
is simply impossible with AT keycodes. linux input layer key codes
should do. maybe usb hid is usable too, need to check.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-20 13:19 ` Anthony Liguori
@ 2010-08-20 14:03 ` Gerd Hoffmann
2010-08-20 14:37 ` Anthony Liguori
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 14:03 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
>> Again this is libspice interface (and I think also wire protocol), so
>> I can't change it. I can convert dz into some button_mask bits before
>> calling kbd_mouse_event, but looking at the vnc code it seems qemu
>> expects the mouse wheel events being passed via dz not button_state.
>
> That's unfortunate but understood. Is Spice considered a stable API/wire
> protocol at this point?
Yes. There are version numbers though, so we can add new messages and
use them in case both server and client are new enough. Unfortunaly
this makes the keyboard f*ckup only slightly less messy as we'll have to
support both old+new way for compatibility reasons. Oh well.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-20 13:56 ` Gerd Hoffmann
@ 2010-08-20 14:15 ` Daniel P. Berrange
2010-08-20 14:49 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrange @ 2010-08-20 14:15 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Fri, Aug 20, 2010 at 03:56:52PM +0200, Gerd Hoffmann wrote:
> Hi,
>
> >It's not actually ps/2 data. It's AT scan codes plus an internal
> >encoding to indicate press vs. release using the high bit. Additionally,
> >some special keys are encoded with two calls to kbd_put_keycode using
> >the 0xe0 prefix (the grey code).
>
> Wheee. From a brief look at the code it seems this *is* the spice wire
> protocol. One more place where spice uses knowledge about qemu
> internals. Unfortunaly this one escaped my attention until now, so it
> didn't got fixed :-(
>
> >So what I'm proposing is that we modify kbd_put_keycode to also reflect
> >this:
> >
> >// normal keys
> >kbd_keycode_press(at_keycode); // PS/2 at2raw(at_keycode)
> >kbd_keycode_release(at_keycode); // PS/2 0xf0, at2raw(at_keycode)
> >
> >// grey keys; PS/2 0xe0, at2raw(at_keycode)
> >kbd_keycode_press(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
> >at2raw(at_keycode)
> >kbd_keycode_release(0x80 | at_keycode); // PS/2 0xe0, 0xf0,
> >at2raw(at_keycode)
> >
> >If it's not already too late, I'd suggest making this the Spice protocol
> >interface.
>
> No. I think for now I have to deal with the mess in case qemu decides
> to change the internal interface. And when ever touching the spice wire
> protocol to fixup this mess I will *not* use AT keycodes. Handling
> anything with extra internet / multimedia / whatever keys in a sane way
> is simply impossible with AT keycodes. linux input layer key codes
> should do. maybe usb hid is usable too, need to check.
AT (well XT) keycodes aren't that bad a choice, at least if you go for the
extended mapping used by the Linux keyboard driver. This supports pretty
much all of the internet/multimedia keys, AFAICT, more than USB hid
does (at least in the Linux USB HID driver). In order to properly support
the VNC keycode extension with GTK-VNC under Xorg + Win32, OS-X and Linux,
as well as native OS-X & Win32, I've created a giant CSV mapping file for
all keycode sets that I've encountered so far.
The master mapping is from Linux keycodes to other sets:
http://git.gnome.org/browse/gtk-vnc/tree/src/keymaps.csv
And a tool that can then create you C arrays for mapping between
arbitrary keycode sets in any direction (potentially lossy
of course, depending on choice of keycode sets):
http://git.gnome.org/browse/gtk-vnc/tree/src/keymap-gen.pl
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 7/9] spice: add mouse
2010-08-20 14:03 ` Gerd Hoffmann
@ 2010-08-20 14:37 ` Anthony Liguori
0 siblings, 0 replies; 43+ messages in thread
From: Anthony Liguori @ 2010-08-20 14:37 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 08/20/2010 09:03 AM, Gerd Hoffmann wrote:
> Hi,
>
>>> Again this is libspice interface (and I think also wire protocol), so
>>> I can't change it. I can convert dz into some button_mask bits before
>>> calling kbd_mouse_event, but looking at the vnc code it seems qemu
>>> expects the mouse wheel events being passed via dz not button_state.
>>
>> That's unfortunate but understood. Is Spice considered a stable API/wire
>> protocol at this point?
>
> Yes. There are version numbers though, so we can add new messages and
> use them in case both server and client are new enough. Unfortunaly
> this makes the keyboard f*ckup only slightly less messy as we'll have
> to support both old+new way for compatibility reasons. Oh well.
One of the things to consider is that we only emulate a 2-axis 5 button
mouse. Buttons one through three are bits in button_state and buttons
4/5 are represented through dz. My mouse IRL is actually a 9 button mouse.
We encode buttons 4/5 as a -1, 0, 1 in dz which corresponds to 10, 00,
and 11. We cannot represent 11 with dz which corresponds to having
buttons 4 and 5 pressed simultaneously. Most mice have a scroll wheel
for 4/5 but there's no strict requirement that that's the case.
So in terms of building a long term stable wire protocol, you'd be much
better off using button_mask to encode buttons 4/5 because it's just a
matter of time before we drop the dz hack in qemu.
Regards,
Anthony Liguori
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-20 14:15 ` Daniel P. Berrange
@ 2010-08-20 14:49 ` Gerd Hoffmann
2010-08-20 15:01 ` Daniel P. Berrange
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-20 14:49 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel
Hi,
> AT (well XT) keycodes aren't that bad a choice, at least if you go for the
> extended mapping used by the Linux keyboard driver.
Hmm, as far I know those extended mappings are not standardized. Uses
linux this just as internal representation? Or can you actually feed a
linux guest with them and expect it to work? How about non-linux guests?
When using it as wire protocol guest compatibility doesn't matter that
much though ...
> The master mapping is from Linux keycodes to other sets:
>
> http://git.gnome.org/browse/gtk-vnc/tree/src/keymaps.csv
>
> And a tool that can then create you C arrays for mapping between
> arbitrary keycode sets in any direction (potentially lossy
> of course, depending on choice of keycode sets):
>
> http://git.gnome.org/browse/gtk-vnc/tree/src/keymap-gen.pl
Nice.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard
2010-08-20 14:49 ` Gerd Hoffmann
@ 2010-08-20 15:01 ` Daniel P. Berrange
0 siblings, 0 replies; 43+ messages in thread
From: Daniel P. Berrange @ 2010-08-20 15:01 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Fri, Aug 20, 2010 at 04:49:20PM +0200, Gerd Hoffmann wrote:
> Hi,
>
> >AT (well XT) keycodes aren't that bad a choice, at least if you go for the
> >extended mapping used by the Linux keyboard driver.
>
> Hmm, as far I know those extended mappings are not standardized. Uses
> linux this just as internal representation? Or can you actually feed a
> linux guest with them and expect it to work? How about non-linux guests?
Internally Linux uses Linux keycodes throughout. The keyboard driver
(drivers/char/keyboard.c) has a table that maps Linux keycodes to XT
keycodes when running in RAW mode that has entries for many of the
internet/multimedia keys.
IIUC, Microsoft sets out standard XT mappings for internet, multimedia keys,
etc as part of WHQL tests for keyboards. There is a doc hiding somewhere
on the web that details these, but its location escapes me currently.
I've not checked whether the extended mappings used by the Linux
keyboard driver in RAW mode, because it wasn't relevant for my immediate
needs - I just needed to know what the mapping was, not who standardized
it :-)
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 8/9] spice: simple display
2010-08-19 15:40 ` Alexander Graf
@ 2010-08-25 11:09 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-25 11:09 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
> In my man page it's declared as
>
> #define _GNU_SOURCE
> #include<stdio.h>
>
> int dprintf(int fd, const char *format, ...);
>
> but maybe it did become POSIX recently. Either way, it's used already :).
Same man-page says:
CONFORMING TO
These functions are GNU extensions that are nowadays
specified in POSIX.1-2008
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [Qemu-devel] [PATCH 5/9] spice: core bits
2010-08-19 14:19 ` Anthony Liguori
2010-08-20 11:54 ` Gerd Hoffmann
@ 2010-08-25 12:37 ` Gerd Hoffmann
1 sibling, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2010-08-25 12:37 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
Hi,
> I think there's two paths we can take here. We can either use
> module_init() with a new class for this location or we can be more
> explicit and pass the option list here.
Actually the device_init() hook does just fine here. Not sure this is a
good idea to hook there though as this is meant for guest device
backends and it could be moved at some point in the future.
I basically need a hook to do host-side setup after all config
processing is done and before guest devices are created.
cheers,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2010-08-25 12:37 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-19 12:40 [Qemu-devel] [PATCH 0/9] initial spice support Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 1/9] add pflib: PixelFormat conversion library Gerd Hoffmann
2010-08-19 14:04 ` Anthony Liguori
2010-08-19 15:34 ` Gerd Hoffmann
2010-08-19 19:09 ` Anthony Liguori
2010-08-20 10:38 ` Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 2/9] configure: add logging Gerd Hoffmann
2010-08-19 14:05 ` Anthony Liguori
2010-08-19 15:44 ` Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 3/9] add spice into the configure file Gerd Hoffmann
2010-08-19 14:06 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 4/9] configure: require spice 0.5.3 Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 5/9] spice: core bits Gerd Hoffmann
2010-08-19 14:19 ` Anthony Liguori
2010-08-20 11:54 ` Gerd Hoffmann
2010-08-25 12:37 ` Gerd Hoffmann
2010-08-19 12:40 ` [Qemu-devel] [PATCH 6/9] spice: add keyboard Gerd Hoffmann
2010-08-19 14:24 ` Anthony Liguori
2010-08-20 12:34 ` Gerd Hoffmann
2010-08-20 13:18 ` Anthony Liguori
2010-08-20 13:56 ` Gerd Hoffmann
2010-08-20 14:15 ` Daniel P. Berrange
2010-08-20 14:49 ` Gerd Hoffmann
2010-08-20 15:01 ` Daniel P. Berrange
2010-08-19 12:40 ` [Qemu-devel] [PATCH 7/9] spice: add mouse Gerd Hoffmann
2010-08-19 14:25 ` Anthony Liguori
2010-08-20 12:42 ` Gerd Hoffmann
2010-08-20 13:19 ` Anthony Liguori
2010-08-20 14:03 ` Gerd Hoffmann
2010-08-20 14:37 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 8/9] spice: simple display Gerd Hoffmann
2010-08-19 14:39 ` Anthony Liguori
2010-08-19 15:23 ` malc
2010-08-19 15:34 ` Anthony Liguori
2010-08-19 15:36 ` malc
2010-08-19 19:03 ` Anthony Liguori
2010-08-19 19:10 ` malc
2010-08-19 15:40 ` Alexander Graf
2010-08-25 11:09 ` Gerd Hoffmann
2010-08-19 16:05 ` Gerd Hoffmann
2010-08-19 19:00 ` Anthony Liguori
2010-08-19 12:40 ` [Qemu-devel] [PATCH 9/9] spice: add tablet support Gerd Hoffmann
2010-08-19 14:40 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).