* [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering @ 2011-02-25 18:41 Shriram Rajagopalan 2011-02-25 19:38 ` Daniel Stodden 0 siblings, 1 reply; 7+ messages in thread From: Shriram Rajagopalan @ 2011-02-25 18:41 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson # 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering 2011-02-25 18:41 [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering Shriram Rajagopalan @ 2011-02-25 19:38 ` Daniel Stodden 2011-02-25 20:11 ` Shriram Rajagopalan 0 siblings, 1 reply; 7+ messages in thread From: Daniel Stodden @ 2011-02-25 19:38 UTC (permalink / raw) To: Shriram Rajagopalan; +Cc: xen-devel@lists.xensource.com 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; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering 2011-02-25 19:38 ` Daniel Stodden @ 2011-02-25 20:11 ` Shriram Rajagopalan 2011-02-28 16:28 ` Shriram Rajagopalan 0 siblings, 1 reply; 7+ messages in thread From: Shriram Rajagopalan @ 2011-02-25 20:11 UTC (permalink / raw) To: Daniel Stodden; +Cc: xen-devel@lists.xensource.com 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering 2011-02-25 20:11 ` Shriram Rajagopalan @ 2011-02-28 16:28 ` Shriram Rajagopalan 2011-03-03 16:48 ` Ian Jackson 0 siblings, 1 reply; 7+ messages in thread From: Shriram Rajagopalan @ 2011-02-28 16:28 UTC (permalink / raw) To: Daniel Stodden; +Cc: xen-devel@lists.xensource.com, Ian Jackson [-- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering 2011-02-28 16:28 ` Shriram Rajagopalan @ 2011-03-03 16:48 ` Ian Jackson 2011-03-03 21:30 ` Daniel Stodden 0 siblings, 1 reply; 7+ messages in thread From: Ian Jackson @ 2011-03-03 16:48 UTC (permalink / raw) To: Daniel Stodden; +Cc: rshriram, xen-devel@lists.xensource.com 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering 2011-03-03 16:48 ` Ian Jackson @ 2011-03-03 21:30 ` Daniel Stodden 2011-03-07 16:16 ` [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering [and 1 more messages] Ian Jackson 0 siblings, 1 reply; 7+ messages in thread From: Daniel Stodden @ 2011-03-03 21:30 UTC (permalink / raw) To: Ian Jackson; +Cc: rshriram@cs.ubc.ca, xen-devel@lists.xensource.com 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering [and 1 more messages] 2011-03-03 21:30 ` Daniel Stodden @ 2011-03-07 16:16 ` Ian Jackson 0 siblings, 0 replies; 7+ messages in thread From: Ian Jackson @ 2011-03-07 16:16 UTC (permalink / raw) To: Daniel Stodden, Shriram Rajagopalan; +Cc: xen-devel 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-03-07 16:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-25 18:41 [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering Shriram Rajagopalan 2011-02-25 19:38 ` Daniel Stodden 2011-02-25 20:11 ` Shriram Rajagopalan 2011-02-28 16:28 ` Shriram Rajagopalan 2011-03-03 16:48 ` Ian Jackson 2011-03-03 21:30 ` Daniel Stodden 2011-03-07 16:16 ` [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering [and 1 more messages] Ian Jackson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).