Conversation
|
@DirectXMan12 @samhed hi there! Is there any plan or ETA for this pull request to be accepted? |
|
Hi, sorry for the delay. Would it be possible to divide the commits by functionality instead of by file? |
|
Yeah, the idea was to get as far away as possible from the existing I can rearrange the commits, sure. But dividing them by functionality On 04/26/2016 03:04 PM, Samuel wrote:
|
|
Fair enough, don't change that. I'll look further into this later this week. |
|
I appreciate that @samhed ! Let me know if you have any questions about the code or with your testing. |
|
I have looked through your code and it looks good! But as I said, I have mainly read your code. The only thing I have tested is that the normal code path works. I do not have the time to test the actual QEMU extension. |
|
I understand. Let me know if you need helping testing it. I believe there are at least 2 people testing it in the #21 bug too. |
|
i got a disconnect after i try press any key... with this fork. |
|
@mwaibel01 what browser type/version have you used? |
|
@danielhb Firefox 46.0.1 / Chrome 50.0.2661.94 m on Windows |
|
Strange, I've tested with Windows 10 (Chrome and Opera) and haven't experienced any disconnect issues. I'll have another look |
|
I've tested here again with Chrome in Windows 10 and I didn't have any disconnect issues. Only way I can see a disconnect is when I open the VNC client from virt-manager with a noVNC session started. In fact, as far as I've tested noVNC does not connect at all when a desktop VNC client is already opened. Can you provide the console logs to help debug what might be happening? Also, does this behavior exists only with these pull request? There are a few bugs about disconnect issues in noVNC ( #567 for example) that aren't related to the work done here. Thanks! |
|
Msg: Connected (encrypted) to: VM Console |
|
yes only with this pull request... |
|
@mwaibel01 first, thanks for your time in testing this pull request and reporting this issue. Only reason I may think about this happening here and not on master was that this pull request was done almost a month ago and there were lots of changes that weren't present here, and at least one of the missing patches seems to be dealing with this issue, I guess. I just updated this pull request with current noVNC master. I appreciate if you can have another go (it was a forced updated so I believe it's easier to just clone it again). If the issue still persists, then I would ask you to copy/paste the console logs again, both from this pull request and from noVNC master, so I can try to understand the difference in behavior. I know it's a lot of work but unfortunately I couldn't reproduce this issue here so, if it still happens, your logs are the only material I have to debug. Thanks again! |
|
@danielhb Hi, after a new clone i cant establish any connection: Data URI scheme cursor supported util.js:216:43
WebSocket on-error event util.js:218:43
Error while connected. util.js:220:43 sock.rQslice(0, 30): [object Uint8Array] util.js:214:43
<< WebSock.onclose |
|
@danielhb Here the console log from novnc-master without the patch: Data URI scheme cursor supported util.js:216:43
|
|
@mwaibel01 thanks for testing. I am seeing a tangible error in the middle of the trace but it's strange I cannot reproduce it here. How are you running noVNC? Are you launching with:
From the command line? You've said you're testing on Windows. Are you running noVNC source code from Windows too? |
|
@danielhb Hi no i run it from centos. with ./launch.sh --cert cert.pem with default setting to 5900 and apache with wsproxy |
|
@mwaibel01 I've updated the pull request. I believe at least one of the error messages you're seeing is gone. If possible please try with a clean clone again. Thanks! |
|
@danielhb after a new clean clone:
WebSocket on-error event util.js:218:43
Error while connected. util.js:220:43 sock.rQslice(0, 30): [object Uint8Array] util.js:214:43
<< WebSock.onclose |
|
@mwaibel01 please do the following test: using my pull request as is, instead of testing using Firefox or Chrome, use Internet Explorer or Edge. This will fail the browser verification and the message "QEMU Key Event extension enabled" would not appear, falling back to regular noVNC code. Let me know if it works (= connects to VNC session, can send any key presses even if wrong). |
|
@danielhb in IE the connection works and keypress with wrong layout. |
|
Any update on this? I'd love to see it merged. |
|
I'll try and do a code review and test in the next few days. I agree -- I'd love to see this as well. |
include/rfb.js
Outdated
| this._viewportHasMoved = false; | ||
|
|
||
| // QEMU Extended Key Event support - default to false | ||
| this._QEMU_Extended_Key_Event = false; |
There was a problem hiding this comment.
please don't mix camelCase and snake_case (e.g. use _qemuExtendedKeyEvent or _qemuExtKeyEventSupported). Additionaly, it's probably not a bad idea to put "supported" somewhere in the field name, as well.
There was a problem hiding this comment.
Good catch, haven't realized I mixed cases here. I will change it to _qemuExtKeyEventSupported in the next pass
This new file contains the XT scancode mapping that the extension will use in rfb.js file. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
In input.js, a new keyboard handler was added to deal exclusively with the QEMU key event extension. '_onKeyPress()' signature was changed to allow the same method to treat both cases. The extension will only be enabled if the browser has support for the KeyboardEvent.code property. Changes in rfb.js: - added a new extension code, QEMUExtendedKeyEvent, value -258. - handleKeyPress now receives 'keyevent' instead of 'keysym' and 'down'. Both values are retrieved from keyevent as they were in the previous signature. This method now can send QEMU RFB extended key messages if the flag was set to 'true'. - tests/test.rfb.js were changed folowing the onKeyPress() signature change. - added a new function to send the QEMU extended key message. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Added a 'QEMUKeyEventDecoder' method to deal with the key events generated when the QEMU extension is active. Another method, 'TrackQEMUKeyState', was also created with this same goal. Although both methods have similaries with the existing methods 'KeyEventDecoder' and 'TrackKeyState', specially when dealing with 'supress' and 'releaseall', the logic behind the QEMU extension does not required keysym generation for most cases (some NumPad keys are an exception) and, as such, there is no need to treat 'keyPressed' events and to handle char modifiers. 'TrackQEMUKeyState' also handles a Windows scenario where the 'AltGR' key generates CtrlLeft and AltRight keystrokes. The solution was to avoid this specific combination to be sent to the VNC server, discarding the extra 'CtrlLeft' key. Considering that the user can send CtrlLeft+AltLeft, CtrlRight+AltRight and even CtrlRight+AltLeft, this workaround to allow Windows users to use AltGR in their noVNC sessions is worthwhile. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
|
@DirectXMan12 updated the pull request with your review comments |
|
thanks! I'll take a look later this evening or early next week, and then get it merged. |
|
right, so, I tested this briefly (switched over to a DE layout locally, and watched as the current noVNC failed to handle the output of my |
|
LGTM |
|
This doesn't appear to work for me. After upgrading to HEAD, I get a bunch of messages in my VMs console: If I enable debug messages, I see a lot of messages like this in the browser's console: This is with Chrome 53 (beta), and the VM is running CentOS 6 |
|
Did some further testing with Firefox 50.0a2 and Ubuntu 16.04 (as the VM) and I'm still seeing the same thing. Is there a special configuration required on the guest in order to get this functional? |
|
If you are using the qemu keyboard option when starting the guest, remove it Em 01/09/2016 6:26 PM, "devicenull" notifications@github.com escreveu:
|
|
I have no keyboard args on my qemu command line. I have this line in my libvirt xml, which was automatically added by libvirt:
|
|
If you provide more information (debug messages from noVNC, which distro you`re using as hypervisor, etc) I can help with debugging |
|
Hello Daniel, I'm using a quest ubuntu 14, with qemu 2.5 from ubuntu cloud archive. Also I have compute nodes that still have variants of started machines with qemu 2.0. As a side note, libvirt.xml is managed by openstack here is an example: <domain type="kvm">
<uuid>e1dd1c22-7007-4ad2-b7e9-6018fd0b3dcd</uuid>
<name>instance-00000005</name>
<memory>524288</memory>
<vcpu>4</vcpu>
<metadata>
<nova:instance xmlns:nova="http://openstack.org/xmlns/libvirt/nova/1.0">
<nova:package version="14.0.0"/>
<nova:name>test</nova:name>
<nova:creationTime>2016-10-13 13:17:55</nova:creationTime>
<nova:flavor name="test">
<nova:memory>512</nova:memory>
<nova:disk>5</nova:disk>
<nova:swap>0</nova:swap>
<nova:ephemeral>0</nova:ephemeral>
<nova:vcpus>4</nova:vcpus>
</nova:flavor>
<nova:owner>
<nova:user uuid="b2a721756d604020b400b7bf9fc51849">admin</nova:user>
<nova:project uuid="6173d514c77f470c9fff46fa5ad6e0ef">admin</nova:project>
</nova:owner>
<nova:root type="image" uuid="b1507ff6-fbce-4ef8-a649-217ba0ec94fb"/>
</nova:instance>
</metadata>
<sysinfo type="smbios">
<system>
<entry name="manufacturer">OpenStack Foundation</entry>
<entry name="product">OpenStack Nova</entry>
<entry name="version">14.0.0</entry>
<entry name="serial">e465bdb1-649d-4ade-89bb-0e7f9a7cc4a6</entry>
<entry name="uuid">e1dd1c22-7007-4ad2-b7e9-6018fd0b3dcd</entry>
<entry name="family">Virtual Machine</entry>
</system>
</sysinfo>
<os>
<type>hvm</type>
<boot dev="hd"/>
<smbios mode="sysinfo"/>
</os>
<features>
<acpi/>
<apic/>
</features>
<cputune>
<shares>4096</shares>
</cputune>
<clock offset="utc">
<timer name="pit" tickpolicy="delay"/>
<timer name="rtc" tickpolicy="catchup"/>
<timer name="hpet" present="no"/>
</clock>
<cpu mode="host-model" match="exact">
<topology sockets="4" cores="1" threads="1"/>
</cpu>
<devices>
<disk type="file" device="disk">
<driver name="qemu" type="raw" cache="none" discard="unmap"/>
<source file="/var/lib/nova/instances/e1dd1c22-7007-4ad2-b7e9-6018fd0b3dcd/disk"/>
<target bus="virtio" dev="vda"/>
</disk>
<interface type="bridge">
<mac address="fa:16:3e:07:f7:f6"/>
<model type="virtio"/>
<source bridge="brq38880a52-82"/>
<target dev="tap3f6a9869-21"/>
</interface>
<serial type="file">
<source path="/var/lib/nova/instances/e1dd1c22-7007-4ad2-b7e9-6018fd0b3dcd/console.log"/>
</serial>
<serial type="pty"/>
<input type="tablet" bus="usb"/>
<graphics type="vnc" autoport="yes" keymap="en-us" listen="ip"/>
<video>
<model type="cirrus"/>
</video>
<memballoon model="virtio">
<stats period="10"/>
</memballoon>
</devices>
</domain> |
|
Sorry for the noise on this pull request. I just found #666 |
These three patches implements the QEMU Extended Key Event message support in noVNC. This extension is able to deal with any keyboard layout when noVNC connects to a VNC server that implements this extension.