[Libguestfs] [PATCH supermin] init: Support root=UUID=... to specify the appliance disk by volume UUID.

[Pino Toscano]

On Wednesday, 19 April 2017 18:43:41 CEST Richard W.M. Jones wrote:
> Instead of specifying a device name (eg. root=/dev/sdb), this permits
> specifying an ext4 volume UUID (root=UUID=12345678-...).  This allows
> the appliance to be robust against the non-determinism of SCSI device
> enumeration.
> ---
> [...]
> +
> +    for (delay_ns = 250000;
> +         delay_ns <= MAX_ROOT_WAIT * UINT64_C(1000000000);
> +         delay_ns *= 2) {
> +      if (parse_dev_file (path, &major, &minor) != -1) {
> +        if (!quiet)
> +          fprintf (stderr, "supermin: picked %s (%d:%d) as root device\n",
> +                   path, major, minor);
> +        break;
>        }
> +
> +      virtio_warning (delay_ns, path);
> +      NANOSLEEP (delay_ns);
>      }

This code for busy waiting could be factored in a common function --
printing either the /dev/path or the UUID.

 +static void
> +parse_root_uuid (const char *root, unsigned char *raw_uuid)
> +{
> +  size_t i;
> +
> +  i = 0;
> +  while (i < 16) {
> +    if (*root == '-') {
> +      ++root;
> +      continue;
> +    }
> +    if (!isxdigit (root[0]) || !isxdigit (root[1])) {
> +      fprintf (stderr, "supermin: root UUID is not a 16 byte UUID string\n");
> +      exit (EXIT_FAILURE);
> +    }
> +    raw_uuid[i++] = hexdigit (root[0]) * 0x10 + hexdigit (root[1]);
> +    root += 2;
> +  }
> +
> +  if (i < 16) {
> +    fprintf (stderr, "supermin: root UUID should be 16 bytes\n");
> +    exit (EXIT_FAILURE);
> +  }

Is this check actually ever going to be true? It looks to me that the
while loop above can be broken only in two cases:
a) the any of the isxdigit checks fails
b) the condition of the while becomes false
In case of (a), there's exit() already, so it will not execute anything
else anyway; in case of (b), then i will always be >= 16, and thus
never going to satisfy the check above.

Also, parse_root_uuid does not fail when the uuid string has more
character than the ones needed.  I'd suggest changing the last check
to something like:

  if (*root) {
    fprintf (stderr, "supermin: root UUID contains more than 16 bytes\n");
    exit (EXIT_FAILURE);
  }

Thanks,
-- 
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/20170425/e8fd0350/attachment.sig>