GNOME Bugzilla – Bug 672268
Add USB redirection
Last modified: 2016-03-31 14:00:56 UTC
Add USB redirection and agent channels to newly created VMs.
Created attachment 209969 [details] [review] Add USB redirection device to newly created domains
Created attachment 209970 [details] [review] Add SPICE agent channel to newly created domains
Review of attachment 209969 [details] [review]: ::: src/vm-configurator.vala @@ +52,3 @@ domain.add_device (graphics); + // USB redirection channel in addition you should set the USB controller to USB2 (the 4 ICH9 Controllers, check virt-inst), otherwise, it won't work well for many devices
Review of attachment 209970 [details] [review]: looks ok, have you checked if the agent is connected after that ? (spicy has agent info in statusbar)
btw, you probably need to bump libvirt-glib dependencies with those patches, no?
(In reply to comment #4) > Review of attachment 209970 [details] [review]: > > looks ok, have you checked if the agent is connected after that ? (spicy has > agent info in statusbar) Seems not. :( Actually none of the patches are having the desired effect. Wonder if libvir-gconfig/libvirt is launching the correct qemu commandline: /home/zeenix/jhbuild/bin/qemu-kvm -S -M pc-1.1 -enable-kvm -m 1152 -smp 1,sockets=1,cores=1,threads=1 -name Fedora 16 -uuid ef8d54dd-343e-099f-a591-c89d9a89a25c -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/home/zeenix/.libvirt/qemu/lib/Fedora 16.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot c -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device lsi,id=scsi0,bus=pci.0,addr=0x5 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive file=/home/zeenix/.local/share/gnome-boxes/images/Fedora 16,if=none,id=drive-ide0-0-0,format=qcow2 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/home/zeenix/ISOs/Fedora-16-x86_64-DVD.iso,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -drive file=/home/zeenix/.cache/gnome-boxes/fedora16-unattended.img,if=none,id=drive-scsi0-0-1,format=raw -device scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 -netdev user,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:fd:d2:61,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0 -device usb-tablet,id=input0 -spice port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -global qxl-vga.vram_size=67108864 -device AC97,id=sound0,bus=pci.0,addr=0x4 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 At least I don't see any '/dev/virtio-ports' in F16 guest. (In reply to comment #5) > btw, you probably need to bump libvirt-glib dependencies with those patches, > no? Yup.
vdagent README says "Qemu will enable the virtio serial port when using the following params: -device virtio-serial-pci,id=virtio-serial0,max_ports=16,bus=pci.0,addr=0x5 \ -chardev spicevmc,name=vdagent,id=vdagent \ -device \ virtserialport,nr=1,bus=virtio-serial0.0,chardev=vdagent,name=com.redhat.spice.0" Your command line has: -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0 vdagent seems to be looking for /dev/virtio-ports/com.redhat.spice.0 so the missing name on the virtserialport device might be the issue. What do you have in /dev/virtio-ports/? http://hansdegoede.livejournal.com/11686.html gives more details about usb redirection. You need 2 things: * enable USB2 support in qemu (not 100% sure what this means) * -chardev spicevmc,name=usbredir,id=usbredirchardev1 \ -device usb-redir,chardev=usbredirchardev1,id=usbredirdev1,debug=3 on qemu cmdline Your command line has: -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 I think this should be fine (?)
(In reply to comment #7) > What do you have in /dev/virtio-ports/? > Just reread comment #6 and noticed the "At least I don't see any '/dev/virtio-ports' in F16 guest." bit :)
(In reply to comment #7) > Your command line has: > -chardev spicevmc,id=charredir0,name=usbredir > -device usb-redir,chardev=charredir0,id=redir0 > I think this should be fine (?) USB2: it's very much recommended to have ICH9 and companions controllers for redirection to work with many devices: it means 4 controllers: ich9-ehci1 & ich9-uhci{1,2,3{
(In reply to comment #9) > (In reply to comment #7) > > Your command line has: > > -chardev spicevmc,id=charredir0,name=usbredir > > -device usb-redir,chardev=charredir0,id=redir0 > > I think this should be fine (?) > > USB2: it's very much recommended to have ICH9 and companions controllers for > redirection to work with many devices: it means 4 controllers: ich9-ehci1 & > ich9-uhci{1,2,3{ I dont see anything about USB2 here: http://libvirt.org/formatdomain.html#elementsUSB
> I dont see anything about USB2 here: > http://libvirt.org/formatdomain.html#elementsUSB http://libvirt.org/formatdomain.html#elementsControllers UBS2 is implemented by the ICH9 controllers. You could lookup a bit Hans documentation or libvirt commits related to USB redirection and controllers (as well as prior implementation in python-virtinst).
Created attachment 210392 [details] [review] Add SPICE agent channel to newly created domains This one actually makes copy&paste work out of the box (tested against F16 guest). However, it seems now the mouse pointer disapears so very hard to copy&paste. :) Not yet sure if this is related to any of these 2 patches yet..
The fact that you had to add channel.set_target_name ("com.redhat.spice.0"); looks like a bug in libvirt. http://libvirt.org/formatdomain.html#elementCharChannel says: "an optional attribute name controls how the guest will have access to the channel, and defaults to name='com.redhat.spice.0'" I don't object to having this workaround in boxes for now.
Created attachment 210436 [details] [review] Add SPICE agent channel to newly created domains
Created attachment 210437 [details] [review] Add USB redirection device to newly created domains
Rebased so that the working 'Add SPICE agent channel to newly created domains' patch can already be merged.
Review of attachment 210437 [details] [review]: it's missing the ich9 controllers
(In reply to comment #17) > Review of attachment 210437 [details] [review]: > > it's missing the ich9 controllers Is it? Its supposed to be handled by libvirt-gconfig..
(In reply to comment #18) > > it's missing the ich9 controllers > > Is it? Its supposed to be handled by libvirt-gconfig.. What makes you think so? You have to set the controllers.
(In reply to comment #19) > (In reply to comment #18) > > > it's missing the ich9 controllers > > > > Is it? Its supposed to be handled by libvirt-gconfig.. > > What makes you think so? You have to set the controllers. Why can't libvirt-config do that? Is there some policy decision involved?
(In reply to comment #14) > Created an attachment (id=210436) [details] [review] > Add SPICE agent channel to newly created domains While the usb-redir stuff can wait till Christophe is done with the needed libvirt-gconfig parts, could we merge this one already?
Comment on attachment 210436 [details] [review] Add SPICE agent channel to newly created domains Attachment 210436 [details] pushed as 2753679 - Add SPICE agent channel to newly created domains
Created attachment 216220 [details] [review] Add USB devices to newly created domains This one adds controllers as well. The code to add controllers is mostly just a port of Christophe's example from libvirt code (Not yet merged). However, USB rediction still doesn't work for me agianst an F17 with spice-vdagent installed/running. Here is the libvirt domain config XML: <domain type='kvm' id='6'> <name>fedora17</name> <uuid>66f052f0-4342-e2ed-b33d-2b262d8d9ea2</uuid> <title>Fedora 17</title> <metadata> <boxes:os-state xmlns:boxes="http://live.gnome.org/Boxes/">installed</boxes:os-state> </metadata> <memory unit='KiB'>1179648</memory> <currentMemory unit='KiB'>1179648</currentMemory> <vcpu placement='static'>1</vcpu> <os> <type arch='x86_64' machine='pc-1.1'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'> <timer name='rtc' tickpolicy='catchup'/> <timer name='pit' tickpolicy='delay'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/home/zeenix/jhbuild/bin/qemu-kvm</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source file='/home/zeenix/.local/share/gnome-boxes/images/fedora17'/> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </disk> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/home/zeenix/.cache/gnome-boxes/fedora17-unattended.img'/> <target dev='sdb' bus='scsi'/> <alias name='scsi0-0-1'/> <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/extra-data/ISOs/Fedora-17-x86_64-DVD.iso' startupPolicy='optional'/> <target dev='hdc' bus='ide'/> <readonly/> <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='usb' index='0' model='ich9-ehci1'> <alias name='usb0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x7'/> </controller> <controller type='usb' index='0' model='ich9-uhci1'> <alias name='usb0'/> <master startport='0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0' multifunction='on'/> </controller> <controller type='usb' index='0' model='ich9-uhci2'> <alias name='usb0'/> <master startport='2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x1'/> </controller> <controller type='usb' index='0' model='ich9-uhci3'> <alias name='usb0'/> <master startport='4'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x2'/> </controller> <controller type='scsi' index='0'> <alias name='scsi0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> <controller type='ide' index='0'> <alias name='ide0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='virtio-serial' index='0'> <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> <interface type='user'> <mac address='52:54:00:20:05:5b'/> <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <source path='/dev/pts/5'/> <target port='0'/> <alias name='serial0'/> </serial> <console type='pty' tty='/dev/pts/5'> <source path='/dev/pts/5'/> <target type='serial' port='0'/> <alias name='serial0'/> </console> <channel type='spicevmc'> <target type='virtio' name='com.redhat.spice.0'/> <alias name='channel0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> <input type='tablet' bus='usb'> <alias name='input0'/> </input> <input type='mouse' bus='ps2'/> <graphics type='spice' port='5900' autoport='yes'/> <sound model='ac97'> <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='qxl' vram='65536' heads='1'/> <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <redirdev bus='usb' type='spicevmc'> <alias name='redir0'/> </redirdev> <memballoon model='virtio'> <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </memballoon> </devices> <seclabel type='dynamic' model='selinux' relabel='yes'> <label>system_u:system_r:svirt_t:s0:c148,c798</label> <imagelabel>system_u:object_r:svirt_image_t:s0:c148,c798</imagelabel> </seclabel> </domain> Qemu commandline launched by libvirt for this configuration: /home/zeenix/jhbuild/bin/qemu-kvm -S -M pc-1.1 -enable-kvm -m 1152 -smp 1,sockets=1,cores=1,threads=1 -name fedora17 -uuid 66f052f0-4342-e2ed-b33d-2b262d8d9ea2 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/home/zeenix/.config/libvirt/qemu/lib/fedora17.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -no-shutdown -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device lsi,id=scsi0,bus=pci.0,addr=0x6 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7 -drive file=/home/zeenix/.local/share/gnome-boxes/images/fedora17,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x8,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/home/zeenix/.cache/gnome-boxes/fedora17-unattended.img,if=none,id=drive-scsi0-0-1,format=raw -device scsi-hd,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 -drive file=/extra-data/ISOs/Fedora-17-x86_64-DVD.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -netdev user,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:20:05:5b,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -device usb-tablet,id=input0 -spice port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -global qxl-vga.vram_size=67108864 -device AC97,id=sound0,bus=pci.0,addr=0x4 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x9
Review of attachment 216220 [details] [review]: ::: src/vm-configurator.vala @@ +75,3 @@ + domain.add_device (usb_redir); + + // USB controllers should we assume all OS support USB2/ICH9? I think XP does, so perhaps we can..
(In reply to comment #24) > Review of attachment 216220 [details] [review]: > > ::: src/vm-configurator.vala > @@ +75,3 @@ > + domain.add_device (usb_redir); > + > + // USB controllers > > should we assume all OS support USB2/ICH9? I think XP does, so perhaps we can.. Based on our discussion on bug#676590, I was thinking we should install all useful/desired devices unless we know it'll cause problems for some OSs.
(In reply to comment #25) > Based on our discussion on bug#676590, I was thinking we should install all > useful/desired devices unless we know it'll cause problems for some OSs. It is not a fair comparison. If the OS ignores USB tablet, there is no drawback adding this device by default. Otoh, if USB controller is ignored, then it may ignore the USB tablet, which is a regression when the agent is not running.
Apparently supported since XP SP3, http://support.microsoft.com/kb/312370 Can we imagine a USB supported version property in libosinfo? Default to USB2, unless stated explicitely?
(In reply to comment #27) > Can we imagine a USB supported version property in libosinfo? Default to USB2, > unless stated explicitely? Not sure it's the best thing to do though, as the user may update it later on. Perhaps we should just go with USB2 by default. Please ask Hans :)
(In reply to comment #26) > (In reply to comment #25) > > Based on our discussion on bug#676590, I was thinking we should install all > > useful/desired devices unless we know it'll cause problems for some OSs. > > It is not a fair comparison. > > If the OS ignores USB tablet, there is no drawback adding this device by > default. > > Otoh, if USB controller is ignored, then it may ignore the USB tablet, which is > a regression when the agent is not running. Did you mistype above or I just failed to comprehend cause in first para you wrote ignoring of USB tablet is OK but in the 2nd one you said the opposite? (In reply to comment #27) > Apparently supported since XP SP3, http://support.microsoft.com/kb/312370 > > Can we imagine a USB supported version property in libosinfo? Default to USB2, > unless stated explicitely? Sure, there seem to be multiple controllers from different vendors listed under http://pciids.sourceforge.net/v2.2/pci.ids for EHCI and UHCI etc. Wonder which one to pick as libosinfo's device db entries are rather specific and I don't see any generic ones there.
(In reply to comment #28) > (In reply to comment #27) > > Can we imagine a USB supported version property in libosinfo? Default to USB2, > > unless stated explicitely? > > Not sure it's the best thing to do though, as the user may update it later on. > Perhaps we should just go with USB2 by default. Please ask Hans :) I sent him a mail today pointing him to latest patch to get his input.
(In reply to comment #28) > (In reply to comment #27) > > Can we imagine a USB supported version property in libosinfo? Default to USB2, > > unless stated explicitely? > > Not sure it's the best thing to do though, as the user may update it later on. > Perhaps we should just go with USB2 by default. Please ask Hans :) Yes, definitely just go with USB 2 by default, I would not spend any effort on making this a per guest OS settings, since the EHCI spec already solves the issue of non EHCI capable operating systems, by default all ehci controller ports are routed to their USB-1 companion controller ports. The OS must first tell the EHCI controller to switch out of this compatibility mode before the EHCI controller will come into play.
(In reply to comment #23) > Created an attachment (id=216220) [details] [review] > Add USB devices to newly created domains > > This one adds controllers as well. The code to add controllers is mostly just a > port of Christophe's example from libvirt code (Not yet merged). > > However, USB rediction still doesn't work for me agianst an F17 with > spice-vdagent installed/running. <snip xml> The generated xml looks good, have you tried connecting to the vm with remote-viewer instead of boxes itself? Note that usbredir not only needs to be enabled on the vm side, but also on the client side! As a minimum you need something like: SpiceSession *session = ...; SpceGtkSession *gtk_session = spice_gtk_session_get(session); g_object_set(gtk_session, "auto-usbredir", TRUE, NULL); Ideally you would also use a SpiceUsbDeviceWidget to allow users to manually redirect / unredirect devices, this is esp. handy for say a webcam build into a laptop as the user cannot unplug / replug that to get it redirected :) Note that SpiceUsbDeviceWidget is probably not a good UI match for boxes, but it uses only public interfaces, so you can copy paste it and modify it as you want. Or even completely start from scratch using it as an example. Regards, Hans
p.s. usbredir has nothing to do with the agent (it does not need it) and visa versa, so that really should be 2 separate bugs, and there is no need to install the agent for testing usbredir.
(In reply to comment #31) > (In reply to comment #28) > > (In reply to comment #27) > > > Can we imagine a USB supported version property in libosinfo? Default to USB2, > > > unless stated explicitely? > > > > Not sure it's the best thing to do though, as the user may update it later on. > > Perhaps we should just go with USB2 by default. Please ask Hans :) > > Yes, definitely just go with USB 2 by default, I would not spend any effort on > making this a per guest OS settings, since the EHCI spec already solves the > issue of non EHCI capable operating systems, by default all ehci controller > ports are routed to their USB-1 companion controller ports. The OS must first > tell the EHCI controller to switch out of this compatibility mode before the > EHCI controller will come into play. Ah, I'll modify the patch accordingly. (In reply to comment #32) > (In reply to comment #23) > > Created an attachment (id=216220) [details] [review] [details] [review] > > Add USB devices to newly created domains > > > > This one adds controllers as well. The code to add controllers is mostly just a > > port of Christophe's example from libvirt code (Not yet merged). > > > > However, USB rediction still doesn't work for me agianst an F17 with > > spice-vdagent installed/running. > > <snip xml> > > The generated xml looks good, have you tried connecting to the vm with > remote-viewer instead of boxes itself? Note that usbredir not only needs to be > enabled on the vm side, but also on the client side! Oh man, I forgot to enable USB redirection in the VM properties (its disabled by default for now). Will try it as soon as i get this libvirt launch issue fixed.. I guess I should enabled it by default as part of this patch. (In reply to comment #33) > p.s. > > usbredir has nothing to do with the agent (it does not need it) and visa versa, > so that really should be 2 separate bugs, and there is no need to install the > agent for testing usbredir. Oh? I didn't know that. Thats actually very good. The patch to add agent has already been pushed so no need to split the bug now. Thanks so much for your comments!
(In reply to comment #34) > (In reply to comment #31) > > (In reply to comment #28) > > > (In reply to comment #27) > > > > Can we imagine a USB supported version property in libosinfo? Default to USB2, > > > > unless stated explicitely? > > > > > > Not sure it's the best thing to do though, as the user may update it later on. > > > Perhaps we should just go with USB2 by default. Please ask Hans :) > > > > Yes, definitely just go with USB 2 by default, I would not spend any effort on > > making this a per guest OS settings, since the EHCI spec already solves the > > issue of non EHCI capable operating systems, by default all ehci controller > > ports are routed to their USB-1 companion controller ports. The OS must first > > tell the EHCI controller to switch out of this compatibility mode before the > > EHCI controller will come into play. > > Ah, I'll modify the patch accordingly. > > (In reply to comment #32) > > (In reply to comment #23) > > > Created an attachment (id=216220) [details] [review] [details] [review] [details] [review] > > > Add USB devices to newly created domains > > > > > > This one adds controllers as well. The code to add controllers is mostly just a > > > port of Christophe's example from libvirt code (Not yet merged). > > > > > > However, USB rediction still doesn't work for me agianst an F17 with > > > spice-vdagent installed/running. > > > > <snip xml> > > > > The generated xml looks good, have you tried connecting to the vm with > > remote-viewer instead of boxes itself? Note that usbredir not only needs to be > > enabled on the vm side, but also on the client side! > > Oh man, I forgot to enable USB redirection in the VM properties (its disabled > by default for now). Will try it as soon as i get this libvirt launch issue > fixed.. I guess I should enabled it by default as part of this patch. Now I did enable it from the UI but I still didn't get it working. The way I tested it was that I inserted a USB stick while guest was running in Boxes and usb redir was enabled in it's properties. I saw these in 'dmesg|tail' on host: [134169.604310] usb 2-1.1: new high-speed USB device number 3 using ehci_hcd [134169.707984] usb 2-1.1: New USB device found, idVendor=1516, idProduct=1603 [134169.707989] usb 2-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [134169.707993] usb 2-1.1: Product: Mandriva Flash [134169.707995] usb 2-1.1: Manufacturer: SKY [134169.707998] usb 2-1.1: SerialNumber: 89900000000000006CB03312 [134169.807399] Initializing USB Mass Storage driver... [134169.807508] scsi6 : usb-storage 2-1.1:1.0 [134169.807596] usbcore: registered new interface driver usb-storage [134169.807598] USB Mass Storage support registered. [134170.810519] scsi 6:0:0:0: Direct-Access SKY Mandriva Flash 1.00 PQ: 0 ANSI: 2 [134170.812824] sd 6:0:0:0: Attached scsi generic sg2 type 0 [134170.813287] sd 6:0:0:0: [sdb] 8069120 512-byte logical blocks: (4.13 GB/3.84 GiB) [134170.813966] sd 6:0:0:0: [sdb] Write Protect is off [134170.813974] sd 6:0:0:0: [sdb] Mode Sense: 23 00 00 00 [134170.814590] sd 6:0:0:0: [sdb] No Caching mode page present [134170.814596] sd 6:0:0:0: [sdb] Assuming drive cache: write through [134170.818446] sd 6:0:0:0: [sdb] No Caching mode page present [134170.818449] sd 6:0:0:0: [sdb] Assuming drive cache: write through [134170.912813] sdb: sdb1 [134170.915406] sd 6:0:0:0: [sdb] No Caching mode page present [134170.915414] sd 6:0:0:0: [sdb] Assuming drive cache: write through [134170.915420] sd 6:0:0:0: [sdb] Attached SCSI removable disk On the guest, I don't see any such messages. I wanted to try it against virt-viewer but had some issues getting it to connect to the VM. I'll try again tomorrow..
Of course! permissions: http://static.fi/~zeenix/tmp/virt-viewer-usb-redir-perm.png $ ls -l /dev/bus/usb/002/004 crw-rw-r--. 1 root root 189, 131 15.6. 15:53 /dev/bus/usb/002/004 this will be yet another permission issue for us. I wonder if Boxes is the only VM management app that needs to run as normal user. :(
USB redir works fine as normal user with remote-viewer.
(In reply to comment #37) > USB redir works fine as normal user with remote-viewer. Yeah, as discussed on IRC, I needed to build spice-gtk with polkit-devel installed.
Created attachment 216521 [details] [review] Add USB devices to newly created domains co-author: Christophe Fergeau <cfergeau@redhat.com>
So once i had the right packages and spice-gtk built correctly, the patch worked in boxes. The new version just doesn't add the ICH9_UHCI3 controller as per comment#31.
(In reply to comment #40) > So once i had the right packages and spice-gtk built correctly, the patch > worked in boxes. The new version just doesn't add the ICH9_UHCI3 controller as > per comment#31. not sure I understand that change, afaik, it is better to have it there, please ask Hans to confirm
(In reply to comment #41) > (In reply to comment #40) > > So once i had the right packages and spice-gtk built correctly, the patch > > worked in boxes. The new version just doesn't add the ICH9_UHCI3 controller as > > per comment#31. > > not sure I understand that change, afaik, it is better to have it there, please > ask Hans to confirm Right the UHCI3 controller is not an USB-3 controller as discussed on irc, it is a USB-1 controller! The EHCI controller is a USB-2 controller with 6 ports, for USB-1 it has 3 USB-1 UHCI controllers, each with 2 ports, to cover its 6 ports.
Seems this breaks VM saving: <zeenix> error: internal error unable to execute QEMU command 'migrate': State blocked by non-migratable device '0000:00:05.7/ehci' Hans tells me that its a known issue thats being worked on so I'll hold this patch until its fixed in a released Qemu.
(In reply to comment #43) > Seems this breaks VM saving: > > <zeenix> error: internal error unable to execute QEMU command 'migrate': State > blocked by non-migratable device '0000:00:05.7/ehci' > > Hans tells me that its a known issue thats being worked on so I'll hold this > patch until its fixed in a released Qemu. Good catch. Is this only happening when there is a device actually redirected? or anytime?
Created attachment 216541 [details] [review] Add USB devices to newly created domains - Adding back the 'ICH9_UHCI3' controller device. - Remvoing credits to Christophe on his request.
(In reply to comment #44) > (In reply to comment #43) > > Seems this breaks VM saving: > > > > <zeenix> error: internal error unable to execute QEMU command 'migrate': State > > blocked by non-migratable device '0000:00:05.7/ehci' > > > > Hans tells me that its a known issue thats being worked on so I'll hold this > > patch until its fixed in a released Qemu. > > Good catch. Is this only happening when there is a device actually redirected? > or anytime? Happens even if USB redirection is disabled in the UI.
(In reply to comment #46) > (In reply to comment #44) > > (In reply to comment #43) > > > Seems this breaks VM saving: > > > > > > <zeenix> error: internal error unable to execute QEMU command 'migrate': State > > > blocked by non-migratable device '0000:00:05.7/ehci' > > > > > > Hans tells me that its a known issue thats being worked on so I'll hold this > > > patch until its fixed in a released Qemu. > > > > Good catch. Is this only happening when there is a device actually redirected? > > or anytime? > > Happens even if USB redirection is disabled in the UI. And no usb device plugged in. Just by having the usb redir device in the domain config.
(In reply to comment #45) > - Remvoing credits to Christophe on his request. Ah, I didn't ask to be removed either, I just mentioned that I don't care being mentioned or not mentioned ;)
Created attachment 218036 [details] [review] Add USB devices to newly created domains Just rebased on current git master.
(In reply to comment #43) > Seems this breaks VM saving: > > <zeenix> error: internal error unable to execute QEMU command 'migrate': State > blocked by non-migratable device '0000:00:05.7/ehci' > > Hans tells me that its a known issue thats being worked on so I'll hold this > patch until its fixed in a released Qemu. The needed patches have landed in qemu/libusbredir/... and have been backported to fedora 18
(In reply to comment #50) > (In reply to comment #43) > > Seems this breaks VM saving: > > > > <zeenix> error: internal error unable to execute QEMU command 'migrate': State > > blocked by non-migratable device '0000:00:05.7/ehci' > > > > Hans tells me that its a known issue thats being worked on so I'll hold this > > patch until its fixed in a released Qemu. > > The needed patches have landed in qemu/libusbredir/... and have been backported > to fedora 18 So shall we get these patches in? I don't think this is exactly a new feature since we have had this "USB redirection" option in the properties since forever so this is more like making a feature actually work.
(In reply to comment #51) > > So shall we get these patches in? I don't think this is exactly a new feature > since we have had this "USB redirection" option in the properties since forever > so this is more like making a feature actually work. Is it working with just this patch? Don't we need to set some automatic usb redirection property on the spice widget? (maybe it's already done? I haven't checked the code). If we want to get this in 3.6.x, imo we need a way to detect whether we can support usb redirection or not, I'm not even sure the needed bits are in qemu 1.2.0. We also need to get designs for the various USB UI issues that were raised during GUADEC, but if autoredir works as expected, this can be addressed later.
(In reply to comment #52) > (In reply to comment #51) > > > > So shall we get these patches in? I don't think this is exactly a new feature > > since we have had this "USB redirection" option in the properties since forever > > so this is more like making a feature actually work. > > Is it working with just this patch? Don't we need to set some automatic usb > redirection property on the spice widget? Correct. > If we want to get this in 3.6.x, imo we need a way to detect whether we can > support usb redirection or not, I'm not even sure the needed bits are in qemu > 1.2.0. They are not in qemu-1.2.x, they are in qemu upstream and in the Fedora-18 qemu pkgs. So we probably only want to enable this on Fedora for now. Would be cool to enable it though. Also currently the migration is only tested with live migration with spice seamless, not with to disk migration. I've testing with to disk migration on my todo and expect this needs some minor qemu changes, but getting those into F-18 should not be a problem.
I've just build a new qemu for F-18+ which should allow to / from disk migration with usb-redir devices present: http://koji.fedoraproject.org/koji/buildinfo?buildID=356966
Created attachment 228213 [details] [review] Add PropertyCreationFlag to IPropertiesProvider.get_properties We will need this later for USB checks
Created attachment 228214 [details] [review] No need for SpiceDisplay to implement Boxes.IPropertiesProvider It already inherits from Display which also implements this interface.
Created attachment 228215 [details] [review] Allow properties to request a refresh of the properties This is useful is changing a property may cause a change in the set of availible properties.
Created attachment 228216 [details] [review] Add HAVE_USBREDIR configuration If this is set we assume that everything works with usb redirection in qemu, including usb device migration to/from disk etc. We auto detect this for qemu >= 1.3.0, otherwise its disabled by default. Fedora 18 has patches for this, so they will need to force it on.
Created attachment 228217 [details] [review] Add USB support in new VMs if supported If qemu is new enough to fully support usb redirection, add support for it (i.e. redirection channel and usb2 controllers) to all new VMs.
Created attachment 228218 [details] [review] Don't show usb properties if no usb support If usb redirection support is not configured in the VM, don't add usb-related options. Also, rename usb swich label to something more descriptive.
Created attachment 228219 [details] [review] Add "add usb support" button to properties This allows converting old pre-USB VMs to support USB redirection.
I'm having second thoughts about how this works. When we use the spice channel for usb redirection the guest will only see the usb devices while we're connected via spice, which is when the VM is shown in the boxes window. When we e.g. go to the collection view we will disconnect and thus lose the usb devices. I don't think its a good idea to keep all the spice connections around as that will send all drawing commands from all running VMs which is kinda stupid. I think we have two other options: usbredir using tcp, and qemu native usb redirection. I don't know what the difference between those would be, or if they are better ideas in general.
Hans says we should keep the spice widgets around when showing the collection view and other VMs, at least if any usb redirects are active. That way the USB devices will be active. This will have some effects though, as the spice server will continue sending drawing commands to all such running vms. It will also cause us to continue playing audio for these VMs, unless we mute them (which doesn't yet seem implemented in spice-gtk).
Created attachment 228547 [details] [review] Add Display.set_enable_audio This is needed to be able to mute non-visible displays
Created attachment 228548 [details] [review] Restructure the show_display() code Merge the Display.show signal handler lambda into the show_display () method that it will eventually call anyway. This centralizes all that code which lets us reuse it later.
Created attachment 228549 [details] [review] Add Display.should_keep_alive() This returns true if the connection needs to be alive even when not visible. ATM it returns true for spice displays that have USB devices connected.
Created attachment 228550 [details] [review] Keep Display objects alive if needed If Display.should_keep_alive() returns true when we leave the VM we keep the display widget around for later and reuse it when connecting again.
Review of attachment 228547 [details] [review]: ::: src/spice-display.vala @@ +109,3 @@ + mute = !enable; + if (playback_channel != null) + playback_channel.mute = mute; I am not sure that's the best way to mute in this case. The intention of this property is to reflect the guest sound card state, which is currently only guest -> client. I think it makes more sense to just disconnect/reconnect the audio channels here if the client is disconnected.
Created attachment 228585 [details] [review] Allow Machine and Display to report errors
Created attachment 228586 [details] [review] Report USB errors
Created attachment 228587 [details] [review] Add support for manual usb redirection in properties view This is not designed in the mockups, but at least it has the basic functinallity there for now.
Review of attachment 228216 [details] [review]: why a build-time check? libvirt provides a run-time capability check (virsh capabilities).
Review of attachment 228218 [details] [review]: ::: src/libvirt-machine.vala @@ +336,3 @@ + foreach (var device in inactive_config.get_devices ()) { + if (device is GVirConfig.DomainRedirdev) { + has_usb_redir = true; Instead of checking libvirt domain XML, I think we should simply rely on Spice channel list spice_session_has_channel_type (..,SPICE_CHANNEL_USBREDIR)
Review of attachment 228547 [details] [review]: ::: src/spice-display.vala @@ +15,3 @@ private BoxConfig.SyncProperty[] gtk_session_sync_properties; private bool connected; + private bool mute; It looks like set_enable_audio() could set the mute or audio property on the base class instead of having each implementation having to implement that (fwiw, I still wonder why we have a "connected" property in SpiceDisplay, but that's not the time to discuss).
Review of attachment 228549 [details] [review]: ::: src/display.vala @@ +23,3 @@ public abstract void set_enable_audio (bool enable); + public virtual bool should_keep_alive () { I don't think we can generalize this, it is specific to spice USB channels. I tend to think Display user shouldn't need to be aware. It can be handled by Spice/Vnc/Rdp only for whatever reason they may need to keep something alive (maybe similar needs for other kind of channels or for ending session properly etc..).
Review of attachment 228550 [details] [review]: ::: src/libvirt-machine.vala @@ +28,3 @@ + } else { + show_display (); + display.set_enable_audio (true); Shouldn't we enable all features/functions on show_display() and not explicitely enable/disable one by one? Imho, this would make it easier to extend and let each remote protocol implementation do what should be done to have background task still running even when the display is removed or disconnected.
Created attachment 228768 [details] [review] Better way of disabling audio Instead of using PlaybackChannel.mute (which is supposed to mirror guest mute state) we use SpiceSession.enable_audio. Neither is implemented atm, but this is the right API at least.
The latest patch uses a better way to enable/disable audio, and will be squashed with other audio patch when commited. Don't want to confuse patch ordering here in this bug though.
Review of attachment 228216 [details] [review]: We need more than USB redirection support, we also need full support for usb migrate and restore to a file. This was only added to qemu post 1.2 and added in patches to fedora.
Review of attachment 228218 [details] [review]: ::: src/libvirt-machine.vala @@ +336,3 @@ + foreach (var device in inactive_config.get_devices ()) { + if (device is GVirConfig.DomainRedirdev) { + has_usb_redir = true; We can't rely on there being a spice connection. The VM might not even be running.
Review of attachment 228218 [details] [review]: ::: src/libvirt-machine.vala @@ +336,3 @@ + foreach (var device in inactive_config.get_devices ()) { + if (device is GVirConfig.DomainRedirdev) { + has_usb_redir = true; right, good point
Review of attachment 228549 [details] [review]: ::: src/display.vala @@ +23,3 @@ public abstract void set_enable_audio (bool enable); + public virtual bool should_keep_alive () { I don't understand what you propose here. Also, its probably gonna be required for e.g. smartcards too.
Review of attachment 228549 [details] [review]: ::: src/display.vala @@ +23,3 @@ public abstract void set_enable_audio (bool enable); + public virtual bool should_keep_alive () { What does should_keep_alive mean? It seems up to implementation to know that (have redirected USB or other long running task) Since it's not clear, I don't see why the Display user needs to know. What the user want atm is the display and everything, to close/hide the display, or to close/disconnect the whole session. Display:should_keep_alive() doesn't fit very clearly imho. (it could down-cast at SpiceDisplay level and enable/disable usb, audio, printers etc.. but then casting is a bad idea, and could be done differently to do implementation specific features in Spice while keeping Machine code clean & generic)
Review of attachment 228549 [details] [review]: ::: src/display.vala @@ +23,3 @@ public abstract void set_enable_audio (bool enable); + public virtual bool should_keep_alive () { Well, it comes down to this: When the user leaves the VM (say goes to the collection view) Machine.disconnect_display() is called. This will disconnect the display and set the display to NULL. We could handle the display.disconnect_it() call and avoid actually disconnecting in case e.g. a USB device is runnning, but we have to know whether to NULL out the display or not. Especially since we can't reconnect with the same display object if we actually disconnected (that caused bugs earlier). I guess we could have a display.is_connected() thing you could check, but in practical terms it would be the same behaviour, the calling code will need to know if the display was actually disconnected.
Created attachment 228821 [details] [review] Don't keep spice connection connected if closed
We currently display the list of local usb devices to forward even when the VM is turned off or we're not connected via spice, but they don't work. Need to make these insensitive or something.
mclasen reported that it only worked for one usb device, then it complains about no 'free usb channel'. Need to look into that.
Review of attachment 228216 [details] [review]: does that mean you are going to rework your patch to use libvirt run-time capabilities?
Review of attachment 228216 [details] [review]: Do you mean that the libvirt runtime detection can handle querying the qemu version? I didn't think that was possible. Also, if so, how would you handle something like fedora where qemu is 1.2 but we have backported the prerequisite patches to make usb migration work. Right now this is meant to work by manually setting --enable-usbredir in the fedora specfile.
(In reply to comment #87) > mclasen reported that it only worked for one usb device, then it complains > about no 'free usb channel'. Need to look into that. You need one usb redir channel per USB device you want to be able to redirect, otherwise you will get this. By USB redir channel, I mean this: + // USB redirection channel + var usb_redir = new DomainRedirdev (); + usb_redir.set_bus (DomainRedirdevBus.USB);
Attachment 228213 [details] pushed as e4f8bb3 - Add PropertyCreationFlag to IPropertiesProvider.get_properties Attachment 228214 [details] pushed as afc6099 - No need for SpiceDisplay to implement Boxes.IPropertiesProvider Attachment 228215 [details] pushed as 811adbd - Allow properties to request a refresh of the properties Attachment 228216 [details] pushed as e812e71 - Add HAVE_USBREDIR configuration Attachment 228217 [details] pushed as 5803906 - Add USB support in new VMs if supported Attachment 228218 [details] pushed as cfde2d7 - Don't show usb properties if no usb support Attachment 228219 [details] pushed as 29e74fb - Add "add usb support" button to properties Attachment 228547 [details] pushed as 0f110c4 - Add Display.set_enable_audio Attachment 228548 [details] pushed as 201f6e6 - Restructure the show_display() code Attachment 228549 [details] pushed as d23082d - Add Display.should_keep_alive() Attachment 228550 [details] pushed as f21e3a5 - Keep Display objects alive if needed Attachment 228585 [details] pushed as 3c60efa - Allow Machine and Display to report errors Attachment 228586 [details] pushed as 6283816 - Report USB errors Attachment 228587 [details] pushed as c0cc69b - Add support for manual usb redirection in properties view
Based on irc discussions I pushed rebased versions of these with further changes: No configure time qemu checks, just default to no usbredir unless enabled. We can enable it later by default once qemu 1.3 is in common use. Add 4 usb redirection channels Don't show usb devices for manual redirection if not connected.
Comment on attachment 228586 [details] [review] Report USB errors This one introduces warnings from valac: spice-display.vala:44.23-44.52: warning: unhandled error `GLib.Error' var manager = UsbDeviceManager.get (session); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Created attachment 229358 [details] [review] Fix warnings about non-handled exception
Review of attachment 229358 [details] [review]: One more error we silently throw away ;) Though in this case I don't think we can do anything useful when it happens.
Comment on attachment 229358 [details] [review] Fix warnings about non-handled exception Attachment 229358 [details] pushed as 14cd13c - Fix warnings about non-handled exception
Created attachment 229467 [details] [review] build-sys: Add usbredir on/off log to configure output
Review of attachment 229467 [details] [review]: ack
Comment on attachment 229467 [details] [review] build-sys: Add usbredir on/off log to configure output Attachment 229467 [details] pushed as ea99a24 - build-sys: Add usbredir on/off log to configure output