>
> On Thu, Aug 22, 2019 at 1:18 PM Victor Toso <victortoso@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Mon, Aug 12, 2019 at 12:57:21PM +0300, Yuri Benditovich wrote:
> > > SpiceUsbBackendDevice structure is extended to support
> > > additional kind of device that is emulated by Spice-GTK
> > > and not present locally (and does not have libusb_device),
> > > such device has instead pointer to SpiceUsbEmulatedDevice
> > > abstract structure. Specific implementation of such device
> > > depends on its device type. Implementation module will define
> > > constructor for specific device type.
> > > Device structure is abstract but always starts from table of
> > > virtual functions required to redirect such virtual device.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > > ---
> > > src/meson.build | 1 +
> > > src/usb-backend.c | 103 +++++++++++++++++++++++++++++++++++++++++++-
> > > src/usb-backend.h | 3 ++
> > > src/usb-emulation.h | 88 +++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 193 insertions(+), 2 deletions(-)
> > > create mode 100644 src/usb-emulation.h
> > >
> > > diff --git a/src/meson.build b/src/meson.build
> > > index b4a6870..fe19c16 100644
> > > --- a/src/meson.build
> > > +++ b/src/meson.build
> > > @@ -122,6 +122,7 @@ spice_client_glib_sources = [
> > > 'usbutil.c',
> > > 'usbutil.h',
> > > 'usb-backend.c',
> > > + 'usb-emulation.h',
> > > 'usb-backend.h',
> > > 'vmcstream.c',
> > > 'vmcstream.h',
> > > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > > index 9ac8595..de2b55e 100644
> > > --- a/src/usb-backend.c
> > > +++ b/src/usb-backend.c
> > > @@ -39,6 +39,7 @@
> > > #include "usbredirparser.h"
> > > #include "spice-util.h"
> > > #include "usb-backend.h"
> > > +#include "usb-emulation.h"
> > > #include "channel-usbredir-priv.h"
> > > #include "spice-channel-priv.h"
> > >
> > > @@ -46,7 +47,10 @@
> > >
> > > struct _SpiceUsbBackendDevice
> > > {
> > > + /* Pointer to device. Either real device (libusb_device)
> > > + * or emulated one (edev) */
> > > libusb_device *libusb_device;
> > > + SpiceUsbEmulatedDevice *edev;
> > > gint ref_count;
> > > SpiceUsbBackendChannel *attached_to;
> > > UsbDeviceInformation device_info;
> > > @@ -66,6 +70,10 @@ struct _SpiceUsbBackend
> > > libusb_device **libusb_device_list;
> > > gint redirecting;
> > > #endif
> > > +
> > > + /* Mask of allocated device, a specific bit set to 1 to indicate
> > > that the device at
> > > + * that address is allocated */
> > > + uint32_t own_devices_mask;
> > > };
> > >
> > > struct _SpiceUsbBackendChannel
> > > @@ -408,6 +416,8 @@ SpiceUsbBackend *spice_usb_backend_new(GError
> > > **error)
> > > libusb_set_option(be->libusb_context, LIBUSB_OPTION_USE_USBDK);
> > > #endif
> > > #endif
> > > + /* exclude addresses 0 (reserved) and 1 (root hub) */
> > > + be->own_devices_mask = 3;
> > > }
> > > SPICE_DEBUG("%s <<", __FUNCTION__);
> > > return be;
> > > @@ -524,8 +534,13 @@ void
> > > spice_usb_backend_device_unref(SpiceUsbBackendDevice *dev)
> > > {
> > > LOUD_DEBUG("%s >> %p(%d)", __FUNCTION__, dev, dev->ref_count);
> > > if (g_atomic_int_dec_and_test(&dev->ref_count)) {
> > > - libusb_unref_device(dev->libusb_device);
> > > - LOUD_DEBUG("%s freeing %p (libusb %p)", __FUNCTION__, dev,
> > > dev->libusb_device);
> > > + if (dev->libusb_device) {
> > > + libusb_unref_device(dev->libusb_device);
> > > + LOUD_DEBUG("%s freeing %p (libusb %p)", __FUNCTION__, dev,
> > > dev->libusb_device);
> > > + }
> > > + if (dev->edev) {
> > > + device_ops(dev->edev)->unrealize(dev->edev);
> > > + }
> > > g_free(dev);
> > > }
> > > }
> > > @@ -824,4 +839,88 @@
> > > spice_usb_backend_channel_get_guest_filter(SpiceUsbBackendChannel *ch,
> > > }
> > > }
> > >
> > > +void spice_usb_backend_device_report_change(SpiceUsbBackend *be,
> > > + SpiceUsbBackendDevice *dev)
> > > +{
> > > + gchar *desc;
> > > + g_return_if_fail(dev && dev->edev);
> > > +
> > > + desc = device_ops(dev->edev)->get_product_description(dev->edev);
> > > + SPICE_DEBUG("%s: %s", __FUNCTION__, desc);
> > > + g_free(desc);
> > > +}
> > > +
> > > +void spice_usb_backend_device_eject(SpiceUsbBackend *be,
> > > SpiceUsbBackendDevice *dev)
> > > +{
> > > + g_return_if_fail(dev);
> > > +
> > > + if (dev->edev) {
> > > + be->own_devices_mask &= ~(1 << dev->device_info.address);
> > > + }
> > > + if (be->hotplug_callback) {
> > > + be->hotplug_callback(be->hotplug_user_data, dev, FALSE);
> > > + }
> > > +}
> > > +
> > > +gboolean
> > > +spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> > > + SpiceUsbEmulatedDeviceCreate
> > > create_proc,
> > > + void *create_params,
> > > + GError **err)
> > > +{
> > > + SpiceUsbEmulatedDevice *edev;
> > > + SpiceUsbBackendDevice *dev;
> > > + struct libusb_device_descriptor *desc;
> > > + uint16_t device_desc_size;
> > > + uint8_t address = 0;
> > > +
> > > + if (be->own_devices_mask == 0xffffffff) {
> > > + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> > > + _("can't create device - limit reached"));
> > > + return FALSE;
> > > + }
> > > + for (address = 0; address < 32; ++address) {
> > > + if (~be->own_devices_mask & (1 << address)) {
> > > + break;
> > > + }
> > > + }
> > > +
> > > + dev = g_new0(SpiceUsbBackendDevice, 1);
> > > + dev->device_info.bus = BUS_NUMBER_FOR_EMULATED_USB;
> > > + dev->device_info.address = address;
> > > + dev->ref_count = 1;
> > > +
> > > + dev->edev = edev = create_proc(be, dev, create_params, err);
> >
> > I take creating emulated devices can take some time, isn't it
> > worth to do async?
>
> I do not see any point where it can take perceptible time.
> Creation of the device is 'go - no go' and fails only in case the file
> name is wrong.
>
I agree with Yuri. Also if the device is really complicated and needs some
long initialization probably would better to return a valid device and
fail later at the USB layer.
For instance in this specific case you can return an emulated CD device
with no emulated disc in it and later send to the guest OS that a disc was
inserted.
> >
> > > + if (edev == NULL) {
> > > + spice_usb_backend_device_unref(dev);
> > > + return FALSE;
> > > + }
> > > +
> > > + if (!device_ops(edev)->get_descriptor(edev, LIBUSB_DT_DEVICE, 0,
> > > + (void **)&desc,
> > > &device_desc_size)
> > > + || device_desc_size != sizeof(*desc)) {
> > > +
> > > + spice_usb_backend_device_unref(dev);
> > > + g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> > > + _("can't create device - internal error"));
> > > + return FALSE;
> > > + }
> > > +
> > > + be->own_devices_mask |= 1 << address;
> > > +
> > > + dev->device_info.vid = desc->idVendor;
> > > + dev->device_info.pid = desc->idProduct;
> > > + dev->device_info.bcdUSB = desc->bcdUSB;
> > > + dev->device_info.class = desc->bDeviceClass;
> > > + dev->device_info.subclass = desc->bDeviceSubClass;
> > > + dev->device_info.protocol = desc->bDeviceProtocol;
> > > +
> > > + if (be->hotplug_callback) {
> > > + be->hotplug_callback(be->hotplug_user_data, dev, TRUE);
> > > + }
> > > + spice_usb_backend_device_unref(dev);
> > > +
> > > + return TRUE;
> > > +}
> > > +
> > > #endif /* USB_REDIR */
> > > diff --git a/src/usb-backend.h b/src/usb-backend.h
> > > index 66e13f5..cd3ff09 100644
> > > --- a/src/usb-backend.h
> > > +++ b/src/usb-backend.h
> > > @@ -30,12 +30,15 @@ typedef struct _SpiceUsbBackend SpiceUsbBackend;
> > > typedef struct _SpiceUsbBackendDevice SpiceUsbBackendDevice;
> > > typedef struct _SpiceUsbBackendChannel SpiceUsbBackendChannel;
> > >
> > > +#define BUS_NUMBER_FOR_EMULATED_USB G_MAXUINT16
> > > +
> > > typedef struct UsbDeviceInformation
> > > {
> > > uint16_t bus;
> > > uint16_t address;
> > > uint16_t vid;
> > > uint16_t pid;
> > > + uint16_t bcdUSB;
> > > uint8_t class;
> > > uint8_t subclass;
> > > uint8_t protocol;
> > > diff --git a/src/usb-emulation.h b/src/usb-emulation.h
> > > new file mode 100644
> > > index 0000000..ac3d8e0
> > > --- /dev/null
> > > +++ b/src/usb-emulation.h
> > > @@ -0,0 +1,88 @@
> > > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > +/*
> > > + Copyright (C) 2019 Red Hat, Inc.
> > > +
> > > + Red Hat Authors:
> > > + Yuri Benditovich<ybendito@xxxxxxxxxx>
> > > +
> > > + This library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later version.
> > > +
> > > + This library is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General Public
> > > + License along with this library; if not, see
> > > <http://www.gnu.org/licenses/>.
> > > +*/
> > > +
> > > +#pragma once
> > > +
> > > +#include "usbredirparser.h"
> > > +#include "usb-backend.h"
> > > +
> > > +typedef struct SpiceUsbEmulatedDevice SpiceUsbEmulatedDevice;
> > > +typedef SpiceUsbEmulatedDevice*
> > > +(*SpiceUsbEmulatedDeviceCreate)(SpiceUsbBackend *be,
> > > + SpiceUsbBackendDevice *parent,
> > > + void *create_params,
> > > + GError **err);
> > > +
> > > +/*
> > > + function table for emulated USB device
> > > + must be first member of device structure
> > > + all functions are mandatory for implementation
> > > +*/
> > > +typedef struct UsbDeviceOps {
> > > + gboolean (*get_descriptor)(SpiceUsbEmulatedDevice *device,
> > > + uint8_t type, uint8_t index,
> > > + void **buffer, uint16_t *size);
> > > + gchar * (*get_product_description)(SpiceUsbEmulatedDevice *device);
> > > + void (*attach)(SpiceUsbEmulatedDevice *device, struct usbredirparser
> > > *parser);
> > > + void (*reset)(SpiceUsbEmulatedDevice *device);
> > > + /*
> > > + processing is synchronous, default = stall:
> > > + - return success without data: set status to 0
> > > + - return error - set status to error
> > > + - return success with data - set status to 0,
> > > + set buffer to some buffer
> > > + set length to out len
> > > + truncation is automatic
> > > + */
> > > + void (*control_request)(SpiceUsbEmulatedDevice *device,
> > > + uint8_t *data, int data_len,
> > > + struct usb_redir_control_packet_header *h,
> > > + void **buffer);
> > > + /*
> > > + processing is synchronous:
> > > + - set h->status to resulting status, default = stall
> > > + */
> > > + void (*bulk_out_request)(SpiceUsbEmulatedDevice *device,
> > > + uint8_t ep, uint8_t *data, int data_len,
> > > + uint8_t *status);
> > > + /*
> > > + if returns true, processing is asynchronous
> > > + otherwise header contains error status
> > > + */
> > > + gboolean (*bulk_in_request)(SpiceUsbEmulatedDevice *device, uint64_t
> > > id,
> > > + struct usb_redir_bulk_packet_header
> > > *bulk_header);
> > > + void (*cancel_request)(SpiceUsbEmulatedDevice *device, uint64_t id);
> > > + void (*detach)(SpiceUsbEmulatedDevice *device);
> > > + void (*unrealize)(SpiceUsbEmulatedDevice *device);
> > > +} UsbDeviceOps;
> > > +
> > > +static inline const UsbDeviceOps *device_ops(SpiceUsbEmulatedDevice
> > > *dev)
> > > +{
> > > + return (const UsbDeviceOps *)dev;
> > > +}
> > > +
> > > +gboolean
> > > +spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> > > + SpiceUsbEmulatedDeviceCreate
> > > create_proc,
> > > + void *create_params,
> > > + GError **err);
> > > +void spice_usb_backend_device_eject(SpiceUsbBackend *be,
> > > SpiceUsbBackendDevice *device);
> > > +void spice_usb_backend_device_report_change(SpiceUsbBackend *be,
> > > SpiceUsbBackendDevice *device);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel