External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 06:05:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

conf: fix zPCI address auto-generation on s390

Browse Source 
Let us fix the issues with zPCI address validation and auto-generation
on s390.

Currently, there are two issues with handling the ZPCI address
extension. Firstly, when the uid is to be auto-generated with a
specified fid, .i.e.:

    ...
    <address type='pci'>
        <zpci fid='0x0000001f'/>
    </address>
    ...

we expect uid='0x0001' (or the next available uid for the domain).
However, we get a parsing error:

    $ virsh define zpci.xml
    error: XML error: Invalid PCI address uid='0x0000', must be > 0x0000
    and <= 0xffff

Secondly, when the uid is specified explicitly with the invalid
numerical value '0x0000', we actually expect the parsing error above.
However, the domain is being defined and the uid value is silently
changed to a valid value.

The first issue is a bug and the second one is undesired behaviour, and
both issues are related to how we (in-band) signal invalid values for
uid and fid. So let's fix the XML parsing to do validation based on what
is actually specified in the XML.

The first issue is also related to the current code behaviour, which
is, 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: Bjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This commit is contained in:
Shalini Chellathurai Saroja 2020-06-18 10:25:15 +02:00 committed by Andrea Bolognani
parent c125556c12
commit 076591009a
11 changed files with 124 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/conf/device_conf.c
View File

@ -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)

112
src/conf/domain_addr.c
View File

@ -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;

10
src/conf/domain_conf.c
View File

@ -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;

3
src/libvirt_private.syms
View File

@ -2839,7 +2839,8 @@ virPCIHeaderTypeToString;
virPCIIsVirtualFunction;
virPCIStubDriverTypeFromString;
virPCIStubDriverTypeToString;
virZPCIDeviceAddressIsEmpty;
virZPCIDeviceAddressIsIncomplete;
virZPCIDeviceAddressIsPresent;
virZPCIDeviceAddressIsValid;

6
src/qemu/qemu_command.c
View File

@ -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);
}

2
src/qemu/qemu_hotplug.c
View File

@ -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;

2
src/qemu/qemu_validate.c
View File

@ -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",

19
src/util/virpci.c
View File

@ -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

14
src/util/virpci.h
View File

@ -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,

20
tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml Normal file
View File

@ -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>

3
tests/qemuxml2argvtest.c
View File

@ -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,