[libvirt] [PATCH v3 23/26] conf: Introduce isolation groups
Laine Stump
laine at laine.org
Wed Jun 28 17:32:22 UTC 2017
On 06/23/2017 11:03 AM, Andrea Bolognani wrote:
> Isolation groups will eventually allow us to make sure certain
> devices, eg. PCI hostdevs, are assigned to guest PCI buses in
> a way that guarantees improved isolation, error detection and
> recovery for machine types and hypervisors that support it,
> eg. pSeries guest on QEMU.
This is a nice introduction of what isolation groups will do, but
doesn't really explain what this patch does. As I understand it, it is
just adding the isolation group to virDomainDeviceInfo (so that each
device can have an isolation group associated with it) and to
virDomainPCIAddressBus (so that each bus can have an isolation group),
then adds an isolationGroup arg (so far unused) to all the PCI address
allocation functions - presumably the next couple patches will fill in
the hypervisor-specific logic to request a particular isolation group
when desired, and the logic in the allocator to honor those requests.
That all makes sense to me.
>
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
Reviewed-by: Laine Stump <laine at laine.org>
> ---
> src/bhyve/bhyve_device.c | 4 ++--
> src/conf/device_conf.h | 10 ++++++++++
> src/conf/domain_addr.c | 17 ++++++++++++-----
> src/conf/domain_addr.h | 9 ++++++++-
> src/qemu/qemu_domain_address.c | 35 ++++++++++++++++++-----------------
> 5 files changed, 50 insertions(+), 25 deletions(-)
>
> diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> index fdfd512..03aa6c9 100644
> --- a/src/bhyve/bhyve_device.c
> +++ b/src/bhyve/bhyve_device.c
> @@ -57,7 +57,7 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> }
>
> if (virDomainPCIAddressReserveAddr(addrs, addr,
> - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) {
> + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) {
> goto cleanup;
> }
>
> @@ -100,7 +100,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
> lpc_addr.slot = 0x1;
>
> if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr,
> - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0) {
> + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) {
> goto error;
> }
>
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index 14be2e3..b9beafd 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -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;
> };
>
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index b9e9145..48af1f5 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -534,6 +534,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
> virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr addr,
> virDomainPCIConnectFlags flags,
> + int isolationGroup ATTRIBUTE_UNUSED,
> bool fromConfig)
> {
> int ret = -1;
> @@ -586,9 +587,11 @@ virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
> int
> virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr addr,
> - virDomainPCIConnectFlags flags)
> + virDomainPCIConnectFlags flags,
> + int isolationGroup)
> {
> - return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags, true);
> + return virDomainPCIAddressReserveAddrInternal(addrs, addr, flags,
> + isolationGroup, true);
> }
>
> int
> @@ -624,7 +627,8 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
> goto cleanup;
>
> ret = virDomainPCIAddressReserveAddrInternal(addrs, &dev->addr.pci,
> - flags, true);
> + flags, dev->isolationGroup,
> + true);
> } else {
> ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1);
> }
> @@ -745,6 +749,7 @@ static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
> virDomainPCIAddressGetNextAddr(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr next_addr,
> virDomainPCIConnectFlags flags,
> + int isolationGroup ATTRIBUTE_UNUSED,
> int function)
> {
> virPCIDeviceAddress a = { 0 };
> @@ -825,10 +830,12 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,
> {
> virPCIDeviceAddress addr;
>
> - if (virDomainPCIAddressGetNextAddr(addrs, &addr, flags, function) < 0)
> + if (virDomainPCIAddressGetNextAddr(addrs, &addr, flags,
> + dev->isolationGroup, function) < 0)
> return -1;
>
> - if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, false) < 0)
> + if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags,
> + dev->isolationGroup, false) < 0)
> return -1;
>
> if (!addrs->dryRun) {
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 9cd1b29..f8d63df 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -100,6 +100,12 @@ typedef struct {
> * bit is set, that function is in use by a device.
> */
> virDomainPCIAddressSlot slot[VIR_PCI_ADDRESS_SLOT_LAST + 1];
> +
> + /* See virDomainDeviceInfo::isolationGroup */
> + int isolationGroup;
> +
> + /* See virDomainDeviceInfo::isolationGroupLocked */
> + bool isolationGroupLocked;
> } virDomainPCIAddressBus;
> typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;
>
> @@ -139,7 +145,8 @@ bool virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs,
>
> int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr addr,
> - virDomainPCIConnectFlags flags)
> + virDomainPCIConnectFlags flags,
> + int isolationGroup)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 50e41a4..3d56900 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1037,7 +1037,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> }
>
> if (virDomainPCIAddressReserveAddr(addrs, addr,
> - info->pciConnectFlags) < 0) {
> + info->pciConnectFlags,
> + info->isolationGroup) < 0) {
> goto cleanup;
> }
>
> @@ -1082,6 +1083,10 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
> if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], cont->model) < 0)
> goto error;
>
> + /* Forward the information about whether or not this bus should
> + * be allowed to change isolation groups */
> + addrs->buses[idx].isolationGroupLocked = cont->info.isolationGroupLocked;
> +
> if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
> hasPCIeRoot = true;
> }
> @@ -1198,7 +1203,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> if (addrs->nbuses) {
> memset(&tmp_addr, 0, sizeof(tmp_addr));
> tmp_addr.slot = 1;
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> }
>
> @@ -1233,7 +1238,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> goto cleanup;
> }
> } else {
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> primaryVideo->info.addr.pci = tmp_addr;
> primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> @@ -1258,7 +1263,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video"
> " device will not be possible without manual"
> " intervention");
> - } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0) {
> + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) {
> goto cleanup;
> }
> }
> @@ -1334,10 +1339,8 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> assign = true;
> }
> if (assign) {
> - if (virDomainPCIAddressReserveAddr(addrs,
> - &tmp_addr, flags) < 0) {
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> - }
>
> cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> cont->info.addr.pci.domain = 0;
> @@ -1359,10 +1362,8 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> memset(&tmp_addr, 0, sizeof(tmp_addr));
> tmp_addr.slot = 0x1E;
> if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
> - if (virDomainPCIAddressReserveAddr(addrs,
> - &tmp_addr, flags) < 0) {
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> - }
>
> cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> cont->info.addr.pci.domain = 0;
> @@ -1385,12 +1386,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> tmp_addr.slot = 0x1F;
> tmp_addr.function = 0;
> tmp_addr.multi = VIR_TRISTATE_SWITCH_ON;
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
>
> tmp_addr.function = 3;
> tmp_addr.multi = VIR_TRISTATE_SWITCH_ABSENT;
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> }
>
> @@ -1424,7 +1425,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> goto cleanup;
> }
> } else {
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
> primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> primaryVideo->info.addr.pci = tmp_addr;
> @@ -1450,8 +1451,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> " device will not be possible without manual"
> " intervention");
> virResetLastError();
> - } else if (virDomainPCIAddressReserveAddr(addrs,
> - &tmp_addr, flags) < 0) {
> + } else if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0) {
> goto cleanup;
> }
> }
> @@ -1472,7 +1472,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
> !virDeviceInfoPCIAddressWanted(&sound->info)) {
> continue;
> }
> - if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags) < 0)
> + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
> goto cleanup;
>
> sound->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> @@ -1689,7 +1689,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
> if (foundAddr) {
> /* Reserve this function on the slot we found */
> if (virDomainPCIAddressReserveAddr(addrs, &addr,
> - cont->info.pciConnectFlags) < 0) {
> + cont->info.pciConnectFlags,
> + cont->info.isolationGroup) < 0) {
> goto error;
> }
>
>
More information about the libvir-list
mailing list