Re: [libvirt] [PATCH v2 RESEND 03/12] conf: Introduce a new PCI address extension flag
在 2018/7/25 下午9:31, 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.
Actually the guest can't startup
with RNG device instead of a running guest
with a non-working RNG device and qemu would report error. The user can
get the information regarding startup failure. I think this is proper.
If RNG could support MSIx in future, we won't need to do any thing in
Libvirt.
>> [...]
>>> +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 :)