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

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?