diff mbox

[RFC,3/8] libxl: add backend_features to libxl_device_disk

Message ID 20171102180616.24084-4-joao.m.martins@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joao Martins Nov. 2, 2017, 6:06 p.m. UTC
The function libxl__device_generic_add will have an additional
argument whereby it adds a second set of entries visible to the
backend only. These entries will then be used for devices
thus overriding backend maximum feature set with this user-defined ones.

libxl_device_disk.backend_features are a key value store storing:
 <feature-name> = <feature-value>

xl|libxl are stateless with respect to feature names therefore is up to the
admin to carefully select those. If backend isn't supported therefore the
features won't be overwritten.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 tools/libxl/libxl.h          |  8 ++++++++
 tools/libxl/libxl_console.c  |  5 +++--
 tools/libxl/libxl_device.c   | 37 +++++++++++++++++++++++++++++++++----
 tools/libxl/libxl_disk.c     | 17 +++++++++++++++--
 tools/libxl/libxl_internal.h |  4 +++-
 tools/libxl/libxl_pci.c      |  2 +-
 tools/libxl/libxl_types.idl  |  1 +
 tools/libxl/libxl_usb.c      |  2 +-
 8 files changed, 65 insertions(+), 11 deletions(-)

Comments

Oleksandr Grytsov Nov. 7, 2017, 11:28 a.m. UTC | #1
On Thu, Nov 2, 2017 at 8:06 PM, Joao Martins <joao.m.martins@oracle.com>
wrote:

> The function libxl__device_generic_add will have an additional
> argument whereby it adds a second set of entries visible to the
> backend only. These entries will then be used for devices
> thus overriding backend maximum feature set with this user-defined ones.
>
> libxl_device_disk.backend_features are a key value store storing:
>  <feature-name> = <feature-value>
>
> xl|libxl are stateless with respect to feature names therefore is up to the
> admin to carefully select those. If backend isn't supported therefore the
> features won't be overwritten.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>  tools/libxl/libxl.h          |  8 ++++++++
>  tools/libxl/libxl_console.c  |  5 +++--
>  tools/libxl/libxl_device.c   | 37 +++++++++++++++++++++++++++++++++----
>  tools/libxl/libxl_disk.c     | 17 +++++++++++++++--
>  tools/libxl/libxl_internal.h |  4 +++-
>  tools/libxl/libxl_pci.c      |  2 +-
>  tools/libxl/libxl_types.idl  |  1 +
>  tools/libxl/libxl_usb.c      |  2 +-
>  8 files changed, 65 insertions(+), 11 deletions(-)
>
>
No need to extend libxl__device_generic_add with additional parameter
(brents).
You can add nested entry in libxl__set_xenstore_<device> as following:

flexarray_append(back, "require/feature-persistent", "0");
Joao Martins Nov. 7, 2017, 11:48 a.m. UTC | #2
On 11/07/2017 11:28 AM, Oleksandr Grytsov wrote:
> On Thu, Nov 2, 2017 at 8:06 PM, Joao Martins <joao.m.martins@oracle.com
> <mailto:joao.m.martins@oracle.com>> wrote:
> 
>     The function libxl__device_generic_add will have an additional
>     argument whereby it adds a second set of entries visible to the
>     backend only. These entries will then be used for devices
>     thus overriding backend maximum feature set with this user-defined ones.
> 
>     libxl_device_disk.backend_features are a key value store storing:
>      <feature-name> = <feature-value>
> 
>     xl|libxl are stateless with respect to feature names therefore is up to the
>     admin to carefully select those. If backend isn't supported therefore the
>     features won't be overwritten.
> 
>     Signed-off-by: Joao Martins <joao.m.martins@oracle.com
>     <mailto:joao.m.martins@oracle.com>>
>     ---
>      tools/libxl/libxl.h          |  8 ++++++++
>      tools/libxl/libxl_console.c  |  5 +++--
>      tools/libxl/libxl_device.c   | 37 +++++++++++++++++++++++++++++++++----
>      tools/libxl/libxl_disk.c     | 17 +++++++++++++++--
>      tools/libxl/libxl_internal.h |  4 +++-
>      tools/libxl/libxl_pci.c      |  2 +-
>      tools/libxl/libxl_types.idl  |  1 +
>      tools/libxl/libxl_usb.c      |  2 +-
>      8 files changed, 65 insertions(+), 11 deletions(-)
> 
> 
> No need to extend libxl__device_generic_add with additional parameter (brents).
> You can add nested entry in libxl__set_xenstore_<device> as following:
> 
> flexarray_append(back, "require/feature-persistent", "0");

Right, although entries on "back" array will have readonly permission to the
frontend. And these newly added "require" directory in this RFC was meant to be
only visible to the backend, hence only having XS_PERM_NONE permission set.

Joao
diff mbox

Patch

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5e9aed739d..82990089ef 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1101,6 +1101,14 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_SET_PARAMETERS 1
 
+/*
+ * LIBXL_HAVE_DISK_BACKEND_FEATURES
+ *
+ * libxl_device_disk contains backend_features which can be used to control
+ * what features are exposed to guest vbds.
+ */
+#define LIBXL_HAVE_DISK_BACKEND_FEATURES 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index c05dc28b99..f40def1276 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -339,7 +339,7 @@  int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
     libxl__device_generic_add(gc, XBT_NULL, device,
                               libxl__xs_kvs_of_flexarray(gc, back),
                               libxl__xs_kvs_of_flexarray(gc, front),
-                              libxl__xs_kvs_of_flexarray(gc, ro_front));
+                              libxl__xs_kvs_of_flexarray(gc, ro_front), NULL);
     rc = 0;
 out:
     return rc;
@@ -385,7 +385,8 @@  int libxl__device_vuart_add(libxl__gc *gc, uint32_t domid,
     rc = libxl__device_generic_add(gc, XBT_NULL, &device,
                                    libxl__xs_kvs_of_flexarray(gc, back),
                                    NULL,
-                                   libxl__xs_kvs_of_flexarray(gc, ro_front));
+                                   libxl__xs_kvs_of_flexarray(gc, ro_front),
+                                   NULL);
     return rc;
 }
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 5438577c3c..05178fb480 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -43,6 +43,15 @@  char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device)
                      device->domid, device->devid);
 }
 
+char *libxl__device_require_path(libxl__gc *gc, libxl__device *device)
+{
+    char *dom_path = libxl__xs_get_dompath(gc, device->backend_domid);
+
+    return GCSPRINTF("%s/backend/%s/%u/%d/require", dom_path,
+                     libxl__device_kind_to_string(device->backend_kind),
+                     device->domid, device->devid);
+}
+
 char *libxl__device_libxl_path(libxl__gc *gc, libxl__device *device)
 {
     char *libxl_dom_path = libxl__xs_libxl_path(gc, device->domid);
@@ -114,13 +123,16 @@  out:
 }
 
 int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents, char **ro_fents)
+        libxl__device *device, char **bents, char **fents, char **ro_fents,
+        char **brents)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    char *frontend_path = NULL, *backend_path = NULL, *libxl_path;
+    char *frontend_path = NULL, *backend_path = NULL, *require_path = NULL,
+         *libxl_path;
     struct xs_permissions frontend_perms[2];
     struct xs_permissions ro_frontend_perms[2];
     struct xs_permissions backend_perms[2];
+    struct xs_permissions require_perms[1];
     int create_transaction = t == XBT_NULL;
     int libxl_only = device->backend_kind == LIBXL__DEVICE_KIND_NONE;
     int rc;
@@ -131,6 +143,7 @@  int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
     } else {
         frontend_path = libxl__device_frontend_path(gc, device);
         backend_path = libxl__device_backend_path(gc, device);
+        require_path = libxl__device_require_path(gc, device);
     }
     libxl_path = libxl__device_libxl_path(gc, device);
 
@@ -144,6 +157,9 @@  int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
     ro_frontend_perms[1].id = backend_perms[1].id = device->domid;
     ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ;
 
+    require_perms[0].id = device->backend_domid;
+    require_perms[0].perms = XS_PERM_NONE;
+
 retry_transaction:
     if (create_transaction)
         t = xs_transaction_start(ctx->xsh);
@@ -200,6 +216,12 @@  retry_transaction:
                      frontend_path, strlen(frontend_path));
             libxl__xs_writev(gc, t, backend_path, bents);
         }
+        if (brents) {
+            xs_mkdir(ctx->xsh, t, require_path);
+            xs_set_permissions(ctx->xsh, t, require_path, require_perms,
+                               ARRAY_SIZE(require_perms));
+            libxl__xs_writev(gc, t, require_path, brents);
+        }
 
         /*
          * We make a copy of everything for the backend in the libxl
@@ -226,6 +248,11 @@  retry_transaction:
          */
         rc = libxl__xs_writev(gc, t, libxl_path, bents);
         if (rc) goto out;
+
+        if (brents) {
+            rc = libxl__xs_writev(gc, t, libxl_path, brents);
+            if (rc) goto out;
+        }
     }
 
     if (!create_transaction)
@@ -1920,7 +1947,8 @@  void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
         libxl__device_generic_add(gc, t, device,
                                   libxl__xs_kvs_of_flexarray(gc, back),
                                   libxl__xs_kvs_of_flexarray(gc, front),
-                                  libxl__xs_kvs_of_flexarray(gc, ro_front));
+                                  libxl__xs_kvs_of_flexarray(gc, ro_front),
+                                  NULL);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
@@ -1984,7 +2012,8 @@  int libxl__device_add(libxl__gc *gc, uint32_t domid,
     rc = libxl__device_generic_add(gc, XBT_NULL, device,
                                    libxl__xs_kvs_of_flexarray(gc, back),
                                    libxl__xs_kvs_of_flexarray(gc, front),
-                                   libxl__xs_kvs_of_flexarray(gc, ro_front));
+                                   libxl__xs_kvs_of_flexarray(gc, ro_front),
+                                   NULL);
     if (rc) goto out;
 
     rc = 0;
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 895bf4f89a..8caf763c88 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -239,14 +239,16 @@  static void device_disk_add(libxl__egc *egc, uint32_t domid,
     STATE_AO_GC(aodev->ao);
     flexarray_t *front = NULL;
     flexarray_t *back = NULL;
+    flexarray_t *require = NULL;
     char *dev = NULL, *script;
     libxl__device *device;
-    int rc;
+    int rc, i;
     libxl_ctx *ctx = gc->owner;
     xs_transaction_t t = XBT_NULL;
     libxl_domain_config d_config;
     libxl_device_disk disk_saved;
     libxl__domain_userdata_lock *lock = NULL;
+    libxl_key_value_list back_features = disk->backend_features;
 
     libxl_domain_config_init(&d_config);
     libxl_device_disk_init(&disk_saved);
@@ -300,6 +302,7 @@  static void device_disk_add(libxl__egc *egc, uint32_t domid,
 
         front = flexarray_make(gc, 16, 1);
         back = flexarray_make(gc, 16, 1);
+        require = flexarray_make(gc, 16, 1);
 
         GCNEW(device);
         rc = libxl__device_from_disk(gc, domid, disk, device);
@@ -433,6 +436,15 @@  static void device_disk_add(libxl__egc *egc, uint32_t domid,
             }
         }
 
+        if (back_features) {
+            for (i = 0; back_features[i] != NULL; i += 2) {
+                flexarray_append(require, libxl__strdup(gc, back_features[i]));
+                if (back_features[i + 1])
+                    flexarray_append(require,
+                                     libxl__strdup(gc, back_features[i + 1]));
+            }
+        }
+
         if (!get_vdev && aodev->update_json) {
             rc = libxl__set_domain_configuration(gc, domid, &d_config);
             if (rc) goto out;
@@ -441,7 +453,8 @@  static void device_disk_add(libxl__egc *egc, uint32_t domid,
         libxl__device_generic_add(gc, t, device,
                                   libxl__xs_kvs_of_flexarray(gc, back),
                                   libxl__xs_kvs_of_flexarray(gc, front),
-                                  NULL);
+                                  NULL,
+                                  libxl__xs_kvs_of_flexarray(gc, require));
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 45e6df6c82..19c02e27a0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1216,8 +1216,10 @@  _hidden int libxl__device_vuart_add(libxl__gc *gc, uint32_t domid,
 _hidden int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
                                  libxl__device *device);
 _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents, char **ro_fents);
+                                      libxl__device *device, char **bents,
+                                      char **fents, char **ro_fents, char **brents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
+_hidden char *libxl__device_require_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_libxl_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
                                       libxl__device *dev);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 88a55ce8bd..1d595513f5 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -106,7 +106,7 @@  int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
     return libxl__device_generic_add(gc, XBT_NULL, &device,
                                      libxl__xs_kvs_of_flexarray(gc, back),
                                      libxl__xs_kvs_of_flexarray(gc, front),
-                                     NULL);
+                                     NULL, NULL);
 }
 
 static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a239324341..949a797402 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -627,6 +627,7 @@  libxl_device_vkb = Struct("device_vkb", [
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", libxl_domid),
     ("backend_domname", string),
+    ("backend_features", libxl_key_value_list),
     ("pdev_path", string),
     ("vdev", string),
     ("backend", libxl_disk_backend),
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index cb0e792724..97e54272f1 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -283,7 +283,7 @@  static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
         libxl__device_generic_add(gc, t, device,
                                   libxl__xs_kvs_of_flexarray(gc, back),
                                   libxl__xs_kvs_of_flexarray(gc, front),
-                                  NULL);
+                                  NULL, NULL);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;