OS-6401: vmm should present zone-aware sdev instances

Resolution

Fixed: A fix for this issue is checked into the tree and tested.
(Resolution Date: 2018-02-20T22:10:12.326Z)

Related Issues

Related Links

Description

Presently, bhyve is using the /devices/... path to access vmmctl and the subsequently created VM instance minors. In the interest of running it in a zone, it would be advantageous to utilize the same sdev hooks used by vnd to present zone-aware character devices.

Comments

Comment by Jerry Jelinek
Created at 2018-02-14T21:30:59.911Z
I assume the description was supposed to say /devices/pseudo/vmm@0:ctl.

Comment by Jerry Jelinek
Created at 2018-02-14T21:54:22.764Z
So far I have not found any code that actually is opening /devices/pseudo/vmm@0:ctl. I see two functions that open /dev/vmm/ctl.
vm_do_ctl
main (in bhhwcompat.c)

Comment by Mike Gerdts
Created at 2018-02-16T15:06:58.024Z
More details from a duplicate bug (OS-6614) I opened:

Currently all /dev/vmm has nodes for all zones in all zones. That's bad.  This needs to be virtualized.  From a discussion with @robert.mustacchi:
> 'zone', and any other future special devices and/or directories become
> reserved.  These reserved names seem to have no impact on vnd as all
> non-ctl vnd nodes end with a number. I'm OK just holding my nose on this.

OK. For most of this, I've been replacing vnd with vmm. I'd suggest that
you just swallow the amount of userland differences that'll come up and
do something like:

   /dev/vmm/ctl
   /dev/vmm/vm/%vm
   /dev/vmm/zone/%zone/vm/%vm (GZ only)

While this does cause some changes, it basically gets you out of the
naming conundrum. Though you'll need to probably still disallow some
amount of things (., .., names with /, probably non-alphanumeric ascii
or some other subset).

Comment by Mike Gerdts
Created at 2018-02-16T15:10:47.368Z
A couple other notes:

 

Comment by Jerry Jelinek
Created at 2018-02-20T18:47:39.283Z
Updated at 2018-02-20T18:47:59.277Z
I know things have evolved over the course of the project, so perhaps the comments above were relevant at one point, but none of that is relevant today. There is no need to restructure anything in the /dev/vmm hierarchy and no need to do anything about the sdev names. The only thing we have to do is ensure that only the ctl instance and the zone-specific instances are visible inside the zone.

In case it's not obvious why the above renaming discussion is incorrect, the reason is that the vmm device names have nothing to do with the zone names.

Comment by Mike Gerdts
Created at 2018-02-20T19:28:43.994Z
Responding to Jerry's earlier comment: I think this comes down to a design decision.  Do we follow the model of /dev/vnd/dev/net, and /dev/zcons or do we follow the model of /dev/dsk and various others that have a history that dates back to devlinks(1M) being state of the art?

The main reason for the hierarchy in the global zone is that a process in the global zone that needs to operate on an instance in a zone should not have to open $zoneroot/dev/vmm/$vm_name. In my previous comment, I alluded to the idea that bhyvectl needs to open device files. Currently it unconditionally opens /dev/vmm/<vmname>. Without a directory hierarchy like is suggested a few comments above, bhyvectl would only be able to open VMs in the current (i.e. global) zone.

Alternatives to the directory hierarchy include:

Comment by Patrick Mooney
Created at 2018-02-20T19:38:24.534Z
It seems like adding a -z <zonename paradigm to bhyvectl would be a pretty simple way to make the /dev/vmm/zones/<zonename>/<vmname> paths (following vnd's example) workable.

Comment by Jerry Jelinek
Created at 2018-02-20T19:52:54.217Z
Updated at 2018-02-20T19:54:25.496Z
Unfortunately I think this ticket has conflated a lot of issues which need to be split out. Currently I am only working on addressing the security concern around having vmm instances for other zones visible within a specific zone's /dev/vmm tree.

If we want to restructure the vmm tree for other reasons, we should file a ticket for that, but I don't see restructuring as necessary for any particular reason, and not as a blocker for initial release. Note that from what I can see, there is currently no way for a global zone process to even access devices under a zone's /dev/vmm tree by using the zoneroot path, so if we want that capability, we need a ticket for that too.

Also, I am not clear how bhyvectl is expected to be run from within a zone.

Comment by Patrick Mooney
Created at 2018-02-20T19:58:00.119Z
Other illumos distributions (or folks doing custom stuff in SmartOS) may choose to expose the bhyve device to "normal" native zones, allowing the management of instances there along side normal work.

Comment by Jerry Jelinek
Created at 2018-02-20T20:08:56.225Z
I'm not sure what "device" the previous comment is referring to (the ctl device or some set of instances from other zones). If it is for exposing all of the vmm instances for all zones into a different zone, then that seems contradictory to the entire point of the sdev plugin restricting access to only those instances specific to a zone, and is independent from whatever naming scheme is used. If it is just for the ctl device, then that seems fine.

Comment by Mike Gerdts
Created at 2018-02-20T20:14:15.959Z
I'd seen these as one chunk of work. You are right, they can be split up. I'll reopen -OS-6614- as the bug to handle the hierarchy.

Comment by Mike Gerdts
Created at 2018-02-20T20:28:39.872Z
Every zone should shall see the ctl device and any instances that were created in that zone.  A zone shall not see any device (other than the ctl device) that was created by another zone.

Until the /dev/vmm/zone hierarchy is established, the behavior in the global zone and non-global zones should be identical.  If the global zone has global visibility, various zones could create instances with the same names, causing a collision in the global zone's namespace.

It is expected that /usr/sbin/bhyve can be run in zone brands other than the bhyve brand.  In this case, /dev/vmm may contain an arbitrary number of non-ctl instances, but only those that were created within that zone.

Comment by John Levon
Created at 2018-02-20T20:38:44.087Z
vmm names are currently required to be globally unique, and it's not at all clear to me we should try to change that.

Comment by Jerry Jelinek
Created at 2018-02-20T20:41:18.308Z
Updated at 2018-02-20T20:41:35.541Z
I agree with Mike's previous comment, and that aligns with a conversation I had with Patrick too. I feel like supporting that use-case (enabling vmm to be used in non-bhyve branded zones) is something we can change in the future, if we ever have a requirement to do that. It seems like we can track that on its own ticket for future work and not delay the current bhyve work though.

Comment by Jira Bot
Created at 2018-03-01T23:15:03.775Z
illumos-joyent commit 17d35cc662a4fc13809703a0ce96563d895743d0 (branch master, by Mike Gerdts)

OS-6668 need preliminary sdev plugin for vmm
OS-6401 vmm should present zone-aware sdev instances
OS-6670 bhyve instances need to be cleaned up on zone halt
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Jerry Jelinek <jerry.jelinek@joyent.com>