RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 616772 - feature request: virtio-serial needs a callback to notify device that to guest queue is not full
Summary: feature request: virtio-serial needs a callback to notify device that to gues...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm-rhev
Version: 7.0
Hardware: All
OS: Linux
low
low
Target Milestone: beta
: ---
Assignee: Amit Shah
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks: 580953
TreeView+ depends on / blocked
 
Reported: 2010-07-21 12:01 UTC by Alon Levy
Modified: 2015-12-04 16:12 UTC (History)
16 users (show)

Fixed In Version: qemu-2.3
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-04 16:12:51 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Proposed patch (2.60 KB, patch)
2014-07-17 14:39 UTC, Amit Shah
amit.shah: review? (alevy)
Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:2546 0 normal SHIPPED_LIVE qemu-kvm-rhev bug fix and enhancement update 2015-12-04 21:11:56 UTC

Description Alon Levy 2010-07-21 12:01:29 UTC
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.

Comment 2 RHEL Program Management 2010-07-21 12:17:15 UTC
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.

Comment 3 Amit Shah 2010-10-11 11:59:47 UTC
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.

Comment 5 Ronen Hod 2013-11-14 12:28:33 UTC
Taking the suggestion to OASIS

Comment 6 Amit Shah 2014-07-17 14:39:51 UTC
Created attachment 918736 [details]
Proposed patch

Hey Alon,

Please take a look and let me know if this is what you wanted.

Thanks,

Comment 7 Amit Shah 2014-07-21 12:17:43 UTC
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.

More details:
 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.

Comment 8 Alon Levy 2014-07-21 12:28:17 UTC
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.

Comment 9 Marc-Andre Lureau 2014-07-21 14:53:22 UTC
(In reply to Alon Levy from comment #8)
> Note for future spice implementation:
> * No need for new api, simply retry writing on
> spice_server_char_device_wakeup.

Ok, so it will need to retry write as well (read only atm)

> * 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.

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.

Comment 10 Amit Shah 2014-07-21 16:56:30 UTC
(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.

Comment 12 Amit Shah 2014-11-10 17:00:11 UTC
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.

Comment 13 Amit Shah 2015-05-15 11:11:23 UTC
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.

Comment 25 juzhang 2015-08-10 05:27:51 UTC
According to comment23 and comment24, set this issue as verified.

Comment 27 errata-xmlrpc 2015-12-04 16:12:51 UTC
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.

https://rhn.redhat.com/errata/RHBA-2015-2546.html


Note You need to log in before you can comment on or make changes to this bug.