Re: [PATCH 23/40] Staging: hv: storvsc: Introduce code to manage IDEdevices using storvsc HBA

[Christoph Hellwig]

On Thu, Jun 30, 2011 at 09:38:34PM +0000, KY Srinivasan wrote:
> > > +#define HV_MAX_IDE_DEVICES	4
> > > +#define HV_IDE_BASE_CHANNEL	10
> > > +#define HV_IDE0_DEV1		HV_IDE_BASE_CHANNEL
> > > +#define HV_IDE0_DEV2		(HV_IDE_BASE_CHANNEL + 1)
> > > +#define HV_IDE1_DEV1		(HV_IDE_BASE_CHANNEL + 2)
> > > +#define HV_IDE1_DEV2		(HV_IDE_BASE_CHANNEL + 3)
> > 
> > This at last needs a good explanation of why these devices are called
> > IDE if they actually aren't.  I know you've explained the reason to me
> > before, but it should also be in the code.
> 
> These devices are configured as IDE devices for the guest. The current
> emulator supports 2 IDE controllers for a total of potentially 4 devices.
> I did this to support all these 4 devices under one scsi host and used the
> channel information to get at the correct device in the I/O path.
> So, if you go to a model with one host per device, this would not be required.

Either way the driver should have a nice comment somewhere explaining
why you have disks that are named IDE but handled spoken to using SCSI
with explicit discovery.

> > but more importanly what does path actually stand for here?  Opencoding
> > this into the caller and adding proper comments explaining the scheme
> > might be more readable.
> 
> In the blkvsc driver, the path/target info was used to properly identify the 
> device - (a) the device was under the first or second IDE controller and (b)
> whether it is the first or second device under the controller. 

Yeah, that's what I got from reading the code.  What confuses me is the
"path" terminology which doesn't really map to any normal nomenclature.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/