Xen project Mailing List

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

From: Olaf Hering <olaf@xxxxxxxxx>

Date: Mon, 5 May 2014 16:04:21 +0200

Cc: Olaf Hering <olaf@xxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

Delivery-date: Mon, 05 May 2014 14:04:40 +0000

List-id: Xen developer discussion <xen-devel.lists.xen.org>

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. v4: rebase ontop of commit 6ec48cf4 (direct_io_safe) rebase ontop of fixup commit for 6ec48cf4 (direct_io_safe) add testcases to tools/libxl/check-xl-disk-parse 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> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx> --- docs/misc/xl-disk-configuration.txt | 15 +++++++ tools/libxl/check-xl-disk-parse | 80 +++++++++++++++++++++++++++++++++---- 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, 112 insertions(+), 7 deletions(-) diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt index 11fee9a..e2a56f3 100644 --- a/docs/misc/xl-disk-configuration.txt +++ b/docs/misc/xl-disk-configuration.txt @@ -217,6 +217,21 @@ If in the future the bug is fixed properly this option will then be silently ignored. +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 0698586..c564e01 100755 --- a/tools/libxl/check-xl-disk-parse +++ b/tools/libxl/check-xl-disk-parse @@ -62,7 +62,8 @@ disk: { "removable": 0, "readwrite": 1, "is_cdrom": 0, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "True" } END @@ -84,7 +85,8 @@ disk: { "removable": 1, "readwrite": 0, "is_cdrom": 1, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "False" } END @@ -107,7 +109,8 @@ disk: { "removable": 0, "readwrite": 1, "is_cdrom": 0, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "True" } EOF @@ -125,7 +128,8 @@ disk: { "removable": 1, "readwrite": 0, "is_cdrom": 1, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "False" } EOF @@ -147,7 +151,8 @@ disk: { "removable": 1, "readwrite": 0, "is_cdrom": 1, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "False" } EOF @@ -166,7 +171,8 @@ disk: { "removable": 0, "readwrite": 1, "is_cdrom": 0, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "True" } EOF @@ -187,7 +193,8 @@ disk: { "removable": 0, "readwrite": 1, "is_cdrom": 0, - "direct_io_safe": false + "direct_io_safe": false, + "discard_enable": "True" } EOF @@ -196,4 +203,63 @@ EOF # http://www.drbd.org/users-guide-emb/s-xen-configure-domu.html one 0 drbd:app01,hda,w +expected <<END +disk: { + "backend_domid": 0, + "backend_domname": null, + "pdev_path": "/some/disk/image.raw", + "vdev": "hda", + "backend": "unknown", + "format": "raw", + "script": null, + "removable": 0, + "readwrite": 1, + "is_cdrom": 0, + "direct_io_safe": false, + "discard_enable": "True" +} + +END +one 0 discard=on vdev=hda target=/some/disk/image.raw +one 0 discard=1 vdev=hda target=/some/disk/image.raw + +expected <<END +disk: { + "backend_domid": 0, + "backend_domname": null, + "pdev_path": "/some/disk/image.raw", + "vdev": "hda", + "backend": "unknown", + "format": "raw", + "script": null, + "removable": 0, + "readwrite": 1, + "is_cdrom": 0, + "direct_io_safe": false, + "discard_enable": "False" +} + +END +one 0 discard=off vdev=hda target=/some/disk/image.raw +one 0 discard=0 vdev=hda target=/some/disk/image.raw + +expected <<END +disk: { + "backend_domid": 0, + "backend_domname": null, + "pdev_path": "/some/disk/image.iso", + "vdev": "hda", + "backend": "unknown", + "format": "raw", + "script": null, + "removable": 1, + "readwrite": 0, + "is_cdrom": 1, + "direct_io_safe": false, + "discard_enable": "False" +} + +END +one 0 cdrom discard=off vdev=hda target=/some/disk/image.iso + complete diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index d59ce0c..2b7352c 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2209,6 +2209,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, flexarray_append(back, "direct-io-safe"); flexarray_append(back, "1"); } + 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 84f9c0e..c7aa817 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -102,6 +102,11 @@ #define LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE 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 8944686..52f1aa9 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -417,6 +417,7 @@ libxl_device_disk = Struct("device_disk", [ ("readwrite", integer), ("is_cdrom", integer), ("direct_io_safe", bool), + ("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 ba8577c..7fb378a 100644 --- a/tools/libxl/libxlu_disk_l.l +++ b/tools/libxl/libxlu_disk_l.l @@ -174,6 +174,10 @@ backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); } script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); } direct-io-safe,? { DPC->disk->direct_io_safe = 1; } +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 1e7cea9..27a88ba 100644 --- a/xen/include/public/io/blkif.h +++ b/xen/include/public/io/blkif.h @@ -197,6 +197,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

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.