On Thu, Aug 8, 2019 at 1:41 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> Change reference counting semantic for emulated devices.
> Make the same as not emulated.
> This fix a leak if spice_usb_backend_device_eject is not called.
> Consistently the reference counter for SpiceUsbBackendDevice is
> now the number of owning pointers.
> ---
> src/usb-backend.c | 19 ++++++++++---------
> src/usb-emulation.h | 2 +-
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/src/usb-backend.c b/src/usb-backend.c
> index d80da177..de2b55ec 100644
> --- a/src/usb-backend.c
> +++ b/src/usb-backend.c
> @@ -852,17 +852,17 @@ void spice_usb_backend_device_report_change(SpiceUsbBackend *be,
>
> void spice_usb_backend_device_eject(SpiceUsbBackend *be, SpiceUsbBackendDevice *dev)
> {
> - g_return_if_fail(dev && dev->edev);
> + 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);
> }
> - be->own_devices_mask &= ~(1 << dev->device_info.address);
> -
> - spice_usb_backend_device_unref(dev);
> }
>
> -SpiceUsbBackendDevice*
> +gboolean
> spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> SpiceUsbEmulatedDeviceCreate create_proc,
> void *create_params,
> @@ -877,7 +877,7 @@ spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> if (be->own_devices_mask == 0xffffffff) {
> g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("can't create device - limit reached"));
> - return NULL;
> + return FALSE;
> }
> for (address = 0; address < 32; ++address) {
> if (~be->own_devices_mask & (1 << address)) {
> @@ -893,7 +893,7 @@ spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> dev->edev = edev = create_proc(be, dev, create_params, err);
> if (edev == NULL) {
> spice_usb_backend_device_unref(dev);
> - return NULL;
> + return FALSE;
> }
>
> if (!device_ops(edev)->get_descriptor(edev, LIBUSB_DT_DEVICE, 0,
> @@ -903,7 +903,7 @@ spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> spice_usb_backend_device_unref(dev);
> g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("can't create device - internal error"));
> - return NULL;
> + return FALSE;
> }
>
> be->own_devices_mask |= 1 << address;
> @@ -918,8 +918,9 @@ spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> if (be->hotplug_callback) {
> be->hotplug_callback(be->hotplug_user_data, dev, TRUE);
> }
> + spice_usb_backend_device_unref(dev);
I was thinking about doing that... What looks for me a little
problematic in such solution is that cd-device keeps a pointer to the
backend device, but this device is referenced only by
usb-device-manager and never referenced by anybody at backend area.
If you're ok with this fixup, let's use it. I see it working in all
functional flows and in live migration.
>
> - return dev;
> + return TRUE;
> }
>
> #endif /* USB_REDIR */
> diff --git a/src/usb-emulation.h b/src/usb-emulation.h
> index 9e626a24..46d54d47 100644
> --- a/src/usb-emulation.h
> +++ b/src/usb-emulation.h
> @@ -80,7 +80,7 @@ static inline const UsbDeviceOps *device_ops(SpiceUsbEmulatedDevice *dev)
> return (const UsbDeviceOps *)dev;
> }
>
> -SpiceUsbBackendDevice*
> +gboolean
> spice_usb_backend_create_emulated_device(SpiceUsbBackend *be,
> SpiceUsbEmulatedDeviceCreate create_proc,
> void *create_params,
> --
> 2.20.1
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel