[Libguestfs] [PATCH VERSION 4] Generic partition creation interface.

[Richard W.M. Jones]

On Thu, Nov 05, 2009 at 05:19:33PM +0100, Jim Meyering wrote:
> > +int
> > +do_part_add (const char *device, const char *prlogex,
> > +             int64_t startsect, int64_t endsect)
> > +{
> > +  char *err;
> > +  int r;
> > +  char startstr[32];
> > +  char endstr[32];
> > +
> > +  /* Check and translate prlogex. */
> > +  if (strcmp (prlogex, "primary") == 0 ||
> > +      strcmp (prlogex, "logical") == 0 ||
> > +      strcmp (prlogex, "extended") == 0)
> > +    ;
> > +  else if (strcmp (prlogex, "p") == 0)
> > +    prlogex = "primary";
> > +  else if (strcmp (prlogex, "l") == 0)
> > +    prlogex = "logical";
> > +  else if (strcmp (prlogex, "e") == 0)
> > +    prlogex = "extended";
> > +  else {
> > +    reply_with_error ("part-add: unknown partition type: %s: this should be \"primary\", \"logical\" or \"extended\"", prlogex);
> > +    return -1;
> 
> Did you consider enforcing this only for msdos partition tables
> and allowing arbitrary names for GPT?

This is a bit tricky from the API point of view.  If we were
going to allow names too, we'd want something like:

  guestfs_part_add (g, device, prlogex, name, startsect, endsect);

with name being an optional string and we'd have to enforce that the
string is not set for msdos and set for gpt.  Of course in
guestfs_part_add we don't know the partition table type, unless we
saved it from an earlier call or look it up, both of which are hairy.

It's even worse though:

$ parted -s /tmp/test.img -- \
  mklabel gpt \
  mkpart primary 34s 256s \
  mkpart primary 257s -34s

$ parted /tmp/test.img print
WARNING: You are not superuser.  Watch out for permissions.
Model:  (file)
Disk /tmp/test.img: 10.5MB
Sector size (logical/physical): 512B/512B
Partition Table: gpt

Number  Start   End     Size    File system  Name     Flags
 1      17.4kB  132kB   114kB                primary
 2      132kB   10.5MB  10.3MB               primary

We've now got two GPT partitions named "primary".  (At least parted
allows this and doesn't barf).

The undocumented name feature of parted mkpart is a minefield ...

I've now added a guestfs_part_set_name call to the API [patch to
follow] so at least people can change the name later.

[in do_part_disk]
> I created many GPT partition tables with "primary" as the name
> of each partition, before I noticed the error.
> To avoid giving the impression that libguestfs is doing
> this out of ignorance, how about using e.g.,
> 
>       strcmp (parttype, "gpt") == 0 ? "p1" : "primary"

I agree in the guestfs_part_disk case we can do better because we do
know the partition name, so this is a very sensible change.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v