Bug 242679 - emulators/open-vm-tools-nox11: fails to build after r355732
Summary: emulators/open-vm-tools-nox11: fails to build after r355732
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Josh Paetzel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-17 09:10 UTC by Ruslan Garipov
Modified: 2020-01-11 16:25 UTC (History)
4 users (show)

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


Attachments
Replace deprecated timeout(9) interface with callout(9) (v0) (1.43 KB, patch)
2019-12-20 08:09 UTC, Ruslan Garipov
no flags Details | Diff
Replace deprecated timeout(9) interface with callout(9) (v1) (1.69 KB, patch)
2020-01-10 10:41 UTC, Ruslan Garipov
no flags Details | Diff
Replace deprecated timeout(9) interface with callout(9) (v2) (1.63 KB, patch)
2020-01-10 20:08 UTC, Ruslan Garipov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ruslan Garipov 2019-12-17 09:10:37 UTC
I'm trying to build emulators/open-vm-tools-nox11 on FreeBSD 13.0-CURRENT r355689 with /usr/src updated to r355843:

# cd /usr/ports/emulators/open-vm-tools-nox11
# make build
...
--- vmmemctl ---
os.c:681:27: error: implicit declaration of function 'timeout' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
      t->callout_handle = timeout(vmmemctl_poll, t, BALLOON_POLL_PERIOD * hz);
                          ^
os.c:681:25: error: assigning to 'struct callout_handle' from incompatible type 'int'
      t->callout_handle = timeout(vmmemctl_poll, t, BALLOON_POLL_PERIOD * hz);
                        ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
os.c:715:4: error: implicit declaration of function 'callout_handle_init' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
   callout_handle_init(&state->timer.callout_handle);
   ^
os.c:723:24: error: implicit declaration of function 'timeout' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
   t->callout_handle = timeout(vmmemctl_poll, t, BALLOON_POLL_PERIOD * hz);
                       ^
os.c:723:22: error: assigning to 'struct callout_handle' from incompatible type 'int'
   t->callout_handle = timeout(vmmemctl_poll, t, BALLOON_POLL_PERIOD * hz);
                     ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
os.c:762:4: error: implicit declaration of function 'untimeout' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
   untimeout(vmmemctl_poll, t, t->callout_handle);
   ^
6 errors generated.
--- os.o ---
*** [os.o] Error code 1

make[4]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules/freebsd/vmmemctl
1 error

make[4]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules/freebsd/vmmemctl
*** [vmmemctl] Error code 2

make[3]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules
--- vmblock ---
A failure has been detected in another branch of the parallel make

make[4]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules/freebsd/vmblock
*** [vmblock] Error code 2

make[3]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules
2 errors

make[3]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools/modules
*** [all-recursive] Error code 1

make[2]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools
1 error

make[2]: stopped in /usr/ports/emulators/open-vm-tools-nox11/work/open-vm-tools-stable-11.0.1/open-vm-tools
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1

Stop.
make[1]: stopped in /usr/ports/emulators/open-vm-tools-nox11
*** Error code 1

Stop.
make: stopped in /usr/ports/emulators/open-vm-tools-nox11

I believe this is caused by r355732[1] which deprecated timeout(9) interfaces.

[1] https://svnweb.freebsd.org/base?view=revision&revision=355732
Comment 1 Josh Paetzel freebsd_committer 2019-12-19 16:03:28 UTC
I agree with your analysis.
Comment 2 Ruslan Garipov 2019-12-20 08:09:03 UTC
Created attachment 210077 [details]
Replace deprecated timeout(9) interface with callout(9) (v0)

Let me propose a patch for this PR.

I used callout_init to initialize the os_timer::callout_handle member
instead of deprecated callout_handle_init.  callout_init(9) doesn't
associate a lock with the callout structure, therefore, I don't know is
it safe to do that replacement.

I also can't check whether scheduling of ballooning tasks works without
issues after my patch, because all my FreeBSD 13.0-CURRENT virtual
machines are hosted on production ESXi host (therefore, I can't force ``out
of memory'' on that host).  The only I can say that two virtual machines
run FreeBSD 13.0-CURRENT with my patch without errors (more than two
days now).  They both are heavily loaded, but of course don't use
balloon memory (due to powerful physical ESXi host).
Comment 3 Phillip R. Jaenke 2019-12-20 22:33:23 UTC
Chiming in here with a 'patch works for me, so far' on -CURRENT in VMware Workstation 15.5.1. (Obviously not an exhaustive test there.) Oddly it's not reporting up to Workstation as installed/running, but that's been a common red herring. Communications are working fine.

root@blackstar:/usr/src # vmware-toolbox-cmd stat speed
3602 MHz
root@blackstar:/usr/src # vmware-toolbox-cmd stat balloon
UpdateInfo failed: VMware Guest API is not enabled on the host
root@blackstar:/usr/src # vmware-toolbox-cmd stat memres
UpdateInfo failed: VMware Guest API is not enabled on the host
Comment 4 Ruslan Garipov 2019-12-21 04:23:58 UTC
(In reply to Phillip R. Jaenke from comment #3)
Thanks for testing!

> root@blackstar:/usr/src # vmware-toolbox-cmd stat balloon
> UpdateInfo failed: VMware Guest API is not enabled on the host
> root@blackstar:/usr/src # vmware-toolbox-cmd stat memres
> UpdateInfo failed: VMware Guest API is not enabled on the host
As `vmware-toolbox-cmd help stat` reports, balloon and memres subcommands are available only for ESXi's guests.  For me both prints ``0 MB''.
Comment 5 Ruslan Garipov 2020-01-10 10:41:15 UTC
Created attachment 210588 [details]
Replace deprecated timeout(9) interface with callout(9) (v1)

I've slightly modified my patch, so it doesn't affect pre-r355732 systems and non-CURRENT branches.
Comment 6 Josh Paetzel freebsd_committer 2020-01-10 15:19:47 UTC
I got a review from jhb@.  His comments were:

The second call to callout_init isn't needed anymore.  That was an old bug that got fixed.

783: callout_stop(&t->callout_handle); Should that be callout_drain?

Also, what is the relation of this patch to the one in 243240?

My plan is to get them committed this weekend.
Comment 7 Ruslan Garipov 2020-01-10 20:00:44 UTC
(In reply to Josh Paetzel from comment #6)
> The second call to callout_init isn't needed anymore.  That was an old bug that
> got fixed.
Oh, does it reinitialize the same handle?  I overlooked that.

> 783: callout_stop(&t->callout_handle); Should that be callout_drain?
Yes, you're right.  I used callout_stop(9) within the vmmemctl_cleanup because the latter is called only when the module is unloading.  I thought it is safe to stop any pending callout(s) in this case (read: avoid any scheduled ballooning tasks).  But because callout_stop doesn't wait for the callout to complete, yes, it can cause issues in the vmmemctl_cleanup after cancelling the callout.  I should read man pages more carefully.

> Also, what is the relation of this patch to the one in 243240?
Those reports are unrelated to each other.  This one caused by r355732, and 243240 -- by r356337.  They both are caused by changes in API, that's the only common thing between them.

> My plan is to get them committed this weekend.
Thanks a lot!  I'll wait for that.
Comment 8 Ruslan Garipov 2020-01-10 20:05:07 UTC
Comment on attachment 210588 [details]
Replace deprecated timeout(9) interface with callout(9) (v1)

I'll replace callout_stop(9) with callout_drain(9) and remove duplicated callout_{handle_,}init.
Comment 9 Ruslan Garipov 2020-01-10 20:08:24 UTC
Created attachment 210600 [details]
Replace deprecated timeout(9) interface with callout(9) (v2)

1. callout_stop is replaced with callout_drain.
2. callout_handle_init(&t->callout_handle) is removed from the original source code and callout_init(&t->callout_handle, 0) is removed from the previous version of my patch.
Comment 10 Josh Paetzel freebsd_committer 2020-01-10 20:09:34 UTC
Perfect, ty.

Also, do you have any objections to me getting these changes upstreamed?  I've signed all the NDAs needed to contribute code and work with the VMware team on a regular basis.
Comment 11 Ruslan Garipov 2020-01-10 20:16:03 UTC
(In reply to Josh Paetzel from comment #10)
> Also, do you have any objections to me getting these changes upstreamed?
I don't, of course.  Please, do that.

Thanks for your help!
Comment 12 commit-hook freebsd_committer 2020-01-11 16:25:11 UTC
A commit references this bug:

Author: jpaetzel
Date: Sat Jan 11 16:24:55 UTC 2020
New revision: 522695
URL: https://svnweb.freebsd.org/changeset/ports/522695

Log:
  Fix build on HEAD

  PR:	242679
  Submitted by:	Ruslan Garipov <brigadir15@gmail.com>
  Reviewed by:	jhb

Changes:
  head/emulators/open-vm-tools/files/patch-modules_freebsd_vmmemctl_os.c
Comment 13 Josh Paetzel freebsd_committer 2020-01-11 16:25:33 UTC
Committed, thanks!