Re: [Xen-devel] [PATCH v1 08/12] libxl: add disk specific remove functions [and 1 more messages]

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("Re: [Xen-devel] [PATCH v1 08/12] libxl: add disk 
specific remove functions"):
> On 13/03/13 17:22, Ian Jackson wrote:
> > This macro is very similar to DEFINE_DEVICE_REMOVE.  The only
> > difference seems to be these extra lines:
> > 
> >> +        aodev->hotplug.version = type->hotplug_version;                 \
> >> +        LOG(DEBUG, "hotplug version: %d", aodev->hotplug.version);      \
> >> +        libxl__initiate_device_remove(egc, aodev);                      \
> > 
> > Perhaps the right answer would be to add a new formal parameter to
> > DEFINE_DEVICE_REMOVE which allows DEFINE_DEVICE_REMOVE's user to
> > specify some extra code here ?
> 
> type->hotplug_version is only available in libxl_device_disk, so even if
> using something like
> 
> if (param)
>       aodev->hotplug.version = type->hotplug_version;

I see I didn't reply to this and your v2 has this patch unchanged.  I
still think we need to find a way not to duplicate this code.  What I
meant above was something like this:

#define DEFINE_DEVICE_REMOVE(type, removedestroy, f, extra)             \
    int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,           \
        uint32_t domid, libxl_device_##type *type,                      \
        const libxl_asyncop_how *ao_how)                                \
    {                                                                   \
        AO_CREATE(ctx, domid, ao_how);                                  \
        libxl__device *device;                                          \
        libxl__ao_device *aodev;                                        \
        int rc;                                                         \
                                                                        \
        GCNEW(device);                                                  \
        rc = libxl__device_from_##type(gc, domid, type, device);        \
        if (rc != 0) goto out;                                          \
                                                                        \
        GCNEW(aodev);                                                   \
        libxl__prepare_ao_device(ao, aodev);                            \
        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;                    \
        aodev->dev = device;                                            \
        aodev->callback = device_addrm_aocomplete;                      \
        aodev->force = f;                                               \
        extra;                                                          \
        libxl__initiate_device_remove(egc, aodev);                      \
                                                                        \
    out:                                                                \
        if (rc) return AO_ABORT(rc);                                    \
        return AO_INPROGRESS;                                           \
    }

#define DEFINE_DISK_REMOVE(type, removedestroy, f) \
 DEFINE_DEVICE_REMOVE(type, removedestroy, f, {
        aodev->hotplug.version = type->hotplug_version;
        LOG(DEBUG, "hotplug version: %d", aodev->hotplug.version);
    })
 
Ian.

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