[PATCH libvirt v1 3/6] conf: fix ZPCI address auto-generation on s390

Currently, if uid/fid is specified as zero by the user, it is treated as if the value is not specified. This bug is fixed by introducing two boolean values to identify if uid and fid are specified by the user. Also, if either uid or fid is specified by the user, it is incorrectly assumed that both uid and fid are specified. This bug is fixed by identifying when the user specified ZPCI address is incomplete and auto-generating the missing ZPCI address. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; Signed-off-by: Shalini Chellathurai Saroja <shalini(a)linux.ibm.com&gt; Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- src/conf/device_conf.c | 12 ++-- src/conf/domain_addr.c | 59 +++++++++++++------ src/conf/domain_conf.c | 2 +- src/libvirt_private.syms | 3 +- src/qemu/qemu_validate.c | 2 +- src/util/virpci.c | 17 ++++-- src/util/virpci.h | 5 +- .../hostdev-vfio-zpci-multidomain-many.args | 6 +- .../hostdev-vfio-zpci-multidomain-many.xml | 6 +- 9 files changed, 75 insertions(+), 37 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 001ed929..c03356e7 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -65,8 +65,7 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node, _("Cannot parse <address> 'uid' attribute")); return -1; } - if (!virZPCIDeviceAddressIsValid(&def)) - return -1; + def.uid_set = true; } if (fid) { @@ -75,8 +74,11 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node, _("Cannot parse <address> 'fid' attribute")); return -1; } + def.fid_set = true; } + if (!virZPCIDeviceAddressIsValid(&def)) + return -1; addr->zpci = def; @@ -191,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 e8629b3e..d1cd0804 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -145,12 +145,18 @@ static void virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) { - if (!zpciIds || virZPCIDeviceAddressIsEmpty(addr)) + if (!zpciIds) return; - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + if (addr->uid_set) { + virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + addr->uid_set = false; + } - virDomainZPCIAddressReleaseFid(zpciIds->fids, addr); + if (addr->fid_set) { + virDomainZPCIAddressReleaseFid(zpciIds->fids, addr); + addr->fid_set = false; + } } @@ -186,12 +192,16 @@ static int virDomainZPCIAddressReserveAddr(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) { - if (virDomainZPCIAddressReserveUid(zpciIds->uids, addr) < 0) + if (addr->uid_set && + virDomainZPCIAddressReserveUid(zpciIds->uids, addr) < 0) return -1; - if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) { - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - return -1; + if (addr->fid_set) { + if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) { + if (addr->uid_set) + virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + return -1; + } } return 0; @@ -202,14 +212,28 @@ static int virDomainZPCIAddressReserveNextAddr(virDomainZPCIAddressIdsPtr zpciIds, virZPCIDeviceAddressPtr addr) { - if (virDomainZPCIAddressReserveNextUid(zpciIds->uids, addr) < 0) - return -1; + bool uid_set, fid_set = false; - if (virDomainZPCIAddressReserveNextFid(zpciIds->fids, addr) < 0) { - virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); - return -1; + if (!addr->uid_set) { + if (virDomainZPCIAddressReserveNextUid(zpciIds->uids, addr) < 0) + return -1; + uid_set = true; + } + + if (!addr->fid_set) { + if (virDomainZPCIAddressReserveNextFid(zpciIds->fids, addr) < 0) { + if (uid_set) + virDomainZPCIAddressReleaseUid(zpciIds->uids, addr); + return -1; + } + fid_set = true; } + if (uid_set) + addr->uid_set = true; + if (fid_set) + addr->fid_set = true; + return 0; } @@ -234,7 +258,7 @@ 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) return -1; @@ -246,6 +270,7 @@ virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs, return 0; } + static int virDomainPCIAddressExtensionEnsureAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) @@ -253,10 +278,10 @@ 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 (virDomainZPCIAddressReserveAddr(addrs->zpciIds, zpci) < 0) + return -1; + + return virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, zpci); } return 0; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c478d795..5df80cae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7471,7 +7471,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virTristateSwitchTypeToString(info->addr.pci.multi)); } - if (!virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci)) { + if (!virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci)) { virBufferAsprintf(&childBuf, "<zpci uid='0x%.4x' fid='0x%.8x'/>\n", info->addr.pci.zpci.uid, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec367653..18687563 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2821,7 +2821,8 @@ virPCIHeaderTypeToString; virPCIIsVirtualFunction; virPCIStubDriverTypeFromString; virPCIStubDriverTypeToString; -virZPCIDeviceAddressIsEmpty; +virZPCIDeviceAddressIsIncomplete; +virZPCIDeviceAddressIsPresent; virZPCIDeviceAddressIsValid; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 149b9e73..22fdfd11 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -952,7 +952,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 6c7e6bbc..b38f717c 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2172,8 +2172,9 @@ 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_set && + (zpci->uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID || + zpci->uid == 0)) { virReportError(VIR_ERR_XML_ERROR, _("Invalid PCI address uid='0x%.4x', " "must be > 0x0000 and <= 0x%.4x"), @@ -2186,11 +2187,19 @@ virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci) } bool -virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr) +virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr) { - return !(addr->uid || addr->fid); + return !addr->uid_set || !addr->fid_set; } + +bool +virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr) +{ + return addr->uid_set || addr->fid_set; +} + + #ifdef __linux__ virPCIDeviceAddressPtr diff --git a/src/util/virpci.h b/src/util/virpci.h index f16d2361..e937348f 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -43,6 +43,8 @@ typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; struct _virZPCIDeviceAddress { unsigned int uid; /* exempt from syntax-check */ unsigned int fid; + bool uid_set; + bool fid_set; /* Don't forget to update virPCIDeviceAddressCopy if needed. */ }; @@ -245,8 +247,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-multidomain-many.args b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args index 3dd9a25f..0fae78db 100644 --- a/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args @@ -27,11 +27,11 @@ server,nowait \ -device vfio-pci,host=0001:00:00.0,id=hostdev0,bus=pci.0,addr=0x3 \ -device zpci,uid=53,fid=104,target=hostdev1,id=zpci53 \ -device vfio-pci,host=0002:00:00.0,id=hostdev1,bus=pci.0,addr=0x1 \ --device zpci,uid=1,fid=1,target=hostdev2,id=zpci1 \ +-device zpci,uid=1,fid=0,target=hostdev2,id=zpci1 \ -device vfio-pci,host=0003:00:00.0,id=hostdev2,bus=pci.0,addr=0x2 \ --device zpci,uid=2,fid=2,target=hostdev3,id=zpci2 \ +-device zpci,uid=2,fid=1,target=hostdev3,id=zpci2 \ -device vfio-pci,host=0004:00:00.0,id=hostdev3,bus=pci.0,addr=0x5 \ --device zpci,uid=83,fid=0,target=hostdev4,id=zpci83 \ +-device zpci,uid=83,fid=2,target=hostdev4,id=zpci83 \ -device vfio-pci,host=0005:00:00.0,id=hostdev4,bus=pci.0,addr=0x7 \ -device zpci,uid=3,fid=114,target=hostdev5,id=zpci3 \ -device vfio-pci,host=0006:00:00.0,id=hostdev5,bus=pci.0,addr=0x9 \ diff --git a/tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml b/tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml index e56106d1..ecfc15c4 100644 --- a/tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml +++ b/tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml @@ -39,7 +39,7 @@ <address domain='0x0003' bus='0x00' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'> - <zpci uid='0x0001' fid='0x00000001'/> + <zpci uid='0x0001' fid='0x00000000'/> </address> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> @@ -48,7 +48,7 @@ <address domain='0x0004' bus='0x00' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'> - <zpci uid='0x0002' fid='0x00000002'/> + <zpci uid='0x0002' fid='0x00000001'/> </address> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> @@ -57,7 +57,7 @@ <address domain='0x0005' bus='0x00' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'> - <zpci uid='0x0053' fid='0x00000000'/> + <zpci uid='0x0053' fid='0x00000002'/> </address> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> -- 2.21.1