From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shriram Rajagopalan Subject: Re: [PATCH] blktap2: fix gap in tapdisk2 disk_type numbering Date: Fri, 25 Feb 2011 12:11:56 -0800 Message-ID: References: <1298662680.19195.2153.camel@agari.van.xensource.com> Reply-To: rshriram@cs.ubc.ca Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1298662680.19195.2153.camel@agari.van.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Daniel Stodden Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On Fri, Feb 25, 2011 at 11:38 AM, Daniel Stodden wrote: > On Fri, 2011-02-25 at 13:41 -0500, Shriram Rajagopalan wrote: >> # HG changeset patch >> # User Shriram Rajagopalan >> # Date 1298659167 28800 >> # Node ID f16d772fdb6c58518299d4c3780b846bcbee6165 >> # Parent =A0d9c73cceb29715bfafada4d80e79787cb4666816 >> 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 >> >> diff -r d9c73cceb297 -r f16d772fdb6c tools/blktap2/drivers/tapdisk-diskt= ype.h >> --- a/tools/blktap2/drivers/tapdisk-disktype.h =A0 =A0 =A0 =A0Fri Feb 25= 10:29:53 2011 -0800 >> +++ b/tools/blktap2/drivers/tapdisk-disktype.h =A0 =A0 =A0 =A0Fri Feb 25= 10:39:27 2011 -0800 >> @@ -37,9 +37,9 @@ >> =A0#define DISK_TYPE_RAM =A0 =A0 =A0 =A0 5 >> =A0#define DISK_TYPE_QCOW =A0 =A0 =A0 =A06 >> =A0#define DISK_TYPE_BLOCK_CACHE 7 >> -#define DISK_TYPE_LOG =A0 =A0 =A0 =A0 9 >> -#define DISK_TYPE_REMUS =A0 =A0 =A0 10 >> -#define DISK_TYPE_VINDEX =A0 =A0 =A011 >> +#define DISK_TYPE_LOG =A0 =A0 =A0 =A0 8 >> +#define DISK_TYPE_REMUS =A0 =A0 =A0 9 >> +#define DISK_TYPE_VINDEX =A0 =A0 =A010 >> >> =A0#define DISK_TYPE_NAME_MAX =A0 =A032 > > 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: =A0 583:d83dab01b6fa > user: =A0 =A0 =A0 =A0Daniel Stodden > date: =A0 =A0 =A0 =A0Tue Feb 15 01:37:44 2011 -0800 > summary: =A0 =A0 Allow for holes in tapdisk_disk_types. > > diff -r 37f55e6d34ca -r d83dab01b6fa drivers/tapdisk-disktype.c > --- a/drivers/tapdisk-disktype.c =A0 =A0 =A0 =A0Tue Feb 15 01:37:44 2011 = -0800 > +++ b/drivers/tapdisk-disktype.c =A0 =A0 =A0 =A0Tue Feb 15 01:37:44 2011 = -0800 > @@ -154,13 +154,19 @@ > =A0 =A0 =A0 =A00, > =A0}; > > +#define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0])) > + > =A0int > =A0tapdisk_disktype_find(const char *name) > =A0{ > - =A0 =A0 =A0 const disk_info_t *info; > =A0 =A0 =A0 =A0int i; > > - =A0 =A0 =A0 for (i =3D 0; info =3D tapdisk_disk_types[i], info !=3D NUL= L; ++i) { > + =A0 =A0 =A0 for (i =3D 0; i < ARRAY_SIZE(tapdisk_disk_types); i++) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 const disk_info_t *info =3D tapdisk_disk_ty= pes[i]; > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!info) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; > + > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(name, info->name)) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue; > > > 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