Hi,

On 04/06/2012 09:50 AM, Charles.Tsai-蔡清海-研究發展部 wrote:
Hans,

This approach did not work and I gave it up already.

Charles, since that approach is already used in usb-redir
devices which are already in use by quite a few people I assure
you that it does work!

You are likely doing something wrong in your code somewhere, it
would help tremendously if you could simply post your qemu patches
here, or publish a git tree with your code.

We understand that the code in question will be a work in progress.
That is not a problem, and it simply is much much easier to discuss
these kind of things when we've code to look at.

Regards,

Hans




-----Original Message-----
From: Hans de Goede [mailto:[email protected]]
Sent: Friday, April 06, 2012 3:50 PM
To: Charles.Tsai-蔡清海-研究發展部; [email protected]
Subject: Re: [Spice-devel] [RFC] register vmc interface early for name != 
vdagent [was: Re: Read data out of the Virtqueue]

Hi,

On 04/05/2012 05:36 PM, Alon Levy wrote:
On Thu, Apr 05, 2012 at 06:22:47AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote:
Alon,

        Can you tell me more specific point what you meant " bottomhalf"?

        I am testing my modified code to send a message thru the main channel 
and it works for me.
        However, if there is a better approach to fix this problem, I will take 
it.

Something like this:


diff --git a/spice-qemu-char.c b/spice-qemu-char.c index
1e735ff..560127e 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -25,6 +25,7 @@ typedef struct SpiceCharDriver {
       uint8_t               *datapos;
       ssize_t               bufsize, datalen;
       uint32_t              debug;
+    QEMUBH                *bh_register;
   } SpiceCharDriver;

   static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t
*buf, int len) @@ -188,6 +189,14 @@ static void print_allowed_subtypes(void)
       fprintf(stderr, "\n");
   }

+static void bh_vmc_register_interface(void *opaque) {
+    SpiceCharDriver *s = opaque;
+
+    vmc_register_interface(s);
+    qemu_bh_delete(s->bh_register);
+}
+
   CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
   {
       CharDriverState *chr;
@@ -226,12 +235,15 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
       chr->chr_guest_open = spice_chr_guest_open;
       chr->chr_guest_close = spice_chr_guest_close;

-#if SPICE_SERVER_VERSION<   0x000901
-    /* See comment in vmc_state() */
       if (strcmp(subtype, "vdagent") == 0) {
+#if SPICE_SERVER_VERSION<   0x000901
+        /* See comment in vmc_state() */
           qemu_chr_generic_open(chr);
-    }
   #endif
+    } else {
+        s->bh_register = qemu_bh_new(bh_vmc_register_interface, s);
+        qemu_bh_schedule(s->bh_register);
+    }

       return chr;
   }



Please don't do this. We've an established mechanism for registering the 
chardev with spice-server, we do this when the chardev frontend signals us it 
is ready. It does this through the guest_open / close chardev callbacks. These 
are somewhat poorly named since their origin lies in virtio_console where they 
indeed reflect opening of the device by the guest.

But in essence they mean the chardev frontend is ready to receive data.

Notice that although the callbacks still have bad names, the way to call them 
from a frontend has much better names now a days:
qemu_chr_fe_open / qemu_chr_fe_close

Any special virtual printer frontend should call qemu_chr_fe_open from its qdev 
init function (like the usb redirection code does) and then the chardev will 
get registered with spice-server, and this will happen after the chardev has 
been registered.

The above code is very clearly wrong since it simply just always register the 
chardev backend with the spice-server, potentially before the frontend has 
called qemu_chr_fe_open(), BAD!

More as the above shows using a bh is not always a magic solution to problems. 
See for example also this commit message where using a bh was specifically 
avoided:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=f76e4c7f16c7ab966a792310b6630d3e240688b3

Regards,

Hans
_______________________________________________
Spice-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to