Re: [libvirt] [PATCH 00/10] conf: Clean up virDomain*Def creation
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.
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)