From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] multi-page blkfront/blkback patch Date: Fri, 28 Nov 2008 08:41:56 +0000 Message-ID: <492FBCE4.76E4.0078.0@novell.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Geoffrey Lefebvre Cc: Andrew Warfield , Dutch Meyer , Samvel Yuri , xen-devel , Samuel Thibault List-Id: xen-devel@lists.xenproject.org Some comments on infrastructural things, not the actual functionality: >+ config 1_PAGE >+ bool "1 page" >+ config 2_PAGE >+ bool "2 pages" >+ config 4_PAGE >+ bool "4 pages" These names are too generic. At least XEN_ (and perhaps also BLKFRONT or some such) should also appear in them. >-static int blkif_reqs =3D 64; >+static int blkif_reqs =3D 128; Is this really related to the functionality introduced here? >+ BUG_ON(BLK_NUM_RING_PAGES !=3D 1 &&=20 >+ BLK_NUM_RING_PAGES !=3D 2 &&=20 >+ BLK_NUM_RING_PAGES !=3D 4); This should be BUILD_BUG_ON(). >+#ifndef CONFIG_XEN_BLKDEV_NUM_RING_PAGES >+#error "CONFIG_XEN_BLKDEV_NUM_RING_PAGES undefined!" >+#endif This won't work when building unmodified_drivers - I'm not sure what value to default to here, though. But for compatibility it should probably be 1, and the unmodified drivers' Kbuild could override it if desired. Jan