On 08/09/2016 08:22 AM, Daniel P. Berrange wrote:
On Tue, Aug 09, 2016 at 02:13:29PM +0200, Michal Privoznik wrote:
On 25.07.2016 22:48, Eric Farman wrote:
The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi
Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
---
.../qemuxml2argv-hostdev-scsi-vhost-scsi.args | 24 ++++++++++++++++
.../qemuxml2argv-hostdev-scsi-vhost-scsi.xml | 33 ++++++++++++++++++++++
tests/qemuxml2argvmock.c | 12 ++++++++
tests/qemuxml2argvtest.c | 3 ++
4 files changed, 72 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
new file mode 100644
index 0000000..a2c7c18
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest2 \
+-S \
+-M pc \
+-m 214 \
+-smp 1 \
Because this patch has been sitting on the list for too long, this part
of command line is now being generated slightly differently. Otherwise
looking good.
Fair enough, that's probably a common thread throughout your comments.
Easy to fixup.
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
new file mode 100644
index 0000000..3fc4ef0
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
@@ -0,0 +1,33 @@
+<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</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'/>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <hostdev mode='subsystem' type='scsi'>
+ <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
+ </hostdev>
I'm not sure this syntax really makes sense.
IIRC, currently <hostdev type="scsi"> is used to passthrough an individual
SCSI LUNs from the host to the guest.
With vhost-scsi, IIUC, you are passing through an entire (virtual) SCSI HBA
to the guest. So I think it better for us to not reuse type="scsi" for that
purpose. Instead it feels like we should be adding a type="scsi_host" for
passthrough of entire HBAs
Would that cause confusion amongst an existing type='scsi' hostdev,
since a "scsi_hostX" turns up in the source tag? Example from the
libvirt doc:
<devices>
<hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'>
<source>
<adapter name='scsi_host0'/>
<address bus='0' target='0' unit='0'/>
</source>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</hostdev>
</devices>
Could make it be "type='vhost_scsi'" or something else, if that
clarifies things.
(Question from my own ignorance; does an iSCSI target with more than one
LUN behind it get rejected if specified here, instead of using a <pool>
tag?)
Obviously this has major implications for almost all patches in this series
Splitting this out as a new hostdev type would mean it's not being
shoehorned into the existing SCSI parts. That would probably make the
entire series less unwieldy by dropping the separate protocol that
exists herein.
- Eric
Regards,
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list