[Xen-devel] [PATCH v3] libxl: add option for discard support to xl disk configuration

[Olaf Hering <olaf@xxxxxxxxx>]

Handle new option discard=on|off for disk configuration. It is supposed
to disable discard support if file based backing storage was
intentionally created non-sparse to avoid fragmentation of the file.

The option is a boolean and intended for the backend driver. A new
boolean property "discard-enable" is written to the backend node. An
upcoming patch for qemu will make use of this property. The kernel
blkback driver may be updated as well to disable discard for phy based
backing storage.

v3:
enable discard unconditionally by always writing discard-enable=1 to xenstore
fix typos in xl-disk-configuration.txt
update description in blkif.h, property should be ignored if unsupported
v2:
rename xenstore property from discard_enable to discard-enable
update description in xl-disk-configuration.txt
use libxl_defbool as type for discard_enable
update check-xl-disk-parse to use "<default>"
add LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE to libxl.h

Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
---
 docs/misc/xl-disk-configuration.txt | 15 +++++++++++++++
 tools/libxl/check-xl-disk-parse     | 21 ++++++++++++++-------
 tools/libxl/libxl.c                 |  2 ++
 tools/libxl/libxl.h                 |  5 +++++
 tools/libxl/libxl_types.idl         |  1 +
 tools/libxl/libxlu_disk.c           |  2 ++
 tools/libxl/libxlu_disk_l.l         |  4 ++++
 xen/include/public/io/blkif.h       | 10 ++++++++++
 8 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xl-disk-configuration.txt 
b/docs/misc/xl-disk-configuration.txt
index cf22397..b4a9376 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -178,6 +178,21 @@ information to be interpreted by the executable program 
<script>,
 These scripts are normally called "block-<script>".
 
 
+discard=<boolean>
+---------------
+
+Description:           Instruct backend to advertise discard support to 
frontend
+Supported values:      on, off, 0, 1
+Mandatory:             No
+Default value:         on
+
+This option is an advisory setting for the backend driver, depending on the
+value, to advertise discard support (TRIM, UNMAP) to the frontend.  The real
+benefit of this option is to be able to force it off rather than on. It allows
+to disable "hole punching" for file based backends which were intentionally
+created non-sparse to avoid fragmentation of the file.
+
+
 
 ============================================
 DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-parse
index 797277c..7ec7c83 100755
--- a/tools/libxl/check-xl-disk-parse
+++ b/tools/libxl/check-xl-disk-parse
@@ -61,7 +61,8 @@ disk: {
     "script": null,
     "removable": 0,
     "readwrite": 1,
-    "is_cdrom": 0
+    "is_cdrom": 0,
+    "discard_enable": "True"
 }
 
 END
@@ -82,7 +83,8 @@ disk: {
     "script": null,
     "removable": 1,
     "readwrite": 0,
-    "is_cdrom": 1
+    "is_cdrom": 1,
+    "discard_enable": "False"
 }
 
 END
@@ -104,7 +106,8 @@ disk: {
     "script": null,
     "removable": 0,
     "readwrite": 1,
-    "is_cdrom": 0
+    "is_cdrom": 0,
+    "discard_enable": "True"
 }
 
 EOF
@@ -121,7 +124,8 @@ disk: {
     "script": null,
     "removable": 1,
     "readwrite": 0,
-    "is_cdrom": 1
+    "is_cdrom": 1,
+    "discard_enable": "False"
 }
 
 EOF
@@ -142,7 +146,8 @@ disk: {
     "script": null,
     "removable": 1,
     "readwrite": 0,
-    "is_cdrom": 1
+    "is_cdrom": 1,
+    "discard_enable": "False"
 }
 
 EOF
@@ -160,7 +165,8 @@ disk: {
     "script": "block-iscsi",
     "removable": 0,
     "readwrite": 1,
-    "is_cdrom": 0
+    "is_cdrom": 0,
+    "discard_enable": "True"
 }
 
 EOF
@@ -180,7 +186,8 @@ disk: {
     "script": "block-drbd",
     "removable": 0,
     "readwrite": 1,
-    "is_cdrom": 0
+    "is_cdrom": 0,
+    "discard_enable": "True"
 }
 
 EOF
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 30b0b06..236a3f7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2205,6 +2205,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
domid,
         flexarray_append(back, disk->readwrite ? "w" : "r");
         flexarray_append(back, "device-type");
         flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+        flexarray_append_pair(back, "discard-enable",
+                              libxl_defbool_val(disk->discard_enable) ? "1" : 
"0");
 
         flexarray_append(front, "backend-id");
         flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index b2c3015..ea94895 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -95,6 +95,11 @@
 #define LIBXL_HAVE_BUILDINFO_EVENT_CHANNELS 1
 
 /*
+ * The libxl_device_disk has the discard_enable field.
+ */
+#define LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 0f7bbf8..80d00a7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -416,6 +416,7 @@ libxl_device_disk = Struct("device_disk", [
     ("removable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),
+    ("discard_enable", libxl_defbool),
     ])
 
 libxl_device_nic = Struct("device_nic", [
diff --git a/tools/libxl/libxlu_disk.c b/tools/libxl/libxlu_disk.c
index 18fe386..5827025 100644
--- a/tools/libxl/libxlu_disk.c
+++ b/tools/libxl/libxlu_disk.c
@@ -79,6 +79,8 @@ int xlu_disk_parse(XLU_Config *cfg,
         if (!disk->pdev_path || !strcmp(disk->pdev_path, ""))
             disk->format = LIBXL_DISK_FORMAT_EMPTY;
     }
+    if (libxl_defbool_is_default(disk->discard_enable))
+        libxl_defbool_setdefault(&disk->discard_enable, !!disk->readwrite);
 
     if (!disk->vdev) {
         xlu__disk_err(&dpc,0, "no vdev specified");
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
index 7c4e7f1..2585bee 100644
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -173,6 +173,10 @@ backendtype=[^,]*,? { STRIP(','); 
setbackendtype(DPC,FROMEQUALS); }
 
 vdev=[^,]*,?   { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
 script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
+discard=on,?   { libxl_defbool_set(&DPC->disk->discard_enable, true); }
+discard=1,?    { libxl_defbool_set(&DPC->disk->discard_enable, true); }
+discard=off,?  { libxl_defbool_set(&DPC->disk->discard_enable, false); }
+discard=0,?    { libxl_defbool_set(&DPC->disk->discard_enable, false); }
 
  /* the target magic parameter, eats the rest of the string */
 
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index c3e169c..032c804 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -175,6 +175,16 @@
  *
  *------------------------- Backend Device Properties -------------------------
  *
+ * discard-enable
+ *      Values:         0/1 (boolean)
+ *      Default Value:  1
+ *
+ *      This optional property, set by the toolstack, instructs the backend to
+ *      offer discard to the frontend. If the property is missing the backend
+ *      should offer discard if the backing storage actually supports it. The
+ *      backend driver should ignore the property if the property is set but
+ *      the backing storage does not support the feature.
+ *
  * discard-alignment
  *      Values:         <uint32_t>
  *      Default Value:  0

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