Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering

[Shriram Rajagopalan <rshriram@cs.ubc.ca>]

On Fri, Feb 25, 2011 at 11:38 AM, Daniel Stodden
<daniel.stodden@citrix.com> wrote:
> On Fri, 2011-02-25 at 13:41 -0500, Shriram Rajagopalan wrote:
>> # HG changeset patch
>> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> # Date 1298659167 28800
>> # Node ID f16d772fdb6c58518299d4c3780b846bcbee6165
>> # Parent  d9c73cceb29715bfafada4d80e79787cb4666816
>> blktap2: fix gap in tapdisk2 disk_type numbering
>>
>> Make the DISK_TYPE_* id numbering in tapdisk-disktypes.h contiguous.
>> Currently, id 8 is unallocated causing a null disk type entry in
>> tapdisk_disk_drivers array in tapdisk-disktypes.c. This causes the
>> function tapdisk_disktype_find() to return an error on encountering
>> disk types >7 (remus:, log:, etc.).
>>
>> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>>
>> diff -r d9c73cceb297 -r f16d772fdb6c tools/blktap2/drivers/tapdisk-disktype.h
>> --- a/tools/blktap2/drivers/tapdisk-disktype.h        Fri Feb 25 10:29:53 2011 -0800
>> +++ b/tools/blktap2/drivers/tapdisk-disktype.h        Fri Feb 25 10:39:27 2011 -0800
>> @@ -37,9 +37,9 @@
>>  #define DISK_TYPE_RAM         5
>>  #define DISK_TYPE_QCOW        6
>>  #define DISK_TYPE_BLOCK_CACHE 7
>> -#define DISK_TYPE_LOG         9
>> -#define DISK_TYPE_REMUS       10
>> -#define DISK_TYPE_VINDEX      11
>> +#define DISK_TYPE_LOG         8
>> +#define DISK_TYPE_REMUS       9
>> +#define DISK_TYPE_VINDEX      10
>>
>>  #define DISK_TYPE_NAME_MAX    32
>
> Ack, unless you think the the option of making tapdisk_disktype_find
> just skip over holes be cleaner?
>
> Not sure if the patch below applies cleanly, but you'll get the idea.
>
> Daniel
>
> changeset:   583:d83dab01b6fa
> user:        Daniel Stodden <daniel.stodden@citrix.com>
> date:        Tue Feb 15 01:37:44 2011 -0800
> summary:     Allow for holes in tapdisk_disk_types.
>
> diff -r 37f55e6d34ca -r d83dab01b6fa drivers/tapdisk-disktype.c
> --- a/drivers/tapdisk-disktype.c        Tue Feb 15 01:37:44 2011 -0800
> +++ b/drivers/tapdisk-disktype.c        Tue Feb 15 01:37:44 2011 -0800
> @@ -154,13 +154,19 @@
>        0,
>  };
>
> +#define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> +
>  int
>  tapdisk_disktype_find(const char *name)
>  {
> -       const disk_info_t *info;
>        int i;
>
> -       for (i = 0; info = tapdisk_disk_types[i], info != NULL; ++i) {
> +       for (i = 0; i < ARRAY_SIZE(tapdisk_disk_types); i++) {
> +               const disk_info_t *info = tapdisk_disk_types[i];
> +
> +               if (!info)
> +                       continue;
> +
>                if (strcmp(name, info->name))
>                        continue;
>
>
>

Yes I agree. I just wanted to avoid code level changes, since atm there
seemed to be only one such gap.
Actually, this patch could go in as another patch, so as to enable "future"
tapdisk2 disk_types to pick any un-allocated id, without worrying
about maintaining
continuity.

shriram