Page MenuHomeFreeBSD

vmm: Add ability to destroy VMs on close
Needs ReviewPublic

Authored by bnovkov on Thu, Nov 13, 11:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 18, 3:40 PM
Unknown Object (File)
Tue, Nov 18, 3:37 PM
Unknown Object (File)
Tue, Nov 18, 3:36 PM
Unknown Object (File)
Sun, Nov 16, 10:41 AM
Unknown Object (File)
Sun, Nov 16, 8:32 AM
Unknown Object (File)
Sun, Nov 16, 8:25 AM
Unknown Object (File)
Sun, Nov 16, 8:25 AM
Unknown Object (File)
Sun, Nov 16, 6:03 AM
Subscribers

Details

Reviewers
markj
Group Reviewers
bhyve
Summary

This change adds the ability to tie a virtual machine's lifecycle to
a /dev/vmmctl file descriptor. A user can request vmmctl to destroy a
virtual machine on close using the VMMCTL_CREATE_DESTROY_ON_CLOSE flag
when creating the virtual machine. vmmctl tracks such virtual machines
in per-descriptor lists.

Sponsored by: The FreeBSD Foundation
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Looks good overall.

sys/dev/vmm/vmm_dev.c
1011

Let's please make it an error to specify undefined flags.

1105

This is the case where we're racing with bhyvectl --destroy, right? Shouldn't we immediately proceed to the next VM in the list in this case?

1111

Can vm_suspend() fail here? I think we don't care about errors there so please add a void cast.

bnovkov marked an inline comment as done.

Address @markj 's comments.

sys/dev/vmm/vmm_dev.c
1105

We could move on to the next element, but after giving this some though I think it would unnecessarily complicate things.

Please correct me if I'm wrong, but IMO there are two problems at play here:

  • We can't defer VM removal (i.e., move to the softc to a stack-based list) without marking sc->cdev with NULL, which forces us to immediately destroy the VM since we'd lose the cdev pointer otherwise,
  • Safe forward traversal of the list using the current element isn't really possible since we drop the lock to allow vmmdev_destroy to run. Other bhyvectl commands might invalidate a previously cached pointer to the next element.

We would also need to wait for pending bhyvectl commands by spinning on !LIST_EMPTY(priv->softcs) before freeing the per-descriptor context.

1111

Done, thanks. I've added a void cast to a similar spot in vmmdev_lookup_and_destroy as well.