Re: [Spice-devel] Windows Guest Tools 0.65
Tested and works well! Big thanks! Fred > -Original Message- > From: spice-devel-bounces+fred_liu=issi@lists.freedesktop.org > [mailto:spice-devel-bounces+fred_liu=issi@lists.freedesktop.org] On > Behalf Of Christophe Fergeau > Sent: 星期二, 十月 08, 2013 0:10 > To: spice-devel@lists.freedesktop.org > Subject: [Spice-devel] Windows Guest Tools 0.65 > > Hi, > > A new release of the SPICE Guest Tools for Windows is now available at > http://spice-space.org/download/windows/spice-guest-tools/spice-guest- > tools-0.65.1.exe > > There is only one change over 0.65, but it's a very important one: the > QXL > driver shipped in this installer now has a WHQL signature, which should > solve the various signature issues that people had :) > > The release is signed with GPG key: > A525 E3EA 186A AB45 DD0F 86AF 24A4 69FB 7A56 F78E > http://spice-space.org/download/windows/spice-guest-tools/spice-guest- > tools-0.65.1.exe.sig > > Cheers, > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
> Well, you were not really explaining your particular case, or were you? Yes, I tried at least, and I even provided a complete, working example code (spiceterm). > bypassing all of server, x & qemu & kernel etc... > > Arbitrary utf8 input can only be handled at user space / agent level (no way > to > pass a X11/gdk or utf8 through hw level) > > And as of today, all agents messages go through the main channel. So it is > quite > normal to recommend to use the main channel. > > Now that I review your patch and look at spiceterm usage, I understand you are > not just passing utf8 input, but you also want regular key events, thus the > synchronization question arise. > > > IMHO, extending the input channel keyboard message format would be the > > right thing to do. Simply send: > > > > - scancode > > - keysym > > - modifier key state > > > > inside a single message. > > That might be a good option too, but that won't be that easy for the spice > server > / agent to deal with. And I also think it is useless to send a single message > with > all values when clearly only one of the 2 is being used. Ok (although I think such separation only makes thing unnecessarily complex). > I am still suggesting adding a utf8 input message (on input channel), > synchronized with other existing XT key events (which don't have unicode > representation). I already sent such patch 2 weeks ago - please can you review? http://lists.freedesktop.org/archives/spice-devel/2013-September/014342.html http://lists.freedesktop.org/archives/spice-devel/2013-September/014339.html http://lists.freedesktop.org/archives/spice-devel/2013-September/014341.html http://lists.freedesktop.org/archives/spice-devel/2013-September/014340.html Many thanks for your help. - Dietmar ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
- Original Message - > > > > Ah, host dma-buf not guest dma-buf. It makes more sense then. > > yes host side for the viewer. > > > > So virgil just opens one of those new render-only drm nodes, asks the > > gpu to process the rendering ops from the guest & store the results in a > > dma-buf, then this dma-buf must be displayed somehow, correct? > > Yes, the viewer would essentially be a compositing process, taking the > outputs > from multiple VMs and deciding where to draw them. I suppose like boxes > does now. Boxes, for the display part, is just plain gtk+, regular x11 cairo (the animation part is using clutter/gl, but we bypass that for display) Although this is limited to local rendering, we could teach the spice-gtk API to provide a gl texture or RBO. The client interface and further integration for this could be prototyped using the spice 2d gl canvas today. For gtk/cairo, there is a cairo_gl_surface_create_for_texture(), but for some reason it is not advertized. I also wonder if somebody tried to teach gtk+ to use a cairo gl surface (apparently not upstream). Without this, cairo will have to do ReadPixels... Or the gtk widget could embedd its own GL window, that's probably the way to go. I wondered how webkit-gtk does webgl. It looks like they use an offscreen gl window. I will try to verify this. > > > >> When displaying locally (so SDL-2 or gtk ui), we want to avoid the read by > >> passing a kernel dma_buf handle from the virtual card (in this case > >> virtio-vga with Virgil) to the ui (in this case SDL-2), so it can then > >> directly ask the GPU to blit from that dma_buf to its own visible surface. > > > > Hmm. Both SDL and gtk ui have the problem that they effectively bind > > your VM to the desktop session. Which is not what you want for anything > > but short-running test VMs. It's also a PITA to maintain them with > > libvirt. > > Yeah I've hit that. So far virgil is only running via libvirt with a very > hacked > insecure config to draw to the local X server of my user. Getting past that > is however going to involve a bit of lifting all over the place. > > > > > Any plans for a separate UI process? Something using a unix socket for > > control commands and to hand over a dma-buf handle using fd descriptor > > passing maybe? > > That would be the local rendering solution I think we'd prefer, > > qemu runs as qemu user, uses EGL to talk to the drm render-nodes, > has some sort of unix socket that the viewer connects to and can hand > fds across, then the client viewer uses EGL or GLX to render on-screen > and import the handles into EGL and displays the contents, there may > be a small bit of sync info to send across. > > For remoting then we'd have an extra readback (slow) from the GPU and > then spice or vnc encoding stages. That would possibly open the possibility to run the remote server out of qemu. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
> > Ah, host dma-buf not guest dma-buf. It makes more sense then. yes host side for the viewer. > > So virgil just opens one of those new render-only drm nodes, asks the > gpu to process the rendering ops from the guest & store the results in a > dma-buf, then this dma-buf must be displayed somehow, correct? Yes, the viewer would essentially be a compositing process, taking the outputs from multiple VMs and deciding where to draw them. I suppose like boxes does now. > >> When displaying locally (so SDL-2 or gtk ui), we want to avoid the read by >> passing a kernel dma_buf handle from the virtual card (in this case >> virtio-vga with Virgil) to the ui (in this case SDL-2), so it can then >> directly ask the GPU to blit from that dma_buf to its own visible surface. > > Hmm. Both SDL and gtk ui have the problem that they effectively bind > your VM to the desktop session. Which is not what you want for anything > but short-running test VMs. It's also a PITA to maintain them with > libvirt. Yeah I've hit that. So far virgil is only running via libvirt with a very hacked insecure config to draw to the local X server of my user. Getting past that is however going to involve a bit of lifting all over the place. > > Any plans for a separate UI process? Something using a unix socket for > control commands and to hand over a dma-buf handle using fd descriptor > passing maybe? That would be the local rendering solution I think we'd prefer, qemu runs as qemu user, uses EGL to talk to the drm render-nodes, has some sort of unix socket that the viewer connects to and can hand fds across, then the client viewer uses EGL or GLX to render on-screen and import the handles into EGL and displays the contents, there may be a small bit of sync info to send across. For remoting then we'd have an extra readback (slow) from the GPU and then spice or vnc encoding stages. Dave. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
> That leaves the question how to do single-card multihead. I think the > most sensible approach here is to go the spice route, i.e. have one big > framebuffer and define scanout rectangles for the virtual monitors. > This is how real hardware works, and is also provides a natural fallback > mode for UIs not supporting scanout rectangles: They show a single > window with the whole framebuffer, simliar to old spice clients. No real hw doesn't work like that, that is how we program real hw at the moment, but for example wayland won't do this, and neither does Windows mostly, real hw can have multiple separate framebuffers with separate strides, and separate scanouts from them, and the kms device drivers fully support this mode of operation, just the X server prevents it from being useable. I'd ideally want to have a window per gpu output, since the idea would be to allow them to be placed on different monitors on the host side, and doing it as a single app might limit the possiblities. The other thing is for virgil to work at all we need to render the whole console using GL interfaces, at the moment I just use glDrawPixels in the SDL ui update when in GL mode, so there is no direct access to the framebuffer in this case anyways, its all just GL rendered. >> 2) The ability for a video-card generating output to pass a dma-buf >> context to the display (ui in qemu terms) to get the contents from, >> rather then requiring the contents to be rendered to some memory >> buffer. This way we can save the quite expensive read-back from gpu >> memory of the rendered result and then copying that back to the >> framebuffer of the gpu for local displays (ie gtk, SDL), > > Hmm? Not sure what you are asking for... > > First, reading from gpu memory isn't expensive. It's all virtual, no > slow read cycles as with real hardware. There is almost no difference > between gpu memory and main memory for kvm guests. It's not clear to me > why you are copying stuff from/to gpu memory. > > Second, you can have your scanout framebuffer in main memory. That > isn't a problem at all. It only needs to be contiguous in guest > physical memory, scatter-gather for the framebuffer isn't going to fly. Scatter-gather for the framebuffer is fine as long as its not VESA LFB. I already have virtio-vga device allocating a non-contig framebuffer and just using DMA operations to move data in/out. Dave. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
> I've already had a quick discussion about this with Dave Airlie, and > our ideas on this aligned perfectly. > > The basic idea is to use qemu's console layer (include/ui/console.h) > as an abstraction between the new virtio-vga device Dave has in mind > (which will include optional 3D rendering capability through VIRGIL), > and various display options, ie SDL, vnc and Spice. > > The console layer would need some extensions for this: > > 1) Multi head support, a question which comes up here, is do we only > add support for multiple heads on a single card, or do we also want > to support multiple cards each driving a head here ? I myself tend > to go with the KISS solution for now and only support a single > card with multiple heads. I'm thinking it shouldn't be a major enhancement to go for multiple-cards with multiple-heads, I'm not sure its a worthy goal in the real world though, but it might be nice for testing corner cases. > > 2) The ability for a video-card generating output to pass a dma-buf > context to the display (ui in qemu terms) to get the contents from, > rather then requiring the contents to be rendered to some memory > buffer. This way we can save the quite expensive read-back from gpu > memory of the rendered result and then copying that back to the > framebuffer of the gpu for local displays (ie gtk, SDL), we would > of course still need the read back of the rendered output for > vnc / spice. Well at the moment I'm just using SDL/GLX inside the qemu process to talk direct to the X server, this isn't suitable long term for VMs that aren't running directly on the desktop, So the longer term plan is to abstract the GLX bits away and hopefully with SDL2.0, use EGL to talk to the GPU device, now it could still use GLX for local testing VMs, but in the libvirt situation the qemu process running as the qemu user would talk to the new drm rendernodes via EGL, then using an EGL extension export the scanout buffer via dma-buf (hand wavy magic not withstanding). There are some EGL extensions in the works for this. Then we'd just need to make the libvirt viewer use EGL/GLX so it can actually render the scanout buffer to the screen. > For proper multi-head support in the ui layer for local displays, > we will need to use SDL-2, either by porting the current SDL ui code > to SDL-2, or by introducing a new SDL-2 ui component. I've done an initial SDL2 port already just using ifdef :) http://cgit.freedesktop.org/~airlied/qemu/commit/?h=virtio-gpu&id=ee44399a3dbce8da810329230f0a439a3b88cd67 however the input side of SDL changed quite a bit and it needs a bit more work, though if people are inclined towards a separate sdl2.c I could do that I suppose. The other reason I wanted SDL2.0 is it supports argb cursors. Dave. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
- Original Message - > > You said it yourself, unicode doesn't have representation for all keyboard > > keys. > > As you have seen in my previous reply, I am looking at the problem from an > > "input-method" level. And I don't know if it's a good idea to depend on > > X11/gdk > > keysyms in general for Spice. > > VNC use that for 20 years now, so I guess the idea is not so bad. > > > > Else I need to listen on two different input channels, and mix them > > > somehow. > > > That does > > > not make any sense to me. > > > > Indeed, that might be a problem since input channel and vdagent/main > > channel > > are not synchronized. > > > > Could we have the utf8 input message on the input channel, and multiplex it > > with > > the rest of vdagent messages, so at least the remote/guest receives event > > in > > order (assuming input channel waits for vdagent before processing key > > event)? > > Sorry, I do not really understand that. I already send a patch extending the > input channel, > but nobody answered. That is why I rewrote the whole thing to use vd_agent > instead. Well, you were not really explaining your particular case, or were you? bypassing all of server, x & qemu & kernel etc... Arbitrary utf8 input can only be handled at user space / agent level (no way to pass a X11/gdk or utf8 through hw level) And as of today, all agents messages go through the main channel. So it is quite normal to recommend to use the main channel. Now that I review your patch and look at spiceterm usage, I understand you are not just passing utf8 input, but you also want regular key events, thus the synchronization question arise. > IMHO, extending the input channel keyboard message format would be the > right thing to do. Simply send: > > - scancode > - keysym > - modifier key state > > inside a single message. That might be a good option too, but that won't be that easy for the spice server / agent to deal with. And I also think it is useless to send a single message with all values when clearly only one of the 2 is being used. I am still suggesting adding a utf8 input message (on input channel), synchronized with other existing XT key events (which don't have unicode representation). It is meant to be processed by the agent by in your case, it can be processed immediately. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Current qemu-master hangs when used with qxl + linux guest
Hi, On 10/08/2013 04:30 PM, Daniel P. Berrange wrote: On Tue, Oct 08, 2013 at 04:27:38PM +0200, Hans de Goede wrote: Hi All, I'm having this weird problem with qemu master + spice/qxl using guests. As soon as the guest starts Xorg, I get the following message from qemu: main-loop: WARNING: I/O thread spun for 1000 iterations And from then on the guest hangs and qemu consumes 100% cpu. The qemu console still works, and I can quit qemu that way. Doing ctrl+c + a thread apply all bt in qemy shows one cpu thread waiting for the iothread-lock, and all other threads waiting in poll. This happens both with non kms guests (tried RHEL-6.5, older Fedoras) as well as with kms guests (tried a fully up2date F-19). Since I've not seen any similar reports, I assume it is something with my setup ... I've tried changing various things: -removing the spice agent channel -changing the number of virtual cpus (tried 1 and 2 virtual cpus) -upgrading spice-server to the latest git master But all to no avail. This is with qemu-master build from source on a fully up2date F-20 system, using the F-20 seabios files. If someone has any clever ideas I'll happily try debugging this further. Does the QEMU build in F20 work correctly ? Ah, yes it does, good dea. If so that'd give you a starting point from which to 'git bisect' the problem. Yep, note I asked for a "clever" idea, iow not the big hammer of bisecting :) But since 1.6.0 in F-20 worked it was not such a large bisect, so I went ahead and bisected anyways, which pointed me to commit 7b595f35d8 : "aio / timers: Convert mainloop to use timeout" After careful review of that commit I had a hunch what might be wrong, and it turned out to be right. So after this mail I'm going to send a patch fixing this. If you want to know the details see the patch, titled: "main-loop: Don't lock starve io-threads when main_loop_tlg has pending events" Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
> You said it yourself, unicode doesn't have representation for all keyboard > keys. > As you have seen in my previous reply, I am looking at the problem from an > "input-method" level. And I don't know if it's a good idea to depend on > X11/gdk > keysyms in general for Spice. VNC use that for 20 years now, so I guess the idea is not so bad. > > Else I need to listen on two different input channels, and mix them somehow. > > That does > > not make any sense to me. > > Indeed, that might be a problem since input channel and vdagent/main channel > are not synchronized. > > Could we have the utf8 input message on the input channel, and multiplex it > with > the rest of vdagent messages, so at least the remote/guest receives event in > order (assuming input channel waits for vdagent before processing key event)? Sorry, I do not really understand that. I already send a patch extending the input channel, but nobody answered. That is why I rewrote the whole thing to use vd_agent instead. IMHO, extending the input channel keyboard message format would be the right thing to do. Simply send: - scancode - keysym - modifier key state inside a single message. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
Hi - Original Message - > > Keys that don't have utf8 equivalent should be sent with the current input, > > I don't > > see a benefit changing that. > > > > Having a utf8 input is mainly useful for utf8 input from client (input > > method, > > browser) and to bypass guest keymaps. > > > > It shouldn't be used for all inputs. > > I need that for spiceterm. > > I do not really understand why you limit yourself by sending such restricted > information > on the input channel. Why do not send all information we have? You said it yourself, unicode doesn't have representation for all keyboard keys. As you have seen in my previous reply, I am looking at the problem from an "input-method" level. And I don't know if it's a good idea to depend on X11/gdk keysyms in general for Spice. > Else I need to listen on two different input channels, and mix them somehow. > That does > not make any sense to me. Indeed, that might be a problem since input channel and vdagent/main channel are not synchronized. Could we have the utf8 input message on the input channel, and multiplex it with the rest of vdagent messages, so at least the remote/guest receives event in order (assuming input channel waits for vdagent before processing key event)? ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
> Keys that don't have utf8 equivalent should be sent with the current input, I > don't > see a benefit changing that. > > Having a utf8 input is mainly useful for utf8 input from client (input method, > browser) and to bypass guest keymaps. > > It shouldn't be used for all inputs. I need that for spiceterm. I do not really understand why you limit yourself by sending such restricted information on the input channel. Why do not send all information we have? Else I need to listen on two different input channels, and mix them somehow. That does not make any sense to me. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] Fix PlaybackeCommand typo
--- server/snd_worker.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/snd_worker.c b/server/snd_worker.c index 5346d96..9156bf5 100644 --- a/server/snd_worker.c +++ b/server/snd_worker.c @@ -52,7 +52,7 @@ #define RECORD_SAMPLES_SIZE (SND_RECEIVE_BUF_SIZE >> 2) -enum PlaybackeCommand { +enum PlaybackCommand { SND_PLAYBACK_MIGRATE, SND_PLAYBACK_MODE, SND_PLAYBACK_CTRL, -- 1.8.3.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
- Original Message - > > > - Original Message - > > > Yes, although I think we should be able to send arbitrary utf8 input in > > > the > > > protocol. If I read your proposal right, you are sending UCS-4 codes, no > > > utf8? > > > > > > I think you should use gdk_keyval_to_unicode() then g_ucs4_to_utf8() for > > > key > > > input and efficiency (in your spice-gtk patch). > > > > Unfortunately, the unicode people forgot to add values for function keys > > and > > other special > > keys! That is why I use the X11 keysmys - they can also represent all keys > > on > > the keyboard. > > AFAIK VNC protocol also use that. > > Keys that don't have utf8 equivalent should be sent with the current input, I > don't see a benefit changing that. > > Having a utf8 input is mainly useful for utf8 input from client (input > method, browser) and to bypass guest keymaps. I realize you may just want to receive client keysyms, and not the XT scancode (because you don't have any keymap and talk directly to a process)... In this case, your patch would make sense, and open the door to higher-level inputs ;) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
Hi > Any plans for a separate UI process? Something using a unix socket for > control commands and to hand over a dma-buf handle using fd descriptor > passing maybe? It sounds to me like this is something that an egl extension should provide, but I can't find it yet. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
- Original Message - > > Yes, although I think we should be able to send arbitrary utf8 input in the > > protocol. If I read your proposal right, you are sending UCS-4 codes, no > > utf8? > > > > I think you should use gdk_keyval_to_unicode() then g_ucs4_to_utf8() for > > key > > input and efficiency (in your spice-gtk patch). > > Unfortunately, the unicode people forgot to add values for function keys and > other special > keys! That is why I use the X11 keysmys - they can also represent all keys on > the keyboard. > AFAIK VNC protocol also use that. Keys that don't have utf8 equivalent should be sent with the current input, I don't see a benefit changing that. Having a utf8 input is mainly useful for utf8 input from client (input method, browser) and to bypass guest keymaps. It shouldn't be used for all inputs. > > > Do not send both to agent and hw > > input layer, because the agent can't easily tell which of these two events > > should > > be discarded. The client however, should know when the agent fully supports > > utf8 input. However, this will probably be more difficult to handle in the > > agent, > > when no input method is available (console etc). Either the utf8 input will > > need > > to be translated and sent to uinput, but that's not very elegant, or the > > agent will > > have to inform the client to switch to hw input. > > I guess this is not a concern for spiceterm, because it is implementing an > > agent > > itself, > > yes > > > and cannot be turned off, and also because you handle all inputs. Perhaps > > you could introduce spiceterm? The repo is lacking a simple README file > > just to > > make things clear what is really about :) > > OK, will send more info tomorrow. > > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
> Yes, although I think we should be able to send arbitrary utf8 input in the > protocol. If I read your proposal right, you are sending UCS-4 codes, no utf8? > > I think you should use gdk_keyval_to_unicode() then g_ucs4_to_utf8() for key > input and efficiency (in your spice-gtk patch). Unfortunately, the unicode people forgot to add values for function keys and other special keys! That is why I use the X11 keysmys - they can also represent all keys on the keyboard. AFAIK VNC protocol also use that. > Do not send both to agent and hw > input layer, because the agent can't easily tell which of these two events > should > be discarded. The client however, should know when the agent fully supports > utf8 input. However, this will probably be more difficult to handle in the > agent, > when no input method is available (console etc). Either the utf8 input will > need > to be translated and sent to uinput, but that's not very elegant, or the > agent will > have to inform the client to switch to hw input. > I guess this is not a concern for spiceterm, because it is implementing an > agent > itself, yes > and cannot be turned off, and also because you handle all inputs. Perhaps > you could introduce spiceterm? The repo is lacking a simple README file just > to > make things clear what is really about :) OK, will send more info tomorrow. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
Hi, > This is mostly Dave's area of expertise, but let me try to explain things > a bit better here. The dma-buf pass-through is for the Virgil case, so > we're passing through 3D rendering commands from the guest to a real, > physcial GPU inside the host, which then renders the final image to show > inside the ui to its own, potentially on card, memory, reading from which > is expensive. Ah, host dma-buf not guest dma-buf. It makes more sense then. So virgil just opens one of those new render-only drm nodes, asks the gpu to process the rendering ops from the guest & store the results in a dma-buf, then this dma-buf must be displayed somehow, correct? > When displaying locally (so SDL-2 or gtk ui), we want to avoid the read by > passing a kernel dma_buf handle from the virtual card (in this case > virtio-vga with Virgil) to the ui (in this case SDL-2), so it can then > directly ask the GPU to blit from that dma_buf to its own visible surface. Hmm. Both SDL and gtk ui have the problem that they effectively bind your VM to the desktop session. Which is not what you want for anything but short-running test VMs. It's also a PITA to maintain them with libvirt. Any plans for a separate UI process? Something using a unix socket for control commands and to hand over a dma-buf handle using fd descriptor passing maybe? cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Current qemu-master hangs when used with qxl + linux guest
Hey Hans, On Tue, Oct 08, 2013 at 04:27:38PM +0200, Hans de Goede wrote: > I'm having this weird problem with qemu master + spice/qxl using > guests. As soon as the guest starts Xorg, I get the following message > from qemu: > > main-loop: WARNING: I/O thread spun for 1000 iterations I've also seen that message when giving a quick try to qemu master today, so it's not just your setup. Christophe pgp3bMIiN38EC.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [patch 0/2] vdagent KEYVAL extension
Hi! - Original Message - > This is my second approach to implement the ability to send utf8 input from > spice-gtk to spiceterm. > https://git.proxmox.com/?p=spiceterm.git;a=summary > > The patch is quite simple. > > I still hope there is some interest to get that working? Yes, although I think we should be able to send arbitrary utf8 input in the protocol. If I read your proposal right, you are sending UCS-4 codes, no utf8? I think you should use gdk_keyval_to_unicode() then g_ucs4_to_utf8() for key input and efficiency (in your spice-gtk patch). Do not send both to agent and hw input layer, because the agent can't easily tell which of these two events should be discarded. The client however, should know when the agent fully supports utf8 input. However, this will probably be more difficult to handle in the agent, when no input method is available (console etc). Either the utf8 input will need to be translated and sent to uinput, but that's not very elegant, or the agent will have to inform the client to switch to hw input. I guess this is not a concern for spiceterm, because it is implementing an agent itself, and cannot be turned off, and also because you handle all inputs. Perhaps you could introduce spiceterm? The repo is lacking a simple README file just to make things clear what is really about :) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Current qemu-master hangs when used with qxl + linux guest
On Tue, Oct 08, 2013 at 04:27:38PM +0200, Hans de Goede wrote: > Hi All, > > I'm having this weird problem with qemu master + spice/qxl using > guests. As soon as the guest starts Xorg, I get the following message > from qemu: > > main-loop: WARNING: I/O thread spun for 1000 iterations > > And from then on the guest hangs and qemu consumes 100% cpu. The qemu > console still works, and I can quit qemu that way. > > Doing ctrl+c + a thread apply all bt in qemy shows one cpu thread waiting > for the iothread-lock, and all other threads waiting in poll. > > This happens both with non kms guests (tried RHEL-6.5, older Fedoras) > as well as with kms guests (tried a fully up2date F-19). > > Since I've not seen any similar reports, I assume it is something > with my setup ... > > I've tried changing various things: > -removing the spice agent channel > -changing the number of virtual cpus (tried 1 and 2 virtual cpus) > -upgrading spice-server to the latest git master > > But all to no avail. > > This is with qemu-master build from source on a fully up2date > F-20 system, using the F-20 seabios files. > > If someone has any clever ideas I'll happily try debugging this further. Does the QEMU build in F20 work correctly ? If so that'd give you a starting point from which to 'git bisect' the problem. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Current qemu-master hangs when used with qxl + linux guest
Hi All, I'm having this weird problem with qemu master + spice/qxl using guests. As soon as the guest starts Xorg, I get the following message from qemu: main-loop: WARNING: I/O thread spun for 1000 iterations And from then on the guest hangs and qemu consumes 100% cpu. The qemu console still works, and I can quit qemu that way. Doing ctrl+c + a thread apply all bt in qemy shows one cpu thread waiting for the iothread-lock, and all other threads waiting in poll. This happens both with non kms guests (tried RHEL-6.5, older Fedoras) as well as with kms guests (tried a fully up2date F-19). Since I've not seen any similar reports, I assume it is something with my setup ... I've tried changing various things: -removing the spice agent channel -changing the number of virtual cpus (tried 1 and 2 virtual cpus) -upgrading spice-server to the latest git master But all to no avail. This is with qemu-master build from source on a fully up2date F-20 system, using the F-20 seabios files. If someone has any clever ideas I'll happily try debugging this further. Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
However, I can see the record path has a fixed 44100 value, which is probably bogus today for != 44100 recording? Yah; there is a fair amount of hard coding (although I'm probably overly biased by my time in the old client :-/). And, afaik, there's not a lot of testing done at anything other than 44100, so we may find corner cases yet. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
- Original Message - > > On old client connection, new qemu/new spice-server should also use celt, > > the old client should be able to decode that even if it's 48kHz. > > Perhaps; but the number of hard coded values, and the fact > that this code has never been run at anything but 44.1 makes me nervous. > > At the very least, won't you have rate drift, so movies, for example, > will get out of sync? The "frequency" (playback and record) is communicated to the client. However, I can see the record path has a fixed 44100 value, which is probably bogus today for != 44100 recording? ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
On Tue, Oct 08, 2013 at 08:32:26AM -0500, Jeremy White wrote: > >On old client connection, new qemu/new spice-server should also use celt, > >the old client should be able to decode that even if it's 48kHz. > > Perhaps; but the number of hard coded values, and the fact > that this code has never been run at anything but 44.1 makes me nervous. I've just built spice-server+qemu with diff --git a/server/spice.h b/server/spice.h index b645112..ffc4a91 100644 --- a/server/spice.h +++ b/server/spice.h @@ -342,7 +342,7 @@ enum { SPICE_INTERFACE_AUDIO_FMT_S16 = 1, }; -#define SPICE_INTERFACE_PLAYBACK_FREQ 44100 +#define SPICE_INTERFACE_PLAYBACK_FREQ 48000 #define SPICE_INTERFACE_PLAYBACK_CHAN 2 #define SPICE_INTERFACE_PLAYBACK_FMT SPICE_INTERFACE_AUDIO_FMT_S16 and basic playback is fine, I didn't test more. > At the very least, won't you have rate drift, so movies, for example, > will get out of sync? Fwiw, I had the same concerns with respect to resampling :) Christophe pgpN6WM2yVT4C.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
Hi, On 10/08/2013 03:18 PM, Gerd Hoffmann wrote: Hi, The basic idea is to use qemu's console layer (include/ui/console.h) as an abstraction between the new virtio-vga device Dave has in mind (which will include optional 3D rendering capability through VIRGIL), and various display options, ie SDL, vnc and Spice. The console layer would need some extensions for this: 2) The ability for a video-card generating output to pass a dma-buf context to the display (ui in qemu terms) to get the contents from, rather then requiring the contents to be rendered to some memory buffer. This way we can save the quite expensive read-back from gpu memory of the rendered result and then copying that back to the framebuffer of the gpu for local displays (ie gtk, SDL), Hmm? Not sure what you are asking for... First, reading from gpu memory isn't expensive. It's all virtual, no slow read cycles as with real hardware. There is almost no difference between gpu memory and main memory for kvm guests. It's not clear to me why you are copying stuff from/to gpu memory. This is mostly Dave's area of expertise, but let me try to explain things a bit better here. The dma-buf pass-through is for the Virgil case, so we're passing through 3D rendering commands from the guest to a real, physcial GPU inside the host, which then renders the final image to show inside the ui to its own, potentially on card, memory, reading from which is expensive. When displaying locally (so SDL-2 or gtk ui), we want to avoid the read by passing a kernel dma_buf handle from the virtual card (in this case virtio-vga with Virgil) to the ui (in this case SDL-2), so it can then directly ask the GPU to blit from that dma_buf to its own visible surface. Second, you can have your scanout framebuffer in main memory. That isn't a problem at all. It only needs to be contiguous in guest physical memory, scatter-gather for the framebuffer isn't going to fly. This is not about the virtual gpu / virtual scanout buffer, this is about a real GPU used to do the (final) rendering and about getting that rendering shown to a local user (ie SDL or gtk ui). So the rendered image is stored in memory owned by the real GPU, and we need to get this "copied" to the window the user is viewing, without using the CPU. For proper multi-head support in the ui layer for local displays, we will need to use SDL-2, either by porting the current SDL ui code to SDL-2, or by introducing a new SDL-2 ui component. /me votes for new SDL-2 ui component, the historical grown SDL code can use a rewrite anyway ;) I was already expecting you would prefer the new SDL-2 ui component solution :) Regards, Hans ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
On old client connection, new qemu/new spice-server should also use celt, the old client should be able to decode that even if it's 48kHz. Perhaps; but the number of hard coded values, and the fact that this code has never been run at anything but 44.1 makes me nervous. At the very least, won't you have rate drift, so movies, for example, will get out of sync? Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
- Original Message - > >> Attached is a patch that suggests my idea. I've done some hacks, > >> and it seems 'safe' to adjust the rate on the fly like this. > > > > line_out_ctl is driven by guest. Why would you change the rate when the > > device start/stop a stream? I don't think that's the right approach. > > Well, if you're going to change the rate dynamically, > you've got to pick a point to do that. My reading of the code > suggested that enable/disable was a point where the > buffering was clean, and a rate switch would be safe. > The alternate would be to add a callback of some kind, > so the spice server would call into qemu code when a > client attaches. That may be a superior approach; but it was > harder for me to eyeball the code and feel I could be sure > to do it safely. I don't think we need to change the rate dynamically. Also, doing this depending on guest state would mean you would have to also handle resampling in spice server, to avoid (possibly large) gaps of different rate. > > > > > I think the suggestion by Gerd & Christophe should be enough. > > > > If qemu is old, it should use 44.1 celt only. > > If qemu is new, it can use 48 celt or opus. > > > > This doesn't have to change dynamically, the client should be able to adapt > > to any of these situations. > > Yes, a new client should be fine either way. It's the old clients that > will break with this approach. That is, an old client > running against a new qemu will need to have no sound. The client, old or new, should be able to adapt to the above. > > But I'm muddying things; I'm playing with options that we may not > need. It should be straight forward to add a resampling option, or > even my crazy dynamic resampling option, to a base implementation. > > I'll go work on the base implementation so we can talk about something > tangible. works for me ;) ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC: Integrating Virgil and Spice
Hi, > The basic idea is to use qemu's console layer (include/ui/console.h) > as an abstraction between the new virtio-vga device Dave has in mind > (which will include optional 3D rendering capability through VIRGIL), > and various display options, ie SDL, vnc and Spice. > > The console layer would need some extensions for this: > > 1) Multi head support, a question which comes up here, is do we only > add support for multiple heads on a single card, or do we also want > to support multiple cards each driving a head here ? I myself tend > to go with the KISS solution for now and only support a single > card with multiple heads. Support for multiple cards is there. Well, at least the groundwork. The ui core can deal with it. spice can deal with it. Secondary qxl cards used to completely bypass the qemu console subsystem. This is no longer the case with qemu 1.5+. Not all UIs can deal with it in a sane way though. With SDL and VNC the secondary qxl card is just another console, so ctrl-alt- can be used to switch to it. Once I had an experimental patch to make the gtk ui open a second window for the secondary card. Didn't end up upstream, not in my git tree any more, IIRC I've dropped it at one of the rebases. Isn't hard to do though. That leaves the question how to do single-card multihead. I think the most sensible approach here is to go the spice route, i.e. have one big framebuffer and define scanout rectangles for the virtual monitors. This is how real hardware works, and is also provides a natural fallback mode for UIs not supporting scanout rectangles: They show a single window with the whole framebuffer, simliar to old spice clients. To get that done we effectively have to handle the monitor config properly at qemu console level instead of having a private channel between qxl and spice. > 2) The ability for a video-card generating output to pass a dma-buf > context to the display (ui in qemu terms) to get the contents from, > rather then requiring the contents to be rendered to some memory > buffer. This way we can save the quite expensive read-back from gpu > memory of the rendered result and then copying that back to the > framebuffer of the gpu for local displays (ie gtk, SDL), Hmm? Not sure what you are asking for... First, reading from gpu memory isn't expensive. It's all virtual, no slow read cycles as with real hardware. There is almost no difference between gpu memory and main memory for kvm guests. It's not clear to me why you are copying stuff from/to gpu memory. Second, you can have your scanout framebuffer in main memory. That isn't a problem at all. It only needs to be contiguous in guest physical memory, scatter-gather for the framebuffer isn't going to fly. > For proper multi-head support in the ui layer for local displays, > we will need to use SDL-2, either by porting the current SDL ui code > to SDL-2, or by introducing a new SDL-2 ui component. /me votes for new SDL-2 ui component, the historical grown SDL code can use a rewrite anyway ;) cheers, Gerd ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
On Tue, Oct 08, 2013 at 08:03:05AM -0500, Jeremy White wrote: > >If qemu is old, it should use 44.1 celt only. > >If qemu is new, it can use 48 celt or opus. > > > >This doesn't have to change dynamically, the client should be able to adapt > >to any of these situations. > > Yes, a new client should be fine either way. It's the old clients that > will break with this approach. That is, an old client > running against a new qemu will need to have no sound. On old client connection, new qemu/new spice-server should also use celt, the old client should be able to decode that even if it's 48kHz. Christophe pgpZtlpAOsncR.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
Attached is a patch that suggests my idea. I've done some hacks, and it seems 'safe' to adjust the rate on the fly like this. line_out_ctl is driven by guest. Why would you change the rate when the device start/stop a stream? I don't think that's the right approach. Well, if you're going to change the rate dynamically, you've got to pick a point to do that. My reading of the code suggested that enable/disable was a point where the buffering was clean, and a rate switch would be safe. The alternate would be to add a callback of some kind, so the spice server would call into qemu code when a client attaches. That may be a superior approach; but it was harder for me to eyeball the code and feel I could be sure to do it safely. I think the suggestion by Gerd & Christophe should be enough. If qemu is old, it should use 44.1 celt only. If qemu is new, it can use 48 celt or opus. This doesn't have to change dynamically, the client should be able to adapt to any of these situations. Yes, a new client should be fine either way. It's the old clients that will break with this approach. That is, an old client running against a new qemu will need to have no sound. But I'm muddying things; I'm playing with options that we may not need. It should be straight forward to add a resampling option, or even my crazy dynamic resampling option, to a base implementation. I'll go work on the base implementation so we can talk about something tangible. Cheers, Jeremy ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Use GByteArray to implement RedsClientMonitorsConfig
On Tue, Oct 08, 2013 at 07:03:20AM -0400, Marc-André Lureau wrote: > > - Original Message - > > reds_on_main_agent_monitor_config() is manually implementing > > a grow-on-append char *. glib's GByteArray can do this for us, > > using it makes the code simpler to read. > > Also I don't clearly see the benefit in this case, > realloc fits pretty well. Overall you saved only 3 lines, and use a more > complex structure. The benefit is readability, you don't need to parse the realloc+memcpy+.. and then understand what they do, g_byte_array_append immediatly tells us what the code intends to do. With the realloc + memcpy, questions that comes to my mind while reading the code are if realloc is used properly (no leak in error case), if the code is really doing an append or doing something subtly different, also, what about that 'buffer_pos' variable, is it unused, is it useful? I don't want to have to worry about all of this when looking at how the monitors config stuff. I really don't like high level code (what on_main_agent_monitors_config() tries to achieve) being mixed with low level bookkeeping stuff. > There are gazillions of places where GLib structures would help. I don't > think we should start rewriting them in master. I would not advise going over the whole codebase doing such replacements, but imo we really should try to incrementally improving things when touching a piece of code where glib would fit. Lack of helper functions, helper classes, ... combined with big functions/files opencoding all of this is one of the things making the server code difficult to follow, so we should improve that little by little ;) Christophe pgpL0hKoBuQzh.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Use GByteArray to implement RedsClientMonitorsConfig
- Original Message - > reds_on_main_agent_monitor_config() is manually implementing > a grow-on-append char *. glib's GByteArray can do this for us, > using it makes the code simpler to read. Given that we don't have test suite, I am not really for such small cleanup changes. Also I don't clearly see the benefit in this case, realloc fits pretty well. Overall you saved only 3 lines, and use a more complex structure. There are gazillions of places where GLib structures would help. I don't think we should start rewriting them in master. I suggest a separate "unstable" branch for this kind of changes. I would also relax reviewing rules for it, because the effort to review that near-0 value change is big. nack > --- > server/reds-private.h | 4 +--- > server/reds.c | 27 +-- > 2 files changed, 14 insertions(+), 17 deletions(-) > > diff --git a/server/reds-private.h b/server/reds-private.h > index 9358d27..9d8b5d1 100644 > --- a/server/reds-private.h > +++ b/server/reds-private.h > @@ -120,9 +120,7 @@ typedef struct SpiceCharDeviceStateItem { > * client, being passed to the guest */ > typedef struct RedsClientMonitorsConfig { > MainChannelClient *mcc; > -uint8_t *buffer; > -int buffer_size; > -int buffer_pos; > +GByteArray *config_data; > } RedsClientMonitorsConfig; > > typedef struct RedsState { > diff --git a/server/reds.c b/server/reds.c > index 1456b75..18743a3 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -1079,10 +1079,10 @@ static void reds_client_monitors_config_cleanup(void) > { > RedsClientMonitorsConfig *cmc = &reds->client_monitors_config; > > -cmc->buffer_size = cmc->buffer_pos = 0; > -free(cmc->buffer); > -cmc->buffer = NULL; > -cmc->mcc = NULL; > +if (cmc->config_data != NULL) { > +g_byte_array_free(cmc->config_data, TRUE); > +} > +cmc->config_data = NULL; > } > > static void reds_on_main_agent_monitors_config( > @@ -1092,19 +1092,18 @@ static void reds_on_main_agent_monitors_config( > VDAgentMonitorsConfig *monitors_config; > RedsClientMonitorsConfig *cmc = &reds->client_monitors_config; > > -cmc->buffer_size += size; > -cmc->buffer = realloc(cmc->buffer, cmc->buffer_size); > -spice_assert(cmc->buffer); > cmc->mcc = mcc; > -memcpy(cmc->buffer + cmc->buffer_pos, message, size); > -cmc->buffer_pos += size; > -msg_header = (VDAgentMessage *)cmc->buffer; > -if (sizeof(VDAgentMessage) > cmc->buffer_size || > -msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) { > -spice_debug("not enough data yet. %d\n", cmc->buffer_size); > +if (cmc->config_data == NULL) { > +cmc->config_data = g_byte_array_new(); > +} I guess there might be a better place for initial allocation. > +g_byte_array_append(cmc->config_data, message, size); > +msg_header = (VDAgentMessage *)cmc->config_data->data; > +if (sizeof(VDAgentMessage) > cmc->config_data->len || > +msg_header->size > cmc->config_data->len - > sizeof(VDAgentMessage)) { > +spice_debug("not enough data yet. %d\n", cmc->config_data->len); > return; > } > -monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + > sizeof(*msg_header)); > +monitors_config = (VDAgentMonitorsConfig *)(cmc->config_data->data + > sizeof(*msg_header)); > spice_debug("%s: %d\n", __func__, monitors_config->num_of_monitors); > red_dispatcher_client_monitors_config(monitors_config); > reds_client_monitors_config_cleanup(); > -- > 1.8.3.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] Namespace RECEIVE_BUF_SIZE
ack all - Original Message - > --- > server/main_channel.h | 4 ++-- > server/red_worker.c | 8 > server/snd_worker.c | 6 +++--- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/server/main_channel.h b/server/main_channel.h > index 29eb8d4..c8e9ade 100644 > --- a/server/main_channel.h > +++ b/server/main_channel.h > @@ -30,12 +30,12 @@ > #define REDS_NUM_INTERNAL_AGENT_MESSAGES 1 > > // approximate max receive message size for main channel > -#define RECEIVE_BUF_SIZE \ > +#define MAIN_CHANNEL_RECEIVE_BUF_SIZE \ > (4096 + (REDS_AGENT_WINDOW_SIZE + REDS_NUM_INTERNAL_AGENT_MESSAGES) * > SPICE_AGENT_MAX_DATA_SIZE) > > typedef struct MainChannel { > RedChannel base; > -uint8_t recv_buf[RECEIVE_BUF_SIZE]; > +uint8_t recv_buf[MAIN_CHANNEL_RECEIVE_BUF_SIZE]; > RedsMigSpice mig_target; // TODO: add refs and release (afrer all > clients completed migration in one way or the other?) > int num_clients_mig_wait; > } MainChannel; > diff --git a/server/red_worker.c b/server/red_worker.c > index 8763c8e..451c8ab 100644 > --- a/server/red_worker.c > +++ b/server/red_worker.c > @@ -383,7 +383,7 @@ typedef struct LocalCursor { > } LocalCursor; > > #define MAX_PIPE_SIZE 50 > -#define RECIVE_BUF_SIZE 1024 > +#define CHANNEL_RECIVE_BUF_SIZE 1024 > > #define WIDE_CLIENT_ACK_WINDOW 40 > #define NARROW_CLIENT_ACK_WINDOW 20 > @@ -654,7 +654,7 @@ typedef struct GlzSharedDictionary { > typedef struct CommonChannel { > RedChannel base; // Must be the first thing > struct RedWorker *worker; > -uint8_t recv_buf[RECIVE_BUF_SIZE]; > +uint8_t recv_buf[CHANNEL_RECIVE_BUF_SIZE]; > uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme. > int during_target_migrate; /* TRUE when the client that is associated > with the channel >is during migration. Turned off when the >vm is started. > @@ -1591,8 +1591,8 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient > *rcc, uint16_t type, uint > return spice_malloc(size); > } > > -if (size > RECIVE_BUF_SIZE) { > -spice_critical("unexpected message size %u (max is %d)", size, > RECIVE_BUF_SIZE); > +if (size > CHANNEL_RECIVE_BUF_SIZE) { > +spice_critical("unexpected message size %u (max is %d)", size, > CHANNEL_RECIVE_BUF_SIZE); > return NULL; > } > return common->recv_buf; > diff --git a/server/snd_worker.c b/server/snd_worker.c > index ebddfcd..19171e3 100644 > --- a/server/snd_worker.c > +++ b/server/snd_worker.c > @@ -42,7 +42,7 @@ > #define IOV_MAX 1024 > #endif > > -#define RECIVE_BUF_SIZE (16 * 1024 * 2) > +#define SND_RECIVE_BUF_SIZE (16 * 1024 * 2) > > #define FRAME_SIZE 256 > #define PLAYBACK_BUF_SIZE (FRAME_SIZE * 4) > @@ -50,7 +50,7 @@ > #define CELT_BIT_RATE (64 * 1024) > #define CELT_COMPRESSED_FRAME_BYTES (FRAME_SIZE * CELT_BIT_RATE / > SPICE_INTERFACE_PLAYBACK_FREQ / 8) > > -#define RECORD_SAMPLES_SIZE (RECIVE_BUF_SIZE >> 2) > +#define RECORD_SAMPLES_SIZE (SND_RECIVE_BUF_SIZE >> 2) > > enum PlaybackeCommand { > SND_PLAYBACK_MIGRATE, > @@ -112,7 +112,7 @@ struct SndChannel { > } send_data; > > struct { > -uint8_t buf[RECIVE_BUF_SIZE]; > +uint8_t buf[SND_RECIVE_BUF_SIZE]; > uint8_t *message_start; > uint8_t *now; > uint8_t *end; > -- > 1.8.3.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] Use GByteArray to implement RedsClientMonitorsConfig
reds_on_main_agent_monitor_config() is manually implementing a grow-on-append char *. glib's GByteArray can do this for us, using it makes the code simpler to read. --- server/reds-private.h | 4 +--- server/reds.c | 27 +-- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/server/reds-private.h b/server/reds-private.h index 9358d27..9d8b5d1 100644 --- a/server/reds-private.h +++ b/server/reds-private.h @@ -120,9 +120,7 @@ typedef struct SpiceCharDeviceStateItem { * client, being passed to the guest */ typedef struct RedsClientMonitorsConfig { MainChannelClient *mcc; -uint8_t *buffer; -int buffer_size; -int buffer_pos; +GByteArray *config_data; } RedsClientMonitorsConfig; typedef struct RedsState { diff --git a/server/reds.c b/server/reds.c index 1456b75..18743a3 100644 --- a/server/reds.c +++ b/server/reds.c @@ -1079,10 +1079,10 @@ static void reds_client_monitors_config_cleanup(void) { RedsClientMonitorsConfig *cmc = &reds->client_monitors_config; -cmc->buffer_size = cmc->buffer_pos = 0; -free(cmc->buffer); -cmc->buffer = NULL; -cmc->mcc = NULL; +if (cmc->config_data != NULL) { +g_byte_array_free(cmc->config_data, TRUE); +} +cmc->config_data = NULL; } static void reds_on_main_agent_monitors_config( @@ -1092,19 +1092,18 @@ static void reds_on_main_agent_monitors_config( VDAgentMonitorsConfig *monitors_config; RedsClientMonitorsConfig *cmc = &reds->client_monitors_config; -cmc->buffer_size += size; -cmc->buffer = realloc(cmc->buffer, cmc->buffer_size); -spice_assert(cmc->buffer); cmc->mcc = mcc; -memcpy(cmc->buffer + cmc->buffer_pos, message, size); -cmc->buffer_pos += size; -msg_header = (VDAgentMessage *)cmc->buffer; -if (sizeof(VDAgentMessage) > cmc->buffer_size || -msg_header->size > cmc->buffer_size - sizeof(VDAgentMessage)) { -spice_debug("not enough data yet. %d\n", cmc->buffer_size); +if (cmc->config_data == NULL) { +cmc->config_data = g_byte_array_new(); +} +g_byte_array_append(cmc->config_data, message, size); +msg_header = (VDAgentMessage *)cmc->config_data->data; +if (sizeof(VDAgentMessage) > cmc->config_data->len || +msg_header->size > cmc->config_data->len - sizeof(VDAgentMessage)) { +spice_debug("not enough data yet. %d\n", cmc->config_data->len); return; } -monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header)); +monitors_config = (VDAgentMonitorsConfig *)(cmc->config_data->data + sizeof(*msg_header)); spice_debug("%s: %d\n", __func__, monitors_config->num_of_monitors); red_dispatcher_client_monitors_config(monitors_config); reds_client_monitors_config_cleanup(); -- 1.8.3.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 3/3] reds: Fix 'asyc' typo
--- server/reds.c | 46 +++--- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/server/reds.c b/server/reds.c index dd1bd68..c2b3fff 100644 --- a/server/reds.c +++ b/server/reds.c @@ -141,7 +141,7 @@ typedef struct AsyncRead { typedef struct RedLinkInfo { RedsStream *stream; -AsyncRead asyc_read; +AsyncRead async_read; SpiceLinkHeader link_header; SpiceLinkMess *link_mess; int mess_pos; @@ -2117,12 +2117,12 @@ static void async_read_handler(int fd, int event, void *data) static void reds_get_spice_ticket(RedLinkInfo *link) { -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; obj->now = (uint8_t *)&link->tiTicketing.encrypted_ticket.encrypted_data; obj->end = obj->now + link->tiTicketing.rsa_size; obj->done = reds_handle_ticket; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } #if HAVE_SASL @@ -2208,7 +2208,7 @@ static void reds_handle_auth_sasl_step(void *opaque) RedLinkInfo *link = (RedLinkInfo *)opaque; RedsSASL *sasl = &link->stream->sasl; uint32_t datalen = sasl->len; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; /* NB, distinction of NULL vs "" is *critical* in SASL */ if (datalen) { @@ -2256,7 +2256,7 @@ static void reds_handle_auth_sasl_step(void *opaque) obj->now = (uint8_t *)&sasl->len; obj->end = obj->now + sizeof(uint32_t); obj->done = reds_handle_auth_sasl_steplen; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } else { int ssf; @@ -2292,7 +2292,7 @@ authabort: static void reds_handle_auth_sasl_steplen(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; RedsSASL *sasl = &link->stream->sasl; spice_info("Got steplen %d", sasl->len); @@ -2309,7 +2309,7 @@ static void reds_handle_auth_sasl_steplen(void *opaque) obj->now = (uint8_t *)sasl->data; obj->end = obj->now + sasl->len; obj->done = reds_handle_auth_sasl_step; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } } @@ -2332,7 +2332,7 @@ static void reds_handle_auth_sasl_steplen(void *opaque) static void reds_handle_auth_sasl_start(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; const char *serverout; unsigned int serveroutlen; int err; @@ -2387,7 +2387,7 @@ static void reds_handle_auth_sasl_start(void *opaque) obj->now = (uint8_t *)&sasl->len; obj->end = obj->now + sizeof(uint32_t); obj->done = reds_handle_auth_sasl_steplen; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } else { int ssf; @@ -2423,7 +2423,7 @@ authabort: static void reds_handle_auth_startlen(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; RedsSASL *sasl = &link->stream->sasl; spice_info("Got client start len %d", sasl->len); @@ -2443,13 +2443,13 @@ static void reds_handle_auth_startlen(void *opaque) obj->now = (uint8_t *)sasl->data; obj->end = obj->now + sasl->len; obj->done = reds_handle_auth_sasl_start; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } static void reds_handle_auth_mechname(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; RedsSASL *sasl = &link->stream->sasl; sasl->mechname[sasl->len] = '\0'; @@ -2487,7 +2487,7 @@ static void reds_handle_auth_mechname(void *opaque) obj->now = (uint8_t *)&sasl->len; obj->end = obj->now + sizeof(uint32_t); obj->done = reds_handle_auth_startlen; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); return; } @@ -2495,7 +2495,7 @@ static void reds_handle_auth_mechname(void *opaque) static void reds_handle_auth_mechlen(void *opaque) { RedLinkInfo *link = (RedLinkInfo *)opaque; -AsyncRead *obj = &link->asyc_read; +AsyncRead *obj = &link->async_read; RedsSASL *sasl = &link->stream->sasl; if (sasl->len < 1 || sasl->len > 100) { @@ -2510,7 +2510,7 @@ static void reds_handle_auth_mechlen(void *opaque) obj->now = (uint8_t *)sasl->mechname; obj->end = obj->now + sasl->len; obj->done = reds_handle_auth_mechname; -async_read_handler(0, 0, &link->asyc_read); +async_read_handler(0, 0, &link->async_read); } static void reds_start_auth_sasl(RedLinkInfo *link) @@ -2520,7 +2520,7 @
[Spice-devel] [PATCH 2/3] Fix 'recive' typo throughout the code base
'receive' was mispelt 'recive' in multiple places. --- server/red_worker.c | 8 +++--- server/reds-private.h | 6 ++--- server/reds.c | 72 +-- server/snd_worker.c | 40 ++-- 4 files changed, 63 insertions(+), 63 deletions(-) diff --git a/server/red_worker.c b/server/red_worker.c index 451c8ab..afbdd91 100644 --- a/server/red_worker.c +++ b/server/red_worker.c @@ -383,7 +383,7 @@ typedef struct LocalCursor { } LocalCursor; #define MAX_PIPE_SIZE 50 -#define CHANNEL_RECIVE_BUF_SIZE 1024 +#define CHANNEL_RECEIVE_BUF_SIZE 1024 #define WIDE_CLIENT_ACK_WINDOW 40 #define NARROW_CLIENT_ACK_WINDOW 20 @@ -654,7 +654,7 @@ typedef struct GlzSharedDictionary { typedef struct CommonChannel { RedChannel base; // Must be the first thing struct RedWorker *worker; -uint8_t recv_buf[CHANNEL_RECIVE_BUF_SIZE]; +uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE]; uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme. int during_target_migrate; /* TRUE when the client that is associated with the channel is during migration. Turned off when the vm is started. @@ -1591,8 +1591,8 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint return spice_malloc(size); } -if (size > CHANNEL_RECIVE_BUF_SIZE) { -spice_critical("unexpected message size %u (max is %d)", size, CHANNEL_RECIVE_BUF_SIZE); +if (size > CHANNEL_RECEIVE_BUF_SIZE) { +spice_critical("unexpected message size %u (max is %d)", size, CHANNEL_RECEIVE_BUF_SIZE); return NULL; } return common->recv_buf; diff --git a/server/reds-private.h b/server/reds-private.h index 9d8b5d1..a33a1d7 100644 --- a/server/reds-private.h +++ b/server/reds-private.h @@ -57,9 +57,9 @@ typedef struct VDIPortState { /* read from agent */ Ring read_bufs; uint32_t read_state; -uint32_t message_recive_len; -uint8_t *recive_pos; -uint32_t recive_len; +uint32_t message_receive_len; +uint8_t *receive_pos; +uint32_t receive_len; VDIReadBuf *current_read_buf; AgentMsgFilter read_filter; diff --git a/server/reds.c b/server/reds.c index 18743a3..dd1bd68 100644 --- a/server/reds.c +++ b/server/reds.c @@ -476,9 +476,9 @@ static void reds_reset_vdp(void) SpiceCharDeviceInterface *sif; state->read_state = VDI_PORT_READ_STATE_READ_HEADER; -state->recive_pos = (uint8_t *)&state->vdi_chunk_header; -state->recive_len = sizeof(state->vdi_chunk_header); -state->message_recive_len = 0; +state->receive_pos = (uint8_t *)&state->vdi_chunk_header; +state->receive_len = sizeof(state->vdi_chunk_header); +state->message_receive_len = 0; if (state->current_read_buf) { vdi_port_read_buf_unref(state->current_read_buf); state->current_read_buf = NULL; @@ -786,43 +786,43 @@ static SpiceCharDeviceMsgToClient *vdi_port_read_one_msg_from_device(SpiceCharDe while (vdagent) { switch (state->read_state) { case VDI_PORT_READ_STATE_READ_HEADER: -n = sif->read(vdagent, state->recive_pos, state->recive_len); +n = sif->read(vdagent, state->receive_pos, state->receive_len); if (!n) { return NULL; } -if ((state->recive_len -= n)) { -state->recive_pos += n; +if ((state->receive_len -= n)) { +state->receive_pos += n; return NULL; } -state->message_recive_len = state->vdi_chunk_header.size; +state->message_receive_len = state->vdi_chunk_header.size; state->read_state = VDI_PORT_READ_STATE_GET_BUFF; case VDI_PORT_READ_STATE_GET_BUFF: { if (!(state->current_read_buf = vdi_port_read_buf_get())) { return NULL; } -state->recive_pos = state->current_read_buf->data; -state->recive_len = MIN(state->message_recive_len, +state->receive_pos = state->current_read_buf->data; +state->receive_len = MIN(state->message_receive_len, sizeof(state->current_read_buf->data)); -state->current_read_buf->len = state->recive_len; -state->message_recive_len -= state->recive_len; +state->current_read_buf->len = state->receive_len; +state->message_receive_len -= state->receive_len; state->read_state = VDI_PORT_READ_STATE_READ_DATA; } case VDI_PORT_READ_STATE_READ_DATA: -n = sif->read(vdagent, state->recive_pos, state->recive_len); +n = sif->read(vdagent, state->receive_pos, state->receive_len); if (!n) { return NULL; } -if ((state->recive_len -= n)) { -state->recive_pos += n; +if ((stat
[Spice-devel] [PATCH 1/3] Namespace RECEIVE_BUF_SIZE
--- server/main_channel.h | 4 ++-- server/red_worker.c | 8 server/snd_worker.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/main_channel.h b/server/main_channel.h index 29eb8d4..c8e9ade 100644 --- a/server/main_channel.h +++ b/server/main_channel.h @@ -30,12 +30,12 @@ #define REDS_NUM_INTERNAL_AGENT_MESSAGES 1 // approximate max receive message size for main channel -#define RECEIVE_BUF_SIZE \ +#define MAIN_CHANNEL_RECEIVE_BUF_SIZE \ (4096 + (REDS_AGENT_WINDOW_SIZE + REDS_NUM_INTERNAL_AGENT_MESSAGES) * SPICE_AGENT_MAX_DATA_SIZE) typedef struct MainChannel { RedChannel base; -uint8_t recv_buf[RECEIVE_BUF_SIZE]; +uint8_t recv_buf[MAIN_CHANNEL_RECEIVE_BUF_SIZE]; RedsMigSpice mig_target; // TODO: add refs and release (afrer all clients completed migration in one way or the other?) int num_clients_mig_wait; } MainChannel; diff --git a/server/red_worker.c b/server/red_worker.c index 8763c8e..451c8ab 100644 --- a/server/red_worker.c +++ b/server/red_worker.c @@ -383,7 +383,7 @@ typedef struct LocalCursor { } LocalCursor; #define MAX_PIPE_SIZE 50 -#define RECIVE_BUF_SIZE 1024 +#define CHANNEL_RECIVE_BUF_SIZE 1024 #define WIDE_CLIENT_ACK_WINDOW 40 #define NARROW_CLIENT_ACK_WINDOW 20 @@ -654,7 +654,7 @@ typedef struct GlzSharedDictionary { typedef struct CommonChannel { RedChannel base; // Must be the first thing struct RedWorker *worker; -uint8_t recv_buf[RECIVE_BUF_SIZE]; +uint8_t recv_buf[CHANNEL_RECIVE_BUF_SIZE]; uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme. int during_target_migrate; /* TRUE when the client that is associated with the channel is during migration. Turned off when the vm is started. @@ -1591,8 +1591,8 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint return spice_malloc(size); } -if (size > RECIVE_BUF_SIZE) { -spice_critical("unexpected message size %u (max is %d)", size, RECIVE_BUF_SIZE); +if (size > CHANNEL_RECIVE_BUF_SIZE) { +spice_critical("unexpected message size %u (max is %d)", size, CHANNEL_RECIVE_BUF_SIZE); return NULL; } return common->recv_buf; diff --git a/server/snd_worker.c b/server/snd_worker.c index ebddfcd..19171e3 100644 --- a/server/snd_worker.c +++ b/server/snd_worker.c @@ -42,7 +42,7 @@ #define IOV_MAX 1024 #endif -#define RECIVE_BUF_SIZE (16 * 1024 * 2) +#define SND_RECIVE_BUF_SIZE (16 * 1024 * 2) #define FRAME_SIZE 256 #define PLAYBACK_BUF_SIZE (FRAME_SIZE * 4) @@ -50,7 +50,7 @@ #define CELT_BIT_RATE (64 * 1024) #define CELT_COMPRESSED_FRAME_BYTES (FRAME_SIZE * CELT_BIT_RATE / SPICE_INTERFACE_PLAYBACK_FREQ / 8) -#define RECORD_SAMPLES_SIZE (RECIVE_BUF_SIZE >> 2) +#define RECORD_SAMPLES_SIZE (SND_RECIVE_BUF_SIZE >> 2) enum PlaybackeCommand { SND_PLAYBACK_MIGRATE, @@ -112,7 +112,7 @@ struct SndChannel { } send_data; struct { -uint8_t buf[RECIVE_BUF_SIZE]; +uint8_t buf[SND_RECIVE_BUF_SIZE]; uint8_t *message_start; uint8_t *now; uint8_t *end; -- 1.8.3.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 0/3]
Hey, The following 3 patches fix a few typos, and also make sure we don't have multiple constants named RECEIVE_BUF_SIZE in the codebase. Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] RFC: Integrating Virgil and Spice
Hi All, I realize that it may be a bit early to start this discussion, given the somewhat preliminary state of Virgil, still I would like to start a discussion about this now for 2 reasons: 1) I believe it would be good to start thinking about this earlier rather then later. 2) I would like to present a general overview of a plan for this at kvm-forum to get input on this from the wider kvm-forum community. I've already had a quick discussion about this with Dave Airlie, and our ideas on this aligned perfectly. The basic idea is to use qemu's console layer (include/ui/console.h) as an abstraction between the new virtio-vga device Dave has in mind (which will include optional 3D rendering capability through VIRGIL), and various display options, ie SDL, vnc and Spice. The console layer would need some extensions for this: 1) Multi head support, a question which comes up here, is do we only add support for multiple heads on a single card, or do we also want to support multiple cards each driving a head here ? I myself tend to go with the KISS solution for now and only support a single card with multiple heads. 2) The ability for a video-card generating output to pass a dma-buf context to the display (ui in qemu terms) to get the contents from, rather then requiring the contents to be rendered to some memory buffer. This way we can save the quite expensive read-back from gpu memory of the rendered result and then copying that back to the framebuffer of the gpu for local displays (ie gtk, SDL), we would of course still need the read back of the rendered output for vnc / spice. For proper multi-head support in the ui layer for local displays, we will need to use SDL-2, either by porting the current SDL ui code to SDL-2, or by introducing a new SDL-2 ui component. The changes needed to the gtk ui for multi-head support are not clear at this moment (no-one has looked into this yet AFAIK). Regards, Hans p.s. Note that having multi-head support in qemu's console layer + a multi-head capable SDL-2 ui, means that we could also use a qxl device together with the SDL-2 ui to do multi-head locally, which could be interesting for a variety of use-cases. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] RFC on sound codec refactoring
- Original Message - > > 2. Modify qemu to change the device sample rate dynamically > > > > This seems like it should be possible. I'm working up a > > hack to try it (basically, we regen the rate at voice_enable > > time). This seems to fly in the face of 'normal' > > practice, so there is probably something horrible wrong with it. > > > > But it's fun to play :-). > > Attached is a patch that suggests my idea. I've done some hacks, > and it seems 'safe' to adjust the rate on the fly like this. line_out_ctl is driven by guest. Why would you change the rate when the device start/stop a stream? I don't think that's the right approach. I think the suggestion by Gerd & Christophe should be enough. If qemu is old, it should use 44.1 celt only. If qemu is new, it can use 48 celt or opus. This doesn't have to change dynamically, the client should be able to adapt to any of these situations. > I'd appreciate some friendly folks glancing it over before I try to > flesh it out and get myself laughed off the qemu mailing list. > > Cheers, > > Jeremy > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel