[libvirt] [PATCH 1/3] qemu: Need to assign PCI address to vhost-scsi

Eric Farman farman at linux.vnet.ibm.com
Wed Dec 13 14:15:36 UTC 2017 


On 12/13/2017 09:05 AM, John Ferlan wrote:
> 
> 
> On 12/13/2017 08:28 AM, Eric Farman wrote:
>>
>>
>> On 12/12/2017 09:22 AM, John Ferlan wrote:
>>> Commit id '70249927b' neglected to cover this case because the test
>>> had taken the "shortcut" to already add the <address>, so I altered
>>> that too just to make sure it doesn't happen again.
>>
>> My bad.  :(
>>
> 
> No worries - I probably encouraged it as part of a test files reduction
> plan. It only became a problem once the code to assign PCI addresses got
> adjusted. Of course I neglected to make that attribution, so I adjusted
> this first paragraph to:
> 
> Commit id '70249927b' neglected to cover this case because the test
> had taken the "shortcut" to already add the <address>; however, when
> the PCI address assignment code was adjusted by commit id '70249927'
> the vhost-scsi (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) wasn't
> covered thus returning a 0 for pciFlags. So I altered the tests too
> to make sure it doesn't happen again.

Looks good.

> 
> Thanks for the quick review!

Sure!  Beats jumping back into yesterday's rabbit hole.  :)

  - Eric

> 
> John
> 
>>>
>>> Previously the qemuxml2xmloutdata was a softlink to the source
>>> qemuxml2argvdata, so I unlinked and recreated the output file to
>>> force generation of the adddress. Without the test changes, an
>>> address generation returns:
>>>
>>>       libvirt: Domain Config error : internal error: Cannot automatically
>>>       add a new PCI bus for a device with connect flags 00
>>>
>>> if an address was supplied in the test, a restart of libvirtd or
>>> edit of a guest would display the following opaque message:
>>>
>>>       warning : qemuDomainCollectPCIAddress:1237 :
>>>       qemuDomainDeviceCalculatePCIConnectFlags() thinks that the device
>>>        with PCI address 0000:00:09.0 should not have a PCI address
>>>
>>> where the address is related to the guest PCI address provided.
>>>
>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>
>> Reviewed-by: Eric Farman <farman at linux.vnet.ibm.com>
>>
>>> ---
>>>    src/qemu/qemu_domain_address.c                     |  3 +-
>>>    .../hostdev-scsi-vhost-scsi-ccw.xml                |  1 -
>>>    .../hostdev-scsi-vhost-scsi-pci.xml                |  1 -
>>>    .../hostdev-scsi-vhost-scsi-ccw.xml                | 35
>>> +++++++++++++++++-
>>>    .../hostdev-scsi-vhost-scsi-pci.xml                | 43
>>> +++++++++++++++++++++-
>>>    5 files changed, 78 insertions(+), 5 deletions(-)
>>>    mode change 120000 => 100644
>>> tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>>    mode change 120000 => 100644
>>> tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>>
>>> diff --git a/src/qemu/qemu_domain_address.c
>>> b/src/qemu/qemu_domain_address.c
>>> index a40cdb399..6e7561d39 100644
>>> --- a/src/qemu/qemu_domain_address.c
>>> +++ b/src/qemu/qemu_domain_address.c
>>> @@ -623,7 +623,8 @@
>>> qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>>>
>>>            if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>>>                (hostdev->source.subsys.type !=
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
>>> -             hostdev->source.subsys.type !=
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)) {
>>> +             hostdev->source.subsys.type !=
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
>>> +             hostdev->source.subsys.type !=
>>> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST)) {
>>>                return 0;
>>>            }
>>>
>>> diff --git a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> index 6012d241c..33234bc3d 100644
>>> --- a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> +++ b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> @@ -24,7 +24,6 @@
>>>        </controller>
>>>        <hostdev mode='subsystem' type='scsi_host' managed='no'>
>>>          <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
>>> -      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/>
>>>        </hostdev>
>>>        <memballoon model='virtio'>
>>>          <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/>
>>> diff --git a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pci.xml
>>> b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pci.xml
>>> index cb97544d3..13455f672 100644
>>> --- a/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pci.xml
>>> +++ b/tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pci.xml
>>> @@ -33,7 +33,6 @@
>>>        <input type='keyboard' bus='ps2'/>
>>>        <hostdev mode='subsystem' type='scsi_host' managed='no'>
>>>          <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
>>> -      <address type='pci' domain='0x0000' bus='0x00' slot='0x05'
>>> function='0x0'/>
>>>        </hostdev>
>>>        <memballoon model='virtio'>
>>>          <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
>>> function='0x0'/>
>>> diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> deleted file mode 120000
>>> index 39ff65968..000000000
>>> --- a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> +++ /dev/null
>>> @@ -1 +0,0 @@
>>> -../qemuxml2argvdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> \ No newline at end of file
>>> diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> new file mode 100644
>>> index 000000000..6012d241c
>>> --- /dev/null
>>> +++ b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>>> @@ -0,0 +1,34 @@
>>> +<domain type='qemu'>
>>> +  <name>QEMUGuest2</name>
>>> +  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
>>> +  <memory unit='KiB'>219100</memory>
>>> +  <currentMemory unit='KiB'>219100</currentMemory>
>>> +  <vcpu placement='static'>1</vcpu>
>>> +  <os>
>>> +    <type arch='s390x' machine='s390-ccw'>hvm</type>
>>> +    <boot dev='hd'/>
>>> +  </os>
>>> +  <clock offset='utc'/>
>>> +  <on_poweroff>destroy</on_poweroff>
>>> +  <on_reboot>restart</on_reboot>
>>> +  <on_crash>destroy</on_crash>
>>> +  <devices>
>>> +    <emulator>/usr/bin/qemu-system-s390x</emulator>
>>> +    <disk type='block' device='disk'>
>>> +      <source dev='/dev/HostVG/QEMUGuest2'/>
>>> +      <target dev='hda' bus='virtio'/>
>>> +      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
>>> +    </disk>
>>> +    <controller type='scsi' index='0' model='virtio-scsi'>
>>> +      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
>>> +    </controller>
>>> +    <hostdev mode='subsystem' type='scsi_host' managed='no'>
>>> +      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
>>> +      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/>
>>> +    </hostdev>
>>> +    <memballoon model='virtio'>
>>> +      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/>
>>> +    </memballoon>
>>> +    <panic model='s390'/>
>>> +  </devices>
>>> +</domain>
>>> diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> deleted file mode 120000
>>> index b17ea17c8..000000000
>>> --- a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> +++ /dev/null
>>> @@ -1 +0,0 @@
>>> -../qemuxml2argvdata/hostdev-scsi-vhost-scsi-pci.xml
>>> \ No newline at end of file
>>> diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> new file mode 100644
>>> index 000000000..cb97544d3
>>> --- /dev/null
>>> +++ b/tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>>> @@ -0,0 +1,42 @@
>>> +<domain type='qemu'>
>>> +  <name>QEMUGuest2</name>
>>> +  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
>>> +  <memory unit='KiB'>219100</memory>
>>> +  <currentMemory unit='KiB'>219100</currentMemory>
>>> +  <vcpu placement='static'>1</vcpu>
>>> +  <os>
>>> +    <type arch='i686' machine='pc'>hvm</type>
>>> +    <boot dev='hd'/>
>>> +  </os>
>>> +  <clock offset='utc'/>
>>> +  <on_poweroff>destroy</on_poweroff>
>>> +  <on_reboot>restart</on_reboot>
>>> +  <on_crash>destroy</on_crash>
>>> +  <devices>
>>> +    <emulator>/usr/bin/qemu-system-i686</emulator>
>>> +    <disk type='block' device='disk'>
>>> +      <source dev='/dev/HostVG/QEMUGuest2'/>
>>> +      <target dev='hda' bus='ide'/>
>>> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>>> +    </disk>
>>> +    <controller type='scsi' index='0' model='virtio-scsi'>
>>> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
>>> function='0x0'/>
>>> +    </controller>
>>> +    <controller type='usb' index='0'>
>>> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
>>> function='0x2'/>
>>> +    </controller>
>>> +    <controller type='ide' index='0'>
>>> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
>>> function='0x1'/>
>>> +    </controller>
>>> +    <controller type='pci' index='0' model='pci-root'/>
>>> +    <input type='mouse' bus='ps2'/>
>>> +    <input type='keyboard' bus='ps2'/>
>>> +    <hostdev mode='subsystem' type='scsi_host' managed='no'>
>>> +      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
>>> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x05'
>>> function='0x0'/>
>>> +    </hostdev>
>>> +    <memballoon model='virtio'>
>>> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
>>> function='0x0'/>
>>> +    </memballoon>
>>> +  </devices>
>>> +</domain>
>>>
>>
>

More information about the libvir-list mailing list