[Spice-devel] [PATCH 2/9] add spice_usb_device_manager shared CD related api functions

[Frediano Ziglio]

> 
> From: Alexander Nezhinsky <anezhins at redhat.com>
> 
> The following functions are added:
> spice_usb_device_manager_create_shared_cd_device
> spice_usb_device_manager_is_device_shared_cd
> spice_usb_device_manager_remove_shared_cd_device
> 
> Signed-off-by: Alexander Nezhinsky <anezhins at redhat.com>
> ---
>  src/map-file             |  3 ++
>  src/usb-device-manager.c | 89 ++++++++++++++++++++++++++++++++++++++++
>  src/usb-device-manager.h | 14 +++++++
>  3 files changed, 106 insertions(+)
> 
> diff --git a/src/map-file b/src/map-file
> index 3cb9873..5ae56c3 100644
> --- a/src/map-file
> +++ b/src/map-file
> @@ -180,6 +180,9 @@ spice_usb_device_manager_get_devices_with_filter;
>  spice_usb_device_manager_get_type;
>  spice_usb_device_manager_is_device_connected;
>  spice_usb_device_manager_is_redirecting;
> +spice_usb_device_manager_create_shared_cd_device;
> +spice_usb_device_manager_remove_shared_cd_device;
> +spice_usb_device_manager_is_device_shared_cd;
>  spice_usb_device_widget_get_type;
>  spice_usb_device_widget_new;
>  spice_usbredir_channel_get_type;
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index 3a9542a..0961ef9 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -32,6 +32,7 @@
>  #endif
>  
>  #include "channel-usbredir-priv.h"
> +#include "usb-device-cd.h"
>  #endif
>  
>  #include "spice-session-priv.h"
> @@ -1439,7 +1440,94 @@ gchar *spice_usb_device_get_description(SpiceUsbDevice
> *device, const gchar *for
>  #endif
>  }
>  
> +/**
> + * spice_usb_device_manager_create_shared_cd_device:
> + * @self: a #SpiceUsbDeviceManager
> + * @filename: image or device path
> + * @err: (allow-none): a return location for a #GError, or %NULL.
> + *
> + * Creates a new shared CD device based on a disk image file
> + * or a physical CD device.
> + *
> + * Returns: %TRUE if device created successfully
> + */
> +gboolean
> +spice_usb_device_manager_create_shared_cd_device(
> +                                         SpiceUsbDeviceManager *self,
> +                                         gchar                 *filename,
> +                                         GError               **err)

style, see https://www.spice-space.org/spice-project-coding-style-and-coding-conventions.html,
here in a lot of declarations

Also for consistency in the other file "manager" is used, not "self".

It's weird that the create does not return the device so you cannot
easily use spice_usb_device_manager_remove_shared_cd_device to remove it.

> +{
> +#ifdef USE_USBREDIR
> +    SpiceUsbDeviceManagerPrivate *priv = self->priv;
> +
> +    CdEmulationParams cd_params = {
> +        .filename = filename,
> +        .delete_on_eject = 1,
> +    };
> +
> +    return create_emulated_cd(priv->context, &cd_params, err);
> +#else
> +    g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> +                        _("USB redirection support not compiled in"));
> +    return FALSE;
> +#endif
> +}
> +
> +/**
> + * spice_usb_device_manager_remove_shared_cd_device:
> + * @self: a #SpiceUsbDeviceManager
> + * @device: a #SpiceUsbDevice to remove
> + * @err: (allow-none): a return location for a #GError, or %NULL.
> + *
> + * Removes a shared CD device.
> + *
> + * Returns: %TRUE if device removed successfully
> + */
> +gboolean
> +spice_usb_device_manager_remove_shared_cd_device(SpiceUsbDeviceManager
> *self,
> +                                                 SpiceUsbDevice
> *device,
> +                                                 GError               **err)
> +{
> +#ifdef USE_USBREDIR
> +    SpiceUsbBackendDevice *bdev;
>  
> +    bdev = spice_usb_device_manager_device_to_bdev(self, device);

see below

> +    spice_usb_backend_device_eject(self->priv->context, bdev);
> +    spice_usb_backend_device_unref(bdev);
> +    return TRUE;
> +#else
> +    g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> +                        _("USB redirection support not compiled in"));
> +    return FALSE;
> +#endif
> +}
> +
> +/**
> + * spice_usb_device_manager_is_device_shared_cd:
> + * @self: a #SpiceUsbDeviceManager
> + * @device: a #SpiceUsbDevice to query
> + *
> + * Checks whether a device is shared CD.
> + *
> + * Returns: %TRUE if the device is shared CD
> + */
> +gboolean
> +spice_usb_device_manager_is_device_shared_cd(SpiceUsbDeviceManager *self,
> +                                             SpiceUsbDevice        *device)
> +{
> +#ifdef USE_USBREDIR
> +    SpiceUsbBackendDevice *bdev;
> +    gboolean is_cd;
> +
> +    bdev = spice_usb_device_manager_device_to_bdev(self, device);

Note that SpiceUsbBackendDevice is defined as

    typedef struct _SpiceUsbDevice SpiceUsbBackendDevice;

no need to call this function.

> +    is_cd = (spice_usb_backend_device_get_libdev(bdev) == NULL) ? TRUE :
> FALSE;

just

    is_cd = (spice_usb_backend_device_get_libdev(bdev) == NULL);

Also the name of the function is wrong, you are not checking if the device
is a shared cd but if is a emulated device.

> +    spice_usb_backend_device_unref(bdev);
> +
> +    return is_cd;
> +#else
> +    return FALSE;
> +#endif
> +}
>  
>  #ifdef USE_USBREDIR
>  /*
> @@ -1499,6 +1587,7 @@ gboolean spice_usb_device_is_isochronous(const
> SpiceUsbDevice *info)
>      return spice_usb_backend_device_isoch((SpiceUsbBackendDevice*) info);
>  }
>  
> +

spurious hunk, remove

>  #ifdef G_OS_WIN32
>  static
>  gboolean _usbdk_hider_prepare(SpiceUsbDeviceManager *manager)
> diff --git a/src/usb-device-manager.h b/src/usb-device-manager.h
> index 773208f..dc1a644 100644
> --- a/src/usb-device-manager.h
> +++ b/src/usb-device-manager.h
> @@ -143,6 +143,20 @@
> spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>  
>  gboolean spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager
>  *self);
>  
> +gboolean
> +spice_usb_device_manager_create_shared_cd_device(
> +                                             SpiceUsbDeviceManager *self,
> +                                             gchar
> *filename,
> +                                             GError               **err);
> +gboolean
> +spice_usb_device_manager_remove_shared_cd_device(
> +                                             SpiceUsbDeviceManager *self,
> +                                             SpiceUsbDevice        *device,
> +                                             GError               **err);
> +gboolean
> +spice_usb_device_manager_is_device_shared_cd(SpiceUsbDeviceManager *self,
> +                                             SpiceUsbDevice        *device);
> +
>  G_END_DECLS
>  
>  #endif /* __SPICE_USB_DEVICE_MANAGER_H__ */

Frediano