linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Process for subsystem maintainers to get Hyper-V code out of staging.
@ 2011-02-14 23:30 Hank Janssen
  2011-02-14 23:45 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hank Janssen @ 2011-02-14 23:30 UTC (permalink / raw)
  To: shemminger@linux-foundation.org, netdev@vger.kernel.org,
	davem@davemloft.net, linux-ide
  Cc: KY Srinivasan, Hashir Abdi, Mike Sterling, Haiyang Zhang,
	gregkh@suse.de


Stephen/James/David,

Greetings to you all. As you might be aware, we submitted Hyper-V drivers to the kernel 2009.  
We have been extending these drivers with additional functionality and our primary focus 
now is doing the work needed to exit the staging area.

To give you some background, the following are Hyper-V specific Linux drivers:

                hv_vmbus           The vmbus driver that is the bridge between guest and the 
			host
                hv_storvsc          The SCSI device driver
                hv_blkvsc            The IDE driver
                hv_netvsc           The network driver

We think our drivers are pretty close to be reviewed by the subsystem maintainers.

We have been  working with Greg on  hv_vmbus, and several other driver issues as it 
relates to exiting staging. And now we are looking for guidance for the other drivers.

	1. Most important thing of course, did we contact the correct subsystem 
	    maintainers?
		i. IDE/Blkvsc		David Miller
		ii. SCSI/Storvsc		James Bottomley
		iii. Network/Netvsc	Stephen Hemminger
	2. What is the process to submit the code for review?
	3. Which mailing list(s) do we need to use for the code reviews
	4. I assume normal patch format is required?
	5. What additional information is needed
	6. Once they leave staging where do they need to go? Because they all
	    pretty much come as a package we were thinking drivers/hyperv might
	    be a good place.
                
There are still a few outstanding items we are currently working on. But they should be 
wrapped up shortly. (There are a few remaining FIXME comments in the code we are
cleaning up as I write this). But if possible we would like to get your feedback even as
we are addressing the issues we currently know about.

Thanks

Hank.





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for subsystem maintainers to get Hyper-V code out of staging.
  2011-02-14 23:30 Process for subsystem maintainers to get Hyper-V code out of staging Hank Janssen
@ 2011-02-14 23:45 ` Greg KH
  2011-02-15 10:21 ` Christoph Hellwig
  2011-02-15 10:33 ` Alan Cox
  2 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2011-02-14 23:45 UTC (permalink / raw)
  To: Hank Janssen
  Cc: shemminger@linux-foundation.org, netdev@vger.kernel.org,
	davem@davemloft.net, linux-ide@vger.kernel.org,
	Jame.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org,
	KY Srinivasan, Hashir Abdi, Mike Sterling, Haiyang Zhang

On Mon, Feb 14, 2011 at 11:30:07PM +0000, Hank Janssen wrote:
> 
> Stephen/James/David,
> 
> Greetings to you all. As you might be aware, we submitted Hyper-V drivers to the kernel 2009.  
> We have been extending these drivers with additional functionality and our primary focus 
> now is doing the work needed to exit the staging area.
> 
> To give you some background, the following are Hyper-V specific Linux drivers:
> 
>                 hv_vmbus           The vmbus driver that is the bridge between guest and the 
> 			host
>                 hv_storvsc          The SCSI device driver
>                 hv_blkvsc            The IDE driver
>                 hv_netvsc           The network driver
> 
> We think our drivers are pretty close to be reviewed by the subsystem maintainers.
> 
> We have been  working with Greg on  hv_vmbus, and several other driver issues as it 
> relates to exiting staging. And now we are looking for guidance for the other drivers.
> 
> 	1. Most important thing of course, did we contact the correct subsystem 
> 	    maintainers?
> 		i. IDE/Blkvsc		David Miller
> 		ii. SCSI/Storvsc		James Bottomley
> 		iii. Network/Netvsc	Stephen Hemminger

That's what the MAINTAINERS file says, right?

> 	2. What is the process to submit the code for review?

Like Documentation/SubmittingPatches shows, send patches.

> 	3. Which mailing list(s) do we need to use for the code reviews

Again, MAINTAINERS shows this.

> 	4. I assume normal patch format is required?

Yes.

> 	5. What additional information is needed

What's normally needed.

> 	6. Once they leave staging where do they need to go? Because they all
> 	    pretty much come as a package we were thinking drivers/hyperv might
> 	    be a good place.

That's up to the subsystem, if they want all network drivers in
drivers/net/ then they go there, same for scsi, it's up to the
maintainer.

> There are still a few outstanding items we are currently working on. But they should be 
> wrapped up shortly. (There are a few remaining FIXME comments in the code we are
> cleaning up as I write this). But if possible we would like to get your feedback even as
> we are addressing the issues we currently know about.

I would wait and only ask for review _after_ you fix the things you know
about first.  Otherwise it just wastes everyone's time.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for subsystem maintainers to get Hyper-V code out of staging.
  2011-02-14 23:30 Process for subsystem maintainers to get Hyper-V code out of staging Hank Janssen
  2011-02-14 23:45 ` Greg KH
@ 2011-02-15 10:21 ` Christoph Hellwig
  2011-02-15 10:33 ` Alan Cox
  2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2011-02-15 10:21 UTC (permalink / raw)
  To: Hank Janssen
  Cc: shemminger@linux-foundation.org, netdev@vger.kernel.org,
	davem@davemloft.net, linux-ide@vger.kernel.org,
	Jame.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org,
	KY Srinivasan, Hashir Abdi, Mike Sterling, Haiyang Zhang,
	gregkh@suse.de

Please give all the driver a unique prefix, hv_ is a bit to generic.
mshv might be better.

What's the point of the blksvc driver?  It is implemented as a block
driver, steals the major number of the IDE driver, which is a big no-no
and speaks a SCSI-like protocol to the host.  In what way does this
protocol differ from the full SCSI protocol in the storvsc driver?

As far as the storsvc driver is converned: please merge the storvsc.c
and storvsc_drv.c, as they are only used together.  Also please try to
clean up the way you use function pointers.  E.g. the
OnDeviceAdd/OnDeviceRemove/OnCleanup methods should be part of a
mshv_driver structure, and not store into individual objects.

Please get rid of the *_context structure that only have a single
intance anyway.  In generaly a Linux driver should not have any global
state except for maybe module paramters or a list of devices in cases
where the device model can't handle that.

Please clean up the calling convention for the init code, there is
absolutely no reason to pass stor_vsc_initialize as a function pointer
to storvsc_drv_init instead of calling it directly, and in fact there
is no reason to not just inline both of those into storvsc_init.  One
all the function pointers are moved into a driver struct and the global
context is gone there will be almost no code left in there anyway.

Similarly the exit routine is implemented entirely wrong.  The core
bus layer should iterate over all devices for the driver beeing
unregister and call back into the ->remove callback just for that
device.  Try to follow common real hardware busses like pci, usb or for
a really simple one eisa in your design.

There is no reason to have a per-device slab cache, a global one is
enough.  But for per-I/O allocations you'll need a mempool to make it
deadlock-free.

Do you really need scsi_scan_host for a virtualized scsi transport?  Is
there no way for the host to tell you which LUNs actually are present?

Why do you bother to linearize S/G lists?  If your host can't handle it
just tell the scsi layer that you have a sg_tablesize of 1, which means
you'll never get multiple S/G elements.  (Not doing SG is really, really
sad for new virtual hardware btw, please take the crack away from the
person who designed it).

Also can you please avoid forward declaration of functions as much as
possible?  They really make the code hard to read if used as much as in
these drivers.

That's it for the first round, I'm pretty sure there will be more
comments once the code is better structured and more readabke.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for subsystem maintainers to get Hyper-V code out of staging.
  2011-02-14 23:30 Process for subsystem maintainers to get Hyper-V code out of staging Hank Janssen
  2011-02-14 23:45 ` Greg KH
  2011-02-15 10:21 ` Christoph Hellwig
@ 2011-02-15 10:33 ` Alan Cox
  2 siblings, 0 replies; 4+ messages in thread
From: Alan Cox @ 2011-02-15 10:33 UTC (permalink / raw)
  To: Hank Janssen
  Cc: shemminger@linux-foundation.org, netdev@vger.kernel.org,
	davem@davemloft.net, linux-ide@vger.kernel.org,
	Jame.Bottomley@HansenPartnership.com, linux-scsi@vger.kernel.org,
	KY Srinivasan, Hashir Abdi, Mike Sterling, Haiyang Zhang,
	gregkh@suse.de

> 	1. Most important thing of course, did we contact the correct subsystem 
> 	    maintainers?
> 		i. IDE/Blkvsc		David Miller

Libata - Jeff Garzik & linux-ide@vger.kernel.org

drivers/ide is obsolete and on its way out.

drivers/ata replaced it as the old code couldn't really cope with things
like SATA NCQ or hotplug. The new stuff can which is probably handy in a
hypervisor.

> 		ii. SCSI/Storvsc		James Bottomley

Yes & linux-scsi@vger.kernel.org

> 		iii. Network/Netvsc	Stephen Hemminger
> 	2. What is the process to submit the code for review?

See: Documentation/SubmittingPatches in your kernel tree

That should answer the rest. If not then keep a tag on things that should
have been in there but weren't and that can also get updated.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-02-15 10:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-14 23:30 Process for subsystem maintainers to get Hyper-V code out of staging Hank Janssen
2011-02-14 23:45 ` Greg KH
2011-02-15 10:21 ` Christoph Hellwig
2011-02-15 10:33 ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).