Re: [PATCH V13 3/5] libxl: add pvusb API

[Olaf Hering <olaf@aepfle.de>]

On Tue, Jan 19, Chunyan Liu wrote:

> +++ b/tools/libxl/libxl.c
> @@ -3204,7 +3204,7 @@ void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
>          aodev->dev = device;
>          aodev->callback = local_device_detach_cb;
>          aodev->force = 0;
> -        libxl__initiate_device_remove(egc, aodev);
> +        libxl__initiate_device_generic_remove(egc, aodev);
>          return;
>      }
>  
> @@ -4172,8 +4172,10 @@ out:
>   * libxl_device_vkb_destroy
>   * libxl_device_vfb_remove
>   * libxl_device_vfb_destroy
> + * libxl_device_usbctrl_remove
> + * libxl_device_usbctrl_destroy

This should be moved down to DEFINE_DEVICE_REMOVE_CUSTOM.

>   */
> -#define DEFINE_DEVICE_REMOVE(type, removedestroy, f)                    \
> +#define DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f)        \
>      int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,           \
>          uint32_t domid, libxl_device_##type *type,                      \
>          const libxl_asyncop_how *ao_how)                                \
> @@ -4193,13 +4195,19 @@ out:
>          aodev->dev = device;                                            \
>          aodev->callback = device_addrm_aocomplete;                      \
>          aodev->force = f;                                               \
> -        libxl__initiate_device_remove(egc, aodev);                      \
> +        libxl__initiate_device_##remtype##_remove(egc, aodev);          \
>                                                                          \
>      out:                                                                \
> -        if (rc) return AO_CREATE_FAIL(rc);                                    \
> +        if (rc) return AO_CREATE_FAIL(rc);                              \
>          return AO_INPROGRESS;                                           \
>      }
>  
> +#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
> +    DEFINE_DEVICE_REMOVE_EXT(type, generic, removedestroy, f)
> +
> +#define DEFINE_DEVICE_REMOVE_CUSTOM(type, removedestroy, f)  \
> +    DEFINE_DEVICE_REMOVE_EXT(type, type, removedestroy, f)
> +
>  /* Define all remove/destroy functions and undef the macro */
>  
>  /* disk */


If this is the way to move forward, please split this out into a
separate change which can be applied to staging independent of any pvusb
changes.

I think the patch needs also the #undef.


> @@ -4223,6 +4231,10 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
>  DEFINE_DEVICE_REMOVE(vtpm, remove, 0)
>  DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
>  
> +/* usbctrl */
> +DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, remove, 0)
> +DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, destroy, 1)
> +
>  /* channel/console hotunplug is not implemented. There are 2 possibilities:
>   * 1. add support for secondary consoles to xenconsoled
>   * 2. dynamically add/remove qemu chardevs via qmp messages. */

A comment should mention both libxl_device_usbctrl_remove/destroy and
libxl__initiate_device_usbctrl_remove/destroy.

Olaf