[Libguestfs] [PATCH] NEW API: add blkid command to print the attributes of the device

[Richard W.M. Jones]

On Sat, Dec 03, 2011 at 12:34:06PM +0800, Wanlong Gao wrote:
> A NEW API blkid.
> It can print the device attributes.
> Use it after list-devices, we can list ower devices and the attributes
> of each device.
> Use it like:
> blkid <device>
> It's should be a usefull function, and needed no test case for it.

Yes, in principle.

However it certainly _does_ need a test, in case the blkid command
breaks or changes in future.

Further comments on the patch below.

> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/blkid.c                 |   75 ++++++++++++++++++++++++++++++++++++++++
>  generator/generator_actions.ml |   73 ++++++++++++++++++++++++++++++++++++++
>  src/MAX_PROC_NR                |    2 +-
>  3 files changed, 149 insertions(+), 1 deletions(-)
> 
> diff --git a/daemon/blkid.c b/daemon/blkid.c
> index 6d395c1..a7fd6bc 100644
> --- a/daemon/blkid.c
> +++ b/daemon/blkid.c
> @@ -83,3 +83,78 @@ do_vfs_uuid (const char *device)
>  {
>    return get_blkid_tag (device, "UUID");
>  }
> +
> +char **
> +do_blkid(const char *device)
> +{
> +  int r;
> +  char *out = NULL, *err = NULL;
> +  char **lines = NULL;
> +
> +  char **ret = NULL;
> +  int size = 0, alloc = 0;
> +
> +  const char *blkid[] = {"blkid", "-p", "-i", "-o", "export", device, NULL};
> +  r = commandv(&out, &err, blkid);
> +  if (r == -1) {
> +    reply_with_error("%s", err);
> +    goto error;
> +  }
> +
> +  /* Split the command output into lines */
> +  lines = split_lines(out);
> +  if (lines == NULL) {
> +    reply_with_perror("malloc");
> +    goto error;
> +  }
> +
> +  /* Parse the output of blkid -p -i -o export:
> +   * UUID=b6d83437-c6b4-4bf0-8381-ef3fc3578590
> +   * VERSION=1.0
> +   * TYPE=ext2
> +   * USAGE=filesystem
> +   * MINIMUM_IO_SIZE=512
> +   * PHYSICAL_SECTOR_SIZE=512
> +   * LOGICAL_SECTOR_SIZE=512
> +   * PART_ENTRY_SCHEME=dos
> +   * PART_ENTRY_TYPE=0x83
> +   * PART_ENTRY_NUMBER=6
> +   * PART_ENTRY_OFFSET=642875153
> +   * PART_ENTRY_SIZE=104857600
> +   * PART_ENTRY_DISK=8:0
> +   */
> +  for (char **i = lines; *i != NULL; i++) {
> +    char *line = *i;
> +
> +    /* Skip blank lines (shouldn't happen) */
> +    if (line[0] == '\0') continue;
> +
> +    /* Split the line in 2 at the equals sign */
> +    char *eq = strchr(line, '=');
> +    if (eq) {
> +      *eq = '\0'; eq++;
> +
> +      /* Add the key/value pair to the output */
> +      if (add_string(&ret, &size, &alloc, line) == -1 ||
> +          add_string(&ret, &size, &alloc, eq) == -1) goto error;
> +    } else {
> +      fprintf(stderr, "blkid: unexpected blkid output ignored: %s", line);
> +    }
> +  }
> +
> +  free(out);
> +  free(err);
> +  free(lines);
> +
> +  if (add_string(&ret, &size, &alloc, NULL) == -1) return NULL;
> +
> +  return ret;
> +
> +error:
> +  free(out);
> +  free(err);
> +  if (lines) free(lines);
> +  if (ret) free_strings(ret);
> +
> +  return NULL;
> +}
> diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
> index 0e39e2f..d624f9b 100644
> --- a/generator/generator_actions.ml
> +++ b/generator/generator_actions.ml
> @@ -6538,6 +6538,79 @@ The name of the MD device.
>  This command deactivates the MD array named C<md>.  The
>  device is stopped, but it is not destroyed or zeroed.");
>  
> +  ("blkid", (RHashtable "info", [Device "device"], []), 303, [],
> +   [],
> +   "print block device attributes",
> +   "\
> +This command exposes the output of 'blkid -p -i -o export <device>'. The following

Don't describe how the command is implemented -- we may change the
implementation in future.  Be less specific, such as "This command
returns block device attributes for C<device>".  Also remember this is
POD documentation, so you can't use "<" and ">" characters without
escaping them.

> +C<device> should be a block device or patition, for example C</dev/sda> or C</dev/sdb1>.

This example is unnecessary -- using Device ensures that users have to
pass a device name here.

> +=over
> +
> +=item C<UUID>
> +
> +The uuid of this device.
> +
> +=item C<UUID_SUB>
> +
> +The sub uuid of this device.
> +
> +=item C<LABEL>
> +
> +The label of this device.
> +
> +=item C<VERSION>
> +
> +The version of blkid command.
> +
> +=item C<TYPE>
> +
> +The filesystem type or RAID of this device.
> +
> +=item C<USAGE>
> +
> +The usage of this device, for example C<filesystem> or C<raid>.
> +
> +=item C<MINIMUM_IO_SIZE>
> +
> +The minimum io size of this device.
> +
> +=item C<PHYSICAL_SECTOR_SIZE>
> +
> +The pyhsical sector size of this device.
> +
> +=item C<LOGICAL_SECTOR_SIZE>
> +
> +The logical sector size of this device.
> +
> +=item C<PART_ENTRY_SCHEME>
> +
> +The scheme entry of this partition(only presented when it's a partition).

^^ Add a space.

[...]

How many of these fields does blkid define?  Are they likely to change
in future?  If we promise that fields are returned then we have to
stick to that promise in future because of libguestfs ABI commitments.
It's better to promise fewer fields, and only ones that we could
implement by hand if blkid disappeared in future.

This also needs a test -- see "tune2fs" in generator/generator_actions.ml
for an example using TestOutputHashtable.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top