Re: [Qemu-devel] [PATCH 1/5] usb-ccid: add CCID bus

[Gerd Hoffmann <kraxel@redhat.com>]

On 12/12/10 19:37, Alon Levy wrote:
> A CCID device is a smart card reader. It is a USB device, defined at [1].
> This patch introduces the usb-ccid device that is a ccid bus. Next patches will
> introduce two card types to use it, a passthru card and an emulated card.

Looks good overall, just some minor nits / questions:

> +struct CCIDCardState {
> +    DeviceState qdev;

Add "uint32_t slot" here?

Also adding a bus property for it would be good, even though it can't be 
anything but '0' right now, to prepare the interfaces for the day when 
we'll support more than a single card slot and thus can attach multiple 
cards to the ccid bus.

> +static VMStateDescription ccid_vmstate = {
> +    .name = CCID_DEV_NAME,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = ccid_post_load,
> +    .pre_save = ccid_pre_save,
> +    .fields = (VMStateField []) {
> +        VMSTATE_STRUCT(dev, USBCCIDState, 1, usb_device_vmstate, USBDevice),

Can we please disable this for now?  USB migration support doesn't 
exist, and when we add it chances are that it isn't compatible with what 
you have here.

> +static struct USBDeviceInfo ccid_info = {
> +    .product_desc   = "QEMU USB CCID",
> +    .qdev.name      = CCID_DEV_NAME,
> +    .qdev.size      = sizeof(USBCCIDState),
> +    .qdev.vmsd      =&ccid_vmstate,

Best leave the vmstate structs in the code (with a comment) and just zap 
this line which winds it up.

thanks,
   Gerd