Re: [PATCH] libxl: add option for discard support to xl disk configuration

[Olaf Hering <olaf@aepfle.de>]

On Wed, Jan 29, Ian Campbell wrote:

> On Tue, 2014-01-28 at 19:24 +0100, Olaf Hering wrote:
> > +    ("discard_enable", integer),
> I have a feeling this should be a libxl_defbool, to allow for the
> possibility of "libxl does what is best/lets the backend decide".
> 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 2845ca4..3633a7d 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -2196,6 +2196,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(back, "discard_enable");
> > +        flexarray_append(back, libxl__sprintf(gc, "%d", (disk->discard_enable) ? 1 : 0));
> And if this were a defbool then you'd want to use libxl_defbool_is_default: i.e.
> 	if (!libxl_defbool_is_default(disk->discard_enable))
> 		flexarray_append(back, ..., libxl_defbool_val(...) ? "1" : "0"))
> 
> (note the lack of libxl_sprintf here too).

Did you have something like this in mind? Its all it takes.

Olaf


diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2845ca4..bbaf450 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2196,6 +2196,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");
+        if (!libxl_defbool_is_default(disk->discard_enable))
+            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_types.idl b/tools/libxl/libxl_types.idl
index 649ce50..6575515 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -415,6 +415,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_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 */