Message ID | 1282221625-29501-9-git-send-email-kraxel@redhat.com |
---|---|
State | New |
Headers | show |
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; >
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..]
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
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..]
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
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
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 >
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..] > >
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..] > > > >
> 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
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;
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