[Libguestfs] [PATCH] common/options: Change drv struct to store drive index instead of device name.
Richard W.M. Jones
rjones at redhat.com
Wed May 3 18:28:18 UTC 2017
On Wed, May 03, 2017 at 06:28:43PM +0200, Pino Toscano wrote:
> 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);
OK, I'll make this change.
> 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.
Indeed it did, but only when I was adding 60k+ drives ... My solution
there was just to increase the size of the stack.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list