On 22.01.19 16:03, Christian Borntraeger wrote:
>
>
> On 22.01.2019 13:52, David Hildenbrand wrote:
>> On 22.01.19 13:44, Cornelia Huck wrote:
>>> On Tue, 22 Jan 2019 10:41:43 +0100
>>> David Hildenbrand <da...@redhat.com> wrote:
>>>
>>>> We decided to always create the PCI host bridge, even if 'zpci' is not
>>>> enabled (due to migration compatibility). This however right now allows
>>>> to add zPCI/PCI devices to a VM although the guest will never actually see
>>>> them, confusing people that are using a simple CPU model that has no
>>>> 'zpci' enabled - "Why isn't this working" (David Hildenbrand)
>>>>
>>>> Let's check for 'zpci' and at least print a warning that this will not
>>>> work as expected. We could also bail out, however that might break
>>>> existing QEMU commandlines.
>>>>
>>>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>>>> ---
>>>> hw/s390x/s390-pci-bus.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>> index b86a8bdcd4..e7d4f49611 100644
>>>> --- a/hw/s390x/s390-pci-bus.c
>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>> @@ -863,6 +863,11 @@ static void s390_pcihost_pre_plug(HotplugHandler
>>>> *hotplug_dev, DeviceState *dev,
>>>> {
>>>> S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
>>>>
>>>> + if (!s390_has_feat(S390_FEAT_ZPCI)) {
>>>> + warn_report("Adding PCI or zPCI devices without the 'zpci' CPU
>>>> feature."
>>>> + " The guest will not be able to see/use these
>>>> devices.");
>>>> + }
>>>> +
>>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>>>> PCIDevice *pdev = PCI_DEVICE(dev);
>>>>
>>>
>>> That's hotplug only, isn't it? IIRC coldplugging already fails?
>>>
>>
>> No, applies also to coldplugging.
>
> Back then we made this a conscious decision, because removing the bridge
> triggered a
> lot of issues regarding migration. And the current behaviour actually is a
> good
> match to the real hardware, there are PCI devices in the system that can not
> be used
> by guests. I understand that this is kind of surprising, so I am fine with
> the warn_report
> but I do not want to have a hard error right now.
>
So you agree to this patch, unmodified, correct? Thanks!
--
Thanks,
David / dhildenb
