On Tue, Apr 03, 2012 at 10:16:46AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote:
> Alon,
>
> I am sorry. I don't see your patch.
> I will take a look at the following code and see how I can apply it for
> printing channel.
The diff below is the patch. It doesn't need any changes, if subtype !=
vdagent it will call vmc_register_interface at chardev creation time,
well before spice starts listening to connections.
> ====================
> > diff --git a/spice-qemu-char.c b/spice-qemu-char.c index
> > 1e735ff..d3ae0b7 100644
> > --- a/spice-qemu-char.c
> > +++ b/spice-qemu-char.c
> > @@ -226,12 +226,14 @@ 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 {
> > + vmc_register_interface(s);
> > + }
> >
> > return chr;
> > }
>
> -----Original Message-----
> From: Alon Levy [mailto:[email protected]]
> Sent: Tuesday, April 03, 2012 5:30 PM
> To: Hans de Goede; Charles.Tsai-蔡清海-研究發展部; [email protected]
> Subject: Re: [Spice-devel] [RFC] register vmc interface early for name !=
> vdagent [was: Re: Read data out of the Virtqueue]
>
> On Tue, Apr 03, 2012 at 12:17:32PM +0300, Alon Levy wrote:
> > On Tue, Apr 03, 2012 at 08:40:26AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote:
> > > Alon,
> > >
> > > Our Windows client code is based on 0.10.1 release and that is, I
> > > believe, the latest one.
> > > "spice-gtk" is running in LINUX and I believe it is not applicable to
> > > Windows platform.
> >
> > That is wrong. It works on windows as well, there is also a prebuilt
> > binary on http://www.spice-space.org/downloads.html of
> > remote-viewer.exe
> >
> > >
> > >
> > > Please refer to the following trace to understand what I said. As
> > > you can see, printing channel is registered to spice server after
> > > the following trace
> > > "spice_server_char_device_add_interface: Connect SPICE_CHANNEL_PRINT".
> > >
> > > Spice server report the number of channels to the client using the
> > > following main channel message "SPICE_MSG_MAIN_CHANNELS_LIST" that
> > > happens before the printing channel registers to the spice server. As
> > > such, the spice client cannot create the "print channel object" when it
> > > is launched.
> >
> > ok, my bad. The problem is not a race, the problem is that the channel
> > only gets registered when the guest opens the device, not during
> > initialization of the vm (before it starts running, and before spice
> > server listens to connections).
> >
> > The channel is registered via spice_server_add_interface:
> > spice_server_add_interface
> > -> spice_server_char_device_add_interface
> > -> spicevmc_device_connect
> > -> reds_register_channel
> >
> > spice_server_add_interface is called by qemu:
> > spice_chr_guest_open / spice_chr_write
> > -> vmc_register_interface
> > -> qemu_spice_add_interface
> > -> spice_server_add_interface
> >
> > So one way would be to get spice_chr_guest_open to be called early.
> > We could have the initialization of the channel done unconditionally
> > during chardev creation. The way it should affect users:
> > usbredir - already triggers the channel creation via doing a smartcard
> > - I think it does the same as usbredir.
> > vdagent - can't do it, server uses the interface registration as guest
> > agent exists. So will have to special case.
> >
> > vdagent - doesn't have a dedicated channel so not really a problem.
>
> Eek, somehow I mangled the above paragraph. It should read:
>
> usbredir - already triggers the channel creation via doing a
> qemu_chr_fe_open in usbredir_initfn (see hw/usb/redirect.c)
>
> vdagent - can't do it, server uses the interface registration as guest
> agent exists. So will have to special case.
>
> >
> > >
> > > I am thinking to remedy such a problem by sending a message to spice
> > > client thru the main channel. After spice client receives that message,
> > > it creates and initialize the "printing object". Although it is not good
> > > approach, it is the easy way to fix this issue for us. Let me know if you
> > > have a better idea. Thanks.
> >
> > So I guess something like the following would be a better solution:
> >
> > diff --git a/spice-qemu-char.c b/spice-qemu-char.c index
> > 1e735ff..d3ae0b7 100644
> > --- a/spice-qemu-char.c
> > +++ b/spice-qemu-char.c
> > @@ -226,12 +226,14 @@ 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 {
> > + vmc_register_interface(s);
> > + }
> >
> > return chr;
> > }
> >
> >
> > Hans, any comments?
> >
> > >
> > > ====================================================================
> > > ==============================================================
> > > ctsai@ctsai-ubuntu:~$ ./Target32.sh
> > > Starting QEMU with...
> > > -localtime -drive file=/var/lib/libvirt/images/Windows-7-32.img -vga
> > > qxl -spice port=5900,disable-ticketing -device
> > > virtio-serial-pci,ioeventfd=off -chardev
> > > spicevmc,id=vdagent,name=vdagent,debug=2 -device
> > > virtserialport,chardev=vdagent,name=com.redhat.spice.0 -chardev
> > > spicevmc,id=vdprint,name=vdprint,debug=2 -device
> > > virtserialport,chardev=vdprint,name=com.redhat.print.0 -smp 1,cores=2 -m
> > > 2048 -enable-kvm ....
> > > do_spice_init: starting 0.10.1
> > > spice_server_add_interface: spice_server_add_interface:
> > > type=migration
> > >
> > > spice_server_add_interface: SPICE_INTERFACE_MIGRATION
> > > spice_server_add_interface: spice_server_add_interface:
> > > type=keyboard
> > >
> > > spice_server_add_interface: SPICE_INTERFACE_KEYBOARD
> > > spice_server_add_interface: spice_server_add_interface: type=mouse
> > >
> > > spice_server_add_interface: SPICE_INTERFACE_MOUSE
> > > spice_server_add_interface: spice_server_add_interface: type=qxl
> > >
> > > spice_server_add_interface: SPICE_INTERFACE_QXL
> > > red_worker_main: begin
> > > display_channel_create: create display channel
> > > cursor_channel_create: create cursor channel
> > > scd: 1: vmc_register_interface
> > > spice_server_add_interface: spice_server_add_interface:
> > > type=char_device
> > >
> > > spice_server_char_device_add_interface: CHAR_DEVICE vdagent
> > > scd: 2: vmc_register_interface
> > > spice_server_add_interface: spice_server_add_interface:
> > > type=char_device
> > >
> > > spice_server_char_device_add_interface: CHAR_DEVICE vdprint
> > > spice_server_char_device_add_interface: Connect SPICE_CHANNEL_PRINT
> > > spice_chr_write
> > > scd: 1: spice_chr_write: 36
> > > scd: 1: vmc_read: vdagent 0x7f7ff3f98c80 8/8/36
> > > scd: 2: vmc_read: vdagent 0x7f7ff3f98c88 28/28/28
> > > scd: 3: vmc_read: vdagent (nil) 8/0/0
> > > scd: 4: vmc_read: vdagent (nil) 8/0/0
> > > spice_chr_write
> > > scd: 2: spice_chr_write: 36
> > > scd: 5: vmc_read: vdagent 0x7f7ff3f98c80 8/8/36
> > > scd: 6: vmc_read: vdagent 0x7f7ff3f98c88 28/28/28
> > > scd: 7: vmc_read: vdagent (nil) 8/0/0
> > > scd: 8: vmc_read: vdagent (nil) 8/0/0
> > >
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Alon Levy [mailto:[email protected]]
> > > Sent: Tuesday, April 03, 2012 3:54 PM
> > > To: Charles.Tsai-蔡清海-研究發展部
> > > Cc: [email protected]
> > > Subject: Re: Read data out of the Virtqueue
> > >
> > > On Mon, Apr 02, 2012 at 07:44:46AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote:
> > > > Alon,
> > > >
> > > > In spice client, the creation of the channel is based on
> > > > "init->num_of_channels" that is returned from the spice server.
> > > > Please see the following function to create the spice channel
> > > > from the client
> > > >
> > > > void RedClient::handle_channels(RedPeer::InMessage* message) {
> > > > SpiceMsgChannels *init = (SpiceMsgChannels *)message->data();
> > > > SpiceChannelId* channels = init->channels;
> > > > for (unsigned int i = 0; i < init->num_of_channels; i++) {
> > > > create_channel(channels[i].type, channels[i].id);
> > > > }
> > > > }
> > > >
> > > > For the printing channel, it is created and registered by function
> > > > spicevmc_device_connect(char_device, SPICE_CHANNEL_PRINT); (see in
> > > > reds.c file) Because the printing channel is registered and created in
> > > > spice server lately after spice client is up, the creation of the
> > > > printing channel is thus skipped by the spice client.
> > >
> > > Two things: First, you are using the old client code base as an example.
> > > All well and good, but if possible it's better to use spice-gtk as a base
> > > for new features, because that's the client we are going with forwward
> > > (or actually the client enabling library, with remote-viewer being the
> > > preffered client).
> > > Second, what do you mean registered after the spice client is up? If
> > > there is some race window where the server is already listening to
> > > clients but the printing channel is not registered, that is bad and
> > > needs fixing, but in general the server is initialized *before* the
> > > client is connected. The right order is 1. server channels are
> > > registered 2. server starts listening 3. client connects
> > >
> > > #3 cannot come before #2 so in general there is no way to have the
> > > situation you describe (unless there is a race and #1 happens after #2
> > > and then both orders with respect to #3 are possible).
> > >
> > > > Can you tell me how to resolve this problem based on the spice
> > > > architecture? Thanks.
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Charles.Tsai-蔡清海-研究發展部
> > > > Sent: Friday, March 30, 2012 9:27 AM
> > > > To: 'Alon Levy'
> > > > Cc: [email protected]
> > > > Subject: RE: Read data out of the Virtqueue
> > > >
> > > > Alon,
> > > >
> > > > It is a bug that I modified for printing channel. BTW, can a spice
> > > > channel be brought up or down based on a request from a spice client?
> > > > The reason that I am asking for such a question is because the printing
> > > > channel might be brought up based on a configuration policy. I would
> > > > like to know if a spice channel can be brought up dynamically during
> > > > the run time.
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Alon Levy [mailto:[email protected]]
> > > > Sent: Friday, March 30, 2012 4:16 AM
> > > > To: Charles.Tsai-蔡清海-研究發展部
> > > > Cc: [email protected]
> > > > Subject: Re: Read data out of the Virtqueue
> > > >
> > > > On Thu, Mar 29, 2012 at 09:42:32AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote:
> > > > > Alon,
> > > > >
> > > > > Forget my previous mail. When I dig into the vdservice, I found there
> > > > > was one bug to check the overlay I/O status after calling the VirtIO
> > > > > write function.
> > > > > After I fixed the bug, I can see more printing raw data inside the
> > > > > Qemu now. Although I still find some issues, hopefully I can fix it
> > > > > as I debug more codes in both spice client and spice server. Thank
> > > > > you for your time.
> > > >
> > > > Great that you managed to progress. Can you send a patch for the
> > > > vdservice issue? Is it a bug in the original or just in the one you
> > > > modified for printer channel?
> > > >
> > > > >
> > > > > -----Original Message-----
> > > > > From: Charles.Tsai-蔡清海-研究發展部
> > > > > Sent: Wednesday, March 28, 2012 6:07 PM
> > > > > To: 'Alon Levy'
> > > > > Cc: [email protected]
> > > > > Subject: Read data out of the Virtqueue
> > > > >
> > > > > Alon,
> > > > >
> > > > > My printer driver can write the printing raw data into virtIO driver.
> > > > > But I cannot see the found the printing raw data in the spice server.
> > > > > For the vdagent, I found the following code segment which is a
> > > > > callback to read the data from the vdi_port. Do I need to add a
> > > > > similar code to read the data from the Virtio?
> > > > >
> > > > > In Qemu, I do see vmc_write is called when the printer driver writes
> > > > > the printing raw data into the virtIOdevice.
> > > > > Consequently, function "spicevmc_red_channel_send_item" should be
> > > > > called to send the payload to the spice client. But I did not see
> > > > > function "spicevmc_red_channel_send_item" is called either. It looks
> > > > > like the printing rawa data is still inside the VirtIO queue. What
> > > > > function I need to add so as to pull the data out of the VirtIO queue?
> > > > >
> > > > > ================================================================
> > > > > ==== == ================= struct SpiceCharDeviceState
> > > > > vdagent_char_device_state = {
> > > > > .wakeup = &vdagent_char_device_wakeup, };
> > > > >
> > > > >
> > > > > void vdagent_char_device_wakeup(SpiceCharDeviceInstance *sin) {
> > > > > while (read_from_vdi_port()); }
> > _______________________________________________
> > Spice-devel mailing list
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/spice-devel
