[fedora-virt] spice and qemu 0.14
Alon Levy
alevy at redhat.com
Sun Mar 13 21:05:32 UTC 2011
On Sun, Mar 13, 2011 at 10:50:45PM +0200, Alon Levy wrote:
> On Sun, Mar 13, 2011 at 08:36:43PM +0100, Gianluca Cecchi wrote:
> > I followed the post and compiled all with
> > rpmbuild -bb ./qemu.spec
> > then
> > - rpm -e qemu-kvm qemu-system-x86 qemu-img qemu-common libvirt
> > - rpm -Uvh qemu-kvm-0.14.0-2.fc14.x86_64.rpm
> > qemu-kvm-tools-0.14.0-2.fc14.x86_64.rpm
> > qemu-system-x86-0.14.0-2.fc14.x86_64.rpm qemu-img-0.14.0-2.fc14.x86_64.rpm
> > qemu-common-0.14.0-2.fc14.x86_64.rpm
> > - yum install libvirt (from virt-preview repo)
> >
> > I'm now able to connect to my w2k3 guest with spicec and can get audio
> > working too, so there is a step forward.
> >
> > Only problem is that in w2k3 I'm forced to have the video card driver set as
> > vga, because if I install the qxl driver I then reboot
> > and the w2k3 guest remains in black and then powers off....
> > In /var/log/libvirt/qemu/w2k3.log I can see:
> > qemu-kvm: /home/gcecchi/rpmbuild/BUILD/qemu-kvm-0.14.0/qemu-kvm.c:1724:
> > kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
> > 2011-03-13 20:22:58.116: shutting down
> > ...
> >
> > I don't know if the qxl driver is supposed to work in w2k3 (I'm using the
> > qxl-win32-0.6.1.zip file for drivers) or if this is an expected behaviour
> >
> > Gianluca
> >
> > On Sun, Mar 13, 2011 at 2:06 PM, Boris Derzhavets <bderzhavets at yahoo.com>wrote:
> >
>
> I'd appreciate if you tried the following branch and tell me if it works
> on your windows 2003 vm. It works here on a fedora 64 bit and winxp sp3:
>
> git://anongit.freedesktop.org/~alon/qemu locking.fixes.candidate.1
>
> Alon
Sorry, that's the qemu upstream branch. For qemu-kvm I'm attaching the patches, but
I haven't tested them yet (will tommorrow).
Alon
>
> > > Gerd's Hoffman patch ( http://patchwork.ozlabs.org/patch/84704/) seems to
> > > path through limited testing :-
> > >
> > >
> > > http://bderzhavets.wordpress.com/2011/03/13/qemu-kvm-0-14-patching-via-gerds-hoffman-spiceqxl-locking-fix-for-qemu-kvm-on-f14/
> > >
> > > I was able to connect via spice to F14,SL6 KVM running with updated
> > > emulator by
> > > virt-manager.
> > >
> > > Boris.
> > > Env : F14+Libvirt Preview
> > >
> > > --- On *Sat, 3/12/11, Gianluca Cecchi <gianluca.cecchi at gmail.com>* wrote:
> > >
> > >
> > > From: Gianluca Cecchi <gianluca.cecchi at gmail.com>
> > > Subject: Re: [fedora-virt] spice and qemu 0.14
> > > To: "Richard W.M. Jones" <rjones at redhat.com>
> > > Cc: "virt at lists.fedoraproject.org" <virt at lists.fedoraproject.org>
> > > Date: Saturday, March 12, 2011, 12:44 PM
> > >
> > > On Saturday, March 12, 2011, Richard W.M. Jones <rjones at redhat.com<http://mc/compose?to=rjones@redhat.com>>
> > > wrote:
> > > > On Sat, Mar 12, 2011 at 10:55:30AM +0100, Gianluca Cecchi wrote:
> > > >> Is there any ETA for spice working again with qemu 0.14?
> > > >
> > > > Sorry, but there are no hard guarantees for anything in Fedora. The
> > > > project is run by volunteers. Even people working for Red Hat work on
> > > > RHEL first and foremost.
> > > >
> > > > Maybe contribute the required patch / updates / work yourself?
> > > >
> > > > Rich.
> > > >
> > > > --
> > > > Richard Jones, Virtualization Group, Red Hat
> > > http://people.redhat.com/~rjones
> > > > Read my programming blog: http://rwmj.wordpress.com
> > > > Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
> > > > http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
> > > >
> > > Actually I didn't mean eta in its formal way... Sorry for my miswrite ...
> > > Not the best acronym for my purposes...
> > > To explain better: I meant that I'm using virt-preview repo in f14 and
> > > would like to further test spice and give feedback again to have then
> > > a better f15...
> > > If I remember correctly there was a post saying that an incoming patch
> > > was arriving ..
> > > Gianluca
> > > _______________________________________________
> > > virt mailing list
> > > virt at lists.fedoraproject.org<http://mc/compose?to=virt@lists.fedoraproject.org>
> > > https://admin.fedoraproject.org/mailman/listinfo/virt
> > >
> > >
> > >
>
> > _______________________________________________
> > virt mailing list
> > virt at lists.fedoraproject.org
> > https://admin.fedoraproject.org/mailman/listinfo/virt
>
> _______________________________________________
> virt mailing list
> virt at lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/virt
-------------- next part --------------
>From 6857ce8b1fc783a9309e6b6d0dee2cc6bd0d6b2f Mon Sep 17 00:00:00 2001
From: Uri Lublin <uril at redhat.com>
Date: Sun, 13 Mar 2011 19:45:45 +0200
Subject: [PATCH 1/3] qxl: fix locking, do not lock from spice server thread
instead request iothread to do the operation, and wait for its response.
---
hw/qxl.c | 70 +++++++++++++++++++++++++++++++++++++++++++++------
hw/qxl.h | 3 +-
ui/spice-display.c | 7 +++--
ui/spice-display.h | 5 +++
4 files changed, 72 insertions(+), 13 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 58d6222..927a625 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -28,6 +28,8 @@
#include "qxl.h"
+#define ASSERT assert
+
#undef SPICE_RING_PROD_ITEM
#define SPICE_RING_PROD_ITEM(r, ret) { \
typeof(r) start = r; \
@@ -117,6 +119,15 @@ static QXLMode qxl_modes[] = {
#endif
};
+
+/*
+ * commands/requests from server thread to iothread.
+ */
+enum {
+ QXL_SERVER_SET_IRQ = 1,
+ QXL_SERVER_CREATE_UPDATE,
+};
+
static PCIQXLDevice *qxl0;
static void qxl_send_events(PCIQXLDevice *d, uint32_t events);
@@ -365,11 +376,24 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
QXLCommandRing *ring;
QXLCommand *cmd;
int notify;
+ int r;
+ unsigned char req, ack;
switch (qxl->mode) {
case QXL_MODE_VGA:
dprint(qxl, 2, "%s: vga\n", __FUNCTION__);
- update = qemu_spice_create_update(&qxl->ssd);
+
+ /* TODO -- 1. check write+read return codes. retry upon e.g. EINTR */
+ /* TODO -- 2. change the imlpementation inside qemu_spice_create_update (option rename+add functions) */
+ req = QXL_SERVER_CREATE_UPDATE;
+ r = write(qxl->pipe[1], &req , 1);
+ ASSERT(r == 1);
+ ack = 0;
+ r = read(qxl->pipe_out[0], &ack, 1);
+ ASSERT(r == 1);
+ ASSERT(ack == QXL_SERVER_CREATE_UPDATE); // ack==req
+ update = qxl->ssd.update;
+ //update = qemu_spice_create_update(&qxl->ssd);
if (update == NULL) {
return false;
}
@@ -1079,16 +1103,27 @@ static void qxl_map(PCIDevice *pci, int region_num,
static void pipe_read(void *opaque)
{
PCIQXLDevice *d = opaque;
- char dummy;
+ unsigned char cmd;
int len;
do {
- len = read(d->pipe[0], &dummy, sizeof(dummy));
- } while (len == sizeof(dummy));
- qxl_set_irq(d);
+ len = read(d->pipe[0], &cmd, sizeof(cmd));
+ } while (len == sizeof(cmd));
+ switch (cmd) {
+ case QXL_SERVER_SET_IRQ:
+ qxl_set_irq(d);
+ break;
+ case QXL_SERVER_CREATE_UPDATE:
+ d->ssd.update = qemu_spice_create_update(&d->ssd);
+ len = write(d->pipe_out[1], &cmd, 1);
+ ASSERT(len == 1);
+ break;
+ default:
+ fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd);
+ abort();
+ }
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1100,10 +1135,17 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
return;
}
if (pthread_self() == d->main) {
+ /* running in io_thread thread */
qxl_set_irq(d);
} else {
- if (write(d->pipe[1], d, 1) != 1) {
- dprint(d, 1, "%s: write to pipe failed\n", __FUNCTION__);
+ /* called from spice-server thread */
+ int ret;
+ unsigned char ack = QXL_SERVER_SET_IRQ;
+ ret = write(d->pipe[1], &ack, 1);
+ if (ret != 1) {
+ fprintf(stderr, "%s: write to iothread pipe failed (%d) %m\n",
+ __FUNCTION__, ret);
+ perror("");
}
}
}
@@ -1111,19 +1153,29 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
static void init_pipe_signaling(PCIQXLDevice *d)
{
if (pipe(d->pipe) < 0) {
- dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__);
+ fprintf(stderr, "%s: pipe creation failed\n", __FUNCTION__);
return;
}
+
+#if 0 // why non-blocking ? code does not check that case
#ifdef CONFIG_IOTHREAD
fcntl(d->pipe[0], F_SETFL, O_NONBLOCK);
#else
fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */);
#endif
fcntl(d->pipe[1], F_SETFL, O_NONBLOCK);
+#endif //(if 0)
+
fcntl(d->pipe[0], F_SETOWN, getpid());
d->main = pthread_self();
qemu_set_fd_handler(d->pipe[0], pipe_read, NULL, d);
+
+
+ if (pipe(d->pipe_out) < 0) {
+ fprintf(stderr, "%s: pipe creation failed\n", __FUNCTION__);
+ return;
+ }
}
/* graphics console */
diff --git a/hw/qxl.h b/hw/qxl.h
index f6c450d..b153e4b 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -57,7 +57,8 @@ typedef struct PCIQXLDevice {
/* thread signaling */
pthread_t main;
- int pipe[2];
+ int pipe[2]; /* to iothread */
+ int pipe_out[2]; /* from iothread */
/* ram pci bar */
QXLRam *ram;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 3f978e5..22a34e6 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -79,9 +79,9 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
uint8_t *src, *dst;
int by, bw, bh;
- qemu_mutex_lock_iothread();
+ /* qemu_mutex_lock_iothread(); */
if (qemu_spice_rect_is_empty(&ssd->dirty)) {
- qemu_mutex_unlock_iothread();
+ /* qemu_mutex_unlock_iothread(); */
return NULL;
};
@@ -142,7 +142,7 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
cmd->data = (intptr_t)drawable;
memset(&ssd->dirty, 0, sizeof(ssd->dirty));
- qemu_mutex_unlock_iothread();
+ /* qemu_mutex_unlock_iothread(); */
return update;
}
@@ -301,6 +301,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
SimpleSpiceUpdate *update;
dprint(3, "%s:\n", __FUNCTION__);
+ assert((const char *)"TODO -- fixme" == NULL);
update = qemu_spice_create_update(ssd);
if (update == NULL) {
return false;
diff --git a/ui/spice-display.h b/ui/spice-display.h
index df74828..37dbbc6 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -31,6 +31,8 @@
#define NUM_SURFACES 1024
+struct SimpleSpiceUpdate;
+
typedef struct SimpleSpiceDisplay {
DisplayState *ds;
void *buf;
@@ -44,6 +46,9 @@ typedef struct SimpleSpiceDisplay {
int notify;
int running;
+ /* ssd updates (one request/command at a time) */
+ struct SimpleSpiceUpdate *update;
+
/* qemu-kvm locking ... */
void *env;
} SimpleSpiceDisplay;
--
1.7.4.1
-------------- next part --------------
>From 659e9f7657e9c6e2944f4eecb1381c940a6cc51e Mon Sep 17 00:00:00 2001
From: Uri Lublin <uril at redhat.com>
Date: Sun, 13 Mar 2011 19:45:46 +0200
Subject: [PATCH 2/3] qxl: read_pipe: keep reading as long as there are things in the queue
---
hw/qxl.c | 39 ++++++++++++++++++++++++---------------
1 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 927a625..1ea66a2 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1104,24 +1104,33 @@ static void pipe_read(void *opaque)
{
PCIQXLDevice *d = opaque;
unsigned char cmd;
- int len;
+ int len, again = 1, set_irq_done = 0;
do {
+ cmd = 0;
len = read(d->pipe[0], &cmd, sizeof(cmd));
- } while (len == sizeof(cmd));
- switch (cmd) {
- case QXL_SERVER_SET_IRQ:
- qxl_set_irq(d);
- break;
- case QXL_SERVER_CREATE_UPDATE:
- d->ssd.update = qemu_spice_create_update(&d->ssd);
- len = write(d->pipe_out[1], &cmd, 1);
- ASSERT(len == 1);
- break;
- default:
- fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd);
- abort();
- }
+ if ((len < 0) && (errno == EINTR)) {
+ continue;
+ }
+ switch (cmd) {
+ case QXL_SERVER_SET_IRQ:
+ if (set_irq_done == 0) {
+ /* no need to do it more than once */
+ set_irq_done = 1;
+ qxl_set_irq(d);
+ }
+ break;
+ case QXL_SERVER_CREATE_UPDATE:
+ again = 0;
+ d->ssd.update = qemu_spice_create_update(&d->ssd);
+ len = write(d->pipe_out[1], &cmd, 1);
+ ASSERT(len == 1);
+ break;
+ default:
+ fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd);
+ abort();
+ }
+ } while (again);
}
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
--
1.7.4.1
-------------- next part --------------
>From e9be77687f9b7c9006ee0593d521c56320a63c26 Mon Sep 17 00:00:00 2001
From: Alon Levy <alevy at redhat.com>
Date: Sun, 13 Mar 2011 21:58:24 +0200
Subject: [PATCH 3/3] qxl vga update: don't read, preventing pipe loop of death
---
hw/qxl.c | 44 +++++++++++++++++++++++++-------------------
ui/spice-display.h | 1 +
2 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 1ea66a2..e66e927 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -377,7 +377,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
QXLCommand *cmd;
int notify;
int r;
- unsigned char req, ack;
+ unsigned char req;
switch (qxl->mode) {
case QXL_MODE_VGA:
@@ -385,21 +385,21 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
/* TODO -- 1. check write+read return codes. retry upon e.g. EINTR */
/* TODO -- 2. change the imlpementation inside qemu_spice_create_update (option rename+add functions) */
- req = QXL_SERVER_CREATE_UPDATE;
- r = write(qxl->pipe[1], &req , 1);
- ASSERT(r == 1);
- ack = 0;
- r = read(qxl->pipe_out[0], &ack, 1);
- ASSERT(r == 1);
- ASSERT(ack == QXL_SERVER_CREATE_UPDATE); // ack==req
update = qxl->ssd.update;
- //update = qemu_spice_create_update(&qxl->ssd);
- if (update == NULL) {
+ if (update != NULL) {
+ qxl->ssd.update = NULL;
+ *ext = update->ext;
+ qxl_log_command(qxl, "vga", ext);
+ return true;
+ } else {
+ if (!qxl->ssd.waiting_for_update) {
+ qxl->ssd.waiting_for_update = 1;
+ req = QXL_SERVER_CREATE_UPDATE;
+ r = write(qxl->pipe[1], &req , 1);
+ ASSERT(r == 1);
+ }
return false;
}
- *ext = update->ext;
- qxl_log_command(qxl, "vga", ext);
- return true;
case QXL_MODE_COMPAT:
case QXL_MODE_NATIVE:
case QXL_MODE_UNDEFINED:
@@ -632,6 +632,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
qemu_spice_create_host_primary(&d->ssd);
d->mode = QXL_MODE_VGA;
memset(&d->ssd.dirty, 0, sizeof(d->ssd.dirty));
+ d->ssd.waiting_for_update = 0;
}
static void qxl_exit_vga_mode(PCIQXLDevice *d)
@@ -1105,6 +1106,7 @@ static void pipe_read(void *opaque)
PCIQXLDevice *d = opaque;
unsigned char cmd;
int len, again = 1, set_irq_done = 0;
+ int create_update = 0;
do {
cmd = 0;
@@ -1112,6 +1114,9 @@ static void pipe_read(void *opaque)
if ((len < 0) && (errno == EINTR)) {
continue;
}
+ if (len <= 0) {
+ break;
+ }
switch (cmd) {
case QXL_SERVER_SET_IRQ:
if (set_irq_done == 0) {
@@ -1121,16 +1126,19 @@ static void pipe_read(void *opaque)
}
break;
case QXL_SERVER_CREATE_UPDATE:
- again = 0;
- d->ssd.update = qemu_spice_create_update(&d->ssd);
- len = write(d->pipe_out[1], &cmd, 1);
- ASSERT(len == 1);
+ create_update = 1;
break;
default:
fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd);
abort();
}
} while (again);
+ if (create_update) {
+ if (d->ssd.update == NULL) {
+ d->ssd.update = qemu_spice_create_update(&d->ssd);
+ }
+ d->ssd.waiting_for_update = 0;
+ }
}
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
@@ -1166,14 +1174,12 @@ static void init_pipe_signaling(PCIQXLDevice *d)
return;
}
-#if 0 // why non-blocking ? code does not check that case
#ifdef CONFIG_IOTHREAD
fcntl(d->pipe[0], F_SETFL, O_NONBLOCK);
#else
fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */);
#endif
fcntl(d->pipe[1], F_SETFL, O_NONBLOCK);
-#endif //(if 0)
fcntl(d->pipe[0], F_SETOWN, getpid());
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 37dbbc6..19ca638 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -48,6 +48,7 @@ typedef struct SimpleSpiceDisplay {
/* ssd updates (one request/command at a time) */
struct SimpleSpiceUpdate *update;
+ int waiting_for_update;
/* qemu-kvm locking ... */
void *env;
--
1.7.4.1
More information about the virt
mailing list