On Fri, Sep 28, 2018 at 12:59 PM Christophe Fergeau <cferg...@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 >
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
