[Spice-devel] [spice-gtk v1 0/2] Add usb backend module

[Yuri Benditovich]

On Fri, Sep 28, 2018 at 12:59 PM Christophe Fergeau <cfergeau at redhat.com>
wrote:

> On Thu, Sep 27, 2018 at 06:56:12PM +0300, Yuri Benditovich wrote:
> Hey,
>
> > Christophe,
> >
> > Our primary goal, if you remember, was to deliver cd-sharing feature.
> > For that it was needed to isolate usb redirection modules from libusb and
> > usbredir to create some abstraction layer for USB devices whether they
> are
> > local or emulated in software.
> > SpiceUsbDevice is type of object that Usb Device Manager uses internally,
> > opaque for other modules and external API, with its own life cycle and
> with
> > system dependent structure. It is referenced by external components (as
> > widget or external application).
> > Introduced by us SpiceUsbBackendDevice is used by abstraction layer
> > internally, with its own life cycle and opaque for all other modules with
> > system-independent structure.
> > Current commit makes minimal changes in existing modules and serves the
> > initial goal.
>
> As you said, there's already a SpiceUsbDevice abstraction which is
> opaque to code outside of usb-device-manager.c. Even
> usb-device-manager.c is not heavily tied to it, as it mostly uses
> accessors when it needs to interact with it.
> Your work adds an additional SpiceUsbDeviceBackend abstraction, which
> indeed abstracts some more SpiceUsbDevice by hiding the libusb_device
> pointer. In doing so, it duplicates most of what SpiceUsbDevice already
> has, I'd rather we don't duplicate too many things without a very good
> reason..
> Regarding the lifecycle when you take a look at channel-usbredir.c,
> references on SpiceUsbDevice and SpiceUsbDeviceBackend are
> ref'ed/unref'ed at the same time, so their lifecycles are not as
> independant as one might think.
>
> > Your suggestion breaks the initial goal and moves us to additional
> > refactoring of already prepared 'engineering stable' code and additional
> > complicated (IMO) reintegration with USB device manager.
>
> The goal is both to have something which works, and something which is
> maintainable upstream.. Limiting the amount of abstractions/duplicate
> code we have to deal with definitely helps with the maintainability.
>
> > This also breaks the original idea of isolation - such a way all the
> > internals of 'backend device' will be visible to usb device manager,
> > including libusb.
>
> This does not sound like what I suggested:
> « In short, I'd move SpiceUsbDeviceInfo definition from
> usb-device-manager.c to a new usb-device.c (aka usb-device-backend.c) »
> in other words, make SpiceUsbDevice opaque from usb-device-manager.c,
> which would not make it much different from SpiceUsbDeviceBackend..
>
>
> > From my point of view, the order should be following:
> > 1. Finalize step 1 patches (isolation, opaque usb backend)
> > 2. Add cd-sharing  feature above it
> > 3. Finalize UI solution that serves cd-sharing
> > 4. Reduce differences between Linux and Windows in Usb device manager
> > (persistent backend device object)
>
> Actually, whether we can have persistent backend device objects on
> Windows or not impacts the isolation work, so it would be good to know
> that before 1... If we need to 'renew' libusb_device pointers before
> using them, then it would be better not to keep it in
> SpiceUsbDeviceBackend when building for Windows (yes, exactly like what
> is done for SpiceUsbDevice at the moment..)
>

Know whether we can have persistent backend and libusb device on Windows
means
preparing the POC that does it.

Do you suggest different order of the changes in usb-redir than I've
suggested?
If yes, can you please share it with us?

Thanks,
Yuri



>
> Christophe
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180928/4f5c8947/attachment.html>