[Spice-devel] [PATCH 3/3] usb-redirection: do not duplicate USB device properties

[Yuri Benditovich]

On Mon, Jun 10, 2019 at 2:15 PM Victor Toso <victortoso at redhat.com> 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 at daynix.com>
> > ---
> >  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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel