[v2,4/7] libxl/FreeBSD: add support for disk hotplug scripts

Commit Message

Roger Pau Monné Feb. 25, 2016, 7:25 p.m. UTC

Allow FreeBSD to execute hotplug scripts when attaching disk devices.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
Cc: Ian Jackson <[e-mail]>
Cc: Ian Campbell <[e-mail]>
Cc: Wei Liu <[e-mail]>
---
 tools/libxl/libxl_freebsd.c | 114 ++++++++++++++++++++++++++++++++------------
 1 file changed, 83 insertions(+), 31 deletions(-)

Comments

Wei Liu March 1, 2016, 12:40 p.m. UTC | #1

On Thu, Feb 25, 2016 at 08:25:15PM +0100, Roger Pau Monne wrote:
> Allow FreeBSD to execute hotplug scripts when attaching disk devices.
> 
> Signed-off-by: Roger Pau Monné <[e-mail]>
> ---
> Cc: Ian Jackson <[e-mail]>
> Cc: Ian Campbell <[e-mail]>
> Cc: Wei Liu <[e-mail]>
> ---
>  tools/libxl/libxl_freebsd.c | 114 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 83 insertions(+), 31 deletions(-)
> 

It's better to either break the refactoring to another patch or
explicitly say there is refactoring in this patch.

> +static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
> +                               char ***args, char ***env,
> +                               libxl__device_action action)
> +{
> +    char *be_path = libxl__device_backend_path(gc, dev);
> +    char *script;
> +    int nr = 0, rc;
> +
> +    script = libxl__xs_read(gc, XBT_NULL,
> +                            GCSPRINTF("%s/%s", be_path, "script"));
> +    if (!script) {
> +        LOGEV(ERROR, errno, "unable to read script from %s", be_path);
> +        rc = ERROR_FAIL;
> +        goto error;
> +    }
> +
> +    const int arraysize = 4;
> +    GCNEW_ARRAY(*args, arraysize);
> +    (*args)[nr++] = script;
> +    (*args)[nr++] = be_path;
> +    (*args)[nr++] = (char *) libxl__device_action_to_string(action);
> +    (*args)[nr++] = NULL;
> +    assert(nr == arraysize);
> +
> +    rc = 1;
> +
> +error:
> +    return rc;
> +}
> +

The code looks fine.

Wei.

Patch

diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c
index 483f36e..8ad347e 100644
--- a/tools/libxl/libxl_freebsd.c
+++ b/tools/libxl/libxl_freebsd.c
@@ -59,14 +59,36 @@  static int libxl__hotplug_env_nic(libxl__gc *gc, libxl__device *dev, char ***env
     return 0;
 }
 
-static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev, char ***args,
-                              libxl__device_action action)
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
+                              char ***args, char ***env,
+                              libxl__device_action action,
+                              int num_exec)
 {
+    libxl_nic_type nictype;
     char *be_path = libxl__device_backend_path(gc, dev);
     char *script;
-    int nr = 0, rc = 0, arraysize = 4;
+    int nr = 0, rc;
 
-    assert(dev->backend_kind == LIBXL__DEVICE_KIND_VIF);
+    rc = libxl__nic_type(gc, dev, &nictype);
+    if (rc) {
+        LOG(ERROR, "error when fetching nic type");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    /*
+     * For PV domains only one pass is needed (because there's no emulated
+     * interface). For HVM domains two passes are needed in order to add
+     * both the PV and the tap interfaces to the bridge.
+     */
+    if (nictype == LIBXL_NIC_TYPE_VIF && num_exec != 0) {
+        rc = 0;
+        goto out;
+    }
+
+    rc = libxl__hotplug_env_nic(gc, dev, env, num_exec);
+    if (rc)
+        goto out;
 
     script = libxl__xs_read(gc, XBT_NULL,
                             GCSPRINTF("%s/%s", be_path, "script"));
@@ -76,53 +98,83 @@  static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev, char ***args,
         goto out;
     }
 
+    const int arraysize = 4;
     GCNEW_ARRAY(*args, arraysize);
     (*args)[nr++] = script;
     (*args)[nr++] = be_path;
-    (*args)[nr++] = GCSPRINTF("%s", action == LIBXL__DEVICE_ACTION_ADD ?
-                                    "add" : "remove");
+    (*args)[nr++] = (char *) libxl__device_action_to_string(action);
     (*args)[nr++] = NULL;
     assert(nr == arraysize);
 
+    rc = 1;
+
 out:
     return rc;
 }
 
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
+                               char ***args, char ***env,
+                               libxl__device_action action)
+{
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script;
+    int nr = 0, rc;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            GCSPRINTF("%s/%s", be_path, "script"));
+    if (!script) {
+        LOGEV(ERROR, errno, "unable to read script from %s", be_path);
+        rc = ERROR_FAIL;
+        goto error;
+    }
+
+    const int arraysize = 4;
+    GCNEW_ARRAY(*args, arraysize);
+    (*args)[nr++] = script;
+    (*args)[nr++] = be_path;
+    (*args)[nr++] = (char *) libxl__device_action_to_string(action);
+    (*args)[nr++] = NULL;
+    assert(nr == arraysize);
+
+    rc = 1;
+
+error:
+    return rc;
+}
+
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
                                    libxl__device_action action,
                                    int num_exec)
 {
-    libxl_nic_type nictype;
     int rc;
 
-    if (dev->backend_kind != LIBXL__DEVICE_KIND_VIF || num_exec == 2)
-        return 0;
-
-    rc = libxl__nic_type(gc, dev, &nictype);
-    if (rc) {
-        LOG(ERROR, "error when fetching nic type");
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
-    /*
-     * For PV domains only one pass is needed (because there's no emulated
-     * interface). For HVM domains two passes are needed in order to add
-     * both the PV and the tap interfaces to the bridge.
-     */
-    if (nictype == LIBXL_NIC_TYPE_VIF && num_exec != 0) {
+    switch (dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VBD:
+        if (num_exec != 0) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug_disk(gc, dev, args, env, action);
+        break;
+    case LIBXL__DEVICE_KIND_VIF:
+        /*
+         * If domain has a stubdom we don't have to execute hotplug scripts
+         * for emulated interfaces
+         */
+        if ((num_exec > 1) ||
+            (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug_nic(gc, dev, args, env, action, num_exec);
+        break;
+    default:
+        /* No need to execute any hotplug scripts */
         rc = 0;
-        goto out;
+        break;
     }
 
-    rc = libxl__hotplug_env_nic(gc, dev, env, num_exec);
-    if (rc)
-        goto out;
-
-    rc = libxl__hotplug_nic(gc, dev, args, action);
-    if (!rc) rc = 1;
-
 out:
     return rc;
 }