[PATCH] blktap2: fix gap in tapdisk2 disk_type numbering

[PATCH] blktap2: fix gap in tapdisk2 disk_type numbering

[Shriram Rajagopalan]

# 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

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

[Daniel Stodden]

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;

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

[Shriram Rajagopalan]

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

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

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 3367 bytes --]

On Fri, Feb 25, 2011 at 12:11 PM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:

> 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?
> >
>
So is the current patch okay? (the one which just re-numbers the the disk
ids)
I could send out another separate patch to handle gaps over disk ids or
replace
this patch with V2 that does just that.
shriram

> > 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
>

[-- Attachment #1.2: Type: text/html, Size: 4659 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

[Ian Jackson]

Shriram Rajagopalan writes ("Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering"):
> So is the current patch okay? (the one which just re-numbers the the
> disk ids) I could send out another separate patch to handle gaps
> over disk ids or replace this patch with V2 that does just that.

Daniel, would you care to advise ?  We are currently in codefreeze for
the Xen 4.1 release and we would certainly like some advice here.

Ideally I'd like an ack/nack on Shriram's patch.

Thanks,
Ian.

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

[Daniel Stodden]

On Thu, 2011-03-03 at 11:48 -0500, Ian Jackson wrote:
> Shriram Rajagopalan writes ("Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering"):
> > So is the current patch okay? (the one which just re-numbers the the
> > disk ids) I could send out another separate patch to handle gaps
> > over disk ids or replace this patch with V2 that does just that.
> 
> Daniel, would you care to advise ?  We are currently in codefreeze for
> the Xen 4.1 release and we would certainly like some advice here.
> 
> Ideally I'd like an ack/nack on Shriram's patch.

Re-Ack. Unverified, but looks low-risk to me.

Daniel

Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering [and 1 more messages]

[Ian Jackson]

Shriram Rajagopalan writes ("[Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering"):
> blktap2: fix gap in tapdisk2 disk_type numbering

Thanks, I have applied this to both xen-unstable and 4.1-testing.

Ian.