: 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

Chris Lalancette writes ("[Xen-devel] [PATCH 0/4]: Expand xvd to support > 16 devices"):

I 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.

(sorry for the delay in responding)

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

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.

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

docs/ChangeLog in the repository.

-- Keir

This patch implements the new extended allocation scheme on the blkfront end. Note that there is a new xenbus node for the extended stuff, called "virtual-device-ext". On a blkfront_probe(), if nothing is found in "virtual-device", then it will go looking in "virtual-device-ext". This prevents old guest kernels from potentially mis-interpreting data in "virtual-device" (and then subsequently crashing).

diff -r 5201a184f513 drivers/xen/blkfront/blkfront.c
@@ -92,8 +92,13 @@ static int blkfront_probe(struct xenbus_
 	err = xenbus_scanf(XBT_NIL, dev->nodename,
 			   "virtual-device", "%i", &vdevice);
 	if (err != 1) {
-		xenbus_dev_fatal(dev, err, "reading virtual-device");
-		return err;
+		/* go looking in the extended area instead */
+		err = xenbus_scanf(XBT_NIL, dev->nodename, "virtual-device-ext",
+			"%i", &vdevice);
+		if (err != 1) {
+			xenbus_dev_fatal(dev, err, "reading virtual-device");
+			return err;
+		}
 	}
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel