Bug 857346 - [Hyper-v]The Hyper-v Linux guest version information got via kvp deamon is not correct
[Hyper-v]The Hyper-v Linux guest version information got via kvp deamon is no...
Status: CLOSED WONTFIX
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: hypervkvpd (Show other bugs)
5.9
Unspecified Unspecified
low Severity medium
: rc
: ---
Assigned To: Tomáš Hozza
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-14 03:40 EDT by Shengnan Wang
Modified: 2013-04-23 11:03 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 874929 (view as bug list)
Environment:
Last Closed: 2013-04-23 11:03:47 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
RHEL5.9 guest version information.jpg (171.00 KB, image/jpeg)
2012-09-14 03:41 EDT, Shengnan Wang
no flags Details
windows guest version information.jpg (67.49 KB, image/jpeg)
2012-09-14 03:42 EDT, Shengnan Wang
no flags Details
[PATCH] Changing the way of obtaining OS informations. (3.65 KB, patch)
2012-09-25 08:14 EDT, Tomáš Hozza
no flags Details | Diff
osversion info.jpg (123.31 KB, image/jpeg)
2012-09-26 04:09 EDT, Shengnan Wang
no flags Details
[PATCH] Changing the way of obtaining OS informations. (3.89 KB, patch)
2012-09-26 11:03 EDT, Tomáš Hozza
no flags Details | Diff
verified OSVersion.jpg (103.70 KB, image/jpeg)
2012-09-27 23:34 EDT, Shengnan Wang
no flags Details

  None (edit)
Description Shengnan Wang 2012-09-14 03:40:36 EDT
Description of problem:
The Linux guest version information is in HV_DRV_VERSION in linux/hyperv.h as K.Y. said in bug 854143 comment 24. Host gets our RHEL5.9 guest version information via kvp daemon. Now the version information is not correct. Take Linux5.9 guest(2.6.18-339.el5) for example, the key value pair should be: 
    OSMajorVersion=2
    OSMinorVersion=6 
    OSBuildNumber=18 
    OSversion=2.6.18
    IntegrationServicesVersion=2.6.18-339
Now the version information we get via kvp daemon is as below (attachment "RHEL5.9 guest version information.jpg"):
    OSMajorVersion=
    OSMinorVersion= 
    OSBuildNumber= 2.6.18
    OSversion= 2.6.18
    IntegrationServicesVersion= 3.1

The version information of the Hyper-v Windows guest is correct. (attachment "Windows guest version information.jpg")



Version-Release number of selected component (if applicable):
Host: Windows 2008 R2 Hyper-V Server Core 
Hyper-V Version: 6.1.7600.16385
Guest: Both RHEL5.9 i386 & RHEL5.9 x86_64 (2.6.18-339.el5)
Hypervkvpd package Name: hypervkvpd-0-0.7.el5
Pv drivers: 
[root@localhost ~]# lsmod |grep hv_*
hv_netvsc              25665  0 
hid_base_hv            68177  1 hid_hyperv
hv_utils               12001  0 
hv_storvsc             17601  2 
hv_vmbus               30265  4 hv_netvsc,hid_hyperv,hv_utils,hv_storvsc

How reproducible:
100%

Steps to Reproduce:
1. Install the hypervkvpd package and start the hypervkvpd service on the hyper-v Linux guest.
2. Login the Hyper-V host. Use powershell via "c:\Windows\System32\powershell" .
3. Get the virtual machine object $vm via powershell. 
   # $Vm = gwmi -namespace root\virtualization -query "Select * From Msvm_ComputerSystem Where ElementName='{$Guest_name}'"
4. Get the kvp object $kvp via powershell.
   # $Kvp = gwmi -namespace root\virtualization -query "Associators of {$Vm} Where AssocClass=Msvm_SystemDevice ResultClass=Msvm_KvpExchangeComponent"
5. There is guest information in the output "GuestIntrinsicExchangeItems". And you can use the script from page http://blogs.msdn.com/b/virtual_pc_guy/archive/2008/11/18/hyper-v-script-looking-at-kvp-guestintrinsicexchangeitems.aspx?Redirected=true to display the results clearly from the xml blob.

Actual results:
At step 5, the version information is :
    OSMajorVersion=
    OSMinorVersion= 
    OSBuildNumber= 2.6.18
    OSversion= 2.6.18
    IntegrationServicesVersion= 3.1

Expected results:
At step 5, the version information should be:
    OSMajorVersion=2
    OSMinorVersion=6 
    OSBuildNumber=18 
    OSversion=2.6.18
    IntegrationServicesVersion=2.6.18-339

Additional info:
Comment 1 Shengnan Wang 2012-09-14 03:41:29 EDT
Created attachment 612750 [details]
RHEL5.9 guest version information.jpg
Comment 2 Shengnan Wang 2012-09-14 03:42:02 EDT
Created attachment 612751 [details]
windows guest version information.jpg
Comment 3 K. Y. Srinivasan 2012-09-22 13:40:40 EDT
The Linux version information is correct and is what we are returning across Linux distributions we currently support. As I had noted in a different bug on the same issue, the LIS version information could be better reflect the fact that the drivers are now part of the kernel.
Comment 4 Bhavna Sarathy 2012-09-24 13:50:38 EDT
K.Y, Jason - 

Is this not a bug then?  Should the BZ be closed?
Comment 5 jason wang 2012-09-24 23:18:44 EDT
(In reply to comment #4)
> K.Y, Jason - 
> 
> Is this not a bug then?  Should the BZ be closed?
CC tomas

Not sure whether it's a bug or at leaset a bug of daemon not kernel.

Have a look at the daemon code, it fetches the Major and Minor version information from /etc/redhat-release. But our /etc/redhat-release does not contains such info. Upstream daemon check /etc/os-release which does not exist in RHEL5. So maybe we could get those from `uname -r` so something else.
Comment 6 Tomáš Hozza 2012-09-25 04:11:23 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > K.Y, Jason - 
> > 
> > Is this not a bug then?  Should the BZ be closed?
> CC tomas
> 
> Not sure whether it's a bug or at leaset a bug of daemon not kernel.
> 
> Have a look at the daemon code, it fetches the Major and Minor version
> information from /etc/redhat-release. But our /etc/redhat-release does not
> contains such info. Upstream daemon check /etc/os-release which does not
> exist in RHEL5. So maybe we could get those from `uname -r` so something
> else.

I'll rewrite the code for obtaining OS Major/Minor/Build version. But we still need to use /etc/redhat-release to obtain OSName (uname returns just "Linux" instead of "Red Hat Enterprise Linux ...").

But as for the IntegrationServicesVersion, there is comment in the daemon source saying that "This key is serviced in the kernel". It is obtained from struct 
kvp_int_msg.body.kvp_register.version when registering with kernel driver. If it shouldn't be done so, I can rewrite it too, so the version will be obtained from uname().
Comment 7 jason wang 2012-09-25 04:20:15 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > K.Y, Jason - 
> > > 
> > > Is this not a bug then?  Should the BZ be closed?
> > CC tomas
> > 
> > Not sure whether it's a bug or at leaset a bug of daemon not kernel.
> > 
> > Have a look at the daemon code, it fetches the Major and Minor version
> > information from /etc/redhat-release. But our /etc/redhat-release does not
> > contains such info. Upstream daemon check /etc/os-release which does not
> > exist in RHEL5. So maybe we could get those from `uname -r` so something
> > else.
> 
> I'll rewrite the code for obtaining OS Major/Minor/Build version. But we
> still need to use /etc/redhat-release to obtain OSName (uname returns just
> "Linux" instead of "Red Hat Enterprise Linux ...").
> 
> But as for the IntegrationServicesVersion, there is comment in the daemon
> source saying that "This key is serviced in the kernel". It is obtained from
> struct 
> kvp_int_msg.body.kvp_register.version when registering with kernel driver.
> If it shouldn't be done so, I can rewrite it too, so the version will be
> obtained from uname().

Hi Tomas:

Not sure it's worth to change the value of IntegrationServicesVersion. Maybe we can just use the current version since our hyperv driver were backported from upstream and have exactly the same function.

Thanks
Comment 8 Tomáš Hozza 2012-09-25 08:14:58 EDT
Created attachment 617002 [details]
[PATCH] Changing the way of obtaining OS informations.

I have rewritten the kvp_get_os_info() function so now it should fill
os_major, os_minor, os_build, os_version using values from uname().
Can you (Jason, K.Y.) please review it?

I prepared scratch build of patched KVP daemon. Can you (Shengnan) 
please try it, if you get right values using it?

https://brewweb.devel.redhat.com/taskinfo?taskID=4905771

Thanks.
Comment 9 jason wang 2012-09-25 23:31:13 EDT
(In reply to comment #8)
> Created attachment 617002 [details]
> [PATCH] Changing the way of obtaining OS informations.
> 
> I have rewritten the kvp_get_os_info() function so now it should fill
> os_major, os_minor, os_build, os_version using values from uname().
> Can you (Jason, K.Y.) please review it?
> 
> I prepared scratch build of patched KVP daemon. Can you (Shengnan) 
> please try it, if you get right values using it?
> 
> https://brewweb.devel.redhat.com/taskinfo?taskID=4905771
> 
> Thanks.

Hi Tomas:

Have a look at the patch, looks good to me except two minor issue:

- The original comment said "The current windows host (win7) expects the build string to be of the form: x.y.z Strip additional information we may have. */, so I'm not sure whether it's correct to add "-rhel.dist" to this value.
- In my RHEL5 machine, the /etc/redhat-release only contains one line, so no need to change the code of analysising /etc/redhat-release?

Thanks
Comment 10 Tomáš Hozza 2012-09-26 03:12:11 EDT
(In reply to comment #9)
> Hi Tomas:
> 
> Have a look at the patch, looks good to me except two minor issue:
>
> - The original comment said "The current windows host (win7) expects the
> build string to be of the form: x.y.z Strip additional information we may
> have. */, so I'm not sure whether it's correct to add "-rhel.dist" to this
> value.

Yes, that's why I built also a scratch build, so we can test it and included K.Y. when asked for review. The build string will be now just "18" (in case the release string in the struct filled using uname() contains for example "2.6.18-339.el5"). I changed it to reflect the expected results stated in comment #0.

I included the comment ("utsname.release" contains: "x.y.x-rel.dist..." so parse it for information) just as an example of what the "release" member of struct utsname may contain, so one can understand better the following part of source, where it is parsed.


K.Y. can you please give us some opinion about the build string?


> - In my RHEL5 machine, the /etc/redhat-release only contains one line, so no
> need to change the code of analysising /etc/redhat-release?

Yes, I know. I checked it too. My opinion is that it is not needed, I just didn't want the "os_major" and "os_minor" to be changed if there will be something else in /etc/redhat-release. It's just sanity check.
Comment 11 Shengnan Wang 2012-09-26 04:08:28 EDT
(In reply to comment #8)

> I prepared scratch build of patched KVP daemon. Can you (Shengnan) 
> please try it, if you get right values using it?
> 
> https://brewweb.devel.redhat.com/taskinfo?taskID=4905771
> 
Hi Tomas,

Test the new rpm package. Now the OSVersion is 18. However, the OSVersion should be 2.6.18. Other information got from guest via kvp deamon is right. (File attached: osversion info.jpg)

Thanks!
Comment 12 Shengnan Wang 2012-09-26 04:09:24 EDT
Created attachment 617440 [details]
osversion info.jpg
Comment 13 Tomáš Hozza 2012-09-26 11:03:45 EDT
Created attachment 617597 [details]
[PATCH] Changing the way of obtaining OS informations.

There was one change missing in the patch. Now it should be good.

Here is new brew scratch build for testing:
https://brewweb.devel.redhat.com/taskinfo?taskID=4911753

Jason, do you think we can reassign this Bug to hypervkvpd? Since the patch is for the daemon and solves the problem and as you stated before we are not changing the IntegrationServicesVersion.
Comment 14 K. Y. Srinivasan 2012-09-26 17:19:45 EDT
Thomas,

As I had noted earlier, we are comfortable with what the code in upstream is returning (on RHEL 5.8)

Empty strings returned for OSMajorVersion and OSMinorVersion.

OSName set to Red Hat Enterprise Linux Server release 5.8 (Tikanga) (on 5.8)

OSBuildNumber: 2.6.18 (we want this to be x.y.z)

OSVersion: 2.6.18

With this we would have presented all the information we have.

So, I am not sure we we are changing anything. This is the kind of information we are returning on all distros currently supporting. If we want to make any changes here (and I am not recommending it) we should first go the upstream route.
Comment 15 jason wang 2012-09-27 02:42:07 EDT
(In reply to comment #13)
> Created attachment 617597 [details]
> [PATCH] Changing the way of obtaining OS informations.
> 
> There was one change missing in the patch. Now it should be good.
> 
> Here is new brew scratch build for testing:
> https://brewweb.devel.redhat.com/taskinfo?taskID=4911753
> 
> Jason, do you think we can reassign this Bug to hypervkvpd? Since the patch
> is for the daemon and solves the problem and as you stated before we are not
> changing the IntegrationServicesVersion.

Sure, and K.Y is not recommending to do the changes, maybe we could also consider to close this bug.
Comment 16 Tomáš Hozza 2012-09-27 03:16:15 EDT
(In reply to comment #15)
> Sure, and K.Y is not recommending to do the changes, maybe we could also
> consider to close this bug.

I agree, since K.Y. stated that "This is the kind of information they are returning on all distros currently supporting". 

We have working patch and if there will be desire to change the version information in the future, it can be reopened.
Comment 18 Shengnan Wang 2012-09-27 23:33:36 EDT
(In reply to comment #13)
> Created attachment 617597 [details]
> [PATCH] Changing the way of obtaining OS informations.
> 
> There was one change missing in the patch. Now it should be good.
> 
> Here is new brew scratch build for testing:
> https://brewweb.devel.redhat.com/taskinfo?taskID=4911753
> 
Hi tomas,

Test the package hypervkvpd-0-0.7.2.el5 on both i386 and x86_64 RHEL5.9 guest. It works well. The version information is correct. Details, please see the attached file 'verified OSVersion.jpg'.

Thanks
Comment 19 Shengnan Wang 2012-09-27 23:34:41 EDT
Created attachment 618384 [details]
verified OSVersion.jpg
Comment 20 jason wang 2012-10-08 01:41:43 EDT
Move component to hypervkvpd.
Comment 21 Tomáš Hozza 2013-03-13 09:38:35 EDT
I'm lowering Severity to MEDIUM. The version information returned by hypervkvpd
is not accurate, but this is for sure NOT urgent or high severity Bug.
Comment 22 Ronen Hod 2013-04-23 11:03:47 EDT
Closing, we decided not to fix it in RHEL5.10

Note You need to log in before you can comment on or make changes to this bug.