[libvirt] [PATCH 0/3] Fix problems found for SCSIVHost
See details in patches. John Ferlan (3): qemu: Need to assign PCI address to vhost-scsi util: Fix error path in virSCSIVHostOpenVhostSCSI util: Report error if vhost-scsi device file cannot be found src/qemu/qemu_domain_address.c | 3 +- src/util/virscsivhost.c | 8 +++- .../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 +++++++++++++++++++++- 6 files changed, 84 insertions(+), 7 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 -- 2.13.6
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. 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@redhat.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> -- 2.13.6
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. :(
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@redhat.com>
Reviewed-by: Eric Farman <farman@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>
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. Thanks for the quick review! 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@redhat.com>
Reviewed-by: Eric Farman <farman@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>
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@redhat.com>
Reviewed-by: Eric Farman <farman@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>
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.
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@redhat.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; }
Sorry I missed this before. The fix above doesn't work for domains with PCIe root (i.e. Q35) (well okay, it *does* work, but only by accident - see below) because it will end up creating a virPCIDevice from "hostAddr", which was initialized to point to hostdev->source.subsys.u.pci.addr, which is only valid for PCI hostdevs, not for SCSI_HOST or MDEV - this is why there is an early return for MDEV devices (right before the calle to virPCIDeviceNew(). In the case of mdev, we make the assumption that pcieFlags should be returned (i.e. the device should be on a PCIe controller in the guest) because all currently existing MDEV devices are in fact Express devices. Since the code as it is now will create a nonsense PCI address and try to read data from the host sysfs based on that gibberish, it will end up returning -1 for virPCIDeviceIsPCIExpress. Hmm, so it *does* end up working (if vhost-scsi devices are Express devices), but only by chance, since -1 == true, so pcieFlags will be returned, which is what you want *iff vhost-scsi devices are Express devices*. But I don't know if vhost-scsi devices are Express or legacy PCI - if you don't have any concrete idea about this, you could create a Q35 guest, add a vhost-scsi device with PCI address that's on a pcie-root-port (it will be, based on my reading of the code), then look at "lspci -v" for the SCSI device in the guest. I can help with looking at the device if you can create a Q35 guest with a vhost-scsi device (I don't have anything to connect such a device to).
We cannot be sure someone initialized the passed *vhostfd and we certainly don't want or need to be calling VIR_FORCE_CLOSE on what probably is -1. So let's just return -1 immediately. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virscsivhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index dc7df757a..d6fbf5ed5 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -87,7 +87,7 @@ int virSCSIVHostOpenVhostSCSI(int *vhostfd) { if (!virFileExists(VHOST_SCSI_DEVICE)) - goto error; + return -1; *vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR); -- 2.13.6
On 12/12/2017 09:22 AM, John Ferlan wrote:
We cannot be sure someone initialized the passed *vhostfd and we certainly don't want or need to be calling VIR_FORCE_CLOSE on what probably is -1. So let's just return -1 immediately.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
--- src/util/virscsivhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index dc7df757a..d6fbf5ed5 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -87,7 +87,7 @@ int virSCSIVHostOpenVhostSCSI(int *vhostfd) { if (!virFileExists(VHOST_SCSI_DEVICE)) - goto error; + return -1;
*vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR);
https://bugzilla.redhat.com/show_bug.cgi?id=1523564 If the vhost-scsi device file cannot be found, the generic error "error: An error occurred, but the cause is unknown" is returned. Let's add a real error message to make it clear why the failure occurred. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virscsivhost.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index d6fbf5ed5..5f176e177 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -86,8 +86,12 @@ VIR_ONCE_GLOBAL_INIT(virSCSIVHost) int virSCSIVHostOpenVhostSCSI(int *vhostfd) { - if (!virFileExists(VHOST_SCSI_DEVICE)) + if (!virFileExists(VHOST_SCSI_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vhost-scsi device file '%s' cannot be found"), + VHOST_SCSI_DEVICE); return -1; + } *vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR); -- 2.13.6
On 12/12/2017 09:22 AM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1523564
If the vhost-scsi device file cannot be found, the generic error
"error: An error occurred, but the cause is unknown"
is returned. Let's add a real error message to make it clear why the failure occurred.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
--- src/util/virscsivhost.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index d6fbf5ed5..5f176e177 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -86,8 +86,12 @@ VIR_ONCE_GLOBAL_INIT(virSCSIVHost) int virSCSIVHostOpenVhostSCSI(int *vhostfd) { - if (!virFileExists(VHOST_SCSI_DEVICE)) + if (!virFileExists(VHOST_SCSI_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vhost-scsi device file '%s' cannot be found"), + VHOST_SCSI_DEVICE); return -1; + }
*vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR);
participants (3)
-
Eric Farman -
John Ferlan -
Laine Stump