lkml.org 
[lkml]   [2011]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH ] Staging: hv: Hyper-V driver cleanup
Date


> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Thursday, February 24, 2011 6:46 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Hank Janssen
> Subject: Re: [PATCH ] Staging: hv: Hyper-V driver cleanup
>
> On Thu, Feb 24, 2011 at 03:20:58PM -0800, K. Y. Srinivasan wrote:
> > This patch cleans up (a lot of the) naming issues that
> > various reviewers have noted. It also gets rid of
> > some unnecessary layering in the code.
>
> Whenever you have a patch description that says "It also..." you know
> you need to break this up into smaller, logical pieces.

The name change was related to the layering issue. For instance I combined the
Vm_device and hv_device abstractions to build the hyperv_device abstraction.
Likewise, I combined the driver_context and the hv_driver abstractions to build the
the hyperv_driver abstraction. Would breaking this patch up into two patches,
one dealing with the device abstraction consolidation and the other dealing with
the consolidation of driver abstractions satisfy your concern. Even if I partition this
patch along these lines, it will still be a large set of patches; since these changes
are pervasive.

>
> As it is, I can not take this patch.
>
> Please break it up into logical patches, each doing only one thing, so
> we can properly review it.
>
> > At the lowest
> > level, we have one abstraction for representing
> > a hyperv device (struct hyperv_device) and one
> > abstraction for representing a hyperv driver
> > (struct hyperv_driver). This collapses an unnecessary
> > layering that existed in the code for historical reasons.
> > While the patch is large, it was generated by a very
> > mechanical process (global search and replace). The code
> > compiles cleanly and I have tested this code on a 2.6.38
> > kernel.
>
> There is no 2.6.38 kernel yet, so I find this very hard to believe :)

My mistake; I did not specify the full output of uname -a on the box
that I tested this code. This box is running the LINUX-NEXT kernel :
2.6.38-rc1-0.2-default.

Regards,

K. Y


>
> thanks,
>
> greg k-h



\
 
 \ /
  Last update: 2011-02-25 01:27    [from the cache]
©2003-2011 Jasper Spaans