2010-02-11 21:32:56

by Hank Janssen

[permalink] [raw]
Subject: [PATCH 1/2] Staging: hv: Add proper versioning to HV drivers


Provide proper versioning information for all HV drivers.


Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Hank Janssen <[email protected]>
Signed-off-by: Haiyang Zhang <[email protected]>


drivers/staging/hv/VersionInfo.h | 11 ++++++++---
drivers/staging/hv/Vmbus.c | 8 ++++----
drivers/staging/hv/blkvsc_drv.c | 2 ++
drivers/staging/hv/netvsc_drv.c | 5 +++--
drivers/staging/hv/storvsc_drv.c | 2 ++
drivers/staging/hv/vmbus_drv.c | 2 ++
6 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/hv/VersionInfo.h b/drivers/staging/hv/VersionInfo.h
index 9c3641d..e729b9e 100644
--- a/drivers/staging/hv/VersionInfo.h
+++ b/drivers/staging/hv/VersionInfo.h
@@ -24,8 +24,13 @@
#ifndef __HV_VERSION_INFO
#define __HV_VERSION_INFO

-static const char VersionDate[] = __DATE__;
-static const char VersionTime[] = __TIME__;
-static const char VersionDesc[] = "Version 2.0";
+static const char hv_build_date[] = __DATE__;
+static const char hv_build_time[] = __TIME__;
+
+/*
+ * We use the same version numbering for all Hyper-V modules.
+ */
+#define HV_DRV_VERSION "3.0.0"
+

#endif
diff --git a/drivers/staging/hv/Vmbus.c b/drivers/staging/hv/Vmbus.c
index 35a023e..20bc1cd 100644
--- a/drivers/staging/hv/Vmbus.c
+++ b/drivers/staging/hv/Vmbus.c
@@ -273,10 +273,10 @@ int VmbusInitialize(struct hv_driver *drv)

DPRINT_ENTER(VMBUS);

- DPRINT_INFO(VMBUS, "+++++++ Build Date=%s %s +++++++",
- VersionDate, VersionTime);
- DPRINT_INFO(VMBUS, "+++++++ Build Description=%s +++++++",
- VersionDesc);
+ DPRINT_INFO(VMBUS, "+++++++ Build Date = %s-%s +++++++",
+ hv_build_date, hv_build_time);
+ DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
+ HV_DRV_VERSION);
DPRINT_INFO(VMBUS, "+++++++ Vmbus supported version = %d +++++++",
VMBUS_REVISION_NUMBER);
DPRINT_INFO(VMBUS, "+++++++ Vmbus using SINT %d +++++++",
diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 62b2828..635692d 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -31,6 +31,7 @@
#include <scsi/scsi_dbg.h>
#include "osd.h"
#include "logging.h"
+#include "VersionInfo.h"
#include "vmbus.h"
#include "StorVscApi.h"

@@ -1507,6 +1508,7 @@ static void __exit blkvsc_exit(void)
}

MODULE_LICENSE("GPL");
+MODULE_VERSION(HV_DRV_VERSION);
module_param(blkvsc_ringbuffer_size, int, S_IRUGO);
module_init(blkvsc_init);
module_exit(blkvsc_exit);
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 3234a7c..0a78067 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -35,11 +35,10 @@
#include <net/pkt_sched.h>
#include "osd.h"
#include "logging.h"
+#include "VersionInfo.h"
#include "vmbus.h"
#include "NetVscApi.h"

-MODULE_LICENSE("GPL");
-
struct net_device_context {
/* point back to our device context */
struct device_context *device_ctx;
@@ -603,6 +602,8 @@ static void __exit netvsc_exit(void)
DPRINT_EXIT(NETVSC_DRV);
}

+MODULE_LICENSE("GPL");
+MODULE_VERSION(HV_DRV_VERSION);
module_param(netvsc_ringbuffer_size, int, S_IRUGO);

module_init(netvsc_init);
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 12f8f3f..a10fa0e 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -32,6 +32,7 @@
#include <scsi/scsi_dbg.h>
#include "osd.h"
#include "logging.h"
+#include "VersionInfo.h"
#include "vmbus.h"
#include "StorVscApi.h"

@@ -990,6 +991,7 @@ static void __exit storvsc_exit(void)
}

MODULE_LICENSE("GPL");
+MODULE_VERSION(HV_DRV_VERSION);
module_param(storvsc_ringbuffer_size, int, S_IRUGO);
module_init(storvsc_init);
module_exit(storvsc_exit);
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 894eecf..92d70cd 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -24,6 +24,7 @@
#include <linux/irq.h>
#include <linux/interrupt.h>
#include <linux/sysctl.h>
+#include "VersionInfo.h"
#include "osd.h"
#include "logging.h"
#include "vmbus.h"
@@ -974,6 +975,7 @@ static void __exit vmbus_exit(void)
}

MODULE_LICENSE("GPL");
+MODULE_VERSION(HV_DRV_VERSION);
module_param(vmbus_irq, int, S_IRUGO);
module_param(vmbus_loglevel, int, S_IRUGO);


2010-02-11 21:39:22

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/2] Staging: hv: Add proper versioning to HV drivers

On Thu, Feb 11, 2010 at 09:32:29PM +0000, Hank Janssen wrote:
>
> Provide proper versioning information for all HV drivers.
>
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Hank Janssen <[email protected]>
> Signed-off-by: Haiyang Zhang <[email protected]>
>
>
> drivers/staging/hv/VersionInfo.h | 11 ++++++++---
> drivers/staging/hv/Vmbus.c | 8 ++++----
> drivers/staging/hv/blkvsc_drv.c | 2 ++
> drivers/staging/hv/netvsc_drv.c | 5 +++--
> drivers/staging/hv/storvsc_drv.c | 2 ++
> drivers/staging/hv/vmbus_drv.c | 2 ++
> 6 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/hv/VersionInfo.h b/drivers/staging/hv/VersionInfo.h
> index 9c3641d..e729b9e 100644
> --- a/drivers/staging/hv/VersionInfo.h
> +++ b/drivers/staging/hv/VersionInfo.h
> @@ -24,8 +24,13 @@
> #ifndef __HV_VERSION_INFO
> #define __HV_VERSION_INFO
>
> -static const char VersionDate[] = __DATE__;
> -static const char VersionTime[] = __TIME__;
> -static const char VersionDesc[] = "Version 2.0";
> +static const char hv_build_date[] = __DATE__;
> +static const char hv_build_time[] = __TIME__;

This should not be needed at all, as it make absolutely no sense.

> +/*
> + * We use the same version numbering for all Hyper-V modules.
> + */
> +#define HV_DRV_VERSION "3.0.0"

Is this really needed? Now that the code is in the kernel tree,
shouldn't you be tracking this based on kernel release version (like 90%
of the drivers are tracked), or is there some requirement for a version
number?

And what decides when this number is changed? What does it "mean"?
What is it tracking?

thanks,

greg k-h

2010-02-11 22:15:42

by Hank Janssen

[permalink] [raw]
Subject: RE: [PATCH 1/2] Staging: hv: Add proper versioning to HV drivers

>>
>> -static const char VersionDate[] = __DATE__;
>> -static const char VersionTime[] = __TIME__;
>> -static const char VersionDesc[] = "Version 2.0";
>> +static const char hv_build_date[] = __DATE__;
>> +static const char hv_build_time[] = __TIME__;
>
>This should not be needed at all, as it make absolutely no sense.

You know, you are actually right. This is leftover from when the drivers
Where standalone (before we submitted them to mainline) when customers
Build the drivers on site themselves. It helped them to know when they
Build it.

>> +/*
>> + * We use the same version numbering for all Hyper-V modules.
>> + */
>> +#define HV_DRV_VERSION "3.0.0"
>
>Is this really needed? Now that the code is in the kernel tree,
>shouldn't you be tracking this based on kernel release version (like 90%
>of the drivers are tracked), or is there some requirement for a version
>number?
>
>And what decides when this number is changed? What does it "mean"?
>What is it tracking?

The definition of the versioning for us is as follows;

Field Description Resolution
1 Major Number Changes for these scenarios;
1. When a new version of Windows Hyper-V
is released.
2. A Major change has occurred in the
Linux IC's.
(For example the merge for the first time
into the kernel) Every time the Major Number
changes, the Minor Number and Revision number
are reset to 0.
2 Minor Number Changes when new functionality is added
to the Linux IC's that is not a bug fix.
If new functionality is added to the IC's
that is in effect also a bug fix (in the case
of SMP). The new functionality will override
the need for changing the Revision number.
Every time the Minor Number changes, the
Revision number will be reset to 0.
For example, The current version is on;
3.0.4
And SMP is added, which fixes a problem as
well. The new versioning will become;
3.1.0
3 Revision Every time a bug fix or code change is
done that is not a Minor or Major number
change, this field increases by 1.

We have had issues in the past trying to piece together what customers
Where running.

The importance of the versioning numbering like this will go away over
Time once we have the code cleaned up and have additional functionality
In a staged manner. For now we are still trying to catch up.

Would you like me to re-roll the patch to remove the DATE/TIME stuff
And add the explanation for versioning as comments to the code?

Thanks,

Hank.

2010-02-11 22:34:39

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/2] Staging: hv: Add proper versioning to HV drivers

On Thu, Feb 11, 2010 at 10:15:08PM +0000, Hank Janssen wrote:
> >>
> >> -static const char VersionDate[] = __DATE__;
> >> -static const char VersionTime[] = __TIME__;
> >> -static const char VersionDesc[] = "Version 2.0";
> >> +static const char hv_build_date[] = __DATE__;
> >> +static const char hv_build_time[] = __TIME__;
> >
> >This should not be needed at all, as it make absolutely no sense.
>
> You know, you are actually right. This is leftover from when the drivers
> Where standalone (before we submitted them to mainline) when customers
> Build the drivers on site themselves. It helped them to know when they
> Build it.

Great, then remove it :)

> >> +/*
> >> + * We use the same version numbering for all Hyper-V modules.
> >> + */
> >> +#define HV_DRV_VERSION "3.0.0"
> >
> >Is this really needed? Now that the code is in the kernel tree,
> >shouldn't you be tracking this based on kernel release version (like 90%
> >of the drivers are tracked), or is there some requirement for a version
> >number?
> >
> >And what decides when this number is changed? What does it "mean"?
> >What is it tracking?
>
> The definition of the versioning for us is as follows;
>
> Field Description Resolution
> 1 Major Number Changes for these scenarios;
> 1. When a new version of Windows Hyper-V
> is released.
> 2. A Major change has occurred in the
> Linux IC's.
> (For example the merge for the first time
> into the kernel) Every time the Major Number
> changes, the Minor Number and Revision number
> are reset to 0.
> 2 Minor Number Changes when new functionality is added
> to the Linux IC's that is not a bug fix.
> If new functionality is added to the IC's
> that is in effect also a bug fix (in the case
> of SMP). The new functionality will override
> the need for changing the Revision number.
> Every time the Minor Number changes, the
> Revision number will be reset to 0.
> For example, The current version is on;
> 3.0.4
> And SMP is added, which fixes a problem as
> well. The new versioning will become;
> 3.1.0

These two look good.

> 3 Revision Every time a bug fix or code change is
> done that is not a Minor or Major number
> change, this field increases by 1.

How are you really going to track this? Do you expect every single
patch to the drivers that changes the code to increment this number?
That's quite unrealistic.

> We have had issues in the past trying to piece together what customers
> Where running.

That is why you can now trigger off of the kernel version used, like
everyone else using Linux drivers does.

Yes, some companies/developers still like to track version numbers,
thinking when they backport things to older kernels for distro releases,
it will somehow properly convey the driver version. But it turns out,
with the -stable releases, and bugfixes being backported all over the
place, that it doesn't really work out.

In short I would strongly advise against doing this, unless you are
willing to attempt to keep this up to date, and in reality, just ignore
the number as it means nothing.

This is also especially relevant given that Linux drivers are tightly
tied to the kernel version they are running on, as APIs change
constantly over time. You can not just take one version of a driver on
one release, and drop it in another kernel version, without usually
changing something. That change then "invalidates" the version number
scheme :)

Does this make sense?

> The importance of the versioning numbering like this will go away over
> Time once we have the code cleaned up and have additional functionality
> In a staged manner. For now we are still trying to catch up.

Catching up doesn't mean fixing the version numbering scheme, you have
much bigger issues to fix, IMHO :)

> Would you like me to re-roll the patch to remove the DATE/TIME stuff
> And add the explanation for versioning as comments to the code?

Yes, please do, and possibly drop the "minor" number, otherwise I am
going to have fun incrementing it with every single code change I do to
the drivers. That would be what, version 264 or something by now? :)

thanks,

greg k-h

2010-02-11 23:12:11

by Hank Janssen

[permalink] [raw]
Subject: RE: [PATCH 1/2] Staging: hv: Add proper versioning to HV drivers



>In short I would strongly advise against doing this, unless you are
>willing to attempt to keep this up to date, and in reality, just ignore
>the number as it means nothing.
>
>This is also especially relevant given that Linux drivers are tightly
>tied to the kernel version they are running on, as APIs change
>constantly over time. You can not just take one version of a driver on
>one release, and drop it in another kernel version, without usually
>changing something. That change then "invalidates" the version number
>scheme :)
>
>Does this make sense?

Absolutely, Trying to satisfy internal requests do not always line up to
external standards in the Linux Kernel. I agree with pretty much all you
said. I think we have come up with something that we all can live with.

>> Would you like me to re-roll the patch to remove the DATE/TIME stuff
>> And add the explanation for versioning as comments to the code?
>
>Yes, please do, and possibly drop the "minor" number, otherwise I am
>going to have fun incrementing it with every single code change I do to
>the drivers. That would be what, version 264 or something by now? :)

I think it should be even higher than that :) But it was never intended to be
used by anybody other than MS devs submitting code.

I just submitted a new patch taking into account all that you requested.

As always, thanks for your help!

thanks,

Hank.