From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdgYr-0004xo-AB for qemu-devel@nongnu.org; Wed, 28 Nov 2012 07:16:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TdgYp-0006TD-6A for qemu-devel@nongnu.org; Wed, 28 Nov 2012 07:16:16 -0500 Received: from mx3-phx2.redhat.com ([209.132.183.24]:58942) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TdgYo-0006T7-UH for qemu-devel@nongnu.org; Wed, 28 Nov 2012 07:16:15 -0500 Date: Wed, 28 Nov 2012 07:16:12 -0500 (EST) From: Alon Levy Message-ID: <415540988.39000387.1354104972985.JavaMail.root@redhat.com> In-Reply-To: <87zk22sztf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3] spice-qemu-char: register interface on post load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: amit shah , qemu-devel@nongnu.org, anthony@codemonkey.ws > Alon Levy writes: > > >> Alon Levy writes: > >> > >> > The target has not seen the guest_connected event via > >> > spice_chr_guest_open or spice_chr_write, and so spice server > >> > wrongly > >> > assumes there is no agent active, while the client continues to > >> > send > >> > motion events only by the agent channel, which the server > >> > ignores. > >> > The > >> > net effect is that the mouse is static in the guest. > >> > > >> > By registering the interface on post load spice server will pass > >> > on > >> > the > >> > agent messages fixing the mouse behavior after migration. > >> > > >> > RHBZ #725965 > >> > > >> > Signed-off-by: Alon Levy > >> > --- > >> > spice-qemu-char.c | 34 ++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 34 insertions(+) > >> > > >> > diff --git a/spice-qemu-char.c b/spice-qemu-char.c > >> > index 09aa22d..08b6ba0 100644 > >> > --- a/spice-qemu-char.c > >> > +++ b/spice-qemu-char.c > >> > @@ -1,6 +1,7 @@ > >> > #include "config-host.h" > >> > #include "trace.h" > >> > #include "ui/qemu-spice.h" > >> > +#include "hw/virtio-serial.h" > >> > #include > >> > #include > >> > > >> > @@ -25,6 +26,7 @@ typedef struct SpiceCharDriver { > >> > uint8_t *datapos; > >> > ssize_t bufsize, datalen; > >> > uint32_t debug; > >> > + QEMUTimer *post_load_timer; > >> > } SpiceCharDriver; > >> > > >> > static int vmc_write(SpiceCharDeviceInstance *sin, const > >> > uint8_t > >> > *buf, int len) > >> > @@ -156,6 +158,7 @@ static void spice_chr_close(struct > >> > CharDriverState *chr) > >> > > >> > printf("%s\n", __func__); > >> > vmc_unregister_interface(s); > >> > + qemu_free_timer(s->post_load_timer); > >> > g_free(s); > >> > } > >> > > >> > @@ -188,6 +191,33 @@ static void print_allowed_subtypes(void) > >> > fprintf(stderr, "\n"); > >> > } > >> > > >> > +static void spice_chr_post_load_cb(void *opaque) > >> > +{ > >> > + SpiceCharDriver *s = opaque; > >> > + > >> > + vmc_register_interface(s); > >> > +} > >> > + > >> > +static int spice_chr_post_load(void *opaque, int version_id) > >> > +{ > >> > + SpiceCharDriver *s = opaque; > >> > + > >> > + if (s && s->chr && qemu_chr_be_connected(s->chr)) { > >> > + qemu_mod_timer(s->post_load_timer, 1); > >> > + } > >> > >> You use the time to delay spice_chr_post_load_cb(), right? Can > >> you > >> explain why you have to delay? > > > > This is a precaution, it ensures vmc_register_interface is called > > when > > the vm is running as opposed to stopped which is the state when > > spice_chr_post_load is called. In theory vmc_register_interface > > could > > lead to an attempt to inject an interrupt into the guest, and we > > know > > that fails with kvm irqchip from a previous bug with virtio-serial. > > So your fixed delay of 1ns (I think) on the vm_clock is a roundabout > way > to delay the callback from "post load" until after the run state > transition to RUN_STATE_RUNNING. Correct? Yes. > > If yes, then a VM change state handler might be cleaner. Not saying that two wrongs make a right, but this is also the way we handled the irq injection in post_load for virtio serial console: http://lists.nongnu.org/archive/html/qemu-devel/2012-11/msg01196.html > > [...] >