From: Chris Lalancette <clalance@redhat.com>
Subject: Re: [PATCH 0/4]: Expand xvd to support > 16 devices
Date: Wed, 25 Jun 2008 11:45:15 +0200
Message-ID: <486213AB.7020401@redhat.com>
References: <485FEAD7.9070206@redhat.com>	<485FEAD4.3090009@redhat.com>
	<18527.63569.430144.628906@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
In-Reply-To: <18527.63569.430144.628906@mariner.uk.xensource.com>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>

(sorry for the delay in responding)

Ian Jackson wrote:
> When the guest is enumerating the devices, it should be sure to
> check that the block device number integer matches one of the expected
> forms, as I wrote in my message.  If the number does not, then that
> vbd should be ignored with a warning message.

OK, yes, I see, that makes sense.  I'll make the appropriate change in xlvbd_add().

> 
> This applies also to the partition numbers which you are currently
> limiting to 15.  That's fine but you should put in a check so that
> out-of-range partition numbers are ignored rather than causing
> unexpected behaviours.  (I'll admit that I haven't analysed your code
> in detail to determine exactly what the behaviour would be ...)

I'm not sure that this one is a problem (although I could be wrong).  During
xlvbd_add() time, we end up doing an alloc_disk() with the number of minors that
we can use.  So I don't think that the rest of the system will allow us to go
beyond that value; empirical evidence seems to support this, as attaching a disk
with 16 partitions to /dev/xvdb only shows the first 15 partitions.

Incidentally, the comment I made in my initial posting about expanding the
partitions is wrong, looking back at the code.  I *did* expand the number of
entries that blkfront will pick up (i.e. increased nr_minors when doing the
alloc_disk()), but I did not change the tools side to accept partitions > 15.
Again, something that can easily be done in the future.

> Also I think you should include an API changelog entry.

Do you mean on the Xen Wiki?  I did find a page about API changes, so if/when
these patches go in, I'm happy to add an entry there.

Thanks for looking,
Chris Lalancette