Re: [Xen-devel] [PATCH v6 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-04-24 at 14:37 +0100, George Dunlap wrote:
> On 24/04/13 13:53, Ian Campbell wrote:
> > On Wed, 2013-04-24 at 13:48 +0100, George Dunlap wrote:
> >
> >>>> +
> >>>> +/*
> >>>>     * libxl ABI compatibility
> >>>>     *
> >>>>     * The only guarantee which libxl makes regarding ABI compatibility
> >>>> @@ -735,6 +741,37 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t 
> >>>> domid, libxl_device_disk *disk,
> >>>>                           const libxl_asyncop_how *ao_how)
> >>>>                           LIBXL_EXTERNAL_CALLERS_ONLY;
> >>>>
> >>>> +/*
> >>>> + * USB
> >>>> + *
> >>>> + * For each device removed or added, one of these protocols is 
> >>>> available:
> >>>> + * - PV (i.e., PVUSB)
> >>>> + * - DEVICEMODEL (i.e, qemu)
> >>>> + *
> >>>> + * PV is available for either PV or HVM domains.  DEVICEMODEL is only
> >>>> + * available for HVM domains.  The caller can additionally specify
> >>>> + * "AUTO", in which case the library will try to determine the best
> >>>> + * protocol automatically.
> >>>> + *
> >>>> + * At the moment, the only protocol implemented is DEVICEMODEL, and the 
> >>>> only
> >>>> + * device type impelmented is HOSTDEV.
> >>>                    implemented
> >>>
> >>> If PV isn't implemented I think we should leave it out of the API for
> >>> now, when it is implemented it will need a LIBXL_HAVE_DEVICE_USB_TYPE_PV
> >>> or whatever anyway.
> >> If we return -ENOTIMP or -ENOSYS from usb_add, would that be sufficient?
> > How would the application know whether it should expose this
> > functionality to its users or not?
> >
> > It'd be pretty lame for them (either the app or the end user) to have to
> > try it and see if it worked.
> 
> On the whole, I'm beginning to think that we should just punt the whole 
> USB thing to 4.4 and do it properly the first time, instead of doing it 
> piecemeal.

I'm also starting to wonder about this amount of code so far into the
freeze.

> 
> >>>> + *
> >>>> + * This uses the qmp functionality, and is thus only available for
> >>>> + * qemu-xen, not qemu-traditional.
> >>>> + */
> >>>> +#define LIBXL_DEVICE_USB_BACKEND_DEFAULT (-1)
> >>>> +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
> >>>> +                         libxl_device_usb *dev,
> >>>> +                         const libxl_asyncop_how *ao_how)
> >>>> +                         LIBXL_EXTERNAL_CALLERS_ONLY;
> >>>> +int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,
> >>>> +                            libxl_device_usb *dev,
> >>>> +                            const libxl_asyncop_how *ao_how)
> >>>> +                            LIBXL_EXTERNAL_CALLERS_ONLY;
> >>> No _destroy or _getinfo?
> >>>
> >>> _getinfo might be optional if there isn't any interesting info, but
> >>> _destroy is a must IMHO.
> >> I'm not exactly sure what the difference at this point would be between
> >> remove and destroy.
> >  From libxl.h:
> >
> >   * libxl_device_<type>_remove(ctx, domid, device):
> >   *
> >   *   Removes the given device from the specified domain by performing
> >   *   an orderly unplug with guest co-operation. This requires that the
> >   *   guest is running.
> >   *
> >   *   This method is currently synchronous and therefore can block
> >   *   while interacting with the guest.
> >   *
> >   * libxl_device_<type>_destroy(ctx, domid, device):
> >   *
> >   *   Removes the given device from the specified domain without guest
> >   *   co-operation. It is guest specific what affect this will have on
> >   *   a running guest.
> >   *
> >   *   This function does not interact with the guest and therefore
> >   *   cannot block on the guest.
> 
> I saw that, but AFAICT there is not and never will be a distinguishable 
> difference.

I expect there will be when the PV USB front and backends are involved.

> I see that the same is true for pci devices, as there are two functions 
> which call the same function internally.  I'll do the same thing. :-)

For now that'll do I think.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel