Re: [PATCH 18/25] Staging: hv: Get rid of struct hv_bus
On Tue, Apr 26, 2011 at 08:23:25PM +, KY Srinivasan wrote:
-Original Message-
From: Greg KH [mailto:gre...@suse.de]
Sent: Tuesday, April 26, 2011 3:41 PM
To: KY Srinivasan
Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
virtualizat...@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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Signed-off-by: Abhishek Kane v-abk...@microsoft.com
Signed-off-by: Hank Janssen hjans...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
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? 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
-Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Tuesday, April 26, 2011 7:29 PM To: KY Srinivasan Cc: gre...@suse.de; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 11/25] Staging: hv: Get rid of the drv field in struct hv_device
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Tuesday, April 26, 2011 6:57 PM
To: KY Srinivasan
Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
de...@linuxdriverproject.org; virtualizat...@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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Signed-off-by: Abhishek Kane v-abk...@microsoft.com
Signed-off-by: Hank Janssen hjans...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 12/25] Staging: hv: Cleanup error handling in vmbus_child_device_register()
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Tuesday, April 26, 2011 6:51 PM
To: KY Srinivasan
Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
de...@linuxdriverproject.org; virtualizat...@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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Signed-off-by: Abhishek Kane v-abk...@microsoft.com
Signed-off-by: Hank Janssen hjans...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
ICAC2011 Call For Participation (8th IEEE International Conference on Autonomic Computing)
** CALL FOR PARTICIPATION == The 8th IEEE International Conference on Autonomic Computing Karlsruhe, Germany June 14-18, 2011 http://icac2011.cis.fiu.edu Sponsored by ACM ** Don't miss Jeff Kephart's keynote talk at ICAC 2011 on Autonomic Computing: The First Decade and benefit from early registration! Online registration is open at http://icac2011.cis.fiu.edu/registration.shtm. Reduced fees are available for those registering by May 1, 2011. Two keynotes by Jeff Kephart and Christian Muller-Schloer will wrap-up a decade of Autonomic Computing and of the related area of Organic Computing and outline some directions and challenges for future research. Five attractive workshops are complementing the program, details are at http://icac2011.cis.fiu.edu and in the attached Call for Participation. Looking forward to meeting you at ICAC 2011 in Karlsruhe Hartmut Schmeck (General Chair of ICAC 2011) This year's program includes: - 20 outstanding research papers and 7 short papers - Poster, demo, and industry presentations - 5 workshops - 2 distinguished keynotes Please join us for the 8th ICAC in Karlsruhe, Germany, on June 14-18, 2011. The conference is held at the AkademieHotel Karlsruhe. More information can be found at: http://icac2011.cis.fiu.edu ** IMPORTANT DATES === Early registration deadline: May 1, 2011 Hotel special rate deadline: May 1, 2011 ** CORPORATE SPONSORS == Gold Level Partner: IBM Conference partner: Google Further supporters: Microsoft, 11, Karlsruhe Institute of Technology, University of Arizona, and TechnologieRegion Karlsruhe. ** PRELIMINARY PROGRAM === MONDAY, JUNE 13, 2011 4:00PM - 7:00PM Registration == TUESDAY, JUNE 14, 2011 - WORKSHOPS FeBID 2011 BADS 2011 ACE 2011 == WEDNESDAY, JUNE 15, 2011 - MAIN CONFERENCE 9:00AM Welcome addresses 9:15AM - 10:30AM Session 1: Keynote Talk 9:15AM Autonomic Computing: The First Decade Jeff Kephart (IBM Thomas Watson Research Center) 10:30AM Coffee break 11:00AM - 12:00PM Session 2: Multicore Systems 11:00AM Applying Autonomic Principles for Workload Management in Multi-Core Systems on Chip Johannes Zeppenfeld; Andreas Herkersdorf 11:30AM Smart Data Structures: A Reinforcement Learning Approach to Multicore Data Structures Jonathan Eastep; David Wingate; Anant Agarwal 12:00PM Lunch -- 1:30PM - 3:00PM Session 3: Power Management 1:30PM How Much Power Oversubscription is Safe and Allowed in Data Centers? Xing Fu; Xiaorui Wang; Charles Lefurgy 2:00PM Memory Power Management via Dynamic Voltage/Frequency Scaling Howard David; Chris Fallin; Eugene Gorbatov; Ulf R. Hanebutte; Onur Mutlu 2:30PM iPOEM: A GPS Tool for Integrated Management in Virtualized Data Centers Hui Zhang; Kenji Yoshihira; Ya-Yunn Su; Guofei Jiang; Ming Chen; Xiaorui Wang 3:00PM Coffee break 3:30PM - 5:00PM Session 4: Distributed Systems 3:30PM A Self-Organizing P2P System with Multi-Dimensional Structure Raffaele Giordanelli; Carlo Mastroianni; Michela Meo 4:00PM SILENCE: Distributed Adaptive Sampling for Sensor-based Autonomic Systems Eun Kyung Lee; Hariharasudhan Viswanathan; Dario Pompili 4:30PM Budget-Constrained Bulk Data Transfer via Internet and Shipping Networks Brian Cho; Indranil Gupta 5:00PM Reception == THURSDAY, JUNE 16, 2011 - MAIN CONFERENCE 9:15AM Sponsoring addresses 9:30AM - 10:30AM Session 5: Testing and Diagnostics 9:30AM Toward Data Center Self-Diagnosis Using a Mobile Robot Jon Lenchner; Canturk Isci; Jeffrey Kephart; Christopher Mansley; Jonathan Connell; Suzanne McIntosh 10:00AM Model-based Performance Testing Cornel Barna; Marin Litoiu; Hamoun Ghanbari 10:30AM Coffee break 11:00AM - 12:00PM Session 6: Malware Detection and Clean-up 11:00AM Inoculation against malware infection using kernel-level software sensors Raymond Canzanese; Spiros Mancoridis; Moshe Kam 11:30AM Automatically Clean Up Malware Impacts When Committing OS-level Virtual Machines Zhiyong Shan; Xin Wang;
Re: [RFC PATCH TRIVIAL] Reading the virtio code...
On Wed, Apr 27, 2011 at 02:59:27PM +0930, Rusty Russell wrote: On Sat, 23 Apr 2011 18:13:34 -0500, Rob Landley rland...@parallels.com wrote: From: Rob Landley rland...@parallels.com Going indirect for only two buffers isn't likely to be a performance win because the kmalloc/kfree overhead for the indirect block can't be cheaper than one extra linked list traversal. Unfortunately it's not completely clear. QEMU sets fairly small rings, and the virtio-net driver uses 2 descriptors minimum. The effect can be a real bottleneck for small packets. Now, virtio-net could often stuff the virtio_net_hdr in the space before the packet data (saving a descriptor) but I think that will need a feature bit since qemu (incorrectly) used to insist on a separate descriptor for that header. Properly tuning the threshold would probably be workload-specific. (One big downside of not going indirect is extra pressure on the table entries, and table size varies.) But I think that in the general case, 2 is a defensible minimum? I'd be tempted to say that once we fill the ring, we should drop the threshold. Michael? Thanks, Rusty. Yes, one idea is to use part of a ring (e.g. 1/4 of a ring) for direct entries, and the rest for indirect. So we end up with a threshold like max(1, vq-num_free - in - out - vq-num * N) (above is pseudo-code, must take care of unsigned vs signed etc) and I think I'd try with N = 3/4 or maybe N = 1/2 -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH] driver, virtio: Modify the err hanlding logic
On Wed, 20 Apr 2011 21:21:59 +0800, Liu Yuan namei.u...@gmail.com wrote:
From: Liu Yuan tailai...@taobao.com
In the function vp_request_msix_vectors(), when
pci_enable_msix() returns 0, there will be
redundant double checks for 'err'. This patch
fixes it to avoid the unnecessary check.
Signed-off-by: Liu Yuan tailai...@taobao.com
---
drivers/virtio/virtio_pci.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 4fb5b2b..2c05376 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -298,10 +298,11 @@ static int vp_request_msix_vectors(struct virtio_device
*vdev, int nvectors,
/* pci_enable_msix returns positive if we can't get this many. */
err = pci_enable_msix(vp_dev-pci_dev, vp_dev-msix_entries, nvectors);
- if (err 0)
- err = -ENOSPC;
- if (err)
+ if (err) {
+ if (err 0)
+ err = -ENOSPC;
goto error;
+ }
vp_dev-msix_vectors = nvectors;
vp_dev-msix_enabled = 1;
This patch is extremely marginal. It theoretically improves efficiency,
but it's in a case we don't care about. The code is quite clear.
My general policy for such marginal improvements is to only accept them
from the maintainer him/herself, so I won't be taking this patch.
Of course, if you produce a series of fixes to the driver with such a
patch as a cleaner, I'm lazy enough that I'd take them all at once :)
Thanks,
Rusty.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
On Wed, Apr 27, 2011 at 01:54:02AM +, 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH TRIVIAL] Reading the virtio code...
On Sat, 23 Apr 2011 18:13:34 -0500, Rob Landley rland...@parallels.com wrote: From: Rob Landley rland...@parallels.com Going indirect for only two buffers isn't likely to be a performance win because the kmalloc/kfree overhead for the indirect block can't be cheaper than one extra linked list traversal. Unfortunately it's not completely clear. QEMU sets fairly small rings, and the virtio-net driver uses 2 descriptors minimum. The effect can be a real bottleneck for small packets. Now, virtio-net could often stuff the virtio_net_hdr in the space before the packet data (saving a descriptor) but I think that will need a feature bit since qemu (incorrectly) used to insist on a separate descriptor for that header. Properly tuning the threshold would probably be workload-specific. (One big downside of not going indirect is extra pressure on the table entries, and table size varies.) But I think that in the general case, 2 is a defensible minimum? I'd be tempted to say that once we fill the ring, we should drop the threshold. Michael? Thanks, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
-Original Message- From: Christoph Hellwig [mailto:h...@infradead.org] Sent: Wednesday, April 27, 2011 2:46 AM To: KY Srinivasan Cc: Greg KH; gre...@suse.de; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; virtualizat...@lists.osdl.org Subject: Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code On Wed, Apr 27, 2011 at 01:54:02AM +, 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 k...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
On Wed, Apr 27, 2011 at 11:47:03AM +, 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 08/25] Staging: hv: vmbus_driver cannot be unloaded; cleanup accordingly
On Wed, Apr 27, 2011 at 02:31:18AM +, KY Srinivasan wrote: -Original Message- From: Greg KH [mailto:g...@kroah.com] Sent: Tuesday, April 26, 2011 6:46 PM To: KY Srinivasan Cc: gre...@suse.de; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/25] Staging: hv: Cleanup vmbus driver code
On Wed, Apr 27, 2011 at 01:54:02AM +, 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
