5:29 a.m.
On Thu, Oct 16, 2008 at 11:52:48AM +0200, Chris Lalancette wrote:
Chris Lalancette wrote:
> To support LVM partitioning in oVirt, one of the things we need is the ability
> to tell what kind of label is currently on a block device. Here, a 'label'
is
> used in the same sense that it is used in parted; namely, it defines which kind
> of partition table is on the disk, whether it be DOS, LVM2, SUN, BSD, etc. Note
> that this is different than the partition type; those are things like Linux,
> FAT16, FAT32, etc.
Updated patch based on jim's feedback. I did not remove the pc98 type; if
everyone else thinks it's a good idea, I'll remove it, but it is part of the ABI
in some sense. Otherwise, I followed all of jim's suggestions.
I think we can keep it for completeness, since partd supports it, it is
conceivable (though unlikely) that we can see it and its not a real
burden to keep it.
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.21
diff -u -r1.21 storage_backend.c
--- src/storage_backend.c 5 Sep 2008 12:03:45 -0000 1.21
+++ src/storage_backend.c 16 Oct 2008 09:49:01 -0000
@@ -192,6 +192,30 @@
return ret;
}
+struct diskType disk_types[] = {
This can be const const - surprised Jim didn't catch this :-)
+ { "lvm2", VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8,
0x31303020324D564CUL },
+ { "gpt", VIR_STORAGE_POOL_DISK_GPT, 0x200, 8, 0x5452415020494645UL },
+ { "dvh", VIR_STORAGE_POOL_DISK_DVH, 0x0, 4, 0x41A9E50BUL },
+ { "mac", VIR_STORAGE_POOL_DISK_MAC, 0x0, 2, 0x5245UL },
+ { "bsd", VIR_STORAGE_POOL_DISK_BSD, 0x40, 4, 0x82564557UL },
+ { "sun", VIR_STORAGE_POOL_DISK_SUN, 0x1fc, 2, 0xBEDAUL },
+ /*
+ * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
+ * we can't use that. At the moment I'm relying on the "dummy"
IPL
+ * bootloader data that comes from parted. Luckily, the chances of running
+ * into a pc98 machine running libvirt are approximately nil.
+ */
+ /*{ "pc98", 0x1fe, 2, 0xAA55UL },*/
+ { "pc98", VIR_STORAGE_POOL_DISK_PC98, 0x0, 8, 0x314C5049000000CBUL },
+ /*
+ * NOTE: the order is important here; some other disk types (like GPT and
+ * and PC98) also have 0x55AA at this offset. For that reason, the DOS
+ * one must be the last one.
+ */
+ { "dos", VIR_STORAGE_POOL_DISK_DOS, 0x1fe, 2, 0xAA55UL },
+ { NULL, -1, 0x0, 0, 0x0UL },
+};
+
Remove the strings from here - they're not required for the probing - only
the constant it needed. The string-ification of the types is better handled
by our enum support
+int
+virStorageBackendDiskLabelFormatFromString(virConnectPtr conn ATTRIBUTE_UNUSED,
+ const char *format) {
+ int i;
+
+ if (format == NULL)
+ return VIR_STORAGE_POOL_DISK_UNKNOWN;
+
+ for (i = 0; disk_types[i].name != NULL; i++) {
+ if (STREQ(format, disk_types[i].name))
+ return disk_types[i].part_table_type;
+ }
+
+ return VIR_STORAGE_POOL_DISK_UNKNOWN;
+}
+
+const char *
+virStorageBackendDiskLabelFormatToString(virConnectPtr conn ATTRIBUTE_UNUSED,
+ int format) {
+ int i;
+
+ for (i = 0; disk_types[i].name != NULL; i++) {
+ if (format == disk_types[i].part_table_type)
+ return disk_types[i].name;
+ }
+
+ return "unknown";
+}
I know you're just replicating the existing code, but both these functions can
be killed off and replaced with auto-generated code from our enum support
macros
VIR_ENUM_IMPL(virStorageBackendDiskLabel,
VIR_STORAGE_POOL_DISK_LAST,
"dos", "dvh", "gpt", "mac",
"bsd", "pc98", "sun", "lvm2"
"unknown")
And in the header file just
VIR_ENUM_DECL(virStorageBackendDiskLabel)
#ifndef __MINGW32__
/*
Index: src/storage_backend.h
===================================================================
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.7
diff -u -r1.7 storage_backend.h
--- src/storage_backend.h 2 Sep 2008 14:15:42 -0000 1.7
+++ src/storage_backend.h 16 Oct 2008 09:49:01 -0000
@@ -56,6 +56,29 @@
VIR_STORAGE_BACKEND_POOL_SOURCE_NAME = (1<<4),
};
+enum partTableType {
+ VIR_STORAGE_POOL_DISK_DOS = 1,
+ VIR_STORAGE_POOL_DISK_DVH,
+ VIR_STORAGE_POOL_DISK_GPT,
+ VIR_STORAGE_POOL_DISK_MAC,
+ VIR_STORAGE_POOL_DISK_BSD,
+ VIR_STORAGE_POOL_DISK_PC98,
+ VIR_STORAGE_POOL_DISK_SUN,
+ VIR_STORAGE_POOL_DISK_LVM2,
+ /* the "unknown" disk is 1 billion (and not, for instance, -1), to make
+ sure it doesn't run afoul of error checking */
+ VIR_STORAGE_POOL_DISK_UNKNOWN = 1 * 1024 * 1024 * 1024,
I don't understand why it has to be 1-billion ? For the enum support to
work correctly, we need this to be contiguous, starting from zero, and
and as the last element have
VIR_STORAGE_POOL_DISK_LAST
This perhaps suggests that DISK_UNKNOWN should be the first entry so
that if you have a zero'd struct with a disk type, it'll get defaulted
to UNKOWN
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|