On Tue, Apr 03, 2012 at 09:37:27AM +0000, Charles.Tsai-蔡清海-研究發展部 wrote: > Alon, > > Do you recommend us to use the new spice client code from > "http://www.spice-space.org/downloads.html"; link? > I will try to use it latter on to see what it is difference. > > You are right. The printing channel is created when the guest OS opens > the virtIO printing channel. That is why it happens so lately.
OK, we agree. > From your last reply, it looks the printing channel must be handled > specially so that the spice client can create the object. No, I don't understand it this way. What is wrong with the patch I sent? could you reply inline about it, it would be easier to see what you are talking about that way, thanks. > > I am writing the code to notify the spice client right after the > printing channel is UP. This is the less code to change to make it work. I don't understand why the patch I supplied is not shorter and simpler. > > > > -----Original Message----- > From: Alon Levy [mailto:[email protected]] > Sent: Tuesday, April 03, 2012 5:18 PM > To: Hans de Goede; Charles.Tsai-蔡清海-研究發展部 > Cc: [email protected] > Subject: [RFC] register vmc interface early for name != vdagent [was: Re: > Read data out of the Virtqueue] > > 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. > > qemu_chr_fe_open in usbredir_initfn (see hw/usb/redirect.c) vdagent - doesn't > have a dedicated channel so not really a problem. > > > > > 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
