On Sat, 2017-07-15 at 22:28 -0400, Laine Stump wrote: > > + /* The isolation group for a host device is its IOMMU group, > > + * increased by one: this is because zero is a valid IOMMU group but > > + * that's also the default isolation group, which we want to save > > + * for emulated devices. Shifting isolation groups for host devices > > + * by one ensures there is no overlap */ > > Which is fine because this is only used internally - we never actually > try to relate this internal number to the external number. > > > Of course, the whole reason you're doing this is so that the default > value can be 0, making it easier to initialize objects. Otherwise, we > could make the default group -1 (or UINT_MAX) and then there would be no > confusion (e.g. if someone was looking at the debug log message a few > lines down from here). > > So if the simplified initialization of objects is a good tradeoff in > exchange for confusion when looking at debug output which probably > nobody will ever look at, then I guess this works. I tried pushing for having explicit allocation/initialization functions for structs rather than using VIR_ALLOC(), which for all intents and purposes means mandating the default value to be zero, but there was pushback. Since this alternative approach ultimately achieves the same result, I thought it would be best to just drop it. Another factor to keep in mind is that I took great care in making sure "isolation groups" is the only name used for the concept outside of this function, which means unless you're looking at this specific bit of code you should not even realize there is a correlation between isolation groups and IOMMU groups. > > +++ b/src/qemu/qemu_hotplug.c > > @@ -1476,6 +1476,13 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, > > > > if (qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1) < 0) > > goto error; > > + > > + if (qemuDomainIsPSeries(vm->def)) { > > + /* Isolation groups are only relevant for pSeries guests */ > > + if (qemuDomainFillDeviceIsolationGroup(vm->def, &dev) < 0) > > + goto error; > > + } > > Hotplug of course will work only if a PHB already exists that has the > right iommu group. It only needs to exist and not have any device with a different isolation group plugged in: an existing, empty PHB will simply change its isolation group to accept the hotplugged device. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list