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 > > <[email protected]>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 <[email protected]>* wrote: > > > > > > > > > From: Gianluca Cecchi <[email protected]> > > > Subject: Re: [fedora-virt] spice and qemu 0.14 > > > To: "Richard W.M. Jones" <[email protected]> > > > Cc: "[email protected]" <[email protected]> > > > Date: Saturday, March 12, 2011, 12:44 PM > > > > > > On Saturday, March 12, 2011, Richard W.M. Jones > > > <[email protected]<http://mc/[email protected]>> > > > 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 > > > [email protected]<http://mc/[email protected]> > > > https://admin.fedoraproject.org/mailman/listinfo/virt > > > > > > > > > > > > _______________________________________________ > > virt mailing list > > [email protected] > > https://admin.fedoraproject.org/mailman/listinfo/virt > > _______________________________________________ > virt mailing list > [email protected] > https://admin.fedoraproject.org/mailman/listinfo/virt
>From 6857ce8b1fc783a9309e6b6d0dee2cc6bd0d6b2f Mon Sep 17 00:00:00 2001 From: Uri Lublin <[email protected]> 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
>From 659e9f7657e9c6e2944f4eecb1381c940a6cc51e Mon Sep 17 00:00:00 2001 From: Uri Lublin <[email protected]> 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
>From e9be77687f9b7c9006ee0593d521c56320a63c26 Mon Sep 17 00:00:00 2001 From: Alon Levy <[email protected]> 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
_______________________________________________ virt mailing list [email protected] https://admin.fedoraproject.org/mailman/listinfo/virt
