On 08/09/2018 03:58 AM, Daniel P. Berrangé wrote: > On Wed, Aug 08, 2018 at 11:41:23PM -0400, Laine Stump wrote: >> On 08/08/2018 11:46 AM, Daniel P. Berrangé wrote: >>> The port allocation APIs are currently called unconditionally for all >>> types of NIC, but (mostly) only do anything for NICs with type=network. >>> >>> The exception is the port allocate API which does some validation even >>> for NICs with type!=network. Relying on this validation is flawed, >>> however, since the network driver may not even be installed, so virt >>> drivers must not delegation validation to it for NICs with >>> type!=network. >> Although I never thought through all the minute details to the end (and >> didn't need to because ,,,AllocateActualDevice() wasn't in a public >> API), I had always figured that these calls into the network driver >> would be where, eventually, we would do all of the plumbing for a >> network device, like creating the tap/macvtap device during startup, and >> disconnecting/deleting devices during domain shutdown. (it also kind of >> makes sense for nwfilters to be added/removed by the network driver >> during these APIs, since the filterref is in the <interface> definition). >> >> That's the reason for the unconditional calls. >> >> (one of the "minute details" that I hadn't thought about was the fact >> that we probably can't do *all* of the plumbing at one time - at the >> very least we need to have one API call for creating the devices and >> hooking them up, and another call that happens right before the guest >> CPUs are started (to bring everything online). Still, if we limit the >> netAllocate API to only being called for type='network' then we >> definitely will need an additional API that will likely be called >> unconditionally just after netAllocation is called just for type='network'.) > Yep, this is a bigger problem than I first considered. We have four > hypervisors using tap devices (qemu, lxc, libxl & bhyve) and all of > them have different requirements for the way the tap devices are > created. This is going to make it hard to delegate everything to the > network driver, as the work is hypervisor specific. The differences between qemu and lxc were on my mind but, without looking, I had naively assumed that "all the others" (including libxl and bhve) were just doing something similar to qemu :-/ > > I'm trying to tackle this is distinct achieveable phases. Even the > current AllocateActualDevice() method is doing too much at the same > time. For example, it deals with allocating the resources inside the > network driver (ie reserving a VF or hostdev), as well as populating > the virDomainNetActualDef struct, and invoking hook scripts. This > makes it too tangled up with the hyervisor drivers - for example > needing a full domain XML is very undesirable. I've always been hopeful that we could figure out some way to consolidate everything needed for setup/teardown into just a few well defined actions (to avoid an explosion of APIs that, as a side effect, are difficult to explain). But as always, "Easy to do" is easy to say (and again I find myself using ":-/") Anyway, as long as I know you're considering this, I'm sure you've got a better picture in your brain than I do, so Reviewed-by: Laine Stump <laine@xxxxxxxxx> for the patch. > >> BTW, once it is the responsibility of the network driver to create tap >> devices and connect them to bridges, the network driver will no longer >> be optional (unless we want to duplicate the code in the hypervisor >> drivers), but that is the price we have to pay in order to give >> unprivileged libvirt the ability to use any type of network device. > Even if the network driver can create tap devices, I'd still want to > deal with type=network vs type=bridge separately for access control > purposes. It is desirable to be able to have ACLs on the opening operation > and the natural place to attach the ACL is against the virNetworkPtr > object. We have no object to represent standalone bridge devices outside > of a virNetworkPtr, so nothing to attach an ACL to for creating tap > devices. So are you saying that the network driver simply wouldn't be able to create the tap devices for type=bridge (or type=direct)? I'm not exactly following you (although I don't doubt what you say). -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
