"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
+struct diskType disk_types[] = {
This can be const const - surprised Jim didn't catch this :-)
+struct diskType disk_types[] = {
Had to leave some for you. :-P
Now that you mention it, though, that should be "static", too.
> +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
Or maybe leave it as = 1, to force explicit definition to an enum value.
Then, wherever it's zeroed but not explicitly defined
it will show up as undefined (possibly triggering some sort
of error), rather than proceeding with a mere UNKNOWN.
I guess it depends on the relative worth of being able to
initialize with calloc-style allocation vs. cost of explicit
definition everywhere.