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

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

[K. Y. Srinivasan]

This patch-set addresses some of the bus/driver model cleanup that
Greg sugested over the last couple of days.  In this patch-set we
deal with the following issues:

	1) Cleanup unnecessary state in struct hv_device and 
	   struct hv_driver to be compliant with the Linux
	   Driver model.

	2) Cleanup the vmbus_match() function to conform with the 
	   Linux Driver model.

	3) Cleanup error handling in the vmbus_probe() and 
	   vmbus_child_device_register() functions. Fixed a 
	   bug in the probe failure path as part of this cleanup.

	4) The Windows host cannot handle the vmbus_driver being 
	   unloaded and subsequently loaded. Cleanup the driver with
	   this in mind.

	5) Get rid of struct hv_bus that embedded struct bus_type to 
	   conform with the LDM.

	6) Add probe/remove/shutdown functions to struct hv_driver to
	   conform to LDM.

	7) On some older Hyper-V hosts, the Linux PCI sub-sytem is not able
	   to allocate irq resources to the vmbus driver. I recently learnt
	   that the vmbus driver is an acpi enumerated device on the Hyper-V
	   platform. Added code to retrieve irq information from DSDT.



Regards,

K. Y

[PATCH 01/25] Staging: hv: Introduce a function to map a generic driver pointer to a pointer to storvsc_driver_object

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
introduce a function that maps a generic struct driver pointer to struct
storvsc_driver_object.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/storvsc_api.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/storvsc_api.h b/drivers/staging/hv/storvsc_api.h
index c98139c..b60a058 100644
--- a/drivers/staging/hv/storvsc_api.h
+++ b/drivers/staging/hv/storvsc_api.h
@@ -28,6 +28,7 @@
 #include <linux/kernel.h>
 #include "vstorage.h"
 #include "vmbus_api.h"
+#include "vmbus.h"
 
 /* Defines */
 #define STORVSC_RING_BUFFER_SIZE			(20*PAGE_SIZE)
@@ -153,6 +154,13 @@ static inline struct storvsc_driver_object *hvdr_to_stordr(struct hv_driver *d)
 	return container_of(d, struct storvsc_driver_object, base);
 }
 
+static inline
+struct storvsc_driver_object *drv_to_stordrv(struct device_driver *d)
+{
+	struct hv_driver *hvdrv = drv_to_hv_drv(d);
+	return hvdr_to_stordr(hvdrv);
+}
+
 /* Interface */
 
 int stor_vsc_on_device_add(struct hv_device *device,
-- 
1.7.4.1

[PATCH 02/25] Staging: hv: Get rid of the references to the priv element of struct hv_driver in block driver

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
get rid of the references to the priv element of struct hv_driver
in blkvsc_drv.c and storvsc.c


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c |   14 +++-----------
 drivers/staging/hv/storvsc.c    |    2 +-
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index acc5435..ec6a761 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -214,10 +214,8 @@ static int blkvsc_submit_request(struct blkvsc_request *blkvsc_req,
 {
 	struct block_device_context *blkdev = blkvsc_req->dev;
 	struct hv_device *device_ctx = blkdev->device_ctx;
-	struct hv_driver *drv =
-			drv_to_hv_drv(device_ctx->device.driver);
 	struct storvsc_driver_object *storvsc_drv_obj =
-			drv->priv;
+			drv_to_stordrv(device_ctx->device.driver);
 	struct hv_storvsc_request *storvsc_req;
 	struct vmscsi_request *vm_srb;
 	int ret;
@@ -541,10 +539,8 @@ out:
  */
 static int blkvsc_remove(struct device *device)
 {
-	struct hv_driver *drv =
-				drv_to_hv_drv(device->driver);
 	struct storvsc_driver_object *storvsc_drv_obj =
-				drv->priv;
+				drv_to_stordrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct block_device_context *blkdev = dev_get_drvdata(device);
 	unsigned long flags;
@@ -881,8 +877,6 @@ static int blkvsc_drv_init(void)
 
 	storvsc_drv_obj->ring_buffer_size = blkvsc_ringbuffer_size;
 
-	drv->priv = storvsc_drv_obj;
-
 	/* Callback to client driver to complete the initialization */
 	blk_vsc_initialize(&storvsc_drv_obj->base);
 
@@ -945,10 +939,8 @@ static void blkvsc_drv_exit(void)
  */
 static int blkvsc_probe(struct device *device)
 {
-	struct hv_driver *drv =
-				drv_to_hv_drv(device->driver);
 	struct storvsc_driver_object *storvsc_drv_obj =
-				drv->priv;
+				drv_to_stordrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 
 	struct block_device_context *blkdev = NULL;
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 85bae5a..d53aa97 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -414,7 +414,7 @@ static int stor_vsc_connect_to_vsp(struct hv_device *device)
 	struct storvsc_driver_object *stor_driver;
 	int ret;
 
-	stor_driver = (struct storvsc_driver_object *)device->drv;
+	stor_driver = drv_to_stordrv(device->device.driver);
 	memset(&props, 0, sizeof(struct vmstorage_channel_properties));
 
 	/* Open the channel */
-- 
1.7.4.1

[PATCH 03/25] Staging: hv: Get rid of the references to the priv element of struct hv_driver in hv_mouse.c

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
get rid of the references to the priv element of struct hv_driver
in hv_mouse.c


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/hv_mouse.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index bd93548..4333247 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -52,6 +52,13 @@ struct mousevsc_drv_obj {
 };
 
 
+static inline
+struct mousevsc_drv_obj *drv_to_mousedrv(struct device_driver *d)
+{
+	struct hv_driver *hvdrv = drv_to_hv_drv(d);
+	return container_of(hvdrv, struct mousevsc_drv_obj, base);
+}
+
 /* The maximum size of a synthetic input message. */
 #define SYNTHHID_MAX_INPUT_REPORT_SIZE 16
 
@@ -428,7 +435,7 @@ static void mousevsc_on_receive_input_report(struct mousevsc_dev *input_device,
 		return;
 	}
 
-	input_drv = (struct mousevsc_drv_obj *)input_device->device->drv;
+	input_drv = drv_to_mousedrv(input_device->device->device.driver);
 
 	inputreport_callback(input_device->device,
 			     input_report->buffer,
@@ -713,7 +720,7 @@ static int mousevsc_on_device_add(struct hv_device *device,
 		return ret;
 	}
 
-	input_drv = (struct mousevsc_drv_obj *)input_dev->device->drv;
+	input_drv = drv_to_mousedrv(input_dev->device->device.driver);
 
 	dev_info.vendor = input_dev->hid_dev_info.vendor;
 	dev_info.product = input_dev->hid_dev_info.product;
@@ -829,9 +836,8 @@ static int mousevsc_probe(struct device *device)
 {
 	int ret = 0;
 
-	struct hv_driver *drv =
-		drv_to_hv_drv(device->driver);
-	struct mousevsc_drv_obj *mousevsc_drv_obj = drv->priv;
+	struct mousevsc_drv_obj *mousevsc_drv_obj =
+		drv_to_mousedrv(device->driver);
 
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct input_device_context *input_dev_ctx;
@@ -857,9 +863,8 @@ static int mousevsc_remove(struct device *device)
 {
 	int ret = 0;
 
-	struct hv_driver *drv =
-		drv_to_hv_drv(device->driver);
-	struct mousevsc_drv_obj *mousevsc_drv_obj = drv->priv;
+	struct mousevsc_drv_obj *mousevsc_drv_obj =
+		drv_to_mousedrv(device->driver);
 
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct input_device_context *input_dev_ctx;
@@ -1017,7 +1022,6 @@ static int __init mousevsc_init(void)
 	mouse_vsc_initialize(&input_drv_obj->base);
 
 	drv->driver.name = input_drv_obj->base.name;
-	drv->priv = input_drv_obj;
 
 	drv->driver.probe = mousevsc_probe;
 	drv->driver.remove = mousevsc_remove;
-- 
1.7.4.1

[PATCH 04/25] Staging: hv: Introduce a function to map a generic driver pointer to a pointer to struct netvsc_driver

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
introduce a function to map a generic driver pointer to a pointer to
struct netvsc_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/netvsc_api.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/netvsc_api.h b/drivers/staging/hv/netvsc_api.h
index b4bed36..48b512b 100644
--- a/drivers/staging/hv/netvsc_api.h
+++ b/drivers/staging/hv/netvsc_api.h
@@ -26,6 +26,7 @@
 #define _NETVSC_API_H_
 
 #include "vmbus_api.h"
+#include "vmbus.h"
 
 /* Fwd declaration */
 struct hv_netvsc_packet;
@@ -103,6 +104,13 @@ struct netvsc_driver {
 	void *ctx;
 };
 
+static inline
+struct netvsc_driver *drv_to_netvscdrv(struct device_driver *d)
+{
+	struct hv_driver *hvdrv = drv_to_hv_drv(d);
+	return container_of(hvdrv, struct netvsc_driver, base);
+}
+
 struct netvsc_device_info {
 	unsigned char mac_adr[6];
 	bool link_state;	/* 0 - link up, 1 - link down */
-- 
1.7.4.1

[PATCH 05/25] Staging: hv: Get rid of the references to the priv element of struct hv_driver in net driver

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
get rid of the references to the priv element of struct
hv_driver in network driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/netvsc.c     |   11 +++++++----
 drivers/staging/hv/netvsc_drv.c |   16 ++++++----------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index b3e6497..27c7449 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -835,6 +835,9 @@ static void netvsc_receive(struct hv_device *device,
 	int i, j;
 	int count = 0, bytes_remain = 0;
 	unsigned long flags;
+	struct netvsc_driver *netvsc_drv =
+		 drv_to_netvscdrv(device->device.driver);
+
 	LIST_HEAD(listHead);
 
 	net_device = get_inbound_net_device(device);
@@ -995,8 +998,7 @@ static void netvsc_receive(struct hv_device *device,
 		}
 
 		/* Pass it to the upper layer */
-		((struct netvsc_driver *)device->drv)->
-			recv_cb(device, netvsc_packet);
+		netvsc_drv->recv_cb(device, netvsc_packet);
 
 		netvsc_receive_completion(netvsc_packet->
 				completion.recv.recv_completion_ctx);
@@ -1102,7 +1104,7 @@ static int netvsc_device_add(struct hv_device *device, void *additional_info)
 	struct netvsc_device *net_device;
 	struct hv_netvsc_packet *packet, *pos;
 	struct netvsc_driver *net_driver =
-				(struct netvsc_driver *)device->drv;
+		drv_to_netvscdrv(device->device.driver);
 
 	net_device = alloc_net_device(device);
 	if (!net_device) {
@@ -1183,7 +1185,8 @@ cleanup:
  */
 int netvsc_initialize(struct hv_driver *drv)
 {
-	struct netvsc_driver *driver = (struct netvsc_driver *)drv;
+	struct netvsc_driver *driver =
+		drv_to_netvscdrv(&drv->driver);
 
 	drv->name = driver_name;
 	memcpy(&drv->dev_type, &netvsc_device_type, sizeof(struct hv_guid));
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 096a732..e61eb7e 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -132,9 +132,8 @@ static void netvsc_xmit_completion(void *context)
 static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 {
 	struct net_device_context *net_device_ctx = netdev_priv(net);
-	struct hv_driver *drv =
-	    drv_to_hv_drv(net_device_ctx->device_ctx->device.driver);
-	struct netvsc_driver *net_drv_obj = drv->priv;
+	struct netvsc_driver *net_drv_obj =
+		drv_to_netvscdrv(net_device_ctx->device_ctx->device.driver);
 	struct hv_netvsc_packet *packet;
 	int ret;
 	unsigned int i, num_pages;
@@ -343,9 +342,8 @@ static void netvsc_send_garp(struct work_struct *w)
 
 static int netvsc_probe(struct device *device)
 {
-	struct hv_driver *drv =
-		drv_to_hv_drv(device->driver);
-	struct netvsc_driver *net_drv_obj = drv->priv;
+	struct netvsc_driver *net_drv_obj =
+		drv_to_netvscdrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct net_device *net = NULL;
 	struct net_device_context *net_device_ctx;
@@ -413,9 +411,8 @@ static int netvsc_probe(struct device *device)
 
 static int netvsc_remove(struct device *device)
 {
-	struct hv_driver *drv =
-		drv_to_hv_drv(device->driver);
-	struct netvsc_driver *net_drv_obj = drv->priv;
+	struct netvsc_driver *net_drv_obj =
+		drv_to_netvscdrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct net_device *net = dev_get_drvdata(&device_obj->device);
 	int ret;
@@ -498,7 +495,6 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	net_drv_obj->ring_buf_size = ring_size * PAGE_SIZE;
 	net_drv_obj->recv_cb = netvsc_recv_callback;
 	net_drv_obj->link_status_change = netvsc_linkstatus_callback;
-	drv->priv = net_drv_obj;
 
 	/* Callback to client driver to complete the initialization */
 	drv_init(&net_drv_obj->base);
-- 
1.7.4.1

[PATCH 06/25] Staging: hv: Get rid of the references to the priv element of struct hv_driver in storvsc_drv.c

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
get rid of the references to the priv element of struct
hv_driver in storvsc_drv.c.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/storvsc_drv.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index f819c6a..5ac2904 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -197,8 +197,6 @@ static int storvsc_drv_init(void)
 	/* Callback to client driver to complete the initialization */
 	stor_vsc_initialize(&storvsc_drv_obj->base);
 
-	drv->priv = storvsc_drv_obj;
-
 	DPRINT_INFO(STORVSC_DRV,
 		    "max outstanding reqs %u",
 		    storvsc_drv_obj->max_outstanding_req_per_channel);
@@ -325,9 +323,8 @@ static void storvsc_drv_exit(void)
 static int storvsc_probe(struct device *device)
 {
 	int ret;
-	struct hv_driver *drv =
-				drv_to_hv_drv(device->driver);
-	struct storvsc_driver_object *storvsc_drv_obj = drv->priv;
+	struct storvsc_driver_object *storvsc_drv_obj =
+				 drv_to_stordrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct Scsi_Host *host;
 	struct host_device_context *host_device_ctx;
@@ -404,9 +401,8 @@ static int storvsc_probe(struct device *device)
  */
 static int storvsc_remove(struct device *device)
 {
-	struct hv_driver *drv =
-			drv_to_hv_drv(device->driver);
-	struct storvsc_driver_object *storvsc_drv_obj = drv->priv;
+	struct storvsc_driver_object *storvsc_drv_obj =
+			 drv_to_stordrv(device->driver);
 	struct hv_device *device_obj = device_to_hv_device(device);
 	struct Scsi_Host *host = dev_get_drvdata(device);
 	struct host_device_context *host_device_ctx =
@@ -692,9 +688,8 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,
 	struct host_device_context *host_device_ctx =
 		(struct host_device_context *)scmnd->device->host->hostdata;
 	struct hv_device *device_ctx = host_device_ctx->device_ctx;
-	struct hv_driver *drv =
-		drv_to_hv_drv(device_ctx->device.driver);
-	struct storvsc_driver_object *storvsc_drv_obj = drv->priv;
+	struct storvsc_driver_object *storvsc_drv_obj =
+		drv_to_stordrv(device_ctx->device.driver);
 	struct hv_storvsc_request *request;
 	struct storvsc_cmd_request *cmd_request;
 	unsigned int request_size = 0;
-- 
1.7.4.1

[PATCH 07/25] Staging: hv: Cleanup vmbus_match()

[K. Y. Srinivasan]

In preparation for getting rid of the priv element from struct hv_driver,
cleanup vmbus_match - the setting of the drv field in struct hv_device
is not needed.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index c85d796..bf124a7 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -307,12 +307,9 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 
 	/* We found our driver ? */
 	if (memcmp(&device_ctx->dev_type, &drv->dev_type,
-		   sizeof(struct hv_guid)) == 0) {
-
-		device_ctx->drv = drv->priv;
-
+		   sizeof(struct hv_guid)) == 0)
 		match = 1;
-	}
+
 	return match;
 }
 
-- 
1.7.4.1

[PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[K. Y. Srinivasan]

The vmbus driver cannot be unloaded; the windows host does not
permit this. Cleanup accordingly.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |   32 --------------------------------
 1 files changed, 0 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index bf124a7..d597dd4 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -622,30 +622,6 @@ cleanup:
 	return ret;
 }
 
-/*
- * vmbus_bus_exit - Terminate the vmbus driver.
- *
- * This routine is opposite of vmbus_bus_init()
- */
-static void vmbus_bus_exit(void)
-{
-
-
-	vmbus_release_unattached_channels();
-	vmbus_disconnect();
-	on_each_cpu(hv_synic_cleanup, NULL, 1);
-
-	hv_cleanup();
-
-	bus_unregister(&hv_bus.bus);
-
-	free_irq(hv_pci_dev->irq, hv_pci_dev);
-
-	tasklet_kill(&hv_bus.msg_dpc);
-	tasklet_kill(&hv_bus.event_dpc);
-}
-
-
 /**
  * vmbus_child_driver_register() - Register a vmbus's child driver
  * @drv:        Pointer to driver structure you want to register
@@ -814,17 +790,9 @@ static int __init hv_pci_init(void)
 	return pci_register_driver(&hv_bus_driver);
 }
 
-static void __exit hv_pci_exit(void)
-{
-	vmbus_bus_exit();
-	pci_unregister_driver(&hv_bus_driver);
-}
-
-
 
 MODULE_LICENSE("GPL");
 MODULE_VERSION(HV_DRV_VERSION);
 module_param(vmbus_loglevel, int, S_IRUGO|S_IWUSR);
 
 module_init(hv_pci_init);
-module_exit(hv_pci_exit);
-- 
1.7.4.1

[PATCH 09/25] Staging: hv: Get rid of vmbus_release_unattached_channels() as it is not used

[K. Y. Srinivasan]

Since vmbus_release_unattached_channels() is only used in module
unload path and since the vmbus driver cannot be unloaded,
get rid of this "dead" code.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/channel_mgmt.c |   33 ---------------------------------
 drivers/staging/hv/channel_mgmt.h |    2 --
 2 files changed, 0 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/hv/channel_mgmt.c b/drivers/staging/hv/channel_mgmt.c
index fe32f7e..1929ab3 100644
--- a/drivers/staging/hv/channel_mgmt.c
+++ b/drivers/staging/hv/channel_mgmt.c
@@ -791,37 +791,4 @@ cleanup:
 	return ret;
 }
 
-/*
- * vmbus_release_unattached_channels - Release channels that are
- * unattached/unconnected ie (no drivers associated)
- */
-void vmbus_release_unattached_channels(void)
-{
-	struct vmbus_channel *channel, *pos;
-	struct vmbus_channel *start = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
-
-	list_for_each_entry_safe(channel, pos, &vmbus_connection.chn_list,
-				 listentry) {
-		if (channel == start)
-			break;
-
-		if (!channel->device_obj->drv) {
-			list_del(&channel->listentry);
-
-			pr_err("Releasing unattached device object\n");
-
-			vmbus_child_device_unregister(channel->device_obj);
-			free_channel(channel);
-		} else {
-			if (!start)
-				start = channel;
-		}
-	}
-
-	spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
-}
-
 /* eof */
diff --git a/drivers/staging/hv/channel_mgmt.h b/drivers/staging/hv/channel_mgmt.h
index 96f74e2..3b2c393 100644
--- a/drivers/staging/hv/channel_mgmt.h
+++ b/drivers/staging/hv/channel_mgmt.h
@@ -315,6 +315,4 @@ void vmbus_onmessage(void *context);
 
 int vmbus_request_offers(void);
 
-void vmbus_release_unattached_channels(void);
-
 #endif /* _CHANNEL_MGMT_H_ */
-- 
1.7.4.1

[PATCH 10/25] Staging: hv: Get rid of the priv pointer in struct hv_driver

[K. Y. Srinivasan]

Now that we have gotten rid of all uses of the priv element in
struct hv_driver, get rid of the priv pointer.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index f0d96eb..51fa952 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -94,16 +94,6 @@ struct hv_driver {
 	/* the device type supported by this driver */
 	struct hv_guid dev_type;
 
-	/*
-	 * Device type specific drivers (net, blk etc.)
-	 * need a mechanism to get a pointer to
-	 * device type specific driver structure given
-	 * a pointer to the base hyperv driver structure.
-	 * The current code solves this problem using
-	 * a hack. Support this need explicitly
-	 */
-	void *priv;
-
 	struct device_driver driver;
 
 	int (*dev_add)(struct hv_device *device, void *data);
-- 
1.7.4.1

[PATCH 11/25] Staging: hv: Get rid of the drv field in struct hv_device

[K. Y. Srinivasan]

Now, we can rid of the drv field in struct hv_device.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 51fa952..02e3587 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -103,9 +103,6 @@ struct hv_driver {
 
 /* Base device object */
 struct hv_device {
-	/* the driver for this device */
-	struct hv_driver *drv;
-
 	char name[64];
 
 	struct work_struct probe_failed_work_item;
-- 
1.7.4.1

[PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()

[K. Y. Srinivasan]

Cleanup error handling in vmbus_child_device_register().

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index d597dd4..4d569ad 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)
 	 */
 	ret = device_register(&child_device_obj->device);
 
+	if (ret)
+		return ret;
+
 	/* vmbus_probe() error does not get propergate to device_register(). */
 	ret = child_device_obj->probe_error;
 
-	if (ret)
+	if (ret) {
 		pr_err("Unable to register child device\n");
+		device_unregister(&child_device_obj->device);
+	}
 	else
 		pr_info("child device %s registered\n",
 			dev_name(&child_device_obj->device));
-- 
1.7.4.1

[PATCH 13/25] Staging: hv: Cleanup vmbus_probe() function

[K. Y. Srinivasan]

The logic for handling probe failure was broken. Now that we have
cleaned up error handling, get rid of the vmbus_probe_failed_cb()
function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |    2 --
 drivers/staging/hv/vmbus_drv.c |   27 +--------------------------
 2 files changed, 1 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 02e3587..c953d6e 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -105,8 +105,6 @@ struct hv_driver {
 struct hv_device {
 	char name[64];
 
-	struct work_struct probe_failed_work_item;
-
 	int probe_error;
 
 	/* the device type id of this device */
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 4d569ad..248dc8a 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -313,27 +313,6 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 	return match;
 }
 
-
-/*
- * vmbus_probe_failed_cb - Callback when a driver probe failed in vmbus_probe()
- *
- * We need a callback because we cannot invoked device_unregister() inside
- * vmbus_probe() since vmbus_probe() may be invoked inside device_register()
- * i.e. we cannot call device_unregister() inside device_register()
- */
-static void vmbus_probe_failed_cb(struct work_struct *context)
-{
-	struct hv_device *device_ctx = (struct hv_device *)context;
-
-	/*
-	 * Kick off the process of unregistering the device.
-	 * This will call vmbus_remove() and eventually vmbus_device_release()
-	 */
-	device_unregister(&device_ctx->device);
-
-	/* put_device(&device_ctx->device); */
-}
-
 /*
  * vmbus_probe - Add the new vmbus's child device
  */
@@ -348,14 +327,10 @@ static int vmbus_probe(struct device *child_device)
 	if (drv->driver.probe) {
 		ret = dev->probe_error =
 		drv->driver.probe(child_device);
-		if (ret != 0) {
+		if (ret != 0)
 			pr_err("probe failed for device %s (%d)\n",
 			       dev_name(child_device), ret);
 
-			INIT_WORK(&dev->probe_failed_work_item,
-				  vmbus_probe_failed_cb);
-			schedule_work(&dev->probe_failed_work_item);
-		}
 	} else {
 		pr_err("probe not set for driver %s\n",
 		       dev_name(child_device));
-- 
1.7.4.1

[PATCH 14/25] Staging: hv: Properly handle errors in hv_pci_probe()

[K. Y. Srinivasan]

Much of the vmbus driver initialization is done within the hv_pci_probe()
function. Properly handle errors in hv_pci_probe so that we can
appropriately deal with loading of the vmbus driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 248dc8a..a2acbce 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -54,6 +54,8 @@ EXPORT_SYMBOL(vmbus_loglevel);
 	/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
 	/* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
 
+static int pci_probe_error;
+static struct completion probe_event;
 
 static void get_channel_info(struct hv_device *device,
 			     struct hv_device_info *info)
@@ -732,19 +734,19 @@ void vmbus_child_device_unregister(struct hv_device *device_obj)
 static int __devinit hv_pci_probe(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
-	int err;
-
 	hv_pci_dev = pdev;
 
-	err = pci_enable_device(pdev);
-	if (err)
-		return err;
+	pci_probe_error = pci_enable_device(pdev);
+	if (pci_probe_error)
+		goto probe_cleanup;
 
-	err = vmbus_bus_init(pdev);
-	if (err)
+	pci_probe_error = vmbus_bus_init(pdev);
+	if (pci_probe_error)
 		pci_disable_device(pdev);
 
-	return err;
+probe_cleanup:
+	complete(&probe_event);
+	return pci_probe_error;
 }
 
 /*
@@ -767,7 +769,21 @@ static struct pci_driver hv_bus_driver = {
 
 static int __init hv_pci_init(void)
 {
-	return pci_register_driver(&hv_bus_driver);
+	int ret;
+	init_completion(&probe_event);
+	ret = pci_register_driver(&hv_bus_driver);
+	if (ret)
+		return ret;
+	/*
+	 * All the vmbus initialization occurs within the
+	 * hv_pci_probe() function. Wait for hv_pci_probe()
+	 * to complete.
+	 */
+	wait_for_completion(&probe_event);
+
+	if (pci_probe_error)
+		pci_unregister_driver(&hv_bus_driver);
+	return pci_probe_error;
 }
 
 
-- 
1.7.4.1

[PATCH 15/25] Staging: hv: Make hv_pci_dev a static variable

[K. Y. Srinivasan]

Make hv_pci_dev a static variable.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index a2acbce..f779d4e 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -40,7 +40,7 @@
 #include "vmbus_private.h"
 
 
-struct pci_dev *hv_pci_dev;
+static struct pci_dev *hv_pci_dev;
 
 /* Main vmbus driver data structure */
 struct hv_bus {
-- 
1.7.4.1

[PATCH 16/25] Staging: hv: Make msg_dpc a global variable

[K. Y. Srinivasan]

In preparation for cleaning up (getting rid of) of the hv_bus structure,
make msg_dpc a global variable.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index f779d4e..028051d 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -42,10 +42,11 @@
 
 static struct pci_dev *hv_pci_dev;
 
+static struct tasklet_struct msg_dpc;
+
 /* Main vmbus driver data structure */
 struct hv_bus {
 	struct bus_type bus;
-	struct tasklet_struct msg_dpc;
 	struct tasklet_struct event_dpc;
 };
 
@@ -517,7 +518,7 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
 	/* Schedules a dpc if necessary */
 	if (ret > 0) {
 		if (test_bit(0, (unsigned long *)&ret))
-			tasklet_schedule(&hv_bus.msg_dpc);
+			tasklet_schedule(&msg_dpc);
 
 		if (test_bit(1, (unsigned long *)&ret))
 			tasklet_schedule(&hv_bus.event_dpc);
@@ -552,7 +553,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
 	hv_bus.bus.name = driver_name;
 
 	/* Initialize the bus context */
-	tasklet_init(&hv_bus.msg_dpc, vmbus_on_msg_dpc, 0);
+	tasklet_init(&msg_dpc, vmbus_on_msg_dpc, 0);
 	tasklet_init(&hv_bus.event_dpc, vmbus_on_event, 0);
 
 	/* Now, register the bus  with LDM */
-- 
1.7.4.1

[PATCH 17/25] Staging: hv: Make event_dpc a global variable

[K. Y. Srinivasan]

In preparation for getting rid of struct hv_bus, Make event_dpc a
global variable.


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 028051d..a3a7741 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -43,11 +43,11 @@
 static struct pci_dev *hv_pci_dev;
 
 static struct tasklet_struct msg_dpc;
+static struct tasklet_struct event_dpc;
 
 /* Main vmbus driver data structure */
 struct hv_bus {
 	struct bus_type bus;
-	struct tasklet_struct event_dpc;
 };
 
 unsigned int vmbus_loglevel = (ALL_MODULES << 16 | INFO_LVL);
@@ -521,7 +521,7 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
 			tasklet_schedule(&msg_dpc);
 
 		if (test_bit(1, (unsigned long *)&ret))
-			tasklet_schedule(&hv_bus.event_dpc);
+			tasklet_schedule(&event_dpc);
 
 		return IRQ_HANDLED;
 	} else {
@@ -554,7 +554,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
 
 	/* Initialize the bus context */
 	tasklet_init(&msg_dpc, vmbus_on_msg_dpc, 0);
-	tasklet_init(&hv_bus.event_dpc, vmbus_on_event, 0);
+	tasklet_init(&event_dpc, vmbus_on_event, 0);
 
 	/* Now, register the bus  with LDM */
 	ret = bus_register(&hv_bus.bus);
-- 
1.7.4.1

[PATCH 18/25] Staging: hv: Get rid of struct hv_bus

[K. Y. Srinivasan]

Now, get rid of struct hv_bus. We will no longer be embedding
struct bus_type.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |   33 ++++++++++++++-------------------
 1 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index a3a7741..515311c 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -45,11 +45,6 @@ static struct pci_dev *hv_pci_dev;
 static struct tasklet_struct msg_dpc;
 static struct tasklet_struct event_dpc;
 
-/* Main vmbus driver data structure */
-struct hv_bus {
-	struct bus_type bus;
-};
-
 unsigned int vmbus_loglevel = (ALL_MODULES << 16 | INFO_LVL);
 EXPORT_SYMBOL(vmbus_loglevel);
 	/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
@@ -405,14 +400,14 @@ static void vmbus_device_release(struct device *device)
 }
 
 /* The one and only one */
-static struct hv_bus  hv_bus = {
-	.bus.name =		"vmbus",
-	.bus.match =		vmbus_match,
-	.bus.shutdown =		vmbus_shutdown,
-	.bus.remove =		vmbus_remove,
-	.bus.probe =		vmbus_probe,
-	.bus.uevent =		vmbus_uevent,
-	.bus.dev_attrs =	vmbus_device_attrs,
+static struct bus_type  hv_bus = {
+	.name =		"vmbus",
+	.match =		vmbus_match,
+	.shutdown =		vmbus_shutdown,
+	.remove =		vmbus_remove,
+	.probe =		vmbus_probe,
+	.uevent =		vmbus_uevent,
+	.dev_attrs =	vmbus_device_attrs,
 };
 
 static const char *driver_name = "hyperv";
@@ -550,14 +545,14 @@ static int vmbus_bus_init(struct pci_dev *pdev)
 		goto cleanup;
 	}
 
-	hv_bus.bus.name = driver_name;
+	hv_bus.name = driver_name;
 
 	/* Initialize the bus context */
 	tasklet_init(&msg_dpc, vmbus_on_msg_dpc, 0);
 	tasklet_init(&event_dpc, vmbus_on_event, 0);
 
 	/* Now, register the bus  with LDM */
-	ret = bus_register(&hv_bus.bus);
+	ret = bus_register(&hv_bus);
 	if (ret) {
 		ret = -1;
 		goto cleanup;
@@ -572,7 +567,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
 		pr_err("Unable to request IRQ %d\n",
 			   pdev->irq);
 
-		bus_unregister(&hv_bus.bus);
+		bus_unregister(&hv_bus);
 
 		ret = -1;
 		goto cleanup;
@@ -588,7 +583,7 @@ static int vmbus_bus_init(struct pci_dev *pdev)
 	ret = vmbus_connect();
 	if (ret) {
 		free_irq(pdev->irq, pdev);
-		bus_unregister(&hv_bus.bus);
+		bus_unregister(&hv_bus);
 		goto cleanup;
 	}
 
@@ -618,7 +613,7 @@ int vmbus_child_driver_register(struct device_driver *drv)
 	pr_info("child driver registering - name %s\n", drv->name);
 
 	/* The child driver on this vmbus */
-	drv->bus = &hv_bus.bus;
+	drv->bus = &hv_bus;
 
 	ret = driver_register(drv);
 
@@ -688,7 +683,7 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)
 		     atomic_inc_return(&device_num));
 
 	/* The new device belongs to this bus */
-	child_device_obj->device.bus = &hv_bus.bus; /* device->dev.bus; */
+	child_device_obj->device.bus = &hv_bus; /* device->dev.bus; */
 	child_device_obj->device.parent = &hv_pci_dev->dev;
 	child_device_obj->device.release = vmbus_device_release;
 
-- 
1.7.4.1

[PATCH 19/25] Staging: hv: Add probe function to struct hv_driver

[K. Y. Srinivasan]

Add probe function to struct hv_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index c953d6e..6f3741d 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -99,6 +99,9 @@ struct hv_driver {
 	int (*dev_add)(struct hv_device *device, void *data);
 	int (*dev_rm)(struct hv_device *device);
 	void (*cleanup)(struct hv_driver *driver);
+
+	int (*probe)(struct hv_device *);
+
 };
 
 /* Base device object */
-- 
1.7.4.1

[PATCH 20/25] Staging: hv: Use the probe function in struct hv_driver

[K. Y. Srinivasan]

Use the newly introduced probe function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c  |   19 +++++++++----------
 drivers/staging/hv/hv_mouse.c    |   11 +++++------
 drivers/staging/hv/netvsc_drv.c  |   19 +++++++++----------
 drivers/staging/hv/storvsc_drv.c |   23 +++++++++++------------
 drivers/staging/hv/vmbus_drv.c   |    5 ++---
 5 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index ec6a761..20b9a53 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -140,7 +140,7 @@ MODULE_PARM_DESC(ring_size, "Ring buffer size (in bytes)");
  * There is a circular dependency involving blkvsc_probe()
  * and block_ops.
  */
-static int blkvsc_probe(struct device *dev);
+static int blkvsc_probe(struct hv_device *dev);
 
 static int blk_vsc_on_device_add(struct hv_device *device,
 				void *additional_info)
@@ -882,7 +882,7 @@ static int blkvsc_drv_init(void)
 
 	drv->driver.name = storvsc_drv_obj->base.name;
 
-	drv->driver.probe = blkvsc_probe;
+	drv->probe = blkvsc_probe;
 	drv->driver.remove = blkvsc_remove;
 	drv->driver.shutdown = blkvsc_shutdown;
 
@@ -937,11 +937,10 @@ static void blkvsc_drv_exit(void)
 /*
  * blkvsc_probe - Add a new device for this driver
  */
-static int blkvsc_probe(struct device *device)
+static int blkvsc_probe(struct hv_device *dev)
 {
 	struct storvsc_driver_object *storvsc_drv_obj =
-				drv_to_stordrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
+			drv_to_stordrv(dev->device.driver);
 
 	struct block_device_context *blkdev = NULL;
 	struct storvsc_device_info device_info;
@@ -961,7 +960,7 @@ static int blkvsc_probe(struct device *device)
 	spin_lock_init(&blkdev->lock);
 
 
-	blkdev->request_pool = kmem_cache_create(dev_name(&device_obj->device),
+	blkdev->request_pool = kmem_cache_create(dev_name(&dev->device),
 					sizeof(struct blkvsc_request), 0,
 					SLAB_HWCACHE_ALIGN, NULL);
 	if (!blkdev->request_pool) {
@@ -971,17 +970,17 @@ static int blkvsc_probe(struct device *device)
 
 
 	/* Call to the vsc driver to add the device */
-	ret = storvsc_drv_obj->base.dev_add(device_obj, &device_info);
+	ret = storvsc_drv_obj->base.dev_add(dev, &device_info);
 	if (ret != 0)
 		goto cleanup;
 
-	blkdev->device_ctx = device_obj;
+	blkdev->device_ctx = dev;
 	/* this identified the device 0 or 1 */
 	blkdev->target = device_info.target_id;
 	/* this identified the ide ctrl 0 or 1 */
 	blkdev->path = device_info.path_id;
 
-	dev_set_drvdata(device, blkdev);
+	dev_set_drvdata(&dev->device, blkdev);
 
 	ret = stor_vsc_get_major_info(&device_info, &major_info);
 
@@ -1041,7 +1040,7 @@ static int blkvsc_probe(struct device *device)
 	return ret;
 
 remove:
-	storvsc_drv_obj->base.dev_rm(device_obj);
+	storvsc_drv_obj->base.dev_rm(dev);
 
 cleanup:
 	if (blkdev) {
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 4333247..e2363b3 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -832,23 +832,22 @@ static void mousevsc_hid_close(struct hid_device *hid)
 {
 }
 
-static int mousevsc_probe(struct device *device)
+static int mousevsc_probe(struct hv_device *dev)
 {
 	int ret = 0;
 
 	struct mousevsc_drv_obj *mousevsc_drv_obj =
-		drv_to_mousedrv(device->driver);
+		drv_to_mousedrv(dev->device.driver);
 
-	struct hv_device *device_obj = device_to_hv_device(device);
 	struct input_device_context *input_dev_ctx;
 
 	input_dev_ctx = kmalloc(sizeof(struct input_device_context),
 				GFP_KERNEL);
 
-	dev_set_drvdata(device, input_dev_ctx);
+	dev_set_drvdata(&dev->device, input_dev_ctx);
 
 	/* Call to the vsc driver to add the device */
-	ret = mousevsc_drv_obj->base.dev_add(device_obj, NULL);
+	ret = mousevsc_drv_obj->base.dev_add(dev, NULL);
 
 	if (ret != 0) {
 		DPRINT_ERR(INPUTVSC_DRV, "unable to add input vsc device");
@@ -1023,7 +1022,7 @@ static int __init mousevsc_init(void)
 
 	drv->driver.name = input_drv_obj->base.name;
 
-	drv->driver.probe = mousevsc_probe;
+	drv->probe = mousevsc_probe;
 	drv->driver.remove = mousevsc_remove;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index e61eb7e..685a6f5 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -340,11 +340,10 @@ static void netvsc_send_garp(struct work_struct *w)
 }
 
 
-static int netvsc_probe(struct device *device)
+static int netvsc_probe(struct hv_device *dev)
 {
 	struct netvsc_driver *net_drv_obj =
-		drv_to_netvscdrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
+		drv_to_netvscdrv(dev->device.driver);
 	struct net_device *net = NULL;
 	struct net_device_context *net_device_ctx;
 	struct netvsc_device_info device_info;
@@ -361,16 +360,16 @@ static int netvsc_probe(struct device *device)
 	netif_carrier_off(net);
 
 	net_device_ctx = netdev_priv(net);
-	net_device_ctx->device_ctx = device_obj;
+	net_device_ctx->device_ctx = dev;
 	net_device_ctx->avail = ring_size;
-	dev_set_drvdata(device, net);
+	dev_set_drvdata(&dev->device, net);
 	INIT_WORK(&net_device_ctx->work, netvsc_send_garp);
 
 	/* Notify the netvsc driver of the new device */
-	ret = net_drv_obj->base.dev_add(device_obj, &device_info);
+	ret = net_drv_obj->base.dev_add(dev, &device_info);
 	if (ret != 0) {
 		free_netdev(net);
-		dev_set_drvdata(device, NULL);
+		dev_set_drvdata(&dev->device, NULL);
 
 		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
 		return ret;
@@ -397,12 +396,12 @@ static int netvsc_probe(struct device *device)
 	net->features = NETIF_F_SG;
 
 	SET_ETHTOOL_OPS(net, &ethtool_ops);
-	SET_NETDEV_DEV(net, device);
+	SET_NETDEV_DEV(net, &dev->device);
 
 	ret = register_netdev(net);
 	if (ret != 0) {
 		/* Remove the device and release the resource */
-		net_drv_obj->base.dev_rm(device_obj);
+		net_drv_obj->base.dev_rm(dev);
 		free_netdev(net);
 	}
 
@@ -501,7 +500,7 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 
 	drv->driver.name = net_drv_obj->base.name;
 
-	drv->driver.probe = netvsc_probe;
+	drv->probe = netvsc_probe;
 	drv->driver.remove = netvsc_remove;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 5ac2904..2060206 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -123,7 +123,7 @@ static int stor_vsc_initialize(struct hv_driver *driver)
 }
 
 /* Static decl */
-static int storvsc_probe(struct device *dev);
+static int storvsc_probe(struct hv_device *dev);
 static int storvsc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd);
 static int storvsc_device_alloc(struct scsi_device *);
 static int storvsc_device_configure(struct scsi_device *);
@@ -213,7 +213,7 @@ static int storvsc_drv_init(void)
 
 	drv->driver.name = storvsc_drv_obj->base.name;
 
-	drv->driver.probe = storvsc_probe;
+	drv->probe = storvsc_probe;
 	drv->driver.remove = storvsc_remove;
 
 	/* The driver belongs to vmbus */
@@ -320,12 +320,11 @@ static void storvsc_drv_exit(void)
 /*
  * storvsc_probe - Add a new device for this driver
  */
-static int storvsc_probe(struct device *device)
+static int storvsc_probe(struct hv_device *device)
 {
 	int ret;
 	struct storvsc_driver_object *storvsc_drv_obj =
-				 drv_to_stordrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
+		 drv_to_stordrv(device->device.driver);
 	struct Scsi_Host *host;
 	struct host_device_context *host_device_ctx;
 	struct storvsc_device_info device_info;
@@ -340,16 +339,16 @@ static int storvsc_probe(struct device *device)
 		return -ENOMEM;
 	}
 
-	dev_set_drvdata(device, host);
+	dev_set_drvdata(&device->device, host);
 
 	host_device_ctx = (struct host_device_context *)host->hostdata;
 	memset(host_device_ctx, 0, sizeof(struct host_device_context));
 
 	host_device_ctx->port = host->host_no;
-	host_device_ctx->device_ctx = device_obj;
+	host_device_ctx->device_ctx = device;
 
 	host_device_ctx->request_pool =
-				kmem_cache_create(dev_name(&device_obj->device),
+				kmem_cache_create(dev_name(&device->device),
 					sizeof(struct storvsc_cmd_request), 0,
 					SLAB_HWCACHE_ALIGN, NULL);
 
@@ -360,8 +359,8 @@ static int storvsc_probe(struct device *device)
 
 	device_info.port_number = host->host_no;
 	/* Call to the vsc driver to add the device */
-	ret = storvsc_drv_obj->base.dev_add(device_obj,
-						(void *)&device_info);
+	ret = storvsc_drv_obj->base.dev_add(device, (void *)&device_info);
+
 	if (ret != 0) {
 		DPRINT_ERR(STORVSC_DRV, "unable to add scsi vsc device");
 		kmem_cache_destroy(host_device_ctx->request_pool);
@@ -381,11 +380,11 @@ static int storvsc_probe(struct device *device)
 	host->max_channel = STORVSC_MAX_CHANNELS - 1;
 
 	/* Register the HBA and start the scsi bus scan */
-	ret = scsi_add_host(host, device);
+	ret = scsi_add_host(host, &device->device);
 	if (ret != 0) {
 		DPRINT_ERR(STORVSC_DRV, "unable to add scsi host device");
 
-		storvsc_drv_obj->base.dev_rm(device_obj);
+		storvsc_drv_obj->base.dev_rm(device);
 
 		kmem_cache_destroy(host_device_ctx->request_pool);
 		scsi_host_put(host);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 515311c..15e6497 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -321,10 +321,9 @@ static int vmbus_probe(struct device *child_device)
 			drv_to_hv_drv(child_device->driver);
 	struct hv_device *dev = device_to_hv_device(child_device);
 
-	/* Let the specific open-source driver handles the probe if it can */
-	if (drv->driver.probe) {
+	if (drv->probe) {
 		ret = dev->probe_error =
-		drv->driver.probe(child_device);
+		drv->probe(dev);
 		if (ret != 0)
 			pr_err("probe failed for device %s (%d)\n",
 			       dev_name(child_device), ret);
-- 
1.7.4.1

[PATCH 21/25] Staging: hv: Add remove() function to struct hv_driver

[K. Y. Srinivasan]

Add remove() function to struct hv_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 6f3741d..1af1b62 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -101,6 +101,7 @@ struct hv_driver {
 	void (*cleanup)(struct hv_driver *driver);
 
 	int (*probe)(struct hv_device *);
+	int (*remove)(struct hv_device *);
 
 };
 
-- 
1.7.4.1

[PATCH 22/25] Staging: hv: Use the remove() function in struct hv_driver

[K. Y. Srinivasan]

Use the newly introduced remove() function in struct hv_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c  |   11 +++++------
 drivers/staging/hv/hv_mouse.c    |   11 +++++------
 drivers/staging/hv/netvsc_drv.c  |   13 ++++++-------
 drivers/staging/hv/storvsc_drv.c |   13 ++++++-------
 drivers/staging/hv/vmbus_drv.c   |    9 +++------
 5 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 20b9a53..80f7c0e 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -537,19 +537,18 @@ out:
 /*
  * blkvsc_remove() - Callback when our device is removed
  */
-static int blkvsc_remove(struct device *device)
+static int blkvsc_remove(struct hv_device *dev)
 {
 	struct storvsc_driver_object *storvsc_drv_obj =
-				drv_to_stordrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
-	struct block_device_context *blkdev = dev_get_drvdata(device);
+				drv_to_stordrv(dev->device.driver);
+	struct block_device_context *blkdev = dev_get_drvdata(&dev->device);
 	unsigned long flags;
 
 	/*
 	 * Call to the vsc driver to let it know that the device is being
 	 * removed
 	 */
-	storvsc_drv_obj->base.dev_rm(device_obj);
+	storvsc_drv_obj->base.dev_rm(dev);
 
 	/* Get to a known state */
 	spin_lock_irqsave(&blkdev->lock, flags);
@@ -883,7 +882,7 @@ static int blkvsc_drv_init(void)
 	drv->driver.name = storvsc_drv_obj->base.name;
 
 	drv->probe = blkvsc_probe;
-	drv->driver.remove = blkvsc_remove;
+	drv->remove = blkvsc_remove;
 	drv->driver.shutdown = blkvsc_shutdown;
 
 	/* The driver belongs to vmbus */
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index e2363b3..d49a51e 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -858,20 +858,19 @@ static int mousevsc_probe(struct hv_device *dev)
 	return 0;
 }
 
-static int mousevsc_remove(struct device *device)
+static int mousevsc_remove(struct hv_device *dev)
 {
 	int ret = 0;
 
 	struct mousevsc_drv_obj *mousevsc_drv_obj =
-		drv_to_mousedrv(device->driver);
+		drv_to_mousedrv(dev->device.driver);
 
-	struct hv_device *device_obj = device_to_hv_device(device);
 	struct input_device_context *input_dev_ctx;
 
 	input_dev_ctx = kmalloc(sizeof(struct input_device_context),
 				GFP_KERNEL);
 
-	dev_set_drvdata(device, input_dev_ctx);
+	dev_set_drvdata(&dev->device, input_dev_ctx);
 
 	if (input_dev_ctx->connected) {
 		hidinput_disconnect(input_dev_ctx->hid_device);
@@ -885,7 +884,7 @@ static int mousevsc_remove(struct device *device)
 	 * Call to the vsc driver to let it know that the device
 	 * is being removed
 	 */
-	ret = mousevsc_drv_obj->base.dev_rm(device_obj);
+	ret = mousevsc_drv_obj->base.dev_rm(dev);
 
 	if (ret != 0) {
 		DPRINT_ERR(INPUTVSC_DRV,
@@ -1023,7 +1022,7 @@ static int __init mousevsc_init(void)
 	drv->driver.name = input_drv_obj->base.name;
 
 	drv->probe = mousevsc_probe;
-	drv->driver.remove = mousevsc_remove;
+	drv->remove = mousevsc_remove;
 
 	/* The driver belongs to vmbus */
 	vmbus_child_driver_register(&drv->driver);
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 685a6f5..f4c6000 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -408,16 +408,15 @@ static int netvsc_probe(struct hv_device *dev)
 	return ret;
 }
 
-static int netvsc_remove(struct device *device)
+static int netvsc_remove(struct hv_device *dev)
 {
 	struct netvsc_driver *net_drv_obj =
-		drv_to_netvscdrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
-	struct net_device *net = dev_get_drvdata(&device_obj->device);
+		drv_to_netvscdrv(dev->device.driver);
+	struct net_device *net = dev_get_drvdata(&dev->device);
 	int ret;
 
 	if (net == NULL) {
-		dev_err(device, "No net device to remove\n");
+		dev_err(&dev->device, "No net device to remove\n");
 		return 0;
 	}
 
@@ -434,7 +433,7 @@ static int netvsc_remove(struct device *device)
 	 * Call to the vsc driver to let it know that the device is being
 	 * removed
 	 */
-	ret = net_drv_obj->base.dev_rm(device_obj);
+	ret = net_drv_obj->base.dev_rm(dev);
 	if (ret != 0) {
 		/* TODO: */
 		netdev_err(net, "unable to remove vsc device (ret %d)\n", ret);
@@ -501,7 +500,7 @@ static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
 	drv->driver.name = net_drv_obj->base.name;
 
 	drv->probe = netvsc_probe;
-	drv->driver.remove = netvsc_remove;
+	drv->remove = netvsc_remove;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(&drv->driver);
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 2060206..e449481 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -128,7 +128,7 @@ static int storvsc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd
 static int storvsc_device_alloc(struct scsi_device *);
 static int storvsc_device_configure(struct scsi_device *);
 static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd);
-static int storvsc_remove(struct device *dev);
+static int storvsc_remove(struct hv_device *dev);
 
 static struct scatterlist *create_bounce_buffer(struct scatterlist *sgl,
 						unsigned int sg_count,
@@ -214,7 +214,7 @@ static int storvsc_drv_init(void)
 	drv->driver.name = storvsc_drv_obj->base.name;
 
 	drv->probe = storvsc_probe;
-	drv->driver.remove = storvsc_remove;
+	drv->remove = storvsc_remove;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(&drv->driver);
@@ -398,12 +398,11 @@ static int storvsc_probe(struct hv_device *device)
 /*
  * storvsc_remove - Callback when our device is removed
  */
-static int storvsc_remove(struct device *device)
+static int storvsc_remove(struct hv_device *dev)
 {
 	struct storvsc_driver_object *storvsc_drv_obj =
-			 drv_to_stordrv(device->driver);
-	struct hv_device *device_obj = device_to_hv_device(device);
-	struct Scsi_Host *host = dev_get_drvdata(device);
+			 drv_to_stordrv(dev->device.driver);
+	struct Scsi_Host *host = dev_get_drvdata(&dev->device);
 	struct host_device_context *host_device_ctx =
 			(struct host_device_context *)host->hostdata;
 
@@ -411,7 +410,7 @@ static int storvsc_remove(struct device *device)
 	 * Call to the vsc driver to let it know that the device is being
 	 * removed
 	 */
-	storvsc_drv_obj->base.dev_rm(device_obj);
+	storvsc_drv_obj->base.dev_rm(dev);
 
 	if (host_device_ctx->request_pool) {
 		kmem_cache_destroy(host_device_ctx->request_pool);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 15e6497..e09b363 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -344,16 +344,13 @@ static int vmbus_remove(struct device *child_device)
 	int ret;
 	struct hv_driver *drv;
 
+	struct hv_device *dev = device_to_hv_device(child_device);
 
 	if (child_device->driver) {
 		drv = drv_to_hv_drv(child_device->driver);
 
-		/*
-		 * Let the specific open-source driver handles the removal if
-		 * it can
-		 */
-		if (drv->driver.remove) {
-			ret = drv->driver.remove(child_device);
+		if (drv->remove) {
+			ret = drv->remove(dev);
 		} else {
 			pr_err("remove not set for driver %s\n",
 				dev_name(child_device));
-- 
1.7.4.1

[PATCH 23/25] Staging: hv: Add shutdown() function to struct hv_driver

[K. Y. Srinivasan]

Add shutdown() function to struct hv_driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_api.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 1af1b62..0025002 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -102,6 +102,7 @@ struct hv_driver {
 
 	int (*probe)(struct hv_device *);
 	int (*remove)(struct hv_device *);
+	void (*shutdown)(struct hv_device *);
 
 };
 
-- 
1.7.4.1

[PATCH 24/25] Staging: hv: Use the shutdown() function in struct hv_driver

[K. Y. Srinivasan]

Use the newly introduced  shutdown() function.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/blkvsc_drv.c |    6 +++---
 drivers/staging/hv/vmbus_drv.c  |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 80f7c0e..db44cf6 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -585,9 +585,9 @@ static int blkvsc_remove(struct hv_device *dev)
 
 }
 
-static void blkvsc_shutdown(struct device *device)
+static void blkvsc_shutdown(struct hv_device *dev)
 {
-	struct block_device_context *blkdev = dev_get_drvdata(device);
+	struct block_device_context *blkdev = dev_get_drvdata(&dev->device);
 	unsigned long flags;
 
 	if (!blkdev)
@@ -883,7 +883,7 @@ static int blkvsc_drv_init(void)
 
 	drv->probe = blkvsc_probe;
 	drv->remove = blkvsc_remove;
-	drv->driver.shutdown = blkvsc_shutdown;
+	drv->shutdown = blkvsc_shutdown;
 
 	/* The driver belongs to vmbus */
 	ret = vmbus_child_driver_register(&drv->driver);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index e09b363..f95ec2b 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -368,6 +368,7 @@ static int vmbus_remove(struct device *child_device)
 static void vmbus_shutdown(struct device *child_device)
 {
 	struct hv_driver *drv;
+	struct hv_device *dev = device_to_hv_device(child_device);
 
 
 	/* The device may not be attached yet */
@@ -376,9 +377,8 @@ static void vmbus_shutdown(struct device *child_device)
 
 	drv = drv_to_hv_drv(child_device->driver);
 
-	/* Let the specific open-source driver handles the removal if it can */
-	if (drv->driver.shutdown)
-		drv->driver.shutdown(child_device);
+	if (drv->shutdown)
+		drv->shutdown(dev);
 
 	return;
 }
-- 
1.7.4.1

[PATCH 25/25] Staging: hv: VMBUS is a acpi enumerated device; get irq value from bios.

[K. Y. Srinivasan]

On some windows hosts, the Linux  PCI sub-system is not allocating irq resources to the
vmbus driver. It looks like VMBUS is an ACPI enumerated device. Retrieve the irq
information from DSDT. Currently we use this bios specified irq, if the PCI
sub-system fails to allocate the irq.


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
 drivers/staging/hv/vmbus_drv.c |  101 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 100 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index f95ec2b..1c5d43a 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -17,8 +17,8 @@
  * Authors:
  *   Haiyang Zhang <haiyangz@microsoft.com>
  *   Hank Janssen  <hjanssen@microsoft.com>
+ *   K. Y. Srinivasan <kys@microsoft.com>
  *
- * 3/9/2011: K. Y. Srinivasan	- Significant restructuring and cleanup
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
@@ -31,6 +31,8 @@
 #include <linux/pci.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
 #include <linux/completion.h>
 #include "version_info.h"
 #include "hv_api.h"
@@ -52,6 +54,7 @@ EXPORT_SYMBOL(vmbus_loglevel);
 
 static int pci_probe_error;
 static struct completion probe_event;
+static int irq;
 
 static void get_channel_info(struct hv_device *device,
 			     struct hv_device_info *info)
@@ -723,6 +726,74 @@ void vmbus_child_device_unregister(struct hv_device *device_obj)
 }
 
 
+/*
+ * VMBUS is an acpi enumerated device. Get the the IRQ information
+ * from DSDT.
+ */
+
+static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *irq)
+{
+
+	if (res->type == ACPI_RESOURCE_TYPE_IRQ) {
+		struct acpi_resource_irq *irqp;
+		irqp = &res->data.irq;
+
+		*((unsigned int *)irq) = irqp->interrupts[0];
+	}
+
+	return AE_OK;
+}
+
+static int vmbus_acpi_add(struct acpi_device *device)
+{
+	acpi_status result;
+
+	result =
+	acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+			vmbus_walk_resources, &irq);
+
+	if (ACPI_FAILURE(result)) {
+		complete(&probe_event);
+		return -ENODEV;
+	}
+	complete(&probe_event);
+	return 0;
+}
+
+static const struct acpi_device_id vmbus_acpi_device_ids[] = {
+	{"VMBUS", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, vmbus_acpi_device_ids);
+
+static struct acpi_driver vmbus_acpi_driver = {
+	.name = "vmbus",
+	.ids = vmbus_acpi_device_ids,
+	.ops = {
+		.add = vmbus_acpi_add,
+	},
+};
+
+static int vmbus_acpi_init(void)
+{
+	int result;
+
+
+	result = acpi_bus_register_driver(&vmbus_acpi_driver);
+	if (result < 0)
+		return result;
+
+	return 0;
+}
+
+static void vmbus_acpi_exit(void)
+{
+	acpi_bus_unregister_driver(&vmbus_acpi_driver);
+
+	return;
+}
+
+
 static int __devinit hv_pci_probe(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
@@ -732,7 +803,16 @@ static int __devinit hv_pci_probe(struct pci_dev *pdev,
 	if (pci_probe_error)
 		goto probe_cleanup;
 
+	/*
+	 * If the PCI sub-sytem did not assign us an
+	 * irq, use the bios provided one.
+	 */
+
+	if (pdev->irq == 0)
+		pdev->irq = irq;
+
 	pci_probe_error = vmbus_bus_init(pdev);
+
 	if (pci_probe_error)
 		pci_disable_device(pdev);
 
@@ -762,6 +842,25 @@ static struct pci_driver hv_bus_driver = {
 static int __init hv_pci_init(void)
 {
 	int ret;
+
+	init_completion(&probe_event);
+
+	/*
+	 * Get irq resources first.
+	 */
+
+	ret = vmbus_acpi_init();
+	if (ret)
+		return ret;
+
+	wait_for_completion(&probe_event);
+
+	if (irq <= 0) {
+		vmbus_acpi_exit();
+		return -ENODEV;
+	}
+
+	vmbus_acpi_exit();
 	init_completion(&probe_event);
 	ret = pci_register_driver(&hv_bus_driver);
 	if (ret)
-- 
1.7.4.1

Re: [PATCH 20/25] Staging: hv: Use the probe function in struct hv_driver

[Christoph Hellwig]

> @@ -882,7 +882,7 @@ static int blkvsc_drv_init(void)
>  
>  	drv->driver.name = storvsc_drv_obj->base.name;
>  
> -	drv->driver.probe = blkvsc_probe;
> +	drv->probe = blkvsc_probe;
>  	drv->driver.remove = blkvsc_remove;
>  	drv->driver.shutdown = blkvsc_shutdown;

Not new in this patch, but you should really declare the driver as a
static object and initialize it at compile time, similar to how it's 
done for PCI and countless other busses, e.g.

struct hv_driver blkvsc_driver {
	.name		= "blkvsc",
	.probe		= blkvsc_probe,
	.remove		= blkvsc_remove,
	.shutdown	= blkvsc_shutdown,
};

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

[Christoph Hellwig]

Do you have a repository containing the current state of your patche
somewhere?  There's been so much cleanup that it's hard to review these
patches against the current mainline codebase.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Tuesday, April 26, 2011 12:57 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> Do you have a repository containing the current state of your patche
> somewhere?  There's been so much cleanup that it's hard to review these
> patches against the current mainline codebase.

Christoph,

Yesterday (April 25, 2011), Greg checked in all of the outstanding hv patches. So, if
You checkout Greg's tree today, you will get the most recent hv codebase. This current
patch-set is against Greg's current tree.

Regards,

K. Y

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

[Greg KH]

On Tue, Apr 26, 2011 at 05:04:36PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Tuesday, April 26, 2011 12:57 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> > 
> > Do you have a repository containing the current state of your patche
> > somewhere?  There's been so much cleanup that it's hard to review these
> > patches against the current mainline codebase.
> 
> Christoph,
> 
> Yesterday (April 25, 2011), Greg checked in all of the outstanding hv patches. So, if
> You checkout Greg's tree today, you will get the most recent hv codebase. This current
> patch-set is against Greg's current tree.

It's also always in the linux-next tree, which is easier for most people
to work off of.

thanks,

greg k-h

Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:35AM -0700, K. Y. Srinivasan wrote:
> Now, get rid of struct hv_bus. We will no longer be embedding
> struct bus_type.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
>  drivers/staging/hv/vmbus_drv.c |   33 ++++++++++++++-------------------
>  1 files changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index a3a7741..515311c 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -45,11 +45,6 @@ static struct pci_dev *hv_pci_dev;
>  static struct tasklet_struct msg_dpc;
>  static struct tasklet_struct event_dpc;
>  
> -/* Main vmbus driver data structure */
> -struct hv_bus {
> -	struct bus_type bus;
> -};
> -
>  unsigned int vmbus_loglevel = (ALL_MODULES << 16 | INFO_LVL);
>  EXPORT_SYMBOL(vmbus_loglevel);
>  	/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
> @@ -405,14 +400,14 @@ static void vmbus_device_release(struct device *device)
>  }
>  
>  /* The one and only one */
> -static struct hv_bus  hv_bus = {
> -	.bus.name =		"vmbus",
> -	.bus.match =		vmbus_match,
> -	.bus.shutdown =		vmbus_shutdown,
> -	.bus.remove =		vmbus_remove,
> -	.bus.probe =		vmbus_probe,
> -	.bus.uevent =		vmbus_uevent,
> -	.bus.dev_attrs =	vmbus_device_attrs,
> +static struct bus_type  hv_bus = {
> +	.name =		"vmbus",
> +	.match =		vmbus_match,
> +	.shutdown =		vmbus_shutdown,
> +	.remove =		vmbus_remove,
> +	.probe =		vmbus_probe,
> +	.uevent =		vmbus_uevent,
> +	.dev_attrs =	vmbus_device_attrs,
>  };
>  
>  static const char *driver_name = "hyperv";
> @@ -550,14 +545,14 @@ static int vmbus_bus_init(struct pci_dev *pdev)
>  		goto cleanup;
>  	}
>  
> -	hv_bus.bus.name = driver_name;
> +	hv_bus.name = driver_name;

Why are you setting the name of the bus again?  Shouldn't this line be
removed?

thanks,

greg k-h

RE: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, April 26, 2011 3:41 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> PVT LTD); Hank Janssen
> Subject: Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus
> 
> On Tue, Apr 26, 2011 at 09:20:35AM -0700, K. Y. Srinivasan wrote:
> > Now, get rid of struct hv_bus. We will no longer be embedding
> > struct bus_type.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> >  drivers/staging/hv/vmbus_drv.c |   33 ++++++++++++++-------------------
> >  1 files changed, 14 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > index a3a7741..515311c 100644
> > --- a/drivers/staging/hv/vmbus_drv.c
> > +++ b/drivers/staging/hv/vmbus_drv.c
> > @@ -45,11 +45,6 @@ static struct pci_dev *hv_pci_dev;
> >  static struct tasklet_struct msg_dpc;
> >  static struct tasklet_struct event_dpc;
> >
> > -/* Main vmbus driver data structure */
> > -struct hv_bus {
> > -	struct bus_type bus;
> > -};
> > -
> >  unsigned int vmbus_loglevel = (ALL_MODULES << 16 | INFO_LVL);
> >  EXPORT_SYMBOL(vmbus_loglevel);
> >  	/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
> > @@ -405,14 +400,14 @@ static void vmbus_device_release(struct device
> *device)
> >  }
> >
> >  /* The one and only one */
> > -static struct hv_bus  hv_bus = {
> > -	.bus.name =		"vmbus",
> > -	.bus.match =		vmbus_match,
> > -	.bus.shutdown =		vmbus_shutdown,
> > -	.bus.remove =		vmbus_remove,
> > -	.bus.probe =		vmbus_probe,
> > -	.bus.uevent =		vmbus_uevent,
> > -	.bus.dev_attrs =	vmbus_device_attrs,
> > +static struct bus_type  hv_bus = {
> > +	.name =		"vmbus",
> > +	.match =		vmbus_match,
> > +	.shutdown =		vmbus_shutdown,
> > +	.remove =		vmbus_remove,
> > +	.probe =		vmbus_probe,
> > +	.uevent =		vmbus_uevent,
> > +	.dev_attrs =	vmbus_device_attrs,
> >  };
> >
> >  static const char *driver_name = "hyperv";
> > @@ -550,14 +545,14 @@ static int vmbus_bus_init(struct pci_dev *pdev)
> >  		goto cleanup;
> >  	}
> >
> > -	hv_bus.bus.name = driver_name;
> > +	hv_bus.name = driver_name;
> 
> Why are you setting the name of the bus again?  Shouldn't this line be
> removed?

You are absolutely right. Since this redundancy was in the existing
code, should I send you a separate patch to fix this?

Regards,

K. Y

> 
> thanks,
> 
> greg k-h

Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus

[Greg KH]

On Tue, Apr 26, 2011 at 08:23:25PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Tuesday, April 26, 2011 3:41 PM
> > To: KY Srinivasan
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> > PVT LTD); Hank Janssen
> > Subject: Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus
> > 
> > On Tue, Apr 26, 2011 at 09:20:35AM -0700, K. Y. Srinivasan wrote:
> > > Now, get rid of struct hv_bus. We will no longer be embedding
> > > struct bus_type.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > ---
> > >  drivers/staging/hv/vmbus_drv.c |   33 ++++++++++++++-------------------
> > >  1 files changed, 14 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > > index a3a7741..515311c 100644
> > > --- a/drivers/staging/hv/vmbus_drv.c
> > > +++ b/drivers/staging/hv/vmbus_drv.c
> > > @@ -45,11 +45,6 @@ static struct pci_dev *hv_pci_dev;
> > >  static struct tasklet_struct msg_dpc;
> > >  static struct tasklet_struct event_dpc;
> > >
> > > -/* Main vmbus driver data structure */
> > > -struct hv_bus {
> > > -	struct bus_type bus;
> > > -};
> > > -
> > >  unsigned int vmbus_loglevel = (ALL_MODULES << 16 | INFO_LVL);
> > >  EXPORT_SYMBOL(vmbus_loglevel);
> > >  	/* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
> > > @@ -405,14 +400,14 @@ static void vmbus_device_release(struct device
> > *device)
> > >  }
> > >
> > >  /* The one and only one */
> > > -static struct hv_bus  hv_bus = {
> > > -	.bus.name =		"vmbus",
> > > -	.bus.match =		vmbus_match,
> > > -	.bus.shutdown =		vmbus_shutdown,
> > > -	.bus.remove =		vmbus_remove,
> > > -	.bus.probe =		vmbus_probe,
> > > -	.bus.uevent =		vmbus_uevent,
> > > -	.bus.dev_attrs =	vmbus_device_attrs,
> > > +static struct bus_type  hv_bus = {
> > > +	.name =		"vmbus",
> > > +	.match =		vmbus_match,
> > > +	.shutdown =		vmbus_shutdown,
> > > +	.remove =		vmbus_remove,
> > > +	.probe =		vmbus_probe,
> > > +	.uevent =		vmbus_uevent,
> > > +	.dev_attrs =	vmbus_device_attrs,
> > >  };
> > >
> > >  static const char *driver_name = "hyperv";
> > > @@ -550,14 +545,14 @@ static int vmbus_bus_init(struct pci_dev *pdev)
> > >  		goto cleanup;
> > >  	}
> > >
> > > -	hv_bus.bus.name = driver_name;
> > > +	hv_bus.name = driver_name;
> > 
> > Why are you setting the name of the bus again?  Shouldn't this line be
> > removed?
> 
> You are absolutely right. Since this redundancy was in the existing
> code, should I send you a separate patch to fix this?

A separate one after this series is fine.

thanks,

greg k-h

RE: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Tuesday, April 26, 2011 4:58 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> PVT LTD); Hank Janssen
> Subject: Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus
> 
> On Tue, Apr 26, 2011 at 08:23:25PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Tuesday, April 26, 2011 3:41 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree
> Consulting
> > > PVT LTD); Hank Janssen
> > > Subject: Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus
> > >
> > > On Tue, Apr 26, 2011 at 09:20:35AM -0700, K. Y. Srinivasan wrote:
> > > > Now, get rid of struct hv_bus. We will no longer be embedding
> > > > struct bus_type.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > > ---
> > > >  drivers/staging/hv/vmbus_drv.c |   33 ++++++++++++++-------------------
> > > >  1 files changed, 14 insertions(+), 19 deletions(-)
> > > >
> > > >
> > > > -	hv_bus.bus.name = driver_name;
> > > > +	hv_bus.name = driver_name;
> > >
> > > Why are you setting the name of the bus again?  Shouldn't this line be
> > > removed?
> >
> > You are absolutely right. Since this redundancy was in the existing
> > code, should I send you a separate patch to fix this?
> 
> A separate one after this series is fine.

Done; I have sent the patch out.

Regards,

K. Y

Re: [PATCH 17/25] Staging: hv: Make event_dpc a global variable

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:34AM -0700, K. Y. Srinivasan wrote:
> In preparation for getting rid of struct hv_bus, Make event_dpc a
> global variable.

It's "static", one for the whole driver, not global.

thanks,

greg k-h

Re: [PATCH 16/25] Staging: hv: Make msg_dpc a global variable

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:33AM -0700, K. Y. Srinivasan wrote:
> In preparation for cleaning up (getting rid of) of the hv_bus structure,
> make msg_dpc a global variable.

It's "static".  Ah, you mean for the whole bus.  Yeah, that's true,
wierd choice of words I guess...

thanks,

greg k-h

Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:25AM -0700, K. Y. Srinivasan wrote:
> The vmbus driver cannot be unloaded; the windows host does not
> permit this. Cleanup accordingly.

Woah, you just prevented this driver from ever being able to be
unloaded.

That's not a "cleanup" that's a major change in how things work.  I'm
sure, if you want to continue down this line, there are more things you
can remove from the code, right?

What is the real issue here?  What happens if you unload the bus?  What
goes wrong?  Can it be fixed?

This is a pretty big commitment...

thanks,

greg k-h

Re: [PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:29AM -0700, K. Y. Srinivasan wrote:
> Cleanup error handling in vmbus_child_device_register().
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
>  drivers/staging/hv/vmbus_drv.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index d597dd4..4d569ad 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)
>  	 */
>  	ret = device_register(&child_device_obj->device);
>  
> +	if (ret)
> +		return ret;
> +
>  	/* vmbus_probe() error does not get propergate to device_register(). */
>  	ret = child_device_obj->probe_error;

Wait, why not?  Why is the probe_error have to be saved off like this?
That seems like something is wrong here, this patch should not be
needed.

Well, you should check the return value of device_register, that is
needed, but this seems broken somehow.

>  
> -	if (ret)
> +	if (ret) {
>  		pr_err("Unable to register child device\n");
> +		device_unregister(&child_device_obj->device);
> +	}
>  	else

	} else
is the preferred way.

Care to send a fixup patch to remove the probe_error field and fix this
formatting error up?

thanks,

greg k-h

Re: [PATCH 11/25] Staging: hv: Get rid of the drv field in struct hv_device

[Greg KH]

On Tue, Apr 26, 2011 at 09:20:28AM -0700, K. Y. Srinivasan wrote:
> Now, we can rid of the drv field in struct hv_device.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
>  drivers/staging/hv/vmbus_api.h |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
> index 51fa952..02e3587 100644
> --- a/drivers/staging/hv/vmbus_api.h
> +++ b/drivers/staging/hv/vmbus_api.h
> @@ -103,9 +103,6 @@ struct hv_driver {
>  
>  /* Base device object */
>  struct hv_device {
> -	/* the driver for this device */
> -	struct hv_driver *drv;
> -
>  	char name[64];

FYI, in the future, you can also remove this name[64] field as well as I
don't think it's ever used...

thanks,

greg k-h

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

[Greg KH]

On Tue, Apr 26, 2011 at 09:19:45AM -0700, K. Y. Srinivasan wrote:
> This patch-set addresses some of the bus/driver model cleanup that
> Greg sugested over the last couple of days.  In this patch-set we
> deal with the following issues:
> 
> 	1) Cleanup unnecessary state in struct hv_device and 
> 	   struct hv_driver to be compliant with the Linux
> 	   Driver model.
> 
> 	2) Cleanup the vmbus_match() function to conform with the 
> 	   Linux Driver model.
> 
> 	3) Cleanup error handling in the vmbus_probe() and 
> 	   vmbus_child_device_register() functions. Fixed a 
> 	   bug in the probe failure path as part of this cleanup.
> 
> 	4) The Windows host cannot handle the vmbus_driver being 
> 	   unloaded and subsequently loaded. Cleanup the driver with
> 	   this in mind.

I've stopped at this patch (well, I applied one more, but you can see
that.)

I'd like to get some confirmation that this is really what you all want
to do here before applying it.  If it is, care to resend them with a bit
more information about this issue and why you all are making it?

Anyway, other than this one, the series looks good.  But you should
follow-up with some driver structure changes like what Christoph said to
do.  After that, do you want another round of review of the code, or do
you have more things you want to send in (like the name[64] removal?)

thanks,

greg k-h

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

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, April 26, 2011 7:29 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Tue, Apr 26, 2011 at 09:19:45AM -0700, K. Y. Srinivasan wrote:
> > This patch-set addresses some of the bus/driver model cleanup that
> > Greg sugested over the last couple of days.  In this patch-set we
> > deal with the following issues:
> >
> > 	1) Cleanup unnecessary state in struct hv_device and
> > 	   struct hv_driver to be compliant with the Linux
> > 	   Driver model.
> >
> > 	2) Cleanup the vmbus_match() function to conform with the
> > 	   Linux Driver model.
> >
> > 	3) Cleanup error handling in the vmbus_probe() and
> > 	   vmbus_child_device_register() functions. Fixed a
> > 	   bug in the probe failure path as part of this cleanup.
> >
> > 	4) The Windows host cannot handle the vmbus_driver being
> > 	   unloaded and subsequently loaded. Cleanup the driver with
> > 	   this in mind.
> 
> I've stopped at this patch (well, I applied one more, but you can see
> that.)
> 
> I'd like to get some confirmation that this is really what you all want
> to do here before applying it.  If it is, care to resend them with a bit
> more information about this issue and why you all are making it?

Greg, this is restriction imposed by the Windows host:  you cannot reload the
Vmbus driver without rebooting the guest. If you cannot re-load, what good is it
to be able to unload? Distros that integrate these drivers will load these drivers 
automatically on boot and there is not much point in being able to unload this since 
most likely the root device will be handled by these drivers. For systems that don't
integrate these drivers; I don't see much point in allowing the driver to be unloaded,
if you cannot reload the driver without rebooting the guest. If and when the Windows 
host supports reloading the vmbus driver, we can very easily add this functionality.
The situation currently at best very misleading - you think you can unload the vmbus
driver, only to discover that you have to reboot the guest!

> 
> Anyway, other than this one, the series looks good.  But you should
> follow-up with some driver structure changes like what Christoph said to
> do. 

I will send you a patch for this.

> After that, do you want another round of review of the code, or do
> you have more things you want to send in (like the name[64] removal?)

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 ...)! 

Regards,

K. Y
> 
> thanks,
> 
> greg k-h

RE: [PATCH 11/25] Staging: hv: Get rid of the drv field in struct hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, April 26, 2011 6:57 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> Subject: Re: [PATCH 11/25] Staging: hv: Get rid of the drv field in struct hv_device
> 
> On Tue, Apr 26, 2011 at 09:20:28AM -0700, K. Y. Srinivasan wrote:
> > Now, we can rid of the drv field in struct hv_device.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> >  drivers/staging/hv/vmbus_api.h |    3 ---
> >  1 files changed, 0 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
> > index 51fa952..02e3587 100644
> > --- a/drivers/staging/hv/vmbus_api.h
> > +++ b/drivers/staging/hv/vmbus_api.h
> > @@ -103,9 +103,6 @@ struct hv_driver {
> >
> >  /* Base device object */
> >  struct hv_device {
> > -	/* the driver for this device */
> > -	struct hv_driver *drv;
> > -
> >  	char name[64];
> 
> FYI, in the future, you can also remove this name[64] field as well as I
> don't think it's ever used...

I will send you a patch for this, once all the patches in this series are applied.

Regards,

K. Y

RE: [PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, April 26, 2011 6:51 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD)
> Subject: Re: [PATCH 12/25] Staging: hv: Cleanup error handling in
> vmbus_child_device_register()
> 
> On Tue, Apr 26, 2011 at 09:20:29AM -0700, K. Y. Srinivasan wrote:
> > Cleanup error handling in vmbus_child_device_register().
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> >  drivers/staging/hv/vmbus_drv.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > index d597dd4..4d569ad 100644
> > --- a/drivers/staging/hv/vmbus_drv.c
> > +++ b/drivers/staging/hv/vmbus_drv.c
> > @@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device
> *child_device_obj)
> >  	 */
> >  	ret = device_register(&child_device_obj->device);
> >
> > +	if (ret)
> > +		return ret;
> > +
> >  	/* vmbus_probe() error does not get propergate to device_register(). */
> >  	ret = child_device_obj->probe_error;
> 
> Wait, why not?  Why is the probe_error have to be saved off like this?
> That seems like something is wrong here, this patch should not be
> needed.
> 
> Well, you should check the return value of device_register, that is
> needed, but this seems broken somehow.

The current code had comments claiming that the probe error was not
correctly propagated. Looking at the kernel side of the code, it was not clear
if device_register() could succeed while the probe might fail. In any event,
if you can guarantee that device_register() can return any probe related 
errors, I agree with you that saving the probe error is an overkill. The current code
saved the probe error and with new check I added with regards to the return 
value of device_register, there is no correctness issue with this patch.
> 
> >
> > -	if (ret)
> > +	if (ret) {
> >  		pr_err("Unable to register child device\n");
> > +		device_unregister(&child_device_obj->device);
> > +	}
> >  	else
> 
> 	} else
> is the preferred way.
> 
> Care to send a fixup patch to remove the probe_error field and fix this
> formatting error up?

I will fix up the formatting issue.

Regards,

K. Y

RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, April 26, 2011 6:46 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD)
> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> cleanup accordingly
> 
> On Tue, Apr 26, 2011 at 09:20:25AM -0700, K. Y. Srinivasan wrote:
> > The vmbus driver cannot be unloaded; the windows host does not
> > permit this. Cleanup accordingly.
> 
> Woah, you just prevented this driver from ever being able to be
> unloaded.

It was never unloadable; while the driver defined an exit routine, 
there were couple of issues unloading the vmbus driver:
 
1) All guest resources given to the host could not be recovered.

2) Windows host would not permit reloading the driver without 
rebooting the guest.

All I did was acknowledge the current state and cleanup 
accordingly. This is not unique to Hyper-V; for what it is worth,
the Xen platform_pci  driver which is equivalent to the vmbus driver
is also not unlodable (the last time I checked).

> 
> That's not a "cleanup" that's a major change in how things work.  I'm
> sure, if you want to continue down this line, there are more things you
> can remove from the code, right?
> 
> What is the real issue here?  What happens if you unload the bus?  What
> goes wrong?  Can it be fixed?

This needs to be fixed on the host side. I have notified them of the issue. 

> 
> This is a pretty big commitment...

These drivers only load when Linux is hosted on  a Hyper-V platform;
I am not sure why it is a "big commitment" given that the host does not 
permit reloading this driver without rebooting the guest. 

Regards,

K. Y

Re:RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[uabuntsu]

unsubscribe linux-kernel
At 2011-04-27 10:31:18，"KY Srinivasan" <kys@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: Greg KH [mailto:greg@kroah.com]
>> Sent: Tuesday, April 26, 2011 6:46 PM
>> To: KY Srinivasan
>> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
>> Abhishek Kane (Mindtree Consulting PVT LTD)
>> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
>> cleanup accordingly
>> 
>> On Tue, Apr 26, 2011 at 09:20:25AM -0700, K. Y. Srinivasan wrote:
>> > The vmbus driver cannot be unloaded; the windows host does not
>> > permit this. Cleanup accordingly.
>> 
>> Woah, you just prevented this driver from ever being able to be
>> unloaded.
>
>It was never unloadable; while the driver defined an exit routine, 
>there were couple of issues unloading the vmbus driver:
> 
>1) All guest resources given to the host could not be recovered.
>
>2) Windows host would not permit reloading the driver without 
>rebooting the guest.
>
>All I did was acknowledge the current state and cleanup 
>accordingly. This is not unique to Hyper-V; for what it is worth,
>the Xen platform_pci  driver which is equivalent to the vmbus driver
>is also not unlodable (the last time I checked).
>
>> 
>> That's not a "cleanup" that's a major change in how things work.  I'm
>> sure, if you want to continue down this line, there are more things you
>> can remove from the code, right?
>> 
>> What is the real issue here?  What happens if you unload the bus?  What
>> goes wrong?  Can it be fixed?
>
>This needs to be fixed on the host side. I have notified them of the issue. 
>
>> 
>> This is a pretty big commitment...
>
>These drivers only load when Linux is hosted on  a Hyper-V platform;
>I am not sure why it is a "big commitment" given that the host does not 
>permit reloading this driver without rebooting the guest. 
>
>Regards,
>
>K. Y
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/
>

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

[Christoph Hellwig]

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.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Wednesday, April 27, 2011 2:46 AM
> To: KY Srinivasan
> Cc: Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> 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?

This topic was discussed at some great length back in Feb/March when I 
did a bunch of cleanup with regards how the driver and device data structures
were layered. At that point, the consensus was to keep the hv prefix.

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

While I am currently cleaning up our block drivers, my goal this go around is to
work on getting the vmbus driver out of  staging. I am hoping when I am ready for
having you guys review the storage drivers, I will have dealt with the issues you
raise here.

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

I had written up a brief note that I had sent out setting the stage for the
first patch-set for cleaning up the block drivers. I am copying it here for your
convenience:

From: K. Y. Srinivasan <kys@microsoft.com>
Date: Tue, 22 Mar 2011 11:54:46 -0700
Subject: [PATCH 00/16] Staging: hv: Cleanup storage drivers - Phase I

This is first in a series of patch-sets aimed at cleaning up the storage
drivers for Hyper-V. Before I get into the details of this patch-set, I think
it is  useful to give a brief overview of the storage related front-end
drivers currently in the tree for Linux on Hyper-V:

On the host side, Windows emulates the  standard PC hardware
to permit hosting of fully virtualized operating systems.
To enhance disk I/O performance, we support a virtual block driver.
This block driver currently handles disks that have been setup as IDE
disks for the guest - as specified in the guest configuration.

On the SCSI side, we emulate a SCSI HBA. Devices configured
under the SCSI controller for the guest are handled via this
emulated HBA (SCSI front-end). So, SCSI disks configured for
the guest are handled through native SCSI upper-level drivers.
If this SCSI front-end driver is not loaded, currently, the guest
cannot see devices that have been configured as SCSI devices.
So, while the virtual block driver described earlier could potentially
handle all block devices, the implementation choices made on the host
will not permit it. Also, the only SCSI device that can be currently configured
for the guest is a disk device.

Both the block device driver (hv_blkvsc) and the SCSI front-end
driver (hv_storvsc) communicate with the host via unique channels 
that are implemented as bi-directional ring buffers. Each (storage) 
channel carries with it enough state to uniquely identify the device on
the host side. Microsoft has chosen to use SCSI verbs for this storage channel 
communication. 
 
> 
> 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.
> 

I will do this as part of storage cleanup I am currently doing. Thank you
for taking the time to review the code.

Regards,

K. Y

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

[Christoph Hellwig]

On Wed, Apr 27, 2011 at 11:47:03AM +0000, KY Srinivasan wrote:
> On the host side, Windows emulates the  standard PC hardware
> to permit hosting of fully virtualized operating systems.
> To enhance disk I/O performance, we support a virtual block driver.
> This block driver currently handles disks that have been setup as IDE
> disks for the guest - as specified in the guest configuration.
> 
> On the SCSI side, we emulate a SCSI HBA. Devices configured
> under the SCSI controller for the guest are handled via this
> emulated HBA (SCSI front-end). So, SCSI disks configured for
> the guest are handled through native SCSI upper-level drivers.
> If this SCSI front-end driver is not loaded, currently, the guest
> cannot see devices that have been configured as SCSI devices.
> So, while the virtual block driver described earlier could potentially
> handle all block devices, the implementation choices made on the host
> will not permit it. Also, the only SCSI device that can be currently configured
> for the guest is a disk device.
> 
> Both the block device driver (hv_blkvsc) and the SCSI front-end
> driver (hv_storvsc) communicate with the host via unique channels 
> that are implemented as bi-directional ring buffers. Each (storage) 
> channel carries with it enough state to uniquely identify the device on
> the host side. Microsoft has chosen to use SCSI verbs for this storage channel 
> communication. 

This doesn't really explain much at all.  The only important piece
of information I can read from this statement is that both blkvsc
and storvsc only support disks, but not any other kind of device,
and that chosing either one is an arbitrary seletin when setting up
a VM configuration.

But this still isn't an excuse to implement a block layer driver for
a SCSI protocol, and it doesn't not explain in what way the two
protocols actually differ.  You really should implement blksvs as a SCSI
LLDD, too - and from the looks of it it doesn't even have to be a
separate one, but just adding the ids to storvsc would do the work.

Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[Greg KH]

On Wed, Apr 27, 2011 at 02:31:18AM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Tuesday, April 26, 2011 6:46 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> > Abhishek Kane (Mindtree Consulting PVT LTD)
> > Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> > cleanup accordingly
> > 
> > On Tue, Apr 26, 2011 at 09:20:25AM -0700, K. Y. Srinivasan wrote:
> > > The vmbus driver cannot be unloaded; the windows host does not
> > > permit this. Cleanup accordingly.
> > 
> > Woah, you just prevented this driver from ever being able to be
> > unloaded.
> 
> It was never unloadable; while the driver defined an exit routine, 
> there were couple of issues unloading the vmbus driver:
>  
> 1) All guest resources given to the host could not be recovered.

Is this a problem in the Linux side?  If so, that could easily be fixed.

> 2) Windows host would not permit reloading the driver without 
> rebooting the guest.

That's a different issue, and one that I am very surprised to hear.
That kind of invalidates ever being able to update the driver in a guest
for a long-running system that you want to migrate and not reboot.  That
sounds like a major bug in hyper-v, don't you agree?

> All I did was acknowledge the current state and cleanup 
> accordingly. This is not unique to Hyper-V; for what it is worth,
> the Xen platform_pci  driver which is equivalent to the vmbus driver
> is also not unlodable (the last time I checked).

Why isn't that allowed to be unloaded?  What happens if it does?

I would like to see the following be possible from Linux:
	- running Linux guest on hyperv
	- need to migrate to a newer version of hyper-v
	- pause long-running userspace processes.
	- unload hyperv modules
	- migrate guest to newer hyperv version (possible different host
	  machine)
	- load newer hyperv modules
	- resume long-running guest processes

If this isn't possible due to hyper-v bugs, then I guess we need to be
able to live with it, but we had better advertise it pretty well as I
know people will want to be able to do the above sequence for their
guest instances.

If so, can you expand this patch to say more in the changelog entry, and
resend the remaining patches that I didn't apply as they are now gone
from my pending-patch queue.

> > That's not a "cleanup" that's a major change in how things work.  I'm
> > sure, if you want to continue down this line, there are more things you
> > can remove from the code, right?
> > 
> > What is the real issue here?  What happens if you unload the bus?  What
> > goes wrong?  Can it be fixed?
> 
> This needs to be fixed on the host side. I have notified them of the issue. 

Ok, so if this is going to be fixed, why do we need to prevent this from
ever being possible to have happen on our side?

thanks,

greg k-h

Re: [PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()

[Greg KH]

On Wed, Apr 27, 2011 at 02:11:48AM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Tuesday, April 26, 2011 6:51 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> > Abhishek Kane (Mindtree Consulting PVT LTD)
> > Subject: Re: [PATCH 12/25] Staging: hv: Cleanup error handling in
> > vmbus_child_device_register()
> > 
> > On Tue, Apr 26, 2011 at 09:20:29AM -0700, K. Y. Srinivasan wrote:
> > > Cleanup error handling in vmbus_child_device_register().
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > ---
> > >  drivers/staging/hv/vmbus_drv.c |    7 ++++++-
> > >  1 files changed, 6 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > > index d597dd4..4d569ad 100644
> > > --- a/drivers/staging/hv/vmbus_drv.c
> > > +++ b/drivers/staging/hv/vmbus_drv.c
> > > @@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device
> > *child_device_obj)
> > >  	 */
> > >  	ret = device_register(&child_device_obj->device);
> > >
> > > +	if (ret)
> > > +		return ret;
> > > +
> > >  	/* vmbus_probe() error does not get propergate to device_register(). */
> > >  	ret = child_device_obj->probe_error;
> > 
> > Wait, why not?  Why is the probe_error have to be saved off like this?
> > That seems like something is wrong here, this patch should not be
> > needed.
> > 
> > Well, you should check the return value of device_register, that is
> > needed, but this seems broken somehow.
> 
> The current code had comments claiming that the probe error was not
> correctly propagated. Looking at the kernel side of the code, it was not clear
> if device_register() could succeed while the probe might fail.

Of course it can, device_register() has nothing to do with the probe
callback of the device itself.  To think otherwise is to not understand
the driver model and assume things that you should never be caring
about.

Think about it, if you register a device, you don't know at that point
in time if a driver is currently loaded for it, and that it will be
bound to that device.  Nor do you care, as any needed notifications for
new drivers will be sent to userspace, and they will be loaded at some
random time in the future.  So a probe() call might never be called for
this device until some other time, running on some other processor, in
some other thread.

Drivers are allowed to return errors from their probe functions for
valid reasons (i.e. this driver shouldn't bind to this device for a
variety of good reasons.)  No one cares about this, as the driver core
handles it properly and will pass on to the next driver in the list that
might be able to be bound to this device.

So why do you care about the return value of the probe() call?  It gets
properly handled already by the driver core, why would your bus ever
care about it?  (Hint, no other bus does, as it makes no sense.)

> In any event, if you can guarantee that device_register() can return
> any probe related errors, I agree with you that saving the probe error
> is an overkill. The current code saved the probe error and with new
> check I added with regards to the return value of device_register,
> there is no correctness issue with this patch.

As explained above, no, it will not return a probe error, as that makes
no sense.  If the code is wanting to rely on this, it is broken and must
be fixed.

thanks,

greg k-h

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

[Greg KH]

On Wed, Apr 27, 2011 at 01:54:02AM +0000, KY Srinivasan wrote:
> > After that, do you want another round of review of the code, or do
> > you have more things you want to send in (like the name[64] removal?)
> 
> I would prefer that we go through the  review process. What is the process for
> this review?

The same as always, just ask.

> Is there a time window for people to respond.

No.  We don't have time limits here, this is a community, we don't have
deadlines, you know that.

> 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 ...)! 

Yes, it would be nice, and I understand your the corporate pressures you
are under to get this done, and I am doing my best to fit the patch
review and apply cycle into my very-very-limited-at-the-moment spare
time.

As always, if you miss this kernel release, there's always another one 3
months away, so it's no big deal in the long-run.

thanks,

greg k-h

RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, April 27, 2011 8:20 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD)
> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> cleanup accordingly
> 
> On Wed, Apr 27, 2011 at 02:31:18AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Tuesday, April 26, 2011 6:46 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> > > Abhishek Kane (Mindtree Consulting PVT LTD)
> > > Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> > > cleanup accordingly
> > >
> > > On Tue, Apr 26, 2011 at 09:20:25AM -0700, K. Y. Srinivasan wrote:
> > > > The vmbus driver cannot be unloaded; the windows host does not
> > > > permit this. Cleanup accordingly.
> > >
> > > Woah, you just prevented this driver from ever being able to be
> > > unloaded.
> >
> > It was never unloadable; while the driver defined an exit routine,
> > there were couple of issues unloading the vmbus driver:
> >
> > 1) All guest resources given to the host could not be recovered.
> 
> Is this a problem in the Linux side?  If so, that could easily be fixed.

This is not an issue on the Linux side. From what I remember, this was
Guest/Host protocol related. Once I confirmed the second issue, I did 
not pursue this issue further.

> 
> > 2) Windows host would not permit reloading the driver without
> > rebooting the guest.
> 
> That's a different issue, and one that I am very surprised to hear.
> That kind of invalidates ever being able to update the driver in a guest
> for a long-running system that you want to migrate and not reboot.  That
> sounds like a major bug in hyper-v, don't you agree?

In practical terms, I am not sure this is a major problem. If the root device
Is managed by a Hyper-V driver, then you cannot unload that driver and 
drivers it depends on anyway.
 
> 
> > All I did was acknowledge the current state and cleanup
> > accordingly. This is not unique to Hyper-V; for what it is worth,
> > the Xen platform_pci  driver which is equivalent to the vmbus driver
> > is also not unlodable (the last time I checked).
> 
> Why isn't that allowed to be unloaded?  What happens if it does?

On the Xen side, from as long as I can remember, the platform_pci 
has been unlodable (does not define an exit routiune). I don't recall 
what the issues have been.

> 
> I would like to see the following be possible from Linux:
> 	- running Linux guest on hyperv
> 	- need to migrate to a newer version of hyper-v
> 	- pause long-running userspace processes.
> 	- unload hyperv modules
> 	- migrate guest to newer hyperv version (possible different host
> 	  machine)
> 	- load newer hyperv modules
> 	- resume long-running guest processes

Many hyper-V modules are un-lodable; it is just the vmbus_driver module 
has this issue. Also, since vmbus_driver happens to be the foundational
driver for most of  the hyper-v drivers,  and since ultimately, we do 
want to handle the root device via hyper-v block driver, vmbus_driver will
become unlodable for other reasons.

> 
> If this isn't possible due to hyper-v bugs, then I guess we need to be
> able to live with it, but we had better advertise it pretty well as I
> know people will want to be able to do the above sequence for their
> guest instances.
> 
> If so, can you expand this patch to say more in the changelog entry, and
> resend the remaining patches that I didn't apply as they are now gone
> from my pending-patch queue.

Will do.
> 
> > > That's not a "cleanup" that's a major change in how things work.  I'm
> > > sure, if you want to continue down this line, there are more things you
> > > can remove from the code, right?
> > >
> > > What is the real issue here?  What happens if you unload the bus?  What
> > > goes wrong?  Can it be fixed?
> >
> > This needs to be fixed on the host side. I have notified them of the issue.
> 
> Ok, so if this is going to be fixed, why do we need to prevent this from
> ever being possible to have happen on our side?

While I have notified the windows team about this issue, I do not know
If/or when this issue may be fixed. As, I noted earlier, having an exit function
gives the impression that the module is unloadable while it is not - today because
of the issue on the host side. Even if the host side issue is fixed, we may not be 
able to unload the vmbus driver for other dependency related issues.
All I did was get rid of the exit function so there 
would be no confusion as to the state of this module. We could easily add the exit 
function, if and when the host side issues are fixed. Even then, this module may
be unlodable if we are driving the root device with the hyper-v block driver.

Greg, I am open to either approach here: 1) I could drop this patch and restore the 
exit function. 2) I could keep the patch as is, but add additional comments to
capture this discussion. Let me know what you prefer and I will send the remaining 
patch-set with the agreed upon changes.

Regards,

K. Y

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

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, April 27, 2011 8:28 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Wed, Apr 27, 2011 at 01:54:02AM +0000, KY Srinivasan wrote:
> > > After that, do you want another round of review of the code, or do
> > > you have more things you want to send in (like the name[64] removal?)
> >
> > I would prefer that we go through the  review process. What is the process for
> > this review?
> 
> The same as always, just ask.
> 
> > Is there a time window for people to respond.
> 
> No.  We don't have time limits here, this is a community, we don't have
> deadlines, you know that.

Perhaps I did not properly formulate my question here. The review
process itself may be open-ended, and that is fine - we will fix all legitimate
issues/concerns  in our drivers whether they are in the staging area or not. 
My question was specifically with regards to the review process that may gate exiting
staging. I am hoping to re-spin the remaining patches of the last patch-set and send it to
you by early next week and ask for a review. I fully intend to address whatever review 
comments I may get in a very timely manner. Assuming at some point in time after I ask
for this review there are no outstanding issues, would that be sufficient to exit staging?  

> 
> > 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 ...)!
> 
> Yes, it would be nice, and I understand your the corporate pressures you
> are under to get this done, and I am doing my best to fit the patch
> review and apply cycle into my very-very-limited-at-the-moment spare
> time.

Greg, I do appreciate your ongoing  help here.

Regards,

K. Y 

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

[Greg KH]

On Fri, Apr 29, 2011 at 02:26:13PM +0000, KY Srinivasan wrote:
> Perhaps I did not properly formulate my question here. The review
> process itself may be open-ended, and that is fine - we will fix all legitimate
> issues/concerns  in our drivers whether they are in the staging area or not. 
> My question was specifically with regards to the review process that may gate exiting
> staging. I am hoping to re-spin the remaining patches of the last patch-set and send it to
> you by early next week and ask for a review. I fully intend to address whatever review 
> comments I may get in a very timely manner. Assuming at some point in time after I ask
> for this review there are no outstanding issues, would that be sufficient to exit staging?  

If it looks acceptable to me, and there are no other objections from
other developers, then yes, that would be sufficient to move it out of
staging.

thanks,

greg k-h

Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[Greg KH]

On Fri, Apr 29, 2011 at 01:49:21PM +0000, KY Srinivasan wrote:
> > > 2) Windows host would not permit reloading the driver without
> > > rebooting the guest.
> > 
> > That's a different issue, and one that I am very surprised to hear.
> > That kind of invalidates ever being able to update the driver in a guest
> > for a long-running system that you want to migrate and not reboot.  That
> > sounds like a major bug in hyper-v, don't you agree?
> 
> In practical terms, I am not sure this is a major problem. If the root device
> Is managed by a Hyper-V driver, then you cannot unload that driver and 
> drivers it depends on anyway.

I don't run my hyper-v guests using the hyper-v driver for my root
devices, so in my setup, it is possible to unload the whole vmbus
subsystem and drivers and reload it without any system interruption.

Now I have never tried that... :)

> Greg, I am open to either approach here: 1) I could drop this patch and restore the 
> exit function. 2) I could keep the patch as is, but add additional comments to
> capture this discussion. Let me know what you prefer and I will send the remaining 
> patch-set with the agreed upon changes.

As you all are happy with not having this be unloaded, I'll go with 2)
as it is your code to maintain and support, not mine.

Just resend it with some more comments as I explained, and the rest, and
I'll queue them up when I get caught up on my patch queue (I only have
381 to get through, the end is near!)

thanks,

greg k-h

RE: [PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, April 27, 2011 8:26 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD)
> Subject: Re: [PATCH 12/25] Staging: hv: Cleanup error handling in
> vmbus_child_device_register()
> 
> On Wed, Apr 27, 2011 at 02:11:48AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Tuesday, April 26, 2011 6:51 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> > > Abhishek Kane (Mindtree Consulting PVT LTD)
> > > Subject: Re: [PATCH 12/25] Staging: hv: Cleanup error handling in
> > > vmbus_child_device_register()
> > >
> > > On Tue, Apr 26, 2011 at 09:20:29AM -0700, K. Y. Srinivasan wrote:
> > > > Cleanup error handling in vmbus_child_device_register().
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > > ---
> > > >  drivers/staging/hv/vmbus_drv.c |    7 ++++++-
> > > >  1 files changed, 6 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/hv/vmbus_drv.c
> b/drivers/staging/hv/vmbus_drv.c
> > > > index d597dd4..4d569ad 100644
> > > > --- a/drivers/staging/hv/vmbus_drv.c
> > > > +++ b/drivers/staging/hv/vmbus_drv.c
> > > > @@ -720,11 +720,16 @@ int vmbus_child_device_register(struct hv_device
> > > *child_device_obj)
> > > >  	 */
> > > >  	ret = device_register(&child_device_obj->device);
> > > >
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > >  	/* vmbus_probe() error does not get propergate to device_register(). */
> > > >  	ret = child_device_obj->probe_error;
> > >
> > > Wait, why not?  Why is the probe_error have to be saved off like this?
> > > That seems like something is wrong here, this patch should not be
> > > needed.
> > >
> > > Well, you should check the return value of device_register, that is
> > > needed, but this seems broken somehow.
> >
> > The current code had comments claiming that the probe error was not
> > correctly propagated. Looking at the kernel side of the code, it was not clear
> > if device_register() could succeed while the probe might fail.
> 
> Of course it can, device_register() has nothing to do with the probe
> callback of the device itself.  To think otherwise is to not understand
> the driver model and assume things that you should never be caring
> about.
> 
> Think about it, if you register a device, you don't know at that point
> in time if a driver is currently loaded for it, and that it will be
> bound to that device.  Nor do you care, as any needed notifications for
> new drivers will be sent to userspace, and they will be loaded at some
> random time in the future.  So a probe() call might never be called for
> this device until some other time, running on some other processor, in
> some other thread.
> 
> Drivers are allowed to return errors from their probe functions for
> valid reasons (i.e. this driver shouldn't bind to this device for a
> variety of good reasons.)  No one cares about this, as the driver core
> handles it properly and will pass on to the next driver in the list that
> might be able to be bound to this device.
> 
> So why do you care about the return value of the probe() call?  It gets
> properly handled already by the driver core, why would your bus ever
> care about it?  (Hint, no other bus does, as it makes no sense.)
> 
> > In any event, if you can guarantee that device_register() can return
> > any probe related errors, I agree with you that saving the probe error
> > is an overkill. The current code saved the probe error and with new
> > check I added with regards to the return value of device_register,
> > there is no correctness issue with this patch.
> 
> As explained above, no, it will not return a probe error, as that makes
> no sense.  If the code is wanting to rely on this, it is broken and must
> be fixed.

I did not say that device_register would return the probe_error. On the 
contrary, the code I added explicitly ensures that proper cleanup is done
for  the failure of  device_register and if the  probe function were
called on the same context executing the device_register call,
 the failure of the probe call as well (looking at the stack trace while in
the probe function, this appeared to be the case currently).

If you look at the existing code; the current  code did not deal with the failure of
device_register() - it over-writes the return value of device_register with
that of the probe error. This patch fixed that problem.

The current code dealt with the probe error by spinning up a work item to
deal with the probe failure. This work item would try to unregister the device 
that was not fully registered and this caused a problem. I tried to fix this
problem by getting rid of the work item and dealing with the probe 
failure cleanup in the same context as the call to device_register.
Since no other driver deals with probe failures this way, I will get rid 
of this code in the next version of this patch.

Regards,

K. Y  

> 
> thanks,
> 
> greg k-h

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Wednesday, April 27, 2011 8:19 AM
> To: KY Srinivasan
> Cc: Christoph Hellwig; Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Wed, Apr 27, 2011 at 11:47:03AM +0000, KY Srinivasan wrote:
> > On the host side, Windows emulates the  standard PC hardware
> > to permit hosting of fully virtualized operating systems.
> > To enhance disk I/O performance, we support a virtual block driver.
> > This block driver currently handles disks that have been setup as IDE
> > disks for the guest - as specified in the guest configuration.
> >
> > On the SCSI side, we emulate a SCSI HBA. Devices configured
> > under the SCSI controller for the guest are handled via this
> > emulated HBA (SCSI front-end). So, SCSI disks configured for
> > the guest are handled through native SCSI upper-level drivers.
> > If this SCSI front-end driver is not loaded, currently, the guest
> > cannot see devices that have been configured as SCSI devices.
> > So, while the virtual block driver described earlier could potentially
> > handle all block devices, the implementation choices made on the host
> > will not permit it. Also, the only SCSI device that can be currently configured
> > for the guest is a disk device.
> >
> > Both the block device driver (hv_blkvsc) and the SCSI front-end
> > driver (hv_storvsc) communicate with the host via unique channels
> > that are implemented as bi-directional ring buffers. Each (storage)
> > channel carries with it enough state to uniquely identify the device on
> > the host side. Microsoft has chosen to use SCSI verbs for this storage channel
> > communication.
> 
> This doesn't really explain much at all.  The only important piece
> of information I can read from this statement is that both blkvsc
> and storvsc only support disks, but not any other kind of device,
> and that chosing either one is an arbitrary seletin when setting up
> a VM configuration.
> 
> But this still isn't an excuse to implement a block layer driver for
> a SCSI protocol, and it doesn't not explain in what way the two
> protocols actually differ.  You really should implement blksvs as a SCSI
> LLDD, too - and from the looks of it it doesn't even have to be a
> separate one, but just adding the ids to storvsc would do the work.

On the host-side, as part of configuring a guest  you can specify block devices
as being under an IDE controller or under a
SCSI controller. Those are the only options you have. Devices configured under
the IDE controller cannot be seen in the guest under the emulated SCSI front-end which is
the scsi driver (storvsc_drv). So, when you do a bus scan in the emulated scsi front-end,
the devices enumerated will not include block devices configured under the IDE 
controller. So, it is not clear to me how I can do what you are proposing given the 
restrictions imposed by the host.

Regards,

K. Y
 

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

[Greg KH]

On Fri, Apr 29, 2011 at 04:32:35PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Wednesday, April 27, 2011 8:19 AM
> > To: KY Srinivasan
> > Cc: Christoph Hellwig; Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> > 
> > On Wed, Apr 27, 2011 at 11:47:03AM +0000, KY Srinivasan wrote:
> > > On the host side, Windows emulates the  standard PC hardware
> > > to permit hosting of fully virtualized operating systems.
> > > To enhance disk I/O performance, we support a virtual block driver.
> > > This block driver currently handles disks that have been setup as IDE
> > > disks for the guest - as specified in the guest configuration.
> > >
> > > On the SCSI side, we emulate a SCSI HBA. Devices configured
> > > under the SCSI controller for the guest are handled via this
> > > emulated HBA (SCSI front-end). So, SCSI disks configured for
> > > the guest are handled through native SCSI upper-level drivers.
> > > If this SCSI front-end driver is not loaded, currently, the guest
> > > cannot see devices that have been configured as SCSI devices.
> > > So, while the virtual block driver described earlier could potentially
> > > handle all block devices, the implementation choices made on the host
> > > will not permit it. Also, the only SCSI device that can be currently configured
> > > for the guest is a disk device.
> > >
> > > Both the block device driver (hv_blkvsc) and the SCSI front-end
> > > driver (hv_storvsc) communicate with the host via unique channels
> > > that are implemented as bi-directional ring buffers. Each (storage)
> > > channel carries with it enough state to uniquely identify the device on
> > > the host side. Microsoft has chosen to use SCSI verbs for this storage channel
> > > communication.
> > 
> > This doesn't really explain much at all.  The only important piece
> > of information I can read from this statement is that both blkvsc
> > and storvsc only support disks, but not any other kind of device,
> > and that chosing either one is an arbitrary seletin when setting up
> > a VM configuration.
> > 
> > But this still isn't an excuse to implement a block layer driver for
> > a SCSI protocol, and it doesn't not explain in what way the two
> > protocols actually differ.  You really should implement blksvs as a SCSI
> > LLDD, too - and from the looks of it it doesn't even have to be a
> > separate one, but just adding the ids to storvsc would do the work.
> 
> On the host-side, as part of configuring a guest  you can specify block devices
> as being under an IDE controller or under a
> SCSI controller. Those are the only options you have. Devices configured under
> the IDE controller cannot be seen in the guest under the emulated SCSI front-end which is
> the scsi driver (storvsc_drv).

Are you sure the libata core can't see this ide controller and connect
to it?  That way you would use the scsi system if you do that and you
would need a much smaller ide driver, perhaps being able to merge it
with your scsi driver.

We really don't want to write new IDE drivers anymore that don't use
libata.

thanks,

greg k-h

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

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, April 29, 2011 12:40 PM
> To: KY Srinivasan
> Cc: Christoph Hellwig; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Fri, Apr 29, 2011 at 04:32:35PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Christoph Hellwig [mailto:hch@infradead.org]
> > > Sent: Wednesday, April 27, 2011 8:19 AM
> > > To: KY Srinivasan
> > > Cc: Christoph Hellwig; Greg KH; gregkh@suse.de; linux-
> kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> > >
> > > On Wed, Apr 27, 2011 at 11:47:03AM +0000, KY Srinivasan wrote:
> > > > On the host side, Windows emulates the  standard PC hardware
> > > > to permit hosting of fully virtualized operating systems.
> > > > To enhance disk I/O performance, we support a virtual block driver.
> > > > This block driver currently handles disks that have been setup as IDE
> > > > disks for the guest - as specified in the guest configuration.
> > > >
> > > > On the SCSI side, we emulate a SCSI HBA. Devices configured
> > > > under the SCSI controller for the guest are handled via this
> > > > emulated HBA (SCSI front-end). So, SCSI disks configured for
> > > > the guest are handled through native SCSI upper-level drivers.
> > > > If this SCSI front-end driver is not loaded, currently, the guest
> > > > cannot see devices that have been configured as SCSI devices.
> > > > So, while the virtual block driver described earlier could potentially
> > > > handle all block devices, the implementation choices made on the host
> > > > will not permit it. Also, the only SCSI device that can be currently configured
> > > > for the guest is a disk device.
> > > >
> > > > Both the block device driver (hv_blkvsc) and the SCSI front-end
> > > > driver (hv_storvsc) communicate with the host via unique channels
> > > > that are implemented as bi-directional ring buffers. Each (storage)
> > > > channel carries with it enough state to uniquely identify the device on
> > > > the host side. Microsoft has chosen to use SCSI verbs for this storage
> channel
> > > > communication.
> > >
> > > This doesn't really explain much at all.  The only important piece
> > > of information I can read from this statement is that both blkvsc
> > > and storvsc only support disks, but not any other kind of device,
> > > and that chosing either one is an arbitrary seletin when setting up
> > > a VM configuration.
> > >
> > > But this still isn't an excuse to implement a block layer driver for
> > > a SCSI protocol, and it doesn't not explain in what way the two
> > > protocols actually differ.  You really should implement blksvs as a SCSI
> > > LLDD, too - and from the looks of it it doesn't even have to be a
> > > separate one, but just adding the ids to storvsc would do the work.
> >
> > On the host-side, as part of configuring a guest  you can specify block devices
> > as being under an IDE controller or under a
> > SCSI controller. Those are the only options you have. Devices configured under
> > the IDE controller cannot be seen in the guest under the emulated SCSI front-
> end which is
> > the scsi driver (storvsc_drv).
> 
> Are you sure the libata core can't see this ide controller and connect
> to it?  That way you would use the scsi system if you do that and you
> would need a much smaller ide driver, perhaps being able to merge it
> with your scsi driver.

If we don't load the blkvsc driver, the emulated IDE controller exposed to
the guest can and will be seen by the libata core. In this case though, your
disk I/O will be taking the emulated path with the usual performance hit.

When you load the blkvsc driver, the device access does not go through the emulated
IDE controller. Blkvsc is truly a generic block driver that registers as a block driver in
the guest and talks to an appropriate device driver on the host, communicating over
the vmbus. In this respect, it is identical to block drivers we have for guests in other
virtualization platforms (Xen etc.). The only difference is that on the host side,
the only way you can assign a scsi disk to the guest is to configure this scsi disk
under the scsi controller. So, while blkvsc is a generic block driver, because of the
restrictions on the host side, it only ends up managing block devices that have IDE majors.   

> 
> We really don't want to write new IDE drivers anymore that don't use
> libata.

As I noted earlier, it is incorrect to view Hyper-V blkvsc driver as an IDE driver. There
is nothing IDE specific about it. It is very much like other block front-end drivers
(like in Xen) that get their device information from the host and register the block
device accordingly with the guest. It just happens that in the current version of the
Windows host, only devices that are configured as IDE devices in the host end up being
managed by this driver. To make this clear, in my recent cleanup of this driver (these patches
have been applied), all IDE major information has been properly consolidated.

Regards,

K. Y  

RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, April 29, 2011 11:11 AM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> PVT LTD)
> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> cleanup accordingly
> 
> On Fri, Apr 29, 2011 at 01:49:21PM +0000, KY Srinivasan wrote:
> 
> Just resend it with some more comments as I explained, and the rest, and
> I'll queue them up when I get caught up on my patch queue (I only have
> 381 to get through, the end is near!)

Thanks Greg; soon after I sent you this patch-set, I had also sent a single patch to
address a redundant assignment:

0001-Staging-hv-Do-not-re-set-the-bus-name.patch

Please drop this patch. I will deal with this issue as part of the remaining patch-set
I will be sending now.

Regards,

K. Y


 
> 
> thanks,
> 
> greg k-h

RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, April 29, 2011 11:11 AM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> PVT LTD)
> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> cleanup accordingly
> 
> 
> Just resend it with some more comments as I explained, and the rest, and
> I'll queue them up when I get caught up on my patch queue (I only have
> 381 to get through, the end is near!)

Done; I just resent the remaining patches with comments and corrections
you had recommended. With regards to asking for a review, should I wait until 
all these patches are applied?

Regards,

K. Y

Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[Greg KH]

On Fri, Apr 29, 2011 at 10:02:43PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Friday, April 29, 2011 11:11 AM
> > To: KY Srinivasan
> > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> > PVT LTD)
> > Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> > cleanup accordingly
> > 
> > 
> > Just resend it with some more comments as I explained, and the rest, and
> > I'll queue them up when I get caught up on my patch queue (I only have
> > 381 to get through, the end is near!)
> 
> Done; I just resent the remaining patches with comments and corrections
> you had recommended. With regards to asking for a review, should I wait until 
> all these patches are applied?

Yes, unless you know of any other changes you want to make to the vmbus
core.

thanks,

greg k-h

RE: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Friday, April 29, 2011 7:14 PM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree Consulting
> PVT LTD)
> Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> cleanup accordingly
> 
> On Fri, Apr 29, 2011 at 10:02:43PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Friday, April 29, 2011 11:11 AM
> > > To: KY Srinivasan
> > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org; Haiyang Zhang; Abhishek Kane (Mindtree
> Consulting
> > > PVT LTD)
> > > Subject: Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded;
> > > cleanup accordingly
> > >
> > >
> > > Just resend it with some more comments as I explained, and the rest, and
> > > I'll queue them up when I get caught up on my patch queue (I only have
> > > 381 to get through, the end is near!)
> >
> > Done; I just resent the remaining patches with comments and corrections
> > you had recommended. With regards to asking for a review, should I wait until
> > all these patches are applied?
> 
> Yes, unless you know of any other changes you want to make to the vmbus
> core.

Thanks. Currently, I am not planning to make any changes to the vmbus core.

Regards,

K. Y

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

[Christoph Hellwig]

On Fri, Apr 29, 2011 at 04:32:35PM +0000, KY Srinivasan wrote:
> On the host-side, as part of configuring a guest  you can specify block devices
> as being under an IDE controller or under a
> SCSI controller. Those are the only options you have. Devices configured under
> the IDE controller cannot be seen in the guest under the emulated SCSI front-end which is
> the scsi driver (storvsc_drv). So, when you do a bus scan in the emulated scsi front-end,
> the devices enumerated will not include block devices configured under the IDE 
> controller. So, it is not clear to me how I can do what you are proposing given the 
> restrictions imposed by the host.

Just because a device is not reported by REPORT_LUNS doesn't mean you
can't talk to it using a SCSI LLDD.  We have SCSI transports with all
kinds of strange ways to discover devices.  Using scsi_add_device you
can add LUNs found by your own discovery methods, and use all the
existing scsi command handling.

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

[Christoph Hellwig]

On Fri, Apr 29, 2011 at 09:40:25AM -0700, Greg KH wrote:
> Are you sure the libata core can't see this ide controller and connect
> to it?  That way you would use the scsi system if you do that and you
> would need a much smaller ide driver, perhaps being able to merge it
> with your scsi driver.
> 
> We really don't want to write new IDE drivers anymore that don't use
> libata.

The blkvsc driver isn't an IDE driver, although it currently claims
the old IDE drivers major numbers, which is a no-no and can't work
in most usual setups.  I'm pretty sure I already complained about
this in a previous review round.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Sunday, May 01, 2011 11:41 AM
> To: Greg KH
> Cc: KY Srinivasan; Christoph Hellwig; gregkh@suse.de; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Fri, Apr 29, 2011 at 09:40:25AM -0700, Greg KH wrote:
> > Are you sure the libata core can't see this ide controller and connect
> > to it?  That way you would use the scsi system if you do that and you
> > would need a much smaller ide driver, perhaps being able to merge it
> > with your scsi driver.
> >
> > We really don't want to write new IDE drivers anymore that don't use
> > libata.
> 
> The blkvsc driver isn't an IDE driver, although it currently claims
> the old IDE drivers major numbers, which is a no-no and can't work
> in most usual setups.

What is the issue here? This is no different than what is done in other
Virtualization platforms. For instance, the Xen blkfront driver is no
different - if you specify the block device to be presented to the guest
as an ide device, it will register for the appropriate ide major number.

Regards,

K. Y 

> I'm pretty sure I already complained about
> this in a previous review round.

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

[Greg KH]

On Sun, May 01, 2011 at 11:39:21AM -0400, Christoph Hellwig wrote:
> On Fri, Apr 29, 2011 at 04:32:35PM +0000, KY Srinivasan wrote:
> > On the host-side, as part of configuring a guest  you can specify block devices
> > as being under an IDE controller or under a
> > SCSI controller. Those are the only options you have. Devices configured under
> > the IDE controller cannot be seen in the guest under the emulated SCSI front-end which is
> > the scsi driver (storvsc_drv). So, when you do a bus scan in the emulated scsi front-end,
> > the devices enumerated will not include block devices configured under the IDE 
> > controller. So, it is not clear to me how I can do what you are proposing given the 
> > restrictions imposed by the host.
> 
> Just because a device is not reported by REPORT_LUNS doesn't mean you
> can't talk to it using a SCSI LLDD.  We have SCSI transports with all
> kinds of strange ways to discover devices.  Using scsi_add_device you
> can add LUNs found by your own discovery methods, and use all the
> existing scsi command handling.

Yeah, it seems to me that no matter how the user specifies the disk
"type" for the guest configuration, we should use the same Linux driver,
with the same naming scheme for both ways.

As Christoph points out, it's just a matter of hooking the device up to
the scsi subsystem.  We do that today for ide, usb, scsi, and loads of
other types of devices all with the common goal of making it easier for
userspace to handle the devices in a standard manner.

thanks,

greg k-h

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

[Christoph Hellwig]

On Sun, May 01, 2011 at 03:46:23PM +0000, KY Srinivasan wrote:
> What is the issue here? This is no different than what is done in other
> Virtualization platforms. For instance, the Xen blkfront driver is no
> different - if you specify the block device to be presented to the guest
> as an ide device, it will register for the appropriate ide major number.

No, it won't - at least not in mainline just because it's so buggy.
If distros keep that crap around I can only recommed you to not use
them.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Sunday, May 01, 2011 12:07 PM
> To: KY Srinivasan
> Cc: Christoph Hellwig; Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Sun, May 01, 2011 at 03:46:23PM +0000, KY Srinivasan wrote:
> > What is the issue here? This is no different than what is done in other
> > Virtualization platforms. For instance, the Xen blkfront driver is no
> > different - if you specify the block device to be presented to the guest
> > as an ide device, it will register for the appropriate ide major number.
> 
> No, it won't - at least not in mainline just because it's so buggy.
> If distros keep that crap around I can only recommed you to not use
> them.

Christoph,

Could you elaborate on the problems/issues when the block driver registers for the 
IDE majors. On the Qemu side, we have a mechanism to disable the emulation when 
PV drivers load. I don't think there is an equivalent mechanism on the Windows side.
So, as far as I know, registering for the IDE majors is the only way to also prevent native
drivers in Linux from taking control of the emulated device. 

Regards,

K. Y 

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

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Sunday, May 01, 2011 11:48 AM
> To: KY Srinivasan
> Cc: Christoph Hellwig; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Sun, May 01, 2011 at 11:39:21AM -0400, Christoph Hellwig wrote:
> > On Fri, Apr 29, 2011 at 04:32:35PM +0000, KY Srinivasan wrote:
> > > On the host-side, as part of configuring a guest  you can specify block devices
> > > as being under an IDE controller or under a
> > > SCSI controller. Those are the only options you have. Devices configured
> under
> > > the IDE controller cannot be seen in the guest under the emulated SCSI front-
> end which is
> > > the scsi driver (storvsc_drv). So, when you do a bus scan in the emulated scsi
> front-end,
> > > the devices enumerated will not include block devices configured under the
> IDE
> > > controller. So, it is not clear to me how I can do what you are proposing given
> the
> > > restrictions imposed by the host.
> >
> > Just because a device is not reported by REPORT_LUNS doesn't mean you
> > can't talk to it using a SCSI LLDD.  We have SCSI transports with all
> > kinds of strange ways to discover devices.  Using scsi_add_device you
> > can add LUNs found by your own discovery methods, and use all the
> > existing scsi command handling.
> 
> Yeah, it seems to me that no matter how the user specifies the disk
> "type" for the guest configuration, we should use the same Linux driver,
> with the same naming scheme for both ways.
> 
> As Christoph points out, it's just a matter of hooking the device up to
> the scsi subsystem.  We do that today for ide, usb, scsi, and loads of
> other types of devices all with the common goal of making it easier for
> userspace to handle the devices in a standard manner.

This is not what is being done in Xen and KVM - they both have a PV front-end
block drivers that is not managed by the scsi stack. The Hyper-V block driver is
equivalent to what we have in Xen and KVM in this respect.

Regards,

K. Y 

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

[Christoph Hellwig]

On Sun, May 01, 2011 at 06:56:58PM +0000, KY Srinivasan wrote:
> > Yeah, it seems to me that no matter how the user specifies the disk
> > "type" for the guest configuration, we should use the same Linux driver,
> > with the same naming scheme for both ways.
> > 
> > As Christoph points out, it's just a matter of hooking the device up to
> > the scsi subsystem.  We do that today for ide, usb, scsi, and loads of
> > other types of devices all with the common goal of making it easier for
> > userspace to handle the devices in a standard manner.
> 
> This is not what is being done in Xen and KVM - they both have a PV front-end
> block drivers that is not managed by the scsi stack. The Hyper-V block driver is
> equivalent to what we have in Xen and KVM in this respect.

Xen also has a PV SCSI driver, although that isn't used very much.
For virtio we think it was a mistake to not speak SCSI these days,
and ponder introducing a virtio-scsi to replace virtio-blk.

But that's not the point here at all.  The point is that blockvsc
speaks a SCSI protocol over the wire, so it should be implemented
as a SCSI LLDD unless you have a good reason not to do it.  This
is especially important to get advanced features like block level
cache flush and FUA support, device topology, discard support, for
free.  Cache flush and FUA are good example for something that blkvsc
currently gets wrong, btw.

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

[Christoph Hellwig]

On Sun, May 01, 2011 at 06:08:37PM +0000, KY Srinivasan wrote:
> Could you elaborate on the problems/issues when the block driver registers for the 
> IDE majors. On the Qemu side, we have a mechanism to disable the emulation when 
> PV drivers load. I don't think there is an equivalent mechanism on the Windows side.
> So, as far as I know, registering for the IDE majors is the only way to also prevent native
> drivers in Linux from taking control of the emulated device. 

What qemu are you talking about for the qemu side?  Upstream qemu
doesn't have any way to provide the same image as multiple devices,
nevermind dynamically unplugging bits in that case.  Nor does it support
the hyperv devices.

When you steal majors you rely on:

 a) loading earlier than the driver you steal them from
 b) the driver not simple using other numbers
 c) if it doesn't preventing it from working at all, also for
    devices you don't "replace" with your PV devices.
 d) that the guest actually uses the minors your claim, e.g. any
    current linux distribution uses libata anyway, so you old IDE
    major claim wouldn't do anything.  Nor would claiming sd majors
    as the low-level libata driver would still drive the hardware
    even if SD doesn't bind to it.

You really must never present the same device as two emulated devices
instead of doing such hacks.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Sunday, May 01, 2011 4:53 PM
> To: KY Srinivasan
> Cc: Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Sun, May 01, 2011 at 06:08:37PM +0000, KY Srinivasan wrote:
> > Could you elaborate on the problems/issues when the block driver registers for
> the
> > IDE majors. On the Qemu side, we have a mechanism to disable the emulation
> when
> > PV drivers load. I don't think there is an equivalent mechanism on the Windows
> side.
> > So, as far as I know, registering for the IDE majors is the only way to also
> prevent native
> > drivers in Linux from taking control of the emulated device.
> 
> What qemu are you talking about for the qemu side?  Upstream qemu
> doesn't have any way to provide the same image as multiple devices,
> nevermind dynamically unplugging bits in that case.  Nor does it support
> the hyperv devices.

I am talking about the qemu that was (is) shipping with Xen.  In Hyper-V,
the block devices configured as IDE devices for the guest will be taken over
by the native drivers if the PV drivers don't load first and  take over the IDE majors.
If you want to have the root device be managed by the PV drivers, this appears to be
the only way to ensure that native IDE drivers don't take over the root device. Granted,
this depends on ensuring the PV drivers load first, but I don't know if there is another way
to achieve this.

> 
> When you steal majors you rely on:
> 
>  a) loading earlier than the driver you steal them from
>  b) the driver not simple using other numbers
>  c) if it doesn't preventing it from working at all, also for
>     devices you don't "replace" with your PV devices.

These are exactly the issues that had to be solved to have the PV 
drivers manage the root device.

>  d) that the guest actually uses the minors your claim, e.g. any
>     current linux distribution uses libata anyway, so you old IDE
>     major claim wouldn't do anything.  Nor would claiming sd majors
>     as the low-level libata driver would still drive the hardware
>     even if SD doesn't bind to it.

By setting up appropriate modprobe rules, this can be addressed.

> 
> You really must never present the same device as two emulated devices
> instead of doing such hacks.

Agreed; I am not sure what the right solution for Hyper-V is other than 
(a) preventing the native IDE drivers from loading and (b) having
the right modprobe rules to ensure libata would not present these
same devices to the guest as scsi devices.

Regards,

K. Y

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

[Christoph Hellwig]

On Mon, May 02, 2011 at 07:48:38PM +0000, KY Srinivasan wrote:
> By setting up appropriate modprobe rules, this can be addressed.

That assumes libata is a module, which it is not for many popular
distributions.

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Monday, May 02, 2011 4:00 PM
> To: KY Srinivasan
> Cc: Christoph Hellwig; Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Mon, May 02, 2011 at 07:48:38PM +0000, KY Srinivasan wrote:
> > By setting up appropriate modprobe rules, this can be addressed.
> 
> That assumes libata is a module, which it is not for many popular
> distributions.
> 
As long as you can prevent ata_piix from loading, it should be fine.

Regards,

K. Y

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

[Christoph Hellwig]

On Mon, May 02, 2011 at 09:16:36PM +0000, KY Srinivasan wrote:
> > That assumes libata is a module, which it is not for many popular
> > distributions.
> > 
> As long as you can prevent ata_piix from loading, it should be fine.

Again, this might very well be built in, e.g. take a look at:

http://pkgs.fedoraproject.org/gitweb/?p=kernel.git;a=blob;f=config-generic;h=779415bcc036b922ba92de9c4b15b9da64e9707c;hb=HEAD

http://gitorious.org/opensuse/kernel-source/blobs/master/config/x86_64/default

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

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Monday, May 02, 2011 5:35 PM
> To: KY Srinivasan
> Cc: Greg KH; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
> 
> On Mon, May 02, 2011 at 09:16:36PM +0000, KY Srinivasan wrote:
> > > That assumes libata is a module, which it is not for many popular
> > > distributions.
> > >
> > As long as you can prevent ata_piix from loading, it should be fine.
> 
> Again, this might very well be built in, e.g. take a look at:
> 
> http://pkgs.fedoraproject.org/gitweb/?p=kernel.git;a=blob;f=config-
> generic;h=779415bcc036b922ba92de9c4b15b9da64e9707c;hb=HEAD
> 
> http://gitorious.org/opensuse/kernel-
> source/blobs/master/config/x86_64/default

Good point! For what it is worth, last night I hacked up code
to present the block devices currently managed by the blkvsc
driver as scsi devices. I have still retained the blkvsc driver to
handshake with the host and sert up the channel etc. Rather than
presenting this device as an IDE device to the guest, as you had 
suggested, I am adding this device as a scsi device under the HBA
implemented by the storvsc driver. I have assigned a special channel
number to distinguish these IDE disks, so that on the I/O paths we can 
communicate over the appropriate channels. Given that the host is 
completely oblivious to this arrangement on the guest, I suspect
we don't need to worry about future versions of Windows breaking this.
From, very minimal testing I have done, things appear to work well.
However, the motherboard emulation in Hyper-V requires the boot
device to be an IDE device and other than taking over the IDE majors, I don't
know of a way to prevent the native drivers from taking over the boot device.
On SLES, I had implemented modprobe rules to deal with the issue you had 
mentioned; it is not clear what the general solution might be for this problem if any,
other than changes to the host.

Regards,

K. Y