On Tue, Aug 09, 2016 at 10:44:44AM -0400, Eric Farman wrote:
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@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../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.
No, it should be type=scsi_host, so that in the future if we want to pass through entire SCSI or iSCSI HBAs without vhost-scsi, then we can do it.
(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?)
The IQN name has to include the LUN in it - if that's left out, QEMU would complain that no LUN was specified.
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.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|