* Dave Boutcher (boutcher at cs.umn.edu) wrote:
> On Tue, 18 Jul 2006 00:00:33 -0700, Chris Wright <chrisw at sous-sol.org> said:
> >
> > The block device frontend driver allows the kernel to access block
> > devices exported exported by a virtual machine containing a physical
> > block device driver.
>
> First, I think this belongs in drivers/block (and the network driver
> belongs in drivers/net). If we're going to bring xen to the party,
> lets not leave it hiding out in a corner.
Yeah, I think so too.
> > + switch (backend_state) {
> > + case XenbusStateUnknown:
> > + case XenbusStateInitialising:
> > + case XenbusStateInitWait:
> > + case XenbusStateInitialised:
> > + case XenbusStateClosed:
>
> This actually should get fixed elsewhere, but SillyCaps???
I really don't care either way. There's no shortage of this style
specifically for enums, in fact there's a wide variety of interesting
styles for enums.
> > +static inline int GET_ID_FROM_FREELIST(
> > + struct blkfront_info *info)
> > +{
> > + unsigned long free = info->shadow_free;
> > + BUG_ON(free > BLK_RING_SIZE);
> > + info->shadow_free = info->shadow[free].req.id;
> > + info->shadow[free].req.id = 0x0fffffee; /* debug */
> > + return free;
> > +}
> > +
> > +static inline void ADD_ID_TO_FREELIST(
> > + struct blkfront_info *info, unsigned long id)
> > +{
> > + info->shadow[id].req.id = info->shadow_free;
> > + info->shadow[id].request = 0;
> > + info->shadow_free = id;
> > +}
>
> A real nit..but why are these routines SHOUTING?
GOOD QUESTION! I had missed that, thanks. Seems likely it was just half
converted from macro. I see no reason not to clean that up the rest of
the way.
> > +int blkif_release(struct inode *inode, struct file *filep)
> > +{
> > + struct blkfront_info *info = inode->i_bdev->bd_disk->private_data;
> > + info->users--;
> > + if (info->users == 0) {
>
> Hrm...this strikes me as racey. Don't you need at least a memory
> barrier here to handle SMP?
>
> > +static struct xlbd_major_info xvd_major_info = {
> > + .major = 201,
> > + .type = &xvd_type_info
> > +};
>
> I've forgotten what the current policy is around new major numbers.
Damn, this is rather funny. There was a number reserved, but somehow
this is the wrong one (should be 202 according to lanana[1]). Thanks,
I'll fix that up.
thanks,
-chris
[1] http://www.lanana.org/docs/device-list/devices-2.6+.txt
