[libvirt] [PATCH 00/10] conf: Clean up virDomain*Def creation

[Peter Krempa]

On Fri, Jun 30, 2017 at 12:24:33 +0200, Andrea Bolognani wrote:
> On Fri, 2017-06-30 at 10:58 +0200, Peter Krempa wrote:
> > > Or we could just, you know, do the sensible thing and
> > > store (IOMMU group + 1) instead of (IOMMU group) in
> > 
> > How is that sensible? That looks as a source of bugs in the long run.
> 
> Isolation groups are used to make sure any given device ends
> up on the same bus as related devices and on a different bus
> as unrelated devices.
> 
> They're an abstract concept, and while working on the initial
> implementation it just happened to be convenient for me to
> have the isolation group match the IOMMU group. There's no
> specific reason that has to be the case.

Fair enough. The documentation you are adding in the linked series is
vague enough to alow this meaning too:

@@ -164,6 +164,16 @@ struct _virDomainDeviceInfo {
      */
     int pciConnectFlags; /* enum virDomainPCIConnectFlags */
     char *loadparm;
+
+    /* PCI devices will only be automatically placed on a PCI bus
+     * that shares the same isolation group */
+    int isolationGroup;
+
+    /* Usually, PCI buses will take on the same isolation group
+     * as the first device that is plugged into them, but in some
+     * cases we might want to prevent that from happening by
+     * locking the isolation group */
+    bool isolationGroupLocked;
 };

> We're never converting back and forth between the two, which
> I agree would end up in misery at some point down the line;
> we just set the isolation group once per device and then just
> perform comparison between isolation groups from there on.

I'd suggest you create a helper to assign those then (be it from IOMMU
group or something else), so there's at least a single point that can be
referenced in the future and which will explain this reasoning.

Also adding a note that 0 means the device is not isolated would make
sense in the structure above.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170630/62c783e5/attachment-0001.sig>