libvmmapi: Use the vmmctl device file to create and destroy VMs

[markj]

Summary

This deprecates the vm_create() and vm_open() interfaces and introduces
vm_openf(), which takes flags controlling its behaviour. In particular,
it will optionally create a VM first, and it can optionally reinitialize
an existing VM. This enables some simplification of existing consumers.

[jhb]

lib/libvmmapi/vmmapi.c

118

I wonder if we want this to be a persistent fd once opened instead that is saved in a private global inside of libvmmapi? That is a pattern we have used other places. One thing to consider is it might be nice to let bhyvectl make use of libvmmapi perhaps or its --destroy operation?

203

The old code was careful here to close the /dev/vmm/<n> fd first before triggering the destroy. I guess it is true that just having the fd open doesn't block destroy_dev() in the kernel though.

[jhb]

lib/libvmmapi/vmmapi.c

118

I guess bhyvectl doesn't destroy by name but instead does an open() and then destroy which is a bit odd. It'd be somewhat nicer if it just did the single ioctl? I guess it is fine though.

[markj]

lib/libvmmapi/vmmapi.c

118

Note that vm_create() is effectively a legacy/deprecated interface now. The "modern" way of creating a VM is to use vm_openf(VMMAPI_OPEN_CREATE), in which case the fd is saved in the returned context structure.

bhyvectl already uses libvmmapi to destroy a VM (via vm_destroy()), so I don't quite follow what you mean.

[markj]

lib/libvmmapi/vmmapi.c

118

It is a bit odd. But, once we have a mode where a VM is automatically destroyed once the corresponding vmmctl fd is closed, I think bhyvectl --destroy will be much less useful anyway...

[mp]

lib/libvmmapi/vmmapi.c

148

Should this be handled via error returns instead of just assert (vm_device_open as well)? And vm should be free'd in the below err: handling.

[markj]

lib/libvmmapi/vmmapi.c

148

We could return NULL instead of crashing if malloc() fails to allocate address space, I don't have a strong opinion on it. IMHO it doesn't matter very much, and I didn't want to change the existing behaviour as it's not related to the goal of the patch.

The vm_close()/vm_destroy() calls handle freeing the vm structure.