Re: [Qemu-devel] [PATCH] usb-redir: Allow to attach USB 2.0 devices to 1.1 host controller

[Hans de Goede]

Hi,

On 09/18/2012 11:18 PM, Anthony Liguori wrote:
> Hans de Goede <address@hidden> writes:
>
> > Hi,
> >
> > On 09/17/2012 11:18 AM, Jan Kiszka wrote:
> > > On 2012-09-17 11:08, Hans de Goede wrote:
> >
> > <snip>
> >
> > > > Although not pretty I'm ok with this, since I actually want to add
> > > > similar code to allow usb-3 (superspeed) devices like a usb-3 usb-stick
> > > > to work with ehci or uhci controllers :)
> > >
> > > Great, that would have been my next question, but I don't have hardware
> > > for that around yet.
> >
> > I do have hardware for that around, so once you've respun your patch to
> > address the issues discussed, then that will give me a nice basis to
> > add usb-3 usb-stick to ehci-controller redirection :)
> >
> > > BTW, I'm facing several incompatibilities with passed-through CDC/ACM
> > > devices (e.g. a Galaxy S2), independent of my patch. Both host-linux and
> > > redir doesn't allow to use them properly but show different symptoms.
> > > Need to analyze and report once time permits.
> >
> > Hmm, there is (was) one know issues with these devices, which has been fixed
> > in usbredir, so first of all make sure that the usbredir on your spice-client
> > / usbredirserver, has this patch:
> > http://cgit.freedesktop.org/spice/usbredir/commit/?id=7783d3db61083bbf7f61b1ea8608c666b4c6a1dd
> >
> > If that does not work, add the debug parameter to the usb-redir device, set it
> > to 4, collect logs of trying to redirect the device and send me the logs
> > please, ie:
> > -device usb-redir,chardev=usbredirchardev1,id=usbredirdev1,debug=4
> >
> > Also be aware that usb-redir relies on chardev flowcontrol working,
> > which it does not upstream! See for example here for the chardev flow
> > control patch set which RHEL / Fedora carry:
> > http://cgit.freedesktop.org/~jwrdegoede/qemu/log/?h=qemu-kvm-1.2-usbredir&ofs=50
>
> Those patches are garbage.

<sigh, very very deep sigh>

No they are not garbage. They:

1) Fix a real issue, which is independent of spice / usb-redir but happens to be
more easily triggerable with them. Note that the original set was developed to
fix a bug which did not involve either!

2) Are disliked by you because they build upon the existing not pretty chardev
code. We all agree that needs a rewrite. But that is not a valid reason to
keep this perfectly fine patchset out of qemu master for years now!

3) You keep saying they are "racy", but you've never given a simple step by
step thread 1 does foo, thread 2 does bar replay of such a theoretical race.

4) Have caused no issues in almost 2 years of deployment in the field in both
RHEL and Fedora.

5) Are kept out of master by you for no valid reasons, you seem to use them
as a tool to strongarm people into rewriting the qemu chardev layer. Well it
seems that no one is biting because of -ENOTIME, so can we now please get
these included, after almost 2 years since their first posting ?

> Are you saying that usb-redir doesn't work in qemu.git?  So why are we
> even carrying it?

It works just as well as virtio-console or any other chardev frontend, any 
chardev
frontent can currently trigger asserts by writing data faster then the backend
can handle. This is just more easily triggerable with USB trafic then with a
serial console.

The original bug-report this patch set once was written for was about 
virtio-console
and is dated 2010-08-05 !

So why are we even carrying virtio-console ?

Regards,

Hans

p.s.

Note that I'm *not* the original author of these patches.