[libvirt] [PATCH v2 RESEND 03/12] conf: Introduce a new PCI address extension flag

[Andrea Bolognani]

On Wed, 2018-07-25 at 16:58 +0800, Yi Min Zhao wrote:
[...]
> > > +    case VIR_DOMAIN_DEVICE_CONTROLLER:
> > > +    case VIR_DOMAIN_DEVICE_DISK:
> > > +    case VIR_DOMAIN_DEVICE_LEASE:
> > > +    case VIR_DOMAIN_DEVICE_FS:
> > > +    case VIR_DOMAIN_DEVICE_NET:
> > > +    case VIR_DOMAIN_DEVICE_INPUT:
> > > +    case VIR_DOMAIN_DEVICE_SOUND:
> > > +    case VIR_DOMAIN_DEVICE_VIDEO:
> > > +    case VIR_DOMAIN_DEVICE_HOSTDEV:
> > > +    case VIR_DOMAIN_DEVICE_WATCHDOG:
> > > +    case VIR_DOMAIN_DEVICE_GRAPHICS:
> > > +    case VIR_DOMAIN_DEVICE_HUB:
> > > +    case VIR_DOMAIN_DEVICE_REDIRDEV:
> > > +    case VIR_DOMAIN_DEVICE_SMARTCARD:
> > > +    case VIR_DOMAIN_DEVICE_MEMBALLOON:
> > > +    case VIR_DOMAIN_DEVICE_NVRAM:
> > > +    case VIR_DOMAIN_DEVICE_RNG:
> > > +    case VIR_DOMAIN_DEVICE_SHMEM:
> > > +    case VIR_DOMAIN_DEVICE_TPM:
> > > +    case VIR_DOMAIN_DEVICE_PANIC:
> > > +    case VIR_DOMAIN_DEVICE_MEMORY:
> > > +    case VIR_DOMAIN_DEVICE_IOMMU:
> > > +    case VIR_DOMAIN_DEVICE_VSOCK:
> > 
> > Did you validate that all of the above can be used with zPCI?
> 
> Yes, I did. But how far can zPCI be used is a question. I make sure that 
> if their address
> type can be PCI type zPCI address could be expanded. But we don't 
> guarantee they can
> be really used in qemu. Like RNG device can't be used because it doesn't 
> support MSIx
> which is required on S390.

If that's the case, I'm not sure the current solution is what we
want: if someone creates a guest and includes a RNG device in the
configuration, they probably expect it to, well, work. IIUC, with
the current implementation they would get a non-working RNG device
instead, which certainly feels suboptimal.

> > [...]
> > > +static virDomainPCIAddressExtensionFlags
> > > +qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr qemuCaps,
> > > +                                                  virDomainDeviceDefPtr dev)
> > > +{
> > > +    virDomainPCIAddressExtensionFlags extFlags = 0;
> > > +
> > > +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
> > > +        qemuDomainDeviceSupportZPCI(dev))
> > > +        extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
> > 
> > The libvirt code style guidelines[1] state that this should be
> > formatted as
> > 
> >    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
> >        qemuDomainDeviceSupportZPCI(dev)) {
> >        extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
> >    }
> > 
> > [1] https://libvirt.org/hacking.html
> 
> I see a lot of code in libvirt use this style. Is it new guideline?

It's certainly been around for the past 3+ years. There's a lot of
code in libvirt that's *way* older than that, though :)

-- 
Andrea Bolognani / Red Hat / Virtualization