On Mon, Jun 10, 2019 at 2:15 PM Victor Toso <victortoso@xxxxxxxxxx> wrote:
>
> On Wed, Apr 10, 2019 at 10:31:39PM +0300, Yuri Benditovich wrote:
> > Do not keep bus, address, vid and pid of the USB device
> > in SpiceUsbDeviceInfo structure. Getters for these properties
> > can easily obtain them from respective backend device.
>
> Why should we keep this structure at all? gboolean isochronous is
> set with spice_usb_backend_device_isoch() so we basically have a
> referece counter and a pointer to SpiceUsbBackendDevice after
> this patch.
SpiceUsbDeviceInfo keeps the info about isochronous in order to
avoid checking it each time we need it, i.e. upon each device redirection
(this involves getting full device configuration from libusb and parsing
all the interfaces to find isoch endpoint). If we place it into
SpiceUsbBackendDevice
we will need to check it each time the SpiceUsbBackendDevice created.
On Linux this could be OK as on Linux we create SpiceUsbBackendDevice
only when new device comes. On Windows this is each time we enumerate
devices (on each 'device change' indication from PnP.
This is a reason why it is placed into SpiceUsbDeviceInfo.
>
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > ---
> > src/usb-device-manager.c | 29 +++++++++++++----------------
> > 1 file changed, 13 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > index 2bee55d..6898472 100644
> > --- a/src/usb-device-manager.c
> > +++ b/src/usb-device-manager.c
> > @@ -129,13 +129,9 @@ enum {
> > #ifdef USE_USBREDIR
> >
> > typedef struct _SpiceUsbDeviceInfo {
> > - guint8 busnum;
> > - guint8 devaddr;
> > - guint16 vid;
> > - guint16 pid;
> > - gboolean isochronous;
> > SpiceUsbBackendDevice *bdev;
> > gint ref;
> > + gboolean isochronous;
> > } SpiceUsbDeviceInfo;
> >
> >
> > @@ -1613,18 +1609,11 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice *device, const gchar *for
> > static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev)
> > {
> > SpiceUsbDeviceInfo *info;
> > - const UsbDeviceInformation *bdev_info;
> >
> > g_return_val_if_fail(bdev != NULL, NULL);
> >
> > - bdev_info = spice_usb_backend_device_get_info(bdev);
> > -
> > info = g_new0(SpiceUsbDeviceInfo, 1);
> >
> > - info->busnum = bdev_info->bus;
> > - info->devaddr = bdev_info->address;
> > - info->vid = bdev_info->vid;
> > - info->pid = bdev_info->pid;
> > info->ref = 1;
> > info->bdev = spice_usb_backend_device_ref(bdev);
> > info->isochronous = spice_usb_backend_device_isoch(bdev);
> > @@ -1635,37 +1624,45 @@ static SpiceUsbDeviceInfo *spice_usb_device_new(SpiceUsbBackendDevice *bdev)
> > guint8 spice_usb_device_get_busnum(const SpiceUsbDevice *device)
> > {
> > const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device;
> > + const UsbDeviceInformation *b_info;
> >
> > g_return_val_if_fail(info != NULL, 0);
> >
> > - return info->busnum;
> > + b_info = spice_usb_backend_device_get_info(info->bdev);
> > + return b_info->bus;
> > }
> >
> > guint8 spice_usb_device_get_devaddr(const SpiceUsbDevice *device)
> > {
> > const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device;
> > + const UsbDeviceInformation *b_info;
> >
> > g_return_val_if_fail(info != NULL, 0);
> >
> > - return info->devaddr;
> > + b_info = spice_usb_backend_device_get_info(info->bdev);
> > + return b_info->address;
> > }
> >
> > guint16 spice_usb_device_get_vid(const SpiceUsbDevice *device)
> > {
> > const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device;
> > + const UsbDeviceInformation *b_info;
> >
> > g_return_val_if_fail(info != NULL, 0);
> >
> > - return info->vid;
> > + b_info = spice_usb_backend_device_get_info(info->bdev);
> > + return b_info->vid;
> > }
> >
> > guint16 spice_usb_device_get_pid(const SpiceUsbDevice *device)
> > {
> > const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device;
> > + const UsbDeviceInformation *b_info;
> >
> > g_return_val_if_fail(info != NULL, 0);
> >
> > - return info->pid;
> > + b_info = spice_usb_backend_device_get_info(info->bdev);
> > + return b_info->pid;
> > }
> >
> > gboolean spice_usb_device_is_isochronous(const SpiceUsbDevice *device)
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel