From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: [PATCH 5/6] libxl: introduce libxl__alloc_vdev Date: Fri, 30 Mar 2012 15:17:36 +0100 Message-ID: <20341.49280.31751.307404@mariner.uk.xensource.com> References: <1332856772-30292-5-git-send-email-stefano.stabellini@eu.citrix.com> <20337.63133.683848.208682@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" , Ian Campbell List-Id: xen-devel@lists.xenproject.org Stefano Stabellini writes ("Re: [PATCH 5/6] libxl: introduce libxl__alloc_vdev"): > On Tue, 27 Mar 2012, Ian Jackson wrote: > > Furthermore, for the purpose for which you're using this, you should > > not start at zero (xvda). You should start at at least 26 (xvdA) and > > possibly later. That way you are less likely to clash with other > > statically-allocated vbds. > > Another interesting observation but I disagree: why should we expect > "statically-allocated vbds" below xvdA? How many do you think we are > going to find? Why 26? I think it is very arbitrary. > We should just make sure that our allocation policy works well and start > from 0. Your design assumes that the system administrator has not configured, for themselves, any disks to be exposed to dom0 (eg by a driver domain). If they have (eg, run "xl block-attach 0 vdev=xvda ...") then they will expect it to work, and they will expect to be able to control the device name that this appears as in dom0. Therefore the namespace of xvd* names in dom0 is not freely available to us as libxl programmers; we need to avoid trampling all over it. Since we need to be able to allocate some vbds dynamically, the right approach is to decree that some portion of the dom0 vbd space is reserved for static allocations by the administrator, and that the remainder is for dynamic allocations by software which picks a free vbd. Naturally the static space should come first. > > Of course you can't then using the resulting vdev as a pathname in > > /dev/ but that's nonportable anyway; different guest OSs (and here we > > are playing the role of the guest) may have different conventions. > > You need a separate function to convert vdev numbers (as returned from > > _disk_dev_number) to pathnames in /dev. > > This is a good point and the key issue here. > > The frontend is actually free to choose whatever name it wants for the > device it is going to create from the dev number. I don't think that is true. On each individual guest platform, the relationship between vbd numbers (the actual interface between frontend and backend), and whatever that guest has for device names, needs to be statically defined. If we are assuming in this case, as we are, a vaguely unixy guest (which we must do as libxl because we do not support libxl on non-unix guests), we are free to assume that vbds which are presented to dom0[*] appear with names in /dev/. ([*] Strictly, to the domain running this particular libxl code. This might be dom0 or it might be some kind of disaggregated builder/toolstack domain.) We are also free to make an assumption, for each individual dom0 platform which we support, about exactly how these vbd numbers map to device names - at least, we are free to make this assumption for dom0 platforms which make suitable promises. If there are dom0 platforms which do not make suitable promises then for this whole scheme to work these platforms need to provide a way for us to find the /dev/ name of a particular vbd exposed to this guest. In the case of Linux, the vbd number to /dev/ name mapping is reasonably fixed. It needs to remain that way in general because changing the device names inside guests, when the host configuration remains the same, disrupts the operation and administration of the guest. So we can write a function which converts vbd numbers to Linux (pvops) device names. Or if we prefer we can probably go grobbling in sysfs to find the same information but that's likely to be more painful. But this same code needs to be different on BSD (say) because the BSD frontend driver has a different scheme for assigning names to vbds based on their numbers. > So I think that the best thing we can do is rely on it to find out the > device name, either doing: > > [1] xvd(a + i) -> libxl__device_disk_dev_number -> xs_read This is not correct, because the device name in the guest is not written to xenstore anywhere. It is private to the guest. (In this case the "guest" is actually dom0, but the point stands.) > or reading the "dev" node under the xenstore backend path, that is > generated using the same libxl function (this is what I am doing now). > I fail to see why one of these two approaches would be better than the > other, but if you think that [1] is the best, I can change my code. Again, you are confusing the virtual name given in the domain config file with the actual device name in the guest's /dev, which may not be the same. > > This is a broken half-reimplementation of > > libxl__device_disk_dev_number. > > No it is not the same thing. This function doesn't return any meaningful > number representing the disk in a unique way, it basically returns the last > letter of the disk name as an integer (minus offset). But you haven't explained why that is correct. > > > + free(disks); > > > + return libxl__index_to_vdev(gc, max_idx); > > > > And you don't need this function because you can use the disk number > > directly (converted to a string of decimal digits) as the vdev. > > These two functions are not the same as libxl__device_disk_dev_number > and friends. They are just helper functions for libxl__alloc_vdev. > I could rename them libxl__vdev_helper_idx and libxl__vdev_helper_name > to avoid confusion. The code would still be wrong. Ian.