Shriram Rajagopalan
2011-Feb-25 18:41 UTC
[Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
# 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Stodden
2011-Feb-25 19:38 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
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 32Ack, 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; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Feb-25 20:11 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Feb-28 16:28 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Mar-03 16:48 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Daniel Stodden
2011-Mar-03 21:30 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Mar-07 16:16 UTC
Re: [Xen-devel] [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering [and 1 more messages]
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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel