Bug 215353 - emulators/open-vm-tools emulators/open-vm-tools-nox11: Fix build after sysctl changes in head r310051
Summary: emulators/open-vm-tools emulators/open-vm-tools-nox11: Fix build after sysctl...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Enji Cooper
URL:
Keywords:
: 215367 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-12-17 14:30 UTC by Dimitry Andric
Modified: 2017-03-14 21:18 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (swills)


Attachments
Fix sysctl_add_oid() for head after r310051 (1.24 KB, patch)
2016-12-17 14:30 UTC, Dimitry Andric
no flags Details | Diff
Fix sysctl_add_oid() for head after r310051 (1.21 KB, patch)
2016-12-17 15:22 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric freebsd_committer 2016-12-17 14:30:56 UTC
Created attachment 178024 [details]
Fix sysctl_add_oid() for head after r310051

After head r310051, where the prototype of sysctl_add_oid(9) has changed, adding one extra parameter, previously built vmmemctl.ko modules insta-panic the kernel.

Additionally, attempting to rebuild the open-vm-tools ports with the new prototype results in errors in the vmmemctl module now:

os.c:894:46: error: too few arguments to function call, expected 11, have 10
                         BALLOON_NAME_VERBOSE);
                                             ^
/usr/src/sys/sys/sysctl.h:1014:1: note: 'sysctl_add_oid' declared here
struct sysctl_oid *sysctl_add_oid(struct sysctl_ctx_list *clist,
^
1 error generated.

For some reason, the vmmemctl module does not make use of the SYSCTL_ADD_OID macro from <sys/sysctl.h>, but directly uses the 'real' declaration of sysctl_add_oid(9).

Fortunately __FreeBSD_version was bumped to 1200019 very soon afterwards, in r310149, though it was for unrelated changes.  We can use this as a stopgap fix, to add an extra NULL parameter.
Comment 1 Ed Schouten freebsd_committer 2016-12-17 15:09:24 UTC
Hi Dimitry,

Alternatively it could be fixed as follows:

--- modules/freebsd/vmmemctl/os.c
+++ modules/freebsd/vmmemctl/os.c
@@ -824,7 +824,7 @@
 static void
 vmmemctl_init_sysctl(void)
 {
-   oid =  sysctl_add_oid(NULL, SYSCTL_STATIC_CHILDREN(_vm), OID_AUTO,
+   oid =  SYSCTL_ADD_OID(NULL, SYSCTL_STATIC_CHILDREN(_vm), OID_AUTO,
                          BALLOON_NAME, CTLTYPE_STRING | CTLFLAG_RD,
                          0, 0, vmmemctl_sysctl, "A",
                          BALLOON_NAME_VERBOSE);

This should keep the module buildable regardless of the version of FreeBSD used. Could you please give it a try and let me know whether it works for you? Thanks!

Ed
Comment 2 Dimitry Andric freebsd_committer 2016-12-17 15:22:08 UTC
Created attachment 178025 [details]
Fix sysctl_add_oid() for head after r310051

(In reply to Ed Schouten from comment #1)
> This should keep the module buildable regardless of the version of FreeBSD
> used. Could you please give it a try and let me know whether it works for
> you? Thanks!

Yes, that also works.  I'm unsure why upstream open-vm-tools didn't use those macros, but they seem to have done so since the beginning.  Probably worth upstreaming this.
Comment 3 Ed Schouten freebsd_committer 2016-12-17 16:59:10 UTC
Pull request sent upstream: https://github.com/vmware/open-vm-tools/pull/125
Comment 4 Enji Cooper freebsd_committer 2016-12-18 00:10:34 UTC
*** Bug 215367 has been marked as a duplicate of this bug. ***
Comment 5 Dimitry Andric freebsd_committer 2017-01-19 18:43:44 UTC
Dear portmgr, the maintainer doesn't respond to this bug, and this patch has been sitting for a month, while the port is broken.  Can I commit it?
Comment 6 Mathieu Arnold freebsd_committer 2017-01-19 22:33:03 UTC
Maintainer timeout is 14 days.

Find any ports committer willing to ok the patch, and you will be able to commit it yourself.
Comment 7 Enji Cooper freebsd_committer 2017-02-21 04:57:51 UTC
(In reply to Mathieu Arnold from comment #6)

I'll work on committing this since it's blocking me from using it on ^/head .
Comment 8 commit-hook freebsd_committer 2017-02-21 19:21:48 UTC
A commit references this bug:

Author: ngie
Date: Tue Feb 21 19:20:48 UTC 2017
New revision: 434550
URL: https://svnweb.freebsd.org/changeset/ports/434550

Log:
  emulators/open-vm-tools: change sysctl_add_oid use to SYSCTL_ADD_OID

  sysctl_add_oid was changed in base/head@r310051 to take a label parameter,
  and open-vm-tools doesn't supply it as it depends on the old KPI signature.
  SYSCTL_ADD_OID doesn't require the label parameter since its inception
  though, thus it's a backwards compatible solution for adding sysctl oids.

  PR:		215353
  Reported by:	dim
  Submitted by:	ed
  Reviewed by:	ed, lwhsu
  Approved by:	lwhsu
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D9698

Changes:
  head/emulators/open-vm-tools/Makefile
  head/emulators/open-vm-tools/files/patch-vmmemctl-os.c
Comment 9 Kurt Jaeger freebsd_committer 2017-03-14 18:41:42 UTC
Is there something left to do or can this ticket be closed ?
Comment 10 Dimitry Andric freebsd_committer 2017-03-14 21:18:16 UTC
(In reply to Kurt Jaeger from comment #9)
> Is there something left to do or can this ticket be closed ?

Nope, it's done now. Closing.