Page MenuHomeFreeBSD

byhve: add option to specify IP address for gdb
Needs ReviewPublic

Authored by oshogbo on Tue, Apr 6, 3:36 PM.

Details

Reviewers
jhb
rgrimes
Group Reviewers
manpages
bhyve
Summary

Allow user to specify the IP address available for gdb
debugger.

I even wonder if we should't change the default from ANY to
localhost.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 38466
Build 35355: arc lint + arc unit

Event Timeline

bcr added a subscriber: bcr.

OK from manpages.

grehan added inline comments.
usr.sbin/bhyve/gdb.c
1854

I'd prefer the default was the loopback address (rfb defaults to that).

usr.sbin/bhyve/gdb.c
1854

I agree, I just wasn't sure if we want to change the default. Currently we are listening to ANY.

usr.sbin/bhyve/gdb.c
1854

I'll say it's fine, and can be changed later if there's disagreement.

Though this is not exactly a normal service, to me listening on any by default is the correct thing to do here. One thing I would like to note, this code appears to only listens on one of IPv4 or IPv6, which is probably a short coming.

This revision is now accepted and ready to land.Wed, Apr 7, 12:07 PM

Rod, why is your opinion so important ?

usr.sbin/bhyve/bhyverun.c
489

I would go ahead and use "gdb.address" as the name.

1445

This won't actually work due to the way get_config_value works (it uses a single static buffer in case it has to do any variable expansion, so the next call to get_config_* invalidates any previously-returned char *. The old code ran atoi() before calling get_config_bool_default() which was ok, but the new code can't do that. Probably should just move the parsing into init_gdb() instead and call init_gdb() unconditionally.

usr.sbin/bhyve/gdb.c
1854

I'll say it's fine, and can be changed later if there's disagreement.

I think qemu defaults to localhost and that's probably better to do (as well as generally safer to not expose the debug port on a network by default). I wonder if it would be better though to use "localhost" rather than the numbers? Then you can get a dual-stack socket from getaddrinfo()?

This revision now requires review to proceed.Fri, Apr 9, 5:57 PM
oshogbo added inline comments.
usr.sbin/bhyve/bhyverun.c
1445

Oh, I didn't notice that. I have look into the function and seen that you are using nvlist under hood and assume that you are just returning the value. As I test it only with the static texts I got two different pointers and everything look good. Your suggestion makes sense for me.

usr.sbin/bhyve/gdb.c
1854

I will change the default in separate commit.