Re: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Sat, Jul 28, 2012 at 12:55:35PM -0700, Greg KH wrote: > On Fri, Jul 27, 2012 at 01:29:27PM -0700, Dmitry Torokhov wrote: > > On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: > > > On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: > > > > > The kernel style is to use lower_case for everything. > > > > > So this would become: > > > > > > > > > > vmci_device_get() > > > > > > > > > > This is obviously a very general comment and applies everywhere. > > > > > > > > I wish I could lower case these symbols but VMCI has already existed > > > > outside the mainline Linux tree for some time now and changing these > > > > exported symbols would mean that other drivers that depend on VMCI > > > > (vSock, vmhgfs) would need to change as well. One thought that did > > > > come to mind was exporting both VMCI_Device_Get and vmci_device_get > > > > but that would likely just confuse people. So in short I have made > > > > function names lower case where possible, but exported symbols could > > > > not be changed. > > > > > > Not true at all. You want those drivers to be merged as well, right? > > > So they will need to have their functions changed, and their code as > > > well. > > > > > > Just wait until we get to the "change your functionality around" > > > requests, those will require those drivers to change. Right now we are > > > at the "silly and obvious things you did wrong" stage of the review > > > process :) > > > > > > So please fix these, and also, post these drivers as well, so we can see > > > how they interact with the core code. > > > > > > Actually, if you are going to need lots of refactoring for these > > > drivers, and the core, I would recommend putting this all in the staging > > > tree, to allow that to happen over time. That would ensure that your > > > users keep having working systems, and let you modify the interfaces > > > better and easier, than having to keep it all out-of-tree. > > > > > > What do you think? > > > > Actually I think that we'd prefer to keep this in a patch-based form, at > > least for now, because majority of our users get these drivers with > > VMware Tools and will continue doing so until ditsributions start > > enabling VMCI in their kernels. Which they probably won't until VMCI > > moves form staging. We'd also have to constantly adjust drivers that we > > are not working on getting upstream at this time to work with the > > rapidly changing version of VMCI in staging, which will just add work > > for us. > > That wouldn't be an issue if you just include all of the drivers in the > tree at the same time, right? Maybe it wouldn't, however at this time we have not scheduled any resources for upstreaming vmhgfs driver. We however do seek feedback on vmci driver (and later vsock) for which we did schedule resources. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Fri, Jul 27, 2012 at 01:29:27PM -0700, Dmitry Torokhov wrote: > On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: > > On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: > > > > The kernel style is to use lower_case for everything. > > > > So this would become: > > > > > > > > vmci_device_get() > > > > > > > > This is obviously a very general comment and applies everywhere. > > > > > > I wish I could lower case these symbols but VMCI has already existed > > > outside the mainline Linux tree for some time now and changing these > > > exported symbols would mean that other drivers that depend on VMCI > > > (vSock, vmhgfs) would need to change as well. One thought that did > > > come to mind was exporting both VMCI_Device_Get and vmci_device_get > > > but that would likely just confuse people. So in short I have made > > > function names lower case where possible, but exported symbols could > > > not be changed. > > > > Not true at all. You want those drivers to be merged as well, right? > > So they will need to have their functions changed, and their code as > > well. > > > > Just wait until we get to the "change your functionality around" > > requests, those will require those drivers to change. Right now we are > > at the "silly and obvious things you did wrong" stage of the review > > process :) > > > > So please fix these, and also, post these drivers as well, so we can see > > how they interact with the core code. > > > > Actually, if you are going to need lots of refactoring for these > > drivers, and the core, I would recommend putting this all in the staging > > tree, to allow that to happen over time. That would ensure that your > > users keep having working systems, and let you modify the interfaces > > better and easier, than having to keep it all out-of-tree. > > > > What do you think? > > Actually I think that we'd prefer to keep this in a patch-based form, at > least for now, because majority of our users get these drivers with > VMware Tools and will continue doing so until ditsributions start > enabling VMCI in their kernels. Which they probably won't until VMCI > moves form staging. We'd also have to constantly adjust drivers that we > are not working on getting upstream at this time to work with the > rapidly changing version of VMCI in staging, which will just add work > for us. That wouldn't be an issue if you just include all of the drivers in the tree at the same time, right? Just like what the hyper-v developers did. greg -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Fri, Jul 27, 2012 at 01:29:27PM -0700, Dmitry Torokhov wrote: On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: The kernel style is to use lower_case for everything. So this would become: vmci_device_get() This is obviously a very general comment and applies everywhere. I wish I could lower case these symbols but VMCI has already existed outside the mainline Linux tree for some time now and changing these exported symbols would mean that other drivers that depend on VMCI (vSock, vmhgfs) would need to change as well. One thought that did come to mind was exporting both VMCI_Device_Get and vmci_device_get but that would likely just confuse people. So in short I have made function names lower case where possible, but exported symbols could not be changed. Not true at all. You want those drivers to be merged as well, right? So they will need to have their functions changed, and their code as well. Just wait until we get to the change your functionality around requests, those will require those drivers to change. Right now we are at the silly and obvious things you did wrong stage of the review process :) So please fix these, and also, post these drivers as well, so we can see how they interact with the core code. Actually, if you are going to need lots of refactoring for these drivers, and the core, I would recommend putting this all in the staging tree, to allow that to happen over time. That would ensure that your users keep having working systems, and let you modify the interfaces better and easier, than having to keep it all out-of-tree. What do you think? Actually I think that we'd prefer to keep this in a patch-based form, at least for now, because majority of our users get these drivers with VMware Tools and will continue doing so until ditsributions start enabling VMCI in their kernels. Which they probably won't until VMCI moves form staging. We'd also have to constantly adjust drivers that we are not working on getting upstream at this time to work with the rapidly changing version of VMCI in staging, which will just add work for us. That wouldn't be an issue if you just include all of the drivers in the tree at the same time, right? Just like what the hyper-v developers did. greg -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Sat, Jul 28, 2012 at 12:55:35PM -0700, Greg KH wrote: On Fri, Jul 27, 2012 at 01:29:27PM -0700, Dmitry Torokhov wrote: On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: The kernel style is to use lower_case for everything. So this would become: vmci_device_get() This is obviously a very general comment and applies everywhere. I wish I could lower case these symbols but VMCI has already existed outside the mainline Linux tree for some time now and changing these exported symbols would mean that other drivers that depend on VMCI (vSock, vmhgfs) would need to change as well. One thought that did come to mind was exporting both VMCI_Device_Get and vmci_device_get but that would likely just confuse people. So in short I have made function names lower case where possible, but exported symbols could not be changed. Not true at all. You want those drivers to be merged as well, right? So they will need to have their functions changed, and their code as well. Just wait until we get to the change your functionality around requests, those will require those drivers to change. Right now we are at the silly and obvious things you did wrong stage of the review process :) So please fix these, and also, post these drivers as well, so we can see how they interact with the core code. Actually, if you are going to need lots of refactoring for these drivers, and the core, I would recommend putting this all in the staging tree, to allow that to happen over time. That would ensure that your users keep having working systems, and let you modify the interfaces better and easier, than having to keep it all out-of-tree. What do you think? Actually I think that we'd prefer to keep this in a patch-based form, at least for now, because majority of our users get these drivers with VMware Tools and will continue doing so until ditsributions start enabling VMCI in their kernels. Which they probably won't until VMCI moves form staging. We'd also have to constantly adjust drivers that we are not working on getting upstream at this time to work with the rapidly changing version of VMCI in staging, which will just add work for us. That wouldn't be an issue if you just include all of the drivers in the tree at the same time, right? Maybe it wouldn't, however at this time we have not scheduled any resources for upstreaming vmhgfs driver. We however do seek feedback on vmci driver (and later vsock) for which we did schedule resources. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: > On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: > > > The kernel style is to use lower_case for everything. > > > So this would become: > > > > > > vmci_device_get() > > > > > > This is obviously a very general comment and applies everywhere. > > > > I wish I could lower case these symbols but VMCI has already existed > > outside the mainline Linux tree for some time now and changing these > > exported symbols would mean that other drivers that depend on VMCI > > (vSock, vmhgfs) would need to change as well. One thought that did > > come to mind was exporting both VMCI_Device_Get and vmci_device_get > > but that would likely just confuse people. So in short I have made > > function names lower case where possible, but exported symbols could > > not be changed. > > Not true at all. You want those drivers to be merged as well, right? > So they will need to have their functions changed, and their code as > well. > > Just wait until we get to the "change your functionality around" > requests, those will require those drivers to change. Right now we are > at the "silly and obvious things you did wrong" stage of the review > process :) > > So please fix these, and also, post these drivers as well, so we can see > how they interact with the core code. > > Actually, if you are going to need lots of refactoring for these > drivers, and the core, I would recommend putting this all in the staging > tree, to allow that to happen over time. That would ensure that your > users keep having working systems, and let you modify the interfaces > better and easier, than having to keep it all out-of-tree. > > What do you think? Actually I think that we'd prefer to keep this in a patch-based form, at least for now, because majority of our users get these drivers with VMware Tools and will continue doing so until ditsributions start enabling VMCI in their kernels. Which they probably won't until VMCI moves form staging. We'd also have to constantly adjust drivers that we are not working on getting upstream at this time to work with the rapidly changing version of VMCI in staging, which will just add work for us. So we'd like to get more feedback and have a chance to address issues and then decide whether staying in staging makes sense or not. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
Hi Alan,
On Fri, Jul 27, 2012 at 10:53:57AM +0100, Alan Cox wrote:
> > +enum {
> > + VMCI_SUCCESS_QUEUEPAIR_ATTACH = 5,
> > + VMCI_SUCCESS_QUEUEPAIR_CREATE = 4,
> > + VMCI_SUCCESS_LAST_DETACH= 3,
> > + VMCI_SUCCESS_ACCESS_GRANTED = 2,
> > + VMCI_SUCCESS_ENTRY_DEAD = 1,
>
> We've got a nice collection of Linux error codes than you, and it would
> make the driver enormously more readable on the Linux side if as low
> level as possible it started using Linux error codes.
If VMCI was only used on Linux we'd definitely do that; however VMCI
core is shared among several operating systems (much like ACPI is) and
we'd like to limit divergencies between them while conforming to the
kernel coding style as much as possible.
We'll make sure that we will not leak VMCI-specific errors to the
standard kernel APIs.
Thanks,
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
Hi Alan,
On Fri, Jul 27, 2012 at 10:53:57AM +0100, Alan Cox wrote:
+enum {
+ VMCI_SUCCESS_QUEUEPAIR_ATTACH = 5,
+ VMCI_SUCCESS_QUEUEPAIR_CREATE = 4,
+ VMCI_SUCCESS_LAST_DETACH= 3,
+ VMCI_SUCCESS_ACCESS_GRANTED = 2,
+ VMCI_SUCCESS_ENTRY_DEAD = 1,
We've got a nice collection of Linux error codes than you, and it would
make the driver enormously more readable on the Linux side if as low
level as possible it started using Linux error codes.
If VMCI was only used on Linux we'd definitely do that; however VMCI
core is shared among several operating systems (much like ACPI is) and
we'd like to limit divergencies between them while conforming to the
kernel coding style as much as possible.
We'll make sure that we will not leak VMCI-specific errors to the
standard kernel APIs.
Thanks,
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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: [Pv-drivers] [vmw_vmci 11/11] Apply the header code to make VMCI build
On Fri, Jul 27, 2012 at 11:16:39AM -0700, Greg KH wrote: On Fri, Jul 27, 2012 at 10:20:43AM -0700, Andrew Stiegmann wrote: The kernel style is to use lower_case for everything. So this would become: vmci_device_get() This is obviously a very general comment and applies everywhere. I wish I could lower case these symbols but VMCI has already existed outside the mainline Linux tree for some time now and changing these exported symbols would mean that other drivers that depend on VMCI (vSock, vmhgfs) would need to change as well. One thought that did come to mind was exporting both VMCI_Device_Get and vmci_device_get but that would likely just confuse people. So in short I have made function names lower case where possible, but exported symbols could not be changed. Not true at all. You want those drivers to be merged as well, right? So they will need to have their functions changed, and their code as well. Just wait until we get to the change your functionality around requests, those will require those drivers to change. Right now we are at the silly and obvious things you did wrong stage of the review process :) So please fix these, and also, post these drivers as well, so we can see how they interact with the core code. Actually, if you are going to need lots of refactoring for these drivers, and the core, I would recommend putting this all in the staging tree, to allow that to happen over time. That would ensure that your users keep having working systems, and let you modify the interfaces better and easier, than having to keep it all out-of-tree. What do you think? Actually I think that we'd prefer to keep this in a patch-based form, at least for now, because majority of our users get these drivers with VMware Tools and will continue doing so until ditsributions start enabling VMCI in their kernels. Which they probably won't until VMCI moves form staging. We'd also have to constantly adjust drivers that we are not working on getting upstream at this time to work with the rapidly changing version of VMCI in staging, which will just add work for us. So we'd like to get more feedback and have a chance to address issues and then decide whether staying in staging makes sense or not. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
