[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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 onwards

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.