diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 1d06981a61..64a713a5f9 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -52,29 +52,32 @@ static int virZPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { - virZPCIDeviceAddress def = { 0 }; + virZPCIDeviceAddress def = { .uid = { 0 }, .fid = { 0 } }; g_autofree char *uid = NULL; g_autofree char *fid = NULL; uid = virXMLPropString(node, "uid"); fid = virXMLPropString(node, "fid"); - if (uid && - virStrToLong_uip(uid, NULL, 0, &def.uid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse
'uid' attribute")); - return -1; + if (uid) { + if (virStrToLong_uip(uid, NULL, 0, &def.uid.value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse
'uid' attribute")); + return -1; + } + def.uid.isSet = true; } - if (fid && - virStrToLong_uip(fid, NULL, 0, &def.fid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse
'fid' attribute")); - return -1; + if (fid) { + if (virStrToLong_uip(fid, NULL, 0, &def.fid.value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse
'fid' attribute")); + return -1; + } + def.fid.isSet = true; } - if (!virZPCIDeviceAddressIsEmpty(&def) && - !virZPCIDeviceAddressIsValid(&def)) + if (!virZPCIDeviceAddressIsValid(&def)) return -1; addr->zpci = def; @@ -190,22 +193,20 @@ virDeviceInfoPCIAddressIsPresent(const virDomainDeviceInfo *info) !virPCIDeviceAddressIsEmpty(&info->addr.pci); } - bool virDeviceInfoPCIAddressExtensionIsWanted(const virDomainDeviceInfo *info) { return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) && - virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci); + virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci); } bool virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo *info) { return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) && - !virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci); + virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci); } - int virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 8623e79daf..2f9ff899d7 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -33,20 +33,27 @@ VIR_LOG_INIT("conf.domain_addr"); static int virDomainZPCIAddressReserveId(virHashTablePtr set, - unsigned int id, + virZPCIDeviceAddressID *id, const char *name) { - if (virHashLookup(set, &id)) { + if (!id->isSet) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("zPCI %s %o is already reserved"), - name, id); + _("No zPCI %s to reserve"), + name); return -1; } - if (virHashAddEntry(set, &id, (void*)1) < 0) { + if (virHashLookup(set, &id->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("zPCI %s %o is already reserved"), + name, id->value); + return -1; + } + + if (virHashAddEntry(set, &id->value, (void*)1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to reserve %s %o"), - name, id); + name, id->value); return -1; } @@ -58,7 +65,7 @@ static int virDomainZPCIAddressReserveUid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) { - return virDomainZPCIAddressReserveId(set, addr->uid, "uid"); + return virDomainZPCIAddressReserveId(set, &addr->uid, "uid"); } @@ -66,17 +73,20 @@ static int virDomainZPCIAddressReserveFid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) { - return virDomainZPCIAddressReserveId(set, addr->fid, "fid"); + return virDomainZPCIAddressReserveId(set, &addr->fid, "fid"); } static int virDomainZPCIAddressAssignId(virHashTablePtr set, - unsigned int *id, + virZPCIDeviceAddressID *id, unsigned int min, unsigned int max, const char *name) { + if (id->isSet) + return 0; + while (virHashLookup(set, &min)) { if (min == max) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -86,7 +96,9 @@ virDomainZPCIAddressAssignId(virHashTablePtr set, } ++min; } - *id = min; + + id->value = min; + id->isSet = true; return 0; } @@ -112,16 +124,20 @@ virDomainZPCIAddressAssignFid(virHashTablePtr set, static void virDomainZPCIAddressReleaseId(virHashTablePtr set, - unsigned int *id, + virZPCIDeviceAddressID *id, const char *name) { - if (virHashRemoveEntry(set, id) < 0) { + if (!id->isSet) + return; + + if (virHashRemoveEntry(set, &id->value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Release %s %o failed"), - name, *id); + name, id->value); } - *id = 0; + id->value = 0; + id->isSet = false; } @@ -145,47 +161,24 @@ static void virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) { - if (!zpciIds || virZPCIDeviceAddressIsEmpty(addr)) + if (!zpciIds) return; virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - virDomainZPCIAddressReleaseFid(zpciIds->fids, addr); } static int -virDomainZPCIAddressReserveNextUid(virHashTablePtr uids, - virZPCIDeviceAddressPtr zpci) -{ - if (virDomainZPCIAddressAssignUid(uids, zpci) < 0) - return -1; - - if (virDomainZPCIAddressReserveUid(uids, zpci) < 0) - return -1; - - return 0; -} - - -static int -virDomainZPCIAddressReserveNextFid(virHashTablePtr fids, - virZPCIDeviceAddressPtr zpci) -{ - if (virDomainZPCIAddressAssignFid(fids, zpci) < 0) - return -1; - - if (virDomainZPCIAddressReserveFid(fids, zpci) < 0) - return -1; - - return 0; -} - - -static int -virDomainZPCIAddressReserveAddr(virDomainZPCIAddressIdsPtr zpciIds, +virDomainZPCIAddressEnsureAddr(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) { + if (virDomainZPCIAddressAssignUid(zpciIds->uids, addr) < 0) + return -1; + + if (virDomainZPCIAddressAssignFid(zpciIds->fids, addr) < 0) + return -1; + if (virDomainZPCIAddressReserveUid(zpciIds->uids, addr) < 0) return -1; @@ -198,22 +191,6 @@ virDomainZPCIAddressReserveAddr(virDomainZPCIAddressIdsPtr zpciIds, } -static int -virDomainZPCIAddressReserveNextAddr(virDomainZPCIAddressIdsPtr zpciIds, - virZPCIDeviceAddressPtr addr) -{ - if (virDomainZPCIAddressReserveNextUid(zpciIds->uids, addr) < 0) - return -1; - - if (virDomainZPCIAddressReserveNextFid(zpciIds->fids, addr) < 0) { - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - return -1; - } - - return 0; -} - - int virDomainPCIAddressExtensionReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) @@ -222,7 +199,7 @@ virDomainPCIAddressExtensionReserveAddr(virDomainPCIAddressSetPtr addrs, /* Reserve uid/fid to ZPCI device which has defined uid/fid * in the domain. */ - return virDomainZPCIAddressReserveAddr(addrs->zpciIds, &addr->zpci); + return virDomainZPCIAddressEnsureAddr(addrs->zpciIds, &addr->zpci); } return 0; @@ -234,9 +211,9 @@ virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) { if (addr->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) { - virZPCIDeviceAddress zpci = { 0 }; + virZPCIDeviceAddress zpci = addr->zpci; - if (virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, &zpci) < 0) + if (virDomainZPCIAddressEnsureAddr(addrs->zpciIds, &zpci) < 0) return -1; if (!addrs->dryRun) @@ -246,6 +223,7 @@ virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs, return 0; } + static int virDomainPCIAddressExtensionEnsureAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) @@ -253,10 +231,8 @@ virDomainPCIAddressExtensionEnsureAddr(virDomainPCIAddressSetPtr addrs, if (addr->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) { virZPCIDeviceAddressPtr zpci = &addr->zpci; - if (virZPCIDeviceAddressIsEmpty(zpci)) - return virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, zpci); - else - return virDomainZPCIAddressReserveAddr(addrs->zpciIds, zpci); + if (virDomainZPCIAddressEnsureAddr(addrs->zpciIds, zpci) < 0) + return -1; } return 0; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fc7fcfb0c6..31ba78b950 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7522,11 +7522,15 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virTristateSwitchTypeToString(info->addr.pci.multi)); } - if (!virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci)) { + if (virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing uid or fid attribute of zPCI address")); + } + if (virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci)) { virBufferAsprintf(&childBuf, "\n", - info->addr.pci.zpci.uid, - info->addr.pci.zpci.fid); + info->addr.pci.zpci.uid.value, + info->addr.pci.zpci.fid.value); } break; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index df85b751ca..83e38dfc9a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2839,7 +2839,8 @@ virPCIHeaderTypeToString; virPCIIsVirtualFunction; virPCIStubDriverTypeFromString; virPCIStubDriverTypeToString; -virZPCIDeviceAddressIsEmpty; +virZPCIDeviceAddressIsIncomplete; +virZPCIDeviceAddressIsPresent; virZPCIDeviceAddressIsValid; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f355ddbfd5..6e7fd59561 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1902,10 +1902,10 @@ qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev) virBufferAsprintf(&buf, "zpci,uid=%u,fid=%u,target=%s,id=zpci%u", - dev->addr.pci.zpci.uid, - dev->addr.pci.zpci.fid, + dev->addr.pci.zpci.uid.value, + dev->addr.pci.zpci.fid.value, dev->alias, - dev->addr.pci.zpci.uid); + dev->addr.pci.zpci.uid.value); return virBufferContentAndReset(&buf); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index dc3bd8245f..3954ad1109 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -157,7 +157,7 @@ qemuDomainDetachZPCIDevice(qemuMonitorPtr mon, { g_autofree char *zpciAlias = NULL; - zpciAlias = g_strdup_printf("zpci%d", info->addr.pci.zpci.uid); + zpciAlias = g_strdup_printf("zpci%d", info->addr.pci.zpci.uid.value); if (qemuMonitorDelDevice(mon, zpciAlias) < 0) return -1; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b13c03759e..07826fb7ab 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1018,7 +1018,7 @@ static int qemuValidateDomainDeviceDefZPCIAddress(virDomainDeviceInfoPtr info, virQEMUCapsPtr qemuCaps) { - if (!virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci) && + if (virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/util/virpci.c b/src/util/virpci.c index eb7ca50108..4843195a67 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2173,12 +2173,13 @@ virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci) /* We don't need to check fid because fid covers * all range of uint32 type. */ - if (zpci->uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID || - zpci->uid == 0) { + if (zpci->uid.isSet && + (zpci->uid.value > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID || + zpci->uid.value == 0)) { virReportError(VIR_ERR_XML_ERROR, _("Invalid PCI address uid='0x%.4x', " "must be > 0x0000 and <= 0x%.4x"), - zpci->uid, + zpci->uid.value, VIR_DOMAIN_DEVICE_ZPCI_MAX_UID); return false; } @@ -2187,11 +2188,19 @@ virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci) } bool -virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr) +virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr) { - return !(addr->uid || addr->fid); + return !addr->uid.isSet || !addr->fid.isSet; } + +bool +virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr) +{ + return addr->uid.isSet || addr->fid.isSet; +} + + #ifdef __linux__ virPCIDeviceAddressPtr diff --git a/src/util/virpci.h b/src/util/virpci.h index f16d23614a..f198df5d7c 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -38,11 +38,18 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIDeviceList, virObjectUnref); #define VIR_DOMAIN_DEVICE_ZPCI_MAX_UID UINT16_MAX #define VIR_DOMAIN_DEVICE_ZPCI_MAX_FID UINT32_MAX +typedef struct _virZPCIDeviceAddressID virZPCIDeviceAddressID; typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; + +struct _virZPCIDeviceAddressID { + unsigned int value; + bool isSet; +}; + struct _virZPCIDeviceAddress { - unsigned int uid; /* exempt from syntax-check */ - unsigned int fid; + virZPCIDeviceAddressID uid; /* exempt from syntax-check */ + virZPCIDeviceAddressID fid; /* Don't forget to update virPCIDeviceAddressCopy if needed. */ }; @@ -245,8 +252,9 @@ char *virPCIDeviceAddressAsString(const virPCIDeviceAddress *addr) int virPCIDeviceAddressParse(char *address, virPCIDeviceAddressPtr bdf); +bool virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr); +bool virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr); bool virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci); -bool virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr); int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, int pfNetDevIdx, diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml b/tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml new file mode 100644 index 0000000000..6bfbfe611b --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml @@ -0,0 +1,20 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219100 + + hvm + + + /usr/bin/qemu-system-s390x + + + +
+ +
+ +
+ + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 248ce07811..6094387646 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1752,6 +1752,9 @@ mymain(void) DO_TEST("hostdev-vfio-zpci-autogenerate", QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI); + DO_TEST_PARSE_ERROR("hostdev-vfio-zpci-uid-set-zero", + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_DEVICE_ZPCI); DO_TEST("hostdev-vfio-zpci-boundaries", QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_PCI_BRIDGE,