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