Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code

From: Christoph Hellwig
Date: Wed Apr 27 2011 - 02:45:59 EST


On Wed, Apr 27, 2011 at 01:54:02AM +0000, KY Srinivasan wrote:
> I would prefer that we go through the review process. What is the process for
> this review? Is there a time window for people to respond. I am hoping I will be able
> to address all the review comments well in advance of the next closing of the tree,
> with the hope of taking the vmbus driver out of staging this go around (hope springs
> eternal in the human breast ...)!

It would be useful if you'd send one driver at a time to the list as the
full source to review.

Did we make any progress on the naming discussion? In my opinion hv is
a far to generic name for your drivers. Why not call it mshv dor the
driver directory and prefixes?

As far as the core code is concerned, can you explain the use of the
dev_add, dev_rm and cleanup methods and how they relate to the
normal probe/remove/shutdown methods?

As far as the storage drivers are concerned I still have issues with the
architecture. I haven't seen any good explanation why you want to have
the blkvsc and storvsc drivers different from each other. They both
speak the same vmbus-level protocol and tunnel scsi commands over it.
Why would you sometimes expose this SCSI protocol as a SCSI LLDD and
sometimes as a block driver? What decides that a device is exported
in a way to that blkvsc is bound to them vs storvsc? How do they look
like on the windows side? From my understanding of the windows driver
models both the recent storport model and the older scsiport model are
more or less talking scsi to the driver anyway, so what is the
difference between the two for a Windows guest?

Also pleae get rid of struct storvsc_driver_object, it's just a very
strange way to store file-scope variables, and useless indirection
for the I/O submission handler.

--
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/