Re: [Libguestfs] [PATCH VERSION 4] Generic partition creation interface.
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