Bug 857346
| Summary: | [Hyper-v]The Hyper-v Linux guest version information got via kvp deamon is not correct | |||
|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 5 | Reporter: | Shengnan Wang <shwang> | |
| Component: | hypervkvpd | Assignee: | Tomáš Hozza <thozza> | |
| Status: | CLOSED WONTFIX | QA Contact: | Virtualization Bugs <virt-bugs> | |
| Severity: | medium | Docs Contact: | ||
| Priority: | low | |||
| Version: | 5.9 | CC: | bsarathy, jasowang, jbian, juzhang, kys, leiwang, qguan, rhod, thozza | |
| Target Milestone: | rc | |||
| Target Release: | --- | |||
| Hardware: | Unspecified | |||
| OS: | Unspecified | |||
| Whiteboard: | ||||
| Fixed In Version: | Doc Type: | Bug Fix | ||
| Doc Text: | Story Points: | --- | ||
| Clone Of: | ||||
| : | 874929 (view as bug list) | Environment: | ||
| Last Closed: | 2013-04-23 15:03:47 UTC | Type: | Bug | |
| Regression: | --- | Mount Type: | --- | |
| Documentation: | --- | CRM: | ||
| Verified Versions: | Category: | --- | ||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
| Cloudforms Team: | --- | Target Upstream Version: | ||
| Embargoed: | ||||
| Attachments: | ||||
|
Description
Shengnan Wang
2012-09-14 07:40:36 UTC
Created attachment 612750 [details]
RHEL5.9 guest version information.jpg
Created attachment 612751 [details]
windows guest version information.jpg
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. K.Y, Jason - Is this not a bug then? Should the BZ be closed? (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. (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(). (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 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. (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 (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. (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! Created attachment 617440 [details]
osversion info.jpg
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. 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. (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. (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. (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 Created attachment 618384 [details]
verified OSVersion.jpg
Move component to hypervkvpd. 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. Closing, we decided not to fix it in RHEL5.10 |