Page MenuHomeFreeBSD

Add lib9p to the build, squashed and rebased off master 20200617
AbandonedPublic

Authored by dch on Jun 17 2020, 11:41 PM.

Details

Reviewers
rgrimes
Group Reviewers
bhyve
Summary

see D10335

Add initial implementation of virtio-9p device.
Report configuration registers size properly.
While here, adjust maximum 9P mount tag length to 255 characters.
Add rfuncs.c to the build.
Adjust lib9p makefile to build the imported version of the library.
Add capabilities support to virtio-9p.
Add lib9p include dir to mtree.
Enable Casper in lib9p.
Remove debug CFLAGS from the Makefile.
Fix mismerge, add DEBUG_FLAGS for lib9p.
Update copyright notice.
Call cap_rights_limit() to make use of rootcap.
While here, fix a few typos and reorder <sys/capsicum.h> header.
Import lib9p @ 081fe6f8c9712b47cb555c8d08ed9651aa6cf3a1.

Address more review comments:

  • Add check for maximum share name length
  • Defer allocation of softc
  • Use memcpy instead of strncmpy to copy tag name
  • Improve option parsing

Mention virtio-9p and its options in the bhyve man page.
Sort lib9p SRCS in the Makefile
virtio-9p: error out on invalid options.
Take out redundant .PATH declaration.
Bring back .PATH in lib9p/Makefile; it is needed for include file installation.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 31787
Build 29348: arc lint + arc unit

Event Timeline

dch requested review of this revision.Jun 17 2020, 11:41 PM

As said in the other review, the lib9p changes should be one commit, and bhyve use of it as a followup commit after that. It doesn't seem that this review has applied any of the review feedback from the earlier review, but I appreciate at least having it refreshed to show the updated Makefile, etc.

lib/lib9p/Makefile
4

The -DWITH_CASPER is redundant with line 6 below. We only need it once.

Is -I${.CURDIR} really needed? The public includes will be staged during buildworld into the WORLDTMP sysroot. Internal headers should use "" includes instead of <>.

7

As said in the other review, this probably needs to not be set. DEBUG_FLAGS is a flag that users set.

usr.sbin/bhyve/Makefile
6

This should not be needed. Buildworld will stage the includes to include/9p already.

13

This should definitely not be set here.