LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE exists since Xen 4.5.0

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 src/libxl/libxl_conf.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 3de2f9f57a..e275996cab 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -923,13 +923,12 @@ libxlMakeVnumaList(virDomainDef *def,
     return ret;
 }
 
-static int
-libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
+static void
+libxlDiskSetDiscard(libxl_device_disk *x_disk, virDomainDiskDiscard discard)
 {
     if (!x_disk->readwrite)
-        return 0;
-#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
-    switch ((virDomainDiskDiscard)discard) {
+        return;
+    switch (discard) {
     case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
     case VIR_DOMAIN_DISK_DISCARD_LAST:
         break;
@@ -940,15 +939,6 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
         libxl_defbool_set(&x_disk->discard_enable, false);
         break;
     }
-    return 0;
-#else
-    if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
-        return 0;
-    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                   _("This version of libxenlight does not support "
-                     "disk 'discard' option passing"));
-    return -1;
-#endif
 }
 
 static void
@@ -1209,8 +1199,7 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk)
     x_disk->removable = 1;
     x_disk->readwrite = !l_disk->src->readonly;
     x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0;
-    if (libxlDiskSetDiscard(x_disk, l_disk->discard) < 0)
-        return -1;
+    libxlDiskSetDiscard(x_disk, l_disk->discard);
     libxlDiskSetScript(x_disk, src);
 
     /* An empty CDROM must have the empty format, otherwise libxl fails. */

On 8/13/21 5:22 AM, Olaf Hering wrote:
> LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE exists since Xen 4.5.0
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>   src/libxl/libxl_conf.c | 21 +++++----------------
>   1 file changed, 5 insertions(+), 16 deletions(-)

This patch doesn't apply to libvirt.git master.

> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 3de2f9f57a..e275996cab 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -923,13 +923,12 @@ libxlMakeVnumaList(virDomainDef *def,
>       return ret;
>   }
>   
> -static int
> -libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
> +static void
> +libxlDiskSetDiscard(libxl_device_disk *x_disk, virDomainDiskDiscard discard)
>   {
>       if (!x_disk->readwrite)
> -        return 0;
> -#if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
> -    switch ((virDomainDiskDiscard)discard) {
> +        return;
> +    switch (discard) {

Why remove the cast?

>       case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
>       case VIR_DOMAIN_DISK_DISCARD_LAST:
>           break;
> @@ -940,15 +939,6 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
>           libxl_defbool_set(&x_disk->discard_enable, false);
>           break;
>       }
> -    return 0;
> -#else
> -    if (discard == VIR_DOMAIN_DISK_DISCARD_DEFAULT)
> -        return 0;
> -    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                   _("This version of libxenlight does not support "
> -                     "disk 'discard' option passing"));
> -    return -1;
> -#endif
>   }
>   
>   static void
> @@ -1209,8 +1199,7 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk)
>       x_disk->removable = 1;
>       x_disk->readwrite = !l_disk->src->readonly;
>       x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0;
> -    if (libxlDiskSetDiscard(x_disk, l_disk->discard) < 0)
> -        return -1;
> +    libxlDiskSetDiscard(x_disk, l_disk->discard);
>       libxlDiskSetScript(x_disk, src);

Ah, looks like it came from a base which included downstream patch 
libxl-support-block-script.patch. It's a hacky patch that includes some 
SUSE-specific stuff, so not really upstream material IMO.

Please rebase on libvirt.git master.

Thanks,
Jim

Am Fri, 13 Aug 2021 07:52:31 -0600
schrieb Jim Fehlig <jfehlig@suse.com>:

> Why remove the cast?

Because ->discard is of type virDomainDiskDiscard, the function can receive such type right away.


Olaf

On 8/13/21 8:00 AM, Olaf Hering wrote:
> Am Fri, 13 Aug 2021 07:52:31 -0600
> schrieb Jim Fehlig <jfehlig@suse.com>:
> 
>> Why remove the cast?
> 
> Because ->discard is of type virDomainDiskDiscard, the function can receive such type right away.

Ah, I missed the change to the function signature.

Jim