diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 1d06981a617b84f2625ae0ec8ac7c25c3a1c9ff9..64a713a5f909f17c624d3d4c8df81cc6e7cc4655 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 <address> 'uid' attribute")); - return -1; + if (uid) { + if (virStrToLong_uip(uid, NULL, 0, &def.uid.value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> '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 <address> 'fid' attribute")); - return -1; + if (fid) { + if (virStrToLong_uip(fid, NULL, 0, &def.fid.value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> '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 8623e79daf8e5c9734b7b98481055582acdab448..2f9ff899d7f293caed16c4d63189bea79358e0f2 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, + _("No zPCI %s to reserve"), + name); + return -1; + } + + if (virHashLookup(set, &id->value)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("zPCI %s %o is already reserved"), - name, id); + name, id->value); return -1; } - if (virHashAddEntry(set, &id, (void*)1) < 0) { + 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) +virDomainZPCIAddressEnsureAddr(virDomainZPCIAddressIdsPtr zpciIds, + virZPCIDeviceAddressPtr addr) { - if (virDomainZPCIAddressAssignFid(fids, zpci) < 0) + if (virDomainZPCIAddressAssignUid(zpciIds->uids, addr) < 0) return -1; - if (virDomainZPCIAddressReserveFid(fids, zpci) < 0) + if (virDomainZPCIAddressAssignFid(zpciIds->fids, addr) < 0) return -1; - return 0; -} - - -static int -virDomainZPCIAddressReserveAddr(virDomainZPCIAddressIdsPtr zpciIds, - virZPCIDeviceAddressPtr addr) -{ 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 fc7fcfb0c6b3e232abebed998fd49fa56446cdcc..31ba78b950059567819d778608ff326f4ab2c066 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, "<zpci uid='0x%.4x' fid='0x%.8x'/>\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 df85b751ca35824ac049ccf46a43fca251b8465d..83e38dfc9ad35035cf492e4aed15eb9d89174d34 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 f355ddbfd50447a1d116a68c3932b8b764331c78..6e7fd59561ea134a1fe366d94d15c9cf0a766a0d 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 dc3bd8245fd5217929e5ba2efebe026f5601f6da..3954ad1109ad43a7aacc6cbcbe80a3d82b6e4106 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 b13c03759e28ea3808235cb016c8a1748e0c5b31..07826fb7ab92ad7b7c2f5ef81cc3f3bd0615846a 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 eb7ca501087cb9f7b1310843e98b62ba253781da..4843195a6701adfcf75934c00c411eec0b633e2a 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 f16d23614a6303a3f84c6030ac7307e95765b9a9..f198df5d7ce4de64bd70beaba83fd45744bbefd0 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 0000000000000000000000000000000000000000..6bfbfe611b5c1e2b424b89e0b400dd6b8225e293 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci'> + <zpci uid='0x0'/> + </address> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 248ce0781155605b773969458df1fa85a438b634..60943876466303e6d59a65ab588489c8f0bd80d6 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,