Bug 196185 - emulators/hyperv-is: Update Hyper-V ports for FreeBSD 10.1
Summary: emulators/hyperv-is: Update Hyper-V ports for FreeBSD 10.1
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords: needs-patch
Depends on:
Blocks:
 
Reported: 2014-12-22 08:30 UTC by Kylie
Modified: 2017-03-01 17:41 UTC (History)
9 users (show)

See Also:
delphij: maintainer-feedback+


Attachments
patch (35.41 KB, patch)
2014-12-23 05:36 UTC, FangJun
no flags Details | Diff
/usr/ports/emulator/hyperv-is (7.67 KB, patch)
2014-12-23 05:44 UTC, FangJun
no flags Details | Diff
10.1 ports code (14.38 KB, text/plain)
2015-01-13 08:24 UTC, andy zhang
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kylie 2014-12-22 08:30:03 UTC
There are below new feature and critical bug fix to get FreeBSD running on Hyper-v. 
1) Scatter/gather I/O for storage 
2) KVP does not work on windows server 2008 R2
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2014-12-22 10:36:43 UTC
Maintainer CC'd
Comment 2 Xin LI freebsd_committer freebsd_triage 2014-12-22 18:18:13 UTC
This was submitted by maintainer.
Comment 3 Xin LI freebsd_committer freebsd_triage 2014-12-22 18:20:05 UTC
(In reply to Kylie from comment #0)
> There are below new feature and critical bug fix to get FreeBSD running on
> Hyper-v. 
> 1) Scatter/gather I/O for storage 
> 2) KVP does not work on windows server 2008 R2

Hrm I can't seem to find the patch for this, could you please attach it to this ticket?

Thanks!
Comment 4 Kylie 2014-12-23 00:10:42 UTC
Jun, please attach patch here. 

Hi Xin, 

Ports are available for all new releases. However when 10.2 is released, we may already get everything upstreamed and nothing is needed for 10.2. Then for 10.2, how to exit gracefully without error message if somebody still tries to run our ports. 
We could add one message to say "no more update for 10.2" but ERROR will still be printed if we exits ports with "IGNORE" signal. 

Thanks,
Kylie Liang
Comment 5 FangJun 2014-12-23 05:36:34 UTC
Created attachment 150893 [details]
patch
Comment 6 FangJun 2014-12-23 05:44:26 UTC
Created attachment 150894 [details]
/usr/ports/emulator/hyperv-is
Comment 7 Kylie 2014-12-23 06:34:22 UTC
Andy is doing internal testing. Once done, please update the bugzilla to move forward.
Comment 8 andy zhang 2015-01-13 08:24:57 UTC
Created attachment 151525 [details]
10.1 ports code

already tested in 10.1, 10.0, 9.x, 8.x
Comment 9 John Marino freebsd_committer freebsd_triage 2015-02-02 16:03:46 UTC
This is a patch to the source code, not a patch to the port.

I see the last attachment is a "shar" of the entire port.  In theory patches could be generated by comparing that to the current port.

However, submitting "shar" instead of patches is not easy for anybody.

Actually, these three attachments are causing me a great deal of confusion.

Attachment 1 [details]: patch, Dec 23, seems to patch source
Attachment 2 [details]: patch, Dec 23, seems to patch port but doesn't reference attachment 1 [details] patch
Attachment 3 [details]: shar, Jan 13, seems to replace port.

It would help this PR along if:

1) all unnecessary attachments are designated "obsolete" so they get hidden and don't add to confusion.
2) Make sure patches to the port itself are present.
3) most importantly: poudriere logs for every platform to verify this port builds on all advertised platforms.  I see "internal testing" is implied, but we want to see external testing, e.g. poudriere for verification purposes.
Comment 10 Xin LI freebsd_committer freebsd_triage 2015-02-02 18:23:32 UTC
(In reply to andy zhang from comment #8)
Hi, Andy,

I have noticed that the port have changed to a way where two new scripts, loader.sh and unloader.sh were added, and in pkg-install/pkg-deinstall scripts, they were mentioned.

Is it possible to collapse the functionality into pkg-install/pkg-deinstall?  This would be helpful for those who use pkg(8) only without a ports tree (as the scripts were not distributed as part of the port).  If the scripts make changes to the base system, they can also be packaged instead (e.g. install to /usr/local/share/hyperv/ or /usr/local/libexec/hyperv/) so that users who install binary package would get these scripts too.
Comment 11 Xin LI freebsd_committer freebsd_triage 2015-02-03 00:19:07 UTC
@FangJun -- There are a few issues with the patch ("patch (35.41 KB, patch)  2014-12-23 05:36 UTC").  Could you please check?

1) src/sys/dev/hyperv/utilities/hv_util.c: in tree driver should never return BUS_PROBE_VENDOR.  Otherwise vendor supplied driver will not take precedence.

I would recommend making a (git or your source code management tool) branch for in-tree driver submission to make it easier for future merge, etc., as the SCM tool would take care for this kind of differences.

2) same file -- the driver name have been changed from "hyperv-utils" to "hyperv-utils-port", is this intentional? (I think it's not but it's ok if it's intentional).

3) hyperv_support.S is missing in the patch.  Please use use '-N' when generating the diff.

I can make minor corrections manually.  If the changes would be larger please generate a new diff.
Comment 12 FangJun 2015-02-03 06:29:11 UTC
(In reply to Xin LI from comment #11)
@Xin, thanks for your comments.

1) In the built-in code, hv_util.c probe() still return BUS_PROBE_DEFAULT. We only use BUS_PROBE_VENDOR with the hyperv-is ports to enable our vendor supplied driver.

2) We use the name "hyperv-utils-port" is intentionally, because it is weird that the loader cannot load the hv_utils.ko but it is OK to load other hyperv modules(eg. hv_storvsc.ko). For example, we modify the /boot/loader.conf as below:
hv_storvsc_load="YES"
hv_utils_load="YES"

And then reboot system, we can find only storvsc can be loaded:
/boot/kernel/kernel text=0xf8f898 data=0x124a30+0x2055c0 syms=[0x8+0x1405e0+0x8+0x15b077]
/boot/kernel/hv_storvsc.ko size 0x6278 at 0x1956000
Booting...

And if we change the name, such as hv_utils_test.ko, hv_utils can be loaded successfully. @Xin, ao you have any advice?
For this reason and in ooder to allow the user to know the ports is in use, we also change hv_storvsc.ko to hv_storvsc_ports.ko.

3) The hyperv_support.S will not be used this time, we will delete it.
Comment 13 Kylie 2015-02-10 06:22:34 UTC
Xin and John, 

Could you please review Jun's reply? Any advice for point 2? 

2) We use the name "hyperv-utils-port" is intentionally, because it is weird that the loader cannot load the hv_utils.ko but it is OK to load other hyperv modules(eg. hv_storvsc.ko).
Comment 14 John Marino freebsd_committer freebsd_triage 2015-02-10 15:30:18 UTC
I am really not suitable to lead this PR.
I primary run DragonFly and don't use FreeBSD as much as most of the other committers.  Before I was mainly assisting a PR that had been stuck for ages.

That said, if changing  the name of the module allows it to load, something else is going on.  That doesn't sound right to me.
Comment 15 Rene Ladan freebsd_committer freebsd_triage 2017-03-01 17:41:44 UTC
Port expired, no update to this PR in two years.