Red Hat Bugzilla – Bug 616772
feature request: virtio-serial needs a callback to notify device that to guest queue is not full
Last modified: 2015-12-04 11:12:51 EST
Description of problem:
when a port is fully connected (host and guest) and after virtio_serial_write returns 0, there is no way that doesn't require polling to find out when the host to guest queue (ivq) is not full and the next write will succeed.
This feature request has been proposed after Feature Freeze and we
are unable to resolve it in time for the current Red Hat Enterprise
Linux release. It has been denied for the current release and
proposed for the next Red Hat Enterprise Linux release.
With the current version of virtio, there doesn't seem to be a way to do this -- the guest can fill up buffers for the host to send data in, but this may not be conveyed to the host at all.
A timer with a few milli-seconds worth of sleep time perhaps is better than polling.
I'll let this bug remain open for now and see if other alternatives come up.
Taking the suggestion to OASIS
Created attachment 918736 [details]
Please take a look and let me know if this is what you wanted.
I discussed that patch in attachment 918736 [details] with Alon, and he thinks it's sufficient as well. We will need to expose the callback via CharDriverState so that spice-qemu-kvm.c can use it. Once this is done, spice_server can get rid of polls in favour of the signal it'll receive via the callback.
in spice-qemu-char.c, vmc_write() calls
int can_write = qemu_chr_be_can_write(scd->chr);
If the value of can_write is 0 here, it could mean the vq is full, and spice can't write anything for the guest to consume right now. In this case, vmc_write() can register the new ->guest_writable() callback, and it will then get a notification when the vq is writable again.
The virtio-serial part of this is done, via the patch above.
The chardev work is still to be done.
Once both of those are upstream, spice-server can then be modified to use the new functionality.
Note for future spice implementation:
* No need for new api, simply retry writing on spice_server_char_device_wakeup.
* Then we can remove the write_to_dev_timer timer in spice/server/char_device.c
Two options for handling backwards compatibility:
* don't remove the timer. So for new implementations we will still do redundant attempts to write but since it's the same process the overhead should be negligble.
* add a capability bit. We don't have capabilities in the spice server interface, so this could be done either by first adding capabilities and then using them, or adding a new symbol to be called by qemu setting the state (defaults to false) of a wakeup on guest write availability.
(In reply to Alon Levy from comment #8)
> Note for future spice implementation:
> * No need for new api, simply retry writing on
Ok, so it will need to retry write as well (read only atm)
> * Then we can remove the write_to_dev_timer timer in
> Two options for handling backwards compatibility:
> * don't remove the timer. So for new implementations we will still do
> redundant attempts to write but since it's the same process the overhead
> should be negligble.
> * add a capability bit. We don't have capabilities in the spice server
> interface, so this could be done either by first adding capabilities and
> then using them, or adding a new symbol to be called by qemu setting the
> state (defaults to false) of a wakeup on guest write availability.
I would add a SpiceCharDeviceInterface.flags = SPICE_CHAR_DEVICE_FLAG_DISABLE_TIMER
that sounds simple, but should go upstream/fedora first.
Amit, any chance you propose your patch to qemu upstream soon?
I think we should open a duplicate for tracking spice-server patch.
(In reply to Marc-Andre Lureau from comment #9)
> Amit, any chance you propose your patch to qemu upstream soon?
Without the chardev part of it, there's not much utility to it, I suppose? It does make a good standalone patch, but I'd rather have a proper patchset that uses the functionality here.
Can't really say when the chardev work will happen, though. I'll discuss with Amos and either re-assign to myself or Amos will comment here.
Marc-Andre posted spice patches upstream, and they look fine. All the patches are upstream, and will likely land in early in the qemu-2.3 cycle.
The patch for virtio-serial is now upstream. Marc-Andre's series for using this API in spice will go in 2.4. Marc-Andre, please open a bug for spice if you'd like to backport them.
According to comment23 and comment24, set this issue as verified.
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.
For information on the advisory, and where to find the updated
files, follow the link below.
If the solution does not work for you, open a new bug report.