[Libguestfs] [PATCH v2 03/18] New API parameter: Add discard parameter to guestfs_add_drive_opts.

[Pino Toscano]

On Tuesday 11 March 2014 23:13:46 Richard W.M. Jones wrote:
> diff --git a/src/drives.c b/src/drives.c
> index 2c85b52..68e37f7 100644
> --- a/src/drives.c
> +++ b/src/drives.c
> @@ -115,7 +115,8 @@ static struct drive *
>  create_drive_file (guestfs_h *g, const char *path,
>                     bool readonly, const char *format,
>                     const char *iface, const char *name,
> -                   const char *disk_label, const char *cachemode)
> +                   const char *disk_label, const char *cachemode,
> +                   enum discard discard)
>  {
>    struct drive *drv = safe_calloc (g, 1, sizeof *drv);
> 

(and others)

It seems like these functions, albeit internal and private to this
file, could need some help in avoid the over-long list of parameters,
which all need to be changed every time there's an argument change
(just like now).

I'll do a small refactor.

> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -231,6 +231,12 @@ struct drive_source {
>    char *secret;
>  };
> 
> +enum discard {
> +  discard_disable = 0,
> +  discard_enable = 1,
> +  discard_besteffort = 2,
> +};

I guess the manual numbering is not needed.

> 
> -      if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe")
> +      if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL,
> +                                                  xo, "qcow2", "unsafe",
> +                                                  discard_disable) == -1)
>          return -1;
>      }

... and ...

> @@ -1499,7 +1544,9 @@ construct_libvirt_xml_appliance (guestfs_h *g,
>        attribute ("bus", "scsi");
>      } end_element ();
> 
> -    if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") == -1)
> +    if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL, xo,
> +                                                "qcow2", "unsafe",
> +                                                discard_disable) == -1) return -1;
> 
>      if (construct_libvirt_xml_disk_address (g, xo,

Even if passing discard_disable makes sure that now "data" and "drv"
are not used within construct_libvirt_xml_disk_driver_qemu, wouldn't be
better to pass them anyway if possible, so further code might use them
(at least "data") safely?

-- 
Pino Toscano