summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoranonym <anonym@riseup.net>2017-01-26 11:08:35 +0100
committeranonym <anonym@riseup.net>2017-01-26 11:08:35 +0100
commitf7957b3a7ca3bd08d46e680b2c7238d8900e528a (patch)
tree6fbd806f66c887e0d158c6cbf4e3af0ec8bdb4a2
parent27370365ad57b5328e3470413f5ba95490c52f8a (diff)
Remote shell: switch from serial to virtio transport.feature/11888-remote-shell-virtio-transport
Let's revive this code and make another attempt at this 1000x (throughput) performance improvement.
-rwxr-xr-xconfig/chroot_local-includes/usr/local/lib/tails-autotest-remote-shell42
-rw-r--r--features/domains/default.xml4
-rwxr-xr-xfeatures/scripts/vm-execute27
-rw-r--r--features/support/helpers/remote_shell.rb31
-rw-r--r--features/support/helpers/vm_helper.rb41
5 files changed, 124 insertions, 21 deletions
diff --git a/config/chroot_local-includes/usr/local/lib/tails-autotest-remote-shell b/config/chroot_local-includes/usr/local/lib/tails-autotest-remote-shell
index 8660cd6..bbd8eca 100755
--- a/config/chroot_local-includes/usr/local/lib/tails-autotest-remote-shell
+++ b/config/chroot_local-includes/usr/local/lib/tails-autotest-remote-shell
@@ -10,7 +10,6 @@ import io
import json
import os
import pwd
-import serial
import signal
import subprocess
import sys
@@ -18,7 +17,7 @@ import systemd.daemon
import textwrap
import traceback
-REMOTE_SHELL_DEV = '/dev/ttyS0'
+REMOTE_SHELL_DEV = '/dev/virtio-ports/org.tails.remote_shell.0'
def mk_switch_user_fn(user):
@@ -124,15 +123,44 @@ def run_cmd_as_user(cmd, user):
def main():
- port = serial.Serial(port = REMOTE_SHELL_DEV, baudrate = 4000000)
python_sessions = dict()
+ fd = os.open(REMOTE_SHELL_DEV, os.O_RDWR)
+ port = open(fd, 'wb+', buffering=0)
+ # In order to avoid busy-waiting when polling the above character
+ # device for new data sent from clients, we borrow the approach used
+ # by python-negotiator (https://github.com/xolox/python-negotiator):
+ # We add O_ASYNC so a SIGIO signal is sent to us whenever data is
+ # ready to be read from the device.
+ flags = fcntl.fcntl(fd, fcntl.F_GETFL)
+ fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_ASYNC)
+ fcntl.fcntl(fd, fcntl.F_SETOWN, os.getpid())
+ # By default receiving a SIGIO terminates the process so we override
+ # it to do nothing instead.
+ signal.signal(signal.SIGIO, lambda *args: None)
# Notify systemd that we're ready
systemd.daemon.notify('READY=1')
systemd.daemon.notify('STATUS=Processing requests...\n')
while True:
+ # We can avoid an unnecessary delay of up to one second during the
+ # first iteration of this loop; if a client sends a request
+ # before we run SETOWN above, the expected signal never
+ # reaches us, and we enter the loop and pass through the
+ # conditional doomed to a full second of mournful waiting
+ # for this lost signal. :_( By trying to read before we do a
+ # timed wait for the signal, this delay is avoided.
line = port.readline().decode('utf-8')
+ if line == "":
+ # In case the SIGIO gets lost for whatever reason
+ # (e.g. the one mentioned above), let's always poll at
+ # least once every second.
+ try:
+ signal.sigtimedwait([signal.SIGIO], 1)
+ except InterruptedError:
+ # Thrown if any other signal is received.
+ pass
+ continue
try:
id, cmd_type, *rest = json.loads(line)
ret = ""
@@ -173,7 +201,13 @@ def main():
else:
raise ValueError("unknown command type")
response = (ret + "\n").encode('utf-8')
- port.write(response)
+ # We can only write 2**15 bytes at a time to the virtio channel
+ # (seems to only affect the guest -> host direction).
+ chunk_size = 2**15
+ chunks = (response[0+i:chunk_size+i] for i in \
+ range(0, len(response), chunk_size))
+ for chunk in chunks:
+ port.write(chunk)
port.flush()
except Exception as e:
print("Error caught while processing line:", file=sys.stderr)
diff --git a/features/domains/default.xml b/features/domains/default.xml
index 431dcde..0b7adf4 100644
--- a/features/domains/default.xml
+++ b/features/domains/default.xml
@@ -27,10 +27,6 @@
<model type='virtio'/>
<link state='up'/>
</interface>
- <serial type='tcp'>
- <source mode="bind" host='127.0.0.1' service='1337'/>
- <target port='0'/>
- </serial>
<input type='tablet' bus='usb'/>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
diff --git a/features/scripts/vm-execute b/features/scripts/vm-execute
index 06ff5a2..951d537 100755
--- a/features/scripts/vm-execute
+++ b/features/scripts/vm-execute
@@ -1,6 +1,8 @@
#!/usr/bin/env ruby
+require 'libvirt'
require 'optparse'
+require 'rexml/document'
begin
require "#{`git rev-parse --show-toplevel`.chomp}/features/support/helpers/remote_shell.rb"
require "#{`git rev-parse --show-toplevel`.chomp}/features/support/helpers/misc_helpers.rb"
@@ -12,8 +14,29 @@ $config = Hash.new
def debug_log(*args) ; end
class FakeVM
- def get_remote_shell_port
- 1337
+ def initialize
+ domain = nil
+ @domain_xml = ""
+ begin
+ virt = Libvirt::open("qemu:///system")
+ domain = virt.lookup_domain_by_name("TailsToaster")
+ @domain_xml = domain.xml_desc
+ rescue
+ raise "There was a problem with the TailsToaster VM (is it running?)"
+ ensure
+ virt.close
+ end
+ end
+
+ def remote_shell_socket_path
+ rexml = REXML::Document.new(@domain_xml)
+ rexml.elements.each('domain/devices/channel') do |e|
+ target = e.elements['target']
+ if target.attribute('name').to_s == 'org.tails.remote_shell.0'
+ return e.elements['source'].attribute('path').to_s
+ end
+ end
+ raise "The running TailsToaster has no remote shell channel!"
end
end
diff --git a/features/support/helpers/remote_shell.rb b/features/support/helpers/remote_shell.rb
index b890578..975c40c 100644
--- a/features/support/helpers/remote_shell.rb
+++ b/features/support/helpers/remote_shell.rb
@@ -7,6 +7,12 @@ module RemoteShell
class ServerFailure < StandardError
end
+ # This exception is *only* supposed to be use internally in
+ # communicate() -- in particular it must not be raised by a
+ # Timeout.timeout() wrapping around communicate() or any use of it.
+ class SocketReadTimeout < Exception
+ end
+
# Used to differentiate vs Timeout::Error, which is thrown by
# try_for() (by default) and often wraps around remote shell usage
# -- in that case we don't want to catch that "outer" exception in
@@ -21,7 +27,7 @@ module RemoteShell
def communicate(vm, *args, **opts)
opts[:timeout] ||= DEFAULT_TIMEOUT
- socket = TCPSocket.new("127.0.0.1", vm.get_remote_shell_port)
+ socket = UNIXSocket.new(vm.remote_shell_socket_path)
id = (@@request_id += 1)
# Since we already have defined our own Timeout in the current
# scope, we have to be more careful when referring to the Timeout
@@ -31,7 +37,28 @@ module RemoteShell
socket.puts(JSON.dump([id] + args))
socket.flush
loop do
- line = socket.readline("\n").chomp("\n")
+ # Calling socket.readline() and then just wait for the data to
+ # arrive is prone to stalling for some reason. A timed read()
+ # can perhaps work around this.
+ line_init = nil
+ begin
+ Object::Timeout.timeout(1, SocketReadTimeout) do
+ line_init = socket.read(1)
+ end
+ rescue SocketReadTimeout
+ next
+ end
+ # There may be a race above: imagine if we time out after
+ # reading one byte from the socket, but before it's stored in
+ # our variable. That would mean we lose that byte. Note that
+ # we always must succeed reading one byte *and* storing it the
+ # next time, so at most one byte can be lost. Luckily, since
+ # we know what the first byte must be, when can easily detect
+ # and correct for this.
+ if line_init != '['
+ line_init = '[' + line_init
+ end
+ line = line_init + socket.readline("\n").chomp("\n")
response_id, status, *rest = JSON.load(line)
if response_id == id
if status != "success"
diff --git a/features/support/helpers/vm_helper.rb b/features/support/helpers/vm_helper.rb
index bbfdfa6..5012338 100644
--- a/features/support/helpers/vm_helper.rb
+++ b/features/support/helpers/vm_helper.rb
@@ -81,6 +81,7 @@ class VM
@display = Display.new(@domain_name, x_display)
set_cdrom_boot(TAILS_ISO)
plug_network
+ add_remote_shell_channel
rescue Exception => e
destroy_and_undefine
raise e
@@ -450,6 +451,37 @@ EOF
end
end
+ def remote_shell_socket_path
+ domain_rexml = REXML::Document.new(@domain.xml_desc)
+ domain_rexml.elements.each('domain/devices/channel') do |e|
+ target = e.elements['target']
+ if target.attribute('name').to_s == 'org.tails.remote_shell.0'
+ return e.elements['source'].attribute('path').to_s
+ end
+ end
+ return nil
+ end
+
+ def add_remote_shell_channel
+ if is_running?
+ raise "The remote shell channel can only be added for inactive vms"
+ end
+ if @remote_shell_socket_path.nil?
+ @remote_shell_socket_path =
+ "/tmp/remote-shell_" + random_alnum_string(8) + ".socket"
+ end
+ channel_xml = <<-EOF
+ <channel type='unix'>
+ <source mode="bind" path='#{@remote_shell_socket_path}'/>
+ <target type='virtio' name='org.tails.remote_shell.0'/>
+ </channel>
+ EOF
+ channel_rexml = REXML::Document.new(channel_xml)
+ domain_xml = REXML::Document.new(@domain.xml_desc)
+ domain_xml.elements['domain/devices'].add_element(channel_rexml)
+ update(domain_xml.to_s)
+ end
+
def execute(cmd, options = {})
options[:user] ||= "root"
options[:spawn] = false unless options.has_key?(:spawn)
@@ -704,13 +736,4 @@ EOF
@display.take_screenshot(description)
end
- def get_remote_shell_port
- domain_xml = REXML::Document.new(@domain.xml_desc)
- domain_xml.elements.each('domain/devices/serial') do |e|
- if e.attribute('type').to_s == "tcp"
- return e.elements['source'].attribute('service').to_s.to_i
- end
- end
- end
-
end