[Libguestfs] [PATCH] common/options: Change drv struct to store drive index instead of device name.

[Pino Toscano]

On Friday, 28 April 2017 11:08:21 CEST Richard W.M. Jones wrote:
> The device name is only used by guestfish (when using the -N option to
> prepare drives).  We constructed the device name very naively,
> basically ‘sprintf ("/dev/sd%c", next_drive)’.
> 
> This stores the device index instead, and only constructs the device
> name in guestfish.  Also the device name is constructed properly using
> guestfs_int_drive_name so it can cope with #drives > 26.
> ---

Looks nice, a couple of notes below.

> index 7ae8adf..26f77fd 100644
> --- a/align/scan.c
> +++ b/align/scan.c
> @@ -236,7 +236,7 @@ main (int argc, char *argv[])
>        error (EXIT_FAILURE, 0, _("--uuid option cannot be used with -a or -d"));
>  
>      /* Add domains/drives from the command line (for a single guest). */
> -    add_drives (drvs, 'a');
> +    add_drives (drvs, 0);

This, and in all the other files, is using 0 as starting index.
Since add_drives is a macro, I'd remove its drive_index parameter, and
always pass 0 to add_drives_handle.  The only change needed would be...

>  char
> -add_drives_handle (guestfs_h *g, struct drv *drv, char next_drive)
> +add_drives_handle (guestfs_h *g, struct drv *drv, size_t drive_index)
>  {
>    int r;
>    struct guestfs_add_drive_opts_argv ad_optargs;
>  
> -  if (next_drive > 'z')
> -    error (EXIT_FAILURE, 0, _("too many drives added on the command line"));
> -
>    if (drv) {
> -    next_drive = add_drives (drv->next, next_drive);
> +    drive_index = add_drives (drv->next, drive_index);

... here, to:

  drive_index = add_drives_handle (g, drv->next, drive_index);

Also, unrelated to these changes: since add_drives_handle is recursive,
I guess it will overflow the stack when trying to add lots of disks,
like the ones (i.e. even 20k!) I see in the experiments on your blog.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170503/c05ef510/attachment.sig>