Re: [libvirt] [PATCH 03/13] conf: Introduce a new PCI address extension flag

On Mon, Jun 04, 2018 at 03:52:31PM +0800, Yi Min Zhao wrote: > > > 在 2018/6/2 下午10:15, Ján Tomko 写道: >> On Thu, May 24, 2018 at 02:24:28PM +0200, Xiao Feng Ren wrote: >>> From: Yi Min Zhao <zyimin(a)linux.ibm.com&gt; >>> >>> This patch introduces a new attribute PCI address extension flag >>> to deal with the extension PCI attributes such as 'uid' and 'fid' >>> on the S390 platform. >>> >>> Signed-off-by: Yi Min Zhao <zyimin(a)linux.ibm.com&gt; >>> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; >>> --- >>> src/conf/device_conf.h         |   1 + >>> src/conf/domain_addr.h         |   5 ++ >>> src/qemu/qemu_domain_address.c | 137 >>> ++++++++++++++++++++++++++++++++++++++++- >>> 3 files changed, 141 insertions(+), 2 deletions(-) >>> >> >>> diff --git a/src/qemu/qemu_domain_address.c >>> b/src/qemu/qemu_domain_address.c >>> index b7c82cb6f1..adce399be6 100644 >>> --- a/src/qemu/qemu_domain_address.c >>> +++ b/src/qemu/qemu_domain_address.c >>> @@ -502,6 +502,60 @@ >>> qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, >>> } >>> >>> >>> +static bool >>> +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, >>> +                            virDomainDeviceDefPtr device) >>> +{ >>> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) >>> +        return false; >> >> There's no need to propagate qemuCaps all the way here, >> if we don't have QEMU_CAPS_DEVICE_ZPCI, we don't care which devices >> support it. >> > For hotplug case, qemuDomainEnsurePCIAddress() should be called. > Then if we don't have QEMU_CAPS_DEVICE_ZPCI, we should not set > zPCI extension flag. Except checking qemuCaps, I don't know there's > any other way to check zPCI support. If we don't check zPCI cap, it > would > return true for those supported device types, and then zPCI device would > be generated but qemu binary doesn't support that. > I meant checking the capability earlier and not even calling the qemuDomainDeviceSupportZPCI if the capability is not there.

>>> + >>> +    switch ((virDomainDeviceType) device->type) { >>> +    case VIR_DOMAIN_DEVICE_CONTROLLER: >>> +    case VIR_DOMAIN_DEVICE_CHR: >>> +        return false; >> >> Are these going to support it later? How will we detect that? > For now though, we won't support these in future. Actually I don't > know how to detect that via qemu interface. But anyway, we could > easily switch on support if any of them is supported on S390. The last two sentences contradict each other - if we cannot easily probe it and the devices do not support it now, by turning on the support later we will break the usage of libvirt with QEMUs where these devices did not support it yet.

Jano