[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] inspect: get windows drive letters for GPT disks.



On Fri, 2016-02-05 at 18:08 +0000, Richard W.M. Jones wrote:
> On Fri, Feb 05, 2016 at 12:15:32PM -0500, Dawid Zamirski wrote:
> > This patch updates the guestfs_inspect_get_drive_mappings API call
> > to
> > also return drive letters for GPT paritions. Previously this worked
> > only for MBR partitions. This is achieved by matching the GPT
> > partition
> > GUID with the info stored in the blob from
> > HKLM\SYSTEM\MountedDevices\DosDevices keys. For GPT partions this
> > blob
> > contains a "DMIO:ID:" prefix followed by a 16 byte binary GUID.
> > ---
> >  src/inspect-fs-windows.c | 98
> > +++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 96 insertions(+), 2 deletions(-)
> 
> Attached is a patch with some stylistic updates.  Please combine it
> with your patch.  More comments below.
> 
> > diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c
> > index ccf5cba..6571d37 100644
> > --- a/src/inspect-fs-windows.c
> > +++ b/src/inspect-fs-windows.c
> > @@ -25,6 +25,7 @@
> >  #include <string.h>
> >  #include <errno.h>
> >  #include <iconv.h>
> > +#include <inttypes.h>
> >  
> >  #ifdef HAVE_ENDIAN_H
> >  #include <endian.h>
> > @@ -57,6 +58,8 @@ static int check_windows_arch (guestfs_h *g,
> > struct inspect_fs *fs);
> >  static int check_windows_software_registry (guestfs_h *g, struct
> > inspect_fs *fs);
> >  static int check_windows_system_registry (guestfs_h *g, struct
> > inspect_fs *fs);
> >  static char *map_registry_disk_blob (guestfs_h *g, const void
> > *blob);
> > +static char *map_registry_disk_blob_gpt(guestfs_h *g, const void
> > *blob);
> > +static char *extract_guid_from_registry_blob(guestfs_h *g, const
> > void *blob);
> >  
> >  /* XXX Handling of boot.ini in the Perl version was pretty
> > broken.  It
> >   * essentially didn't do anything for modern Windows guests.
> > @@ -386,6 +389,7 @@ check_windows_system_registry (guestfs_h *g,
> > struct inspect_fs *fs)
> >    int r;
> >    size_t len = strlen (fs->windows_systemroot) + 64;
> >    char system[len];
> > +  char gpt_prefix[] = "DMIO:ID:";
> >    snprintf (system, len, "%s/system32/config/system",
> >              fs->windows_systemroot);
> >  
> > @@ -493,9 +497,14 @@ check_windows_system_registry (guestfs_h *g,
> > struct inspect_fs *fs)
> >  
> >        type = guestfs_hivex_value_type (g, v);
> >        blob = guestfs_hivex_value_value (g, v, &len);
> > -      if (blob != NULL && type == 3 && len == 12) {
> > +      bool is_gpt = memcmp(blob, gpt_prefix, 8) == 0;
> > +      if (blob != NULL && type == 3 && (len == 12 || is_gpt)) {
> >          /* Try to map the blob to a known disk and partition. */
> > -        device = map_registry_disk_blob (g, blob);
> > +        if (is_gpt)
> > +          device = map_registry_disk_blob_gpt (g, blob);
> > +        else
> > +          device = map_registry_disk_blob (g, blob);
> > +
> >          if (device != NULL) {
> >            fs->drive_mappings[count++] = safe_strndup (g, &key[12],
> > 1);
> >            fs->drive_mappings[count++] = device;
> > @@ -605,6 +614,91 @@ map_registry_disk_blob (guestfs_h *g, const
> > void *blob)
> >    return safe_asprintf (g, "%s%d", devices[i], partitions-
> > >val[j].part_num);
> >  }
> >  
> > +/* Matches Windows registry HKLM\SYSYTEM\MountedDevices\DosDevices
> > blob to
> > + * to libguestfs GPT partition device. For GPT disks, the blob is
> > made of
> > + * "DMIO:ID:" prefix followed by the GPT parition GUID.
> > + */
> > +static char *
> > +map_registry_disk_blob_gpt (guestfs_h *g, const void *blob)
> > +{
> > +  CLEANUP_FREE_STRING_LIST char **fses = NULL;
> > +  size_t i;
> > +
> > +  fses = guestfs_list_filesystems (g);
> > +  if (fses == NULL)
> > +    return NULL;
> 
> Did you really mean to call guestfs_list_filesystems here?  If you're
> trying to get the list of partitions, then guestfs_list_partitions is
> what you should be calling.
> 

First, thank you for a prompt review. Yes, I meant to use
guestfs_list_filesystems for save myself from doing nested loops - one
to pull list of devices (guestfs_list_devices) and another to loop
through partitions on each device. If that's a problem, please let me
know and I'll change it to follow what map_registry_blob does.

> > +  for (i = 0; fses[i] != NULL; i += 2) {
> > +    CLEANUP_FREE char *fs_guid = NULL;
> > +    CLEANUP_FREE char *blob_guid = NULL;
> > +    CLEANUP_FREE char *fs_dev  = NULL;
> > +
> > +    fs_dev = guestfs_canonical_device_name (g, fses[i]);
> > +
> > +    if (fs_dev == NULL)
> > +        continue;
> > +
> > +    int partnum = guestfs_part_to_partnum (g, fs_dev);
> 
> Need to check that partnum != -1 and do something.
> 
> > +    CLEANUP_FREE char *device = guestfs_part_to_dev (g, fs_dev);
> > +    CLEANUP_FREE char *type = guestfs_part_get_parttype (g,
> > device);
> > +
> > +    if (STRCASENEQ(type, "gpt"))
> > +        continue;
> > +
> > +    /* get the GPT parition GUID from the partition block device
> > */
> > +    fs_guid = guestfs_part_get_gpt_guid (g, device, partnum);
> > +    
> > +    if (fs_guid == NULL)
> > +      continue;
> > +
> > +    /* extract the GUID from the Windows registry blob */
> > +    blob_guid = extract_guid_from_registry_blob (g, blob);
> > +
> > +    /* if both GUIDs match, we have found mapping for out device
> > */
> > +    if (STRCASEEQ (fs_guid, blob_guid))
> > +        return safe_strdup (g, fs_dev);
> > +  }
> > +
> > +  return NULL;
> > +}
> > +
> > +/* Extracts the binary GUID stored in blob from Windows registry 
> > + * HKLM\SYSTYEM\MountedDevices\DosDevices value and converts it to
> > a GUID string
> > + * so that it can be matched against libguestfs partition device
> > GPT GUID.
> > + */
> > +static char *
> > +extract_guid_from_registry_blob (guestfs_h *g, const void *blob)
> > +{
> > +  char guid_bytes[16];
> > +  size_t len = 37;
> > +  uint32_t data1;
> > +  uint16_t data2, data3;
> > +  uint64_t data4;
> > +
> > +  char *guid = (char *) safe_calloc(g, len, sizeof (char * ));
> > +
> > +  /* get the GUID bytes from blob (skip 8 byte "DMIO:ID:" prefix)
> > */
> > +  memcpy (&guid_bytes, (char *) blob + 8, sizeof (guid_bytes));
> > +
> > +  /* copy relevant sections from blob to respective ints */
> > +  memcpy (&data1, guid_bytes, sizeof (data1));
> > +  memcpy (&data2, guid_bytes + 4, sizeof (data2));
> > +  memcpy (&data3, guid_bytes + 6, sizeof (data3));
> > +  memcpy (&data4, guid_bytes + 8, sizeof (data4));
> > +
> > +  /* ensure proper endianness */
> > +  data1 = htole32 (data1);
> 
> Did you mean to use htole32?  Surely it should be le32toh?

yep it seems I got it backwards.
> 
> > +  data2 = htole16 (data2);
> > +  data3  = htole16 (data3);
> > +  data4 = htobe64 (data4);
> 
> And is this really big endian?

yes it is:
https://en.wikipedia.org/wiki/Globally_unique_identifier#Binary_encodin
g

though it should have been be64toh. Please note that data1-3 are
documented as native, but I assume LE as this data comes from Windows
which runs on x86 in most cases.

> 
> Rich.
> 

I'll send v2 soon.

Regards,
Dawid Zamirski


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]