Chris Lalancette
2008-Jun-23 18:26 UTC
[Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices
All, Current blktap and blkfront are limited to 16 xvd devices (xvda -> xvdp). This is enforced in the userland dom0 tools, but is also hard-coded into the blkfront kernel code (even though modern dev_t can hold many more than 256 minors). Based on the discussion that we had here: http://lists.xensource.com/archives/html/xen-devel/2008-05/msg00128.html I ended up implementing Ian Jackson''s suggestion here: http://lists.xensource.com/archives/html/xen-devel/2008-05/msg00231.html Basically, I left the old format alone, but added a new format that looks like: 1 << 28 | disk << 8 | partition xvd, disks or partitions 16 onwards This format is used for any disks xvdq onward. Note that blktap has a hardcoded limit of 100 devices that I did not change with this patch series; if that ends up being a problem, then that''s just a simple #define to change. I did not expand the number of partitions available (it''s still 15), although there is space in the allocation to do that if someone wishes. More details are in each individual patch. Note that I developed this against RHEL-5 kernels and ported it over to xen-unstable, and only compile tested it there. Chris Lalancette _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jun-23 19:24 UTC
Re: [Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices
Chris Lalancette writes ("[Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices"):> I ended up implementing Ian Jackson''s suggestion here: > http://lists.xensource.com/archives/html/xen-devel/2008-05/msg00231.html > Basically, I left the old format alone, but added a new format that > looks like: > > 1 << 28 | disk << 8 | partition xvd, disks or partitions 16 onwardsI approve of this, obviously. But I think your patch lacks some error checks. These are most critical in the guest, as the guest''s interpretation of the interface will effectively be frozen. 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. 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 ...) (Obviously even adding this check now won''t prevent attempts to specify out of range devices from totally breaking even older guests which lack proper checking. But there''s no reason to perpetuate these bugs.) Also I think you should include an API changelog entry. Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Chris Lalancette
2008-Jun-25 09:45 UTC
Re: [Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices
(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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Jun-25 11:19 UTC
Re: [Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices
On 25/6/08 10:45, "Chris Lalancette" <clalance@redhat.com> wrote:>> 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.docs/ChangeLog in the repository. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel