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

Re: [Libguestfs] [PATCH] part-list: add support for show partition type



On Friday 13 March 2015 05:04:56 Chen Hanxiao wrote:
> We lack of showing parttition type for part-list command.
> This patch will add support for this.

You cannot extend the struct returned by part_init with new members,
that will cause compatibility issues with existing C users of that API.

> Also 'parted -m' did not provide partition type info,
> remove code section for 'parted -m' process.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao cn fujitsu com>
> ---
>  daemon/parted.c      | 155 ++++++++++++++++++++++++++-------------------------
>  generator/structs.ml |   1 +
>  resize/resize.ml     |   2 +-
>  3 files changed, 82 insertions(+), 76 deletions(-)
> 
> diff --git a/daemon/parted.c b/daemon/parted.c
> index a7bcb99..312b15f 100644
> --- a/daemon/parted.c
> +++ b/daemon/parted.c
> @@ -33,6 +33,10 @@ GUESTFSD_EXT_CMD(str_parted, parted);
>  GUESTFSD_EXT_CMD(str_sfdisk, sfdisk);
>  GUESTFSD_EXT_CMD(str_sgdisk, sgdisk);
>  
> +#ifndef PARTED_NO_M
> +# define PARTED_NO_M 0
> +#endif
> +
>  /* Notes:
>   *
>   * Parted 1.9 sends error messages to stdout, hence use of the
> @@ -451,11 +455,7 @@ do_part_get_parttype (const char *device)
>  guestfs_int_partition_list *
>  do_part_list (const char *device)
>  {
> -  int parted_has_m_opt = test_parted_m_opt ();
> -  if (parted_has_m_opt == -1)
> -    return NULL;
> -
> -  CLEANUP_FREE char *out = print_partition_table (device, parted_has_m_opt);
> +  CLEANUP_FREE char *out = print_partition_table (device, PARTED_NO_M);
>    if (!out)
>      return NULL;
>  
> @@ -466,88 +466,86 @@ do_part_list (const char *device)
>  
>    guestfs_int_partition_list *r;
>  
> -  if (parted_has_m_opt) {
> -    /* New-style parsing using the "machine-readable" format from
> -     * 'parted -m'.
> -     *
> -     * lines[0] is "BYT;", lines[1] is the device line which we ignore,
> -     * lines[2..] are the partitions themselves.  Count how many.
> -     */
> -    size_t nr_rows = 0, row;
> -    for (row = 2; lines[row] != NULL; ++row)
> -      ++nr_rows;
> -
> -    r = malloc (sizeof *r);
> -    if (r == NULL) {
> -      reply_with_perror ("malloc");
> -      return NULL;
> +  /* Old-style.  Start at the line following "^Number", up to the
> +   * next blank line.
> +   */
> +  size_t start = 0, end = 0, has_type = 0, row;
> +
> +  for (row = 0; lines[row] != NULL; ++row)
> +    if (STRPREFIX (lines[row], "Number")) {
> +      start = row+1;
> +    /* check whether output of parted has 'Type' field */
> +    if (strstr (lines[row], "Type"))
> +      has_type = 1;
> +      break;
>      }
> -    r->guestfs_int_partition_list_len = nr_rows;
> -    r->guestfs_int_partition_list_val =
> -      malloc (nr_rows * sizeof (guestfs_int_partition));
> -    if (r->guestfs_int_partition_list_val == NULL) {
> -      reply_with_perror ("malloc");
> -      goto error2;
> +
> +  if (start == 0) {
> +    reply_with_error ("parted output has no \"Number\" line");
> +    return NULL;
> +  }
> +
> +  for (row = start; lines[row] != NULL; ++row)
> +    if (STREQ (lines[row], "")) {
> +      end = row;
> +      break;
>      }
>  
> -    /* Now parse the lines. */
> -    size_t i;
> -    for (i = 0, row = 2; lines[row] != NULL; ++i, ++row) {
> -      if (sscanf (lines[row], "%d:%" SCNi64 "B:%" SCNi64 "B:%" SCNi64 "B",
> +  if (end == 0) {
> +    reply_with_error ("parted output has no blank after end of table");
> +    return NULL;
> +  }
> +
> +  size_t nr_rows = end - start;
> +
> +  r = malloc (sizeof *r);
> +  if (r == NULL) {
> +    reply_with_perror ("malloc");
> +    return NULL;
> +  }
> +  r->guestfs_int_partition_list_len = nr_rows;
> +  r->guestfs_int_partition_list_val =
> +    malloc (nr_rows * sizeof (guestfs_int_partition));
> +  if (r->guestfs_int_partition_list_val == NULL) {
> +    reply_with_perror ("malloc");
> +    goto error2;
> +  }
> +
> +  /* Now parse the lines. */
> +  size_t i, k;
> +  if (has_type) {
> +    /* hold type such as primary, logical and extended */
> +    char type_temp[16];
> +    for (i = 0, row = start; row < end; ++i, ++row) {
> +      if (sscanf (lines[row], " %d %" SCNi64 "B %" SCNi64 "B %" SCNi64 "B" "%s",
>                    &r->guestfs_int_partition_list_val[i].part_num,
>                    &r->guestfs_int_partition_list_val[i].part_start,
>                    &r->guestfs_int_partition_list_val[i].part_end,
> -                  &r->guestfs_int_partition_list_val[i].part_size) != 4) {
> +                  &r->guestfs_int_partition_list_val[i].part_size,
> +                  type_temp) != 5) {
>          reply_with_error ("could not parse row from output of parted print command: %s", lines[row]);
>          goto error3;
>        }
> -    }
> -  }
> -  else {
> -    /* Old-style.  Start at the line following "^Number", up to the
> -     * next blank line.
> -     */
> -    size_t start = 0, end = 0, row;
>  
> -    for (row = 0; lines[row] != NULL; ++row)
> -      if (STRPREFIX (lines[row], "Number")) {
> -        start = row+1;
> -        break;
> +      if (STRPREFIX (type_temp, "primary")) {
> +        r->guestfs_int_partition_list_val[i].part_type = strdup("primary");
> +        if (r->guestfs_int_partition_list_val[i].part_type == NULL)
> +          goto error4;
> +      } else if (STRPREFIX (type_temp, "logical")) {
> +        r->guestfs_int_partition_list_val[i].part_type = strdup("logical");
> +        if (r->guestfs_int_partition_list_val[i].part_type == NULL)
> +          goto error4;
> +      } else if (STRPREFIX (type_temp, "extended")) {
> +        r->guestfs_int_partition_list_val[i].part_type = strdup("extended");
> +        if (r->guestfs_int_partition_list_val[i].part_type == NULL)
> +          goto error4;
> +      } else {
> +        r->guestfs_int_partition_list_val[i].part_type = strdup("");
> +        if (r->guestfs_int_partition_list_val[i].part_type == NULL)
> +          goto error4;
>        }
> -
> -    if (start == 0) {
> -      reply_with_error ("parted output has no \"Number\" line");
> -      return NULL;
>      }
> -
> -    for (row = start; lines[row] != NULL; ++row)
> -      if (STREQ (lines[row], "")) {
> -        end = row;
> -        break;
> -      }
> -
> -    if (end == 0) {
> -      reply_with_error ("parted output has no blank after end of table");
> -      return NULL;
> -    }
> -
> -    size_t nr_rows = end - start;
> -
> -    r = malloc (sizeof *r);
> -    if (r == NULL) {
> -      reply_with_perror ("malloc");
> -      return NULL;
> -    }
> -    r->guestfs_int_partition_list_len = nr_rows;
> -    r->guestfs_int_partition_list_val =
> -      malloc (nr_rows * sizeof (guestfs_int_partition));
> -    if (r->guestfs_int_partition_list_val == NULL) {
> -      reply_with_perror ("malloc");
> -      goto error2;
> -    }
> -
> -    /* Now parse the lines. */
> -    size_t i;
> +  } else {
>      for (i = 0, row = start; row < end; ++i, ++row) {
>        if (sscanf (lines[row], " %d %" SCNi64 "B %" SCNi64 "B %" SCNi64 "B",
>                    &r->guestfs_int_partition_list_val[i].part_num,
> @@ -557,11 +555,18 @@ do_part_list (const char *device)
>          reply_with_error ("could not parse row from output of parted print command: %s", lines[row]);
>          goto error3;
>        }
> +
> +      r->guestfs_int_partition_list_val[i].part_type = strdup("");
> +      if (r->guestfs_int_partition_list_val[i].part_type == NULL)
> +        goto error4;
>      }
>    }
>  
>    return r;
>  
> + error4:
> + for (k = 0; k <= i; k++)
> +  free (r->guestfs_int_partition_list_val[k].part_type);
>   error3:
>    free (r->guestfs_int_partition_list_val);
>   error2:
> diff --git a/generator/structs.ml b/generator/structs.ml
> index ea110a1..e7a9fa6 100644
> --- a/generator/structs.ml
> +++ b/generator/structs.ml
> @@ -240,6 +240,7 @@ let structs = [
>      "part_start", FBytes;
>      "part_end", FBytes;
>      "part_size", FBytes;
> +    "part_type", FString;
>      ];
>      s_camel_name = "Partition" };
>  

I'd just add a new part_get_type (or something similar), which would
call parted to return "primary"/etc. This would match what we do
already with other attributes of partitions, and not force part_list
to use a non-machine output type (which is not really meant for
machine parsing).

> diff --git a/resize/resize.ml b/resize/resize.ml
> index 84fd6d4..8f8f67f 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -1078,7 +1078,7 @@ read the man page virt-resize(1).
>             *)
>            p_name = "";
>            p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
> -                     part_size = 0L };
> +                     part_size = 0L; part_type = "" };
>            p_bootable = false; p_id = No_ID; p_type = ContentUnknown;
>            p_label = None; p_guid = None;

Better add this information directly in the type partition, possibly
also making use of type partition_type.

Also this could go in a different patch than the above changes to get
the primary/extended/etc type of a partition.

Thanks,
-- 
Pino Toscano


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