Re: [Xen-devel] [PATCH v2 13/15] libxl: call hotplug scripts for nic devices from libxl

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v2 13/15] libxl: call hotplug scripts for nic 
devices from libxl"):
> Since most of the needed work is already done in previous patches,
> this patch only contains the necessary code to call hotplug scripts
> for nic devices, that should be called when the device is added or
> removed from a guest.

> +libxl_nic_type libxl__nic_type(libxl__gc *gc, libxl__device *dev)
> +{
> +    char *snictype, *be_path;
> +    libxl_nic_type nictype;
> +
> +    be_path = libxl__device_backend_path(gc, dev);
> +    snictype = libxl__xs_read(gc, XBT_NULL,
> +                              GCSPRINTF("%s/%s", be_path, "type"));
> +    if (!snictype) {
> +        LOGE(ERROR, "unable to read nictype from %s", be_path);
> +        return -1;

Please let us not have any more functions that return -1.  Do you mean
ERROR_FAIL ?

> @@ -806,7 +827,18 @@ static void device_hotplug_fork_cb(libxl__egc *egc, 
> libxl__ev_child *child,
>                                                       : LIBXL__LOG_WARNING,
>                                        aodev->what, pid, status);
>          aodev->rc = ERROR_FAIL;
> +        goto out;
>      }
> +
> +    if (aodev->dev->backend_kind == LIBXL__DEVICE_KIND_VIF &&
> +        libxl__nic_type(gc, aodev->dev) == LIBXL_NIC_TYPE_IOEMU &&

And you need to handle the error here.

> +        !aodev->vif_executed) {
> +        aodev->vif_executed = 1;
> +        device_hotplug(egc, aodev);
> +        return;
> +    }

This logic surrounding vif_executed is rather opaque.  Are you trying
to run this whole lot twice so that you can run two lots of scripts ?

If so then perhaps the hotplug helper should simply take a counter, so
that we don't expose this vif abstraction thing ?  And it would be
applicable to everything, not just vifs.

Ian.

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