Page MenuHomeFreeBSD

libvmm: add vm_close() to vmmapi library
Needs ReviewPublic

Authored by gusev.vitaliy_gmail.com on Apr 27 2022, 9:53 AM.

Details

Reviewers
jhb
markj
Group Reviewers
bhyve
Summary

Currently there is no way to safely free vm structure w/o fd leaking. Another function vm_destroy() offers destroying whereas in some cases vm needs to be opened (vm_open) and then closed (vm_close).

--commit message--

libvmm: add vm_close() to vmmapi library

It also adds BEGIN_DECLS and END_DECLS to satisfy binding
with c++ binaries.

Sponsored by vStack.

Test Plan
vmopen.c:
#include <dlfcn.h>
#include <err.h>
#include <vmmapi.h>

int main(int argc, char *argv[])
{
	const char *vmname = argc > 1 ? argv[1] : "-";
	struct vmctx *ctx;
	void (*__vm_close)(struct vmctx *ctx) = dlsym(RTLD_NEXT, "vm_close");

	ctx = vm_open(vmname);

	if (ctx == NULL)
		err(1, "cannot open vm device: %s", vmname);

	if (__vm_close)
		__vm_close(ctx);
}

Compile:
$ cc -W -Wall -Wextra vmopen.c -l dl -lvmmapi -o vmopen
Create "testvm" env:
$ sysctl hw.vmm.create=testvm
Run with original libvmmapi.so:
$ valgrind ./vmopen testvm
...
==95593== HEAP SUMMARY:
==95593==     in use at exit: 55 bytes in 1 blocks
==95593==   total heap usage: 2 allocs, 1 frees, 71 bytes allocated
==95593== 
==95593== LEAK SUMMARY:
==95593==    definitely lost: 55 bytes in 1 blocks  
   
Run with libvmmapi.so having vm_close()
LD_PRELOAD=/data/lib/libvmmapi.so valgrind ./vmopen testvm

==95674== HEAP SUMMARY:
==95674==     in use at exit: 0 bytes in 0 blocks
==95674==   total heap usage: 2 allocs, 2 frees, 71 bytes allocated
==95674== 
==95674== All heap blocks were freed -- no leaks are possible

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

gusev.vitaliy_gmail.com edited the test plan for this revision. (Show Details)
gusev.vitaliy_gmail.com edited the test plan for this revision. (Show Details)

(I would do the C++ bits as a separate commit)

lib/libvmmapi/vmmapi.h
36

Why is vmm.h needed?

79

I think we typically leave a blank line after this and before __END_DECLS in other headers.

This revision is now accepted and ready to land.May 17 2022, 6:40 PM
In D35073#798694, @jhb wrote:

(I would do the C++ bits as a separate commit)

It makes sense. Do you want I create separate review for that?

lib/libvmmapi/vmmapi.h
36

Because w/o that compiling produces a lot errors:

$ cc vmopen.c
In file included from vmopen.c:3:
In file included from /usr/include/vmmapi.h:37:
/usr/include/machine/vmm_dev.h:61:13: error: use of undeclared identifier 'VM_MAX_SUFFIXLEN'
        char            name[VM_MAX_SUFFIXLEN + 1];
                             ^
/usr/include/machine/vmm_dev.h:80:18: error: field has incomplete type 'struct seg_desc'
        struct seg_desc desc;
 ...

I guess including "vmmapi.h" should self sufficient to be compiled.

79

Will do, thanks.

gusev.vitaliy_gmail.com marked 2 inline comments as done.

Added blank lines before and after BEGIN_DECLS/END_DECLS

This revision now requires review to proceed.May 18 2022, 1:22 PM

Removed c++ BEGIN_DECLS/END_DECLS.

@jhb Please land this version and I will create separate review with c++ related changes.

vm_close() is widely used at vStack platform already
it would be nice to use work hours sponsorship :shrug