Feature #11887

Make the remote shell's file operations robust

Added by anonym 2016-10-25 15:21:10 . Updated 2017-03-09 13:59:13 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Start date:
2016-10-25
Due date:
% Done:

100%

Feature Branch:
test/11887-remote-shell-file-operations
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

I.e. don’t depend on quoting hacks + echo + cat + IO redirection in the shell for file operations (which makes binary data ugly business). We already have a command type, currently call and spawn, which we could extend with read, write, append with different parameters like path and data (for writes only).


Subtasks


Related issues

Blocks Tails - Feature #11888: Improve the remote shell's performance by switching to a virtio channel In Progress 2016-10-25
Blocks Tails - Feature #12059: Improve Dogtail's performance Resolved 2016-12-21

History

#1 Updated by intrigeri 2016-10-26 09:28:38

  • related to Feature #11888: Improve the remote shell's performance by switching to a virtio channel added

#2 Updated by anonym 2016-11-29 12:39:13

  • Status changed from Confirmed to In Progress
  • Target version changed from Tails_2.9.1 to Tails 2.10
  • % Done changed from 0 to 30
  • Feature Branch changed from test/remote-shell-file-operations to test/11887-remote-shell-file-operations

I dropped the virtio parts (Feature #11888) for now since it’s plagued by lockups, so we can proceed with getting this merged faster.

#3 Updated by anonym 2016-11-29 12:40:34

  • related to deleted (Feature #11888: Improve the remote shell's performance by switching to a virtio channel)

#4 Updated by anonym 2016-11-29 12:40:48

  • blocks Feature #11888: Improve the remote shell's performance by switching to a virtio channel added

#5 Updated by anonym 2017-01-12 19:11:40

  • Assignee changed from anonym to intrigeri
  • % Done changed from 30 to 50
  • QA Check changed from Dev Needed to Ready for QA

Jenkin’s look good. Please review’n’merge!

#6 Updated by intrigeri 2017-01-16 10:11:41

  • Assignee changed from intrigeri to anonym
  • Target version changed from Tails 2.10 to Tails_2.12
  • QA Check changed from Ready for QA to Dev Needed

The branch hasn’t built successfully in Jenkins since a week (and actually was failing since 3 days when this ticket was set as Ready for QA), and it has a merge conflict with current devel.

Also, it’s based on devel and wasn’t ready in time for the 2.10 freeze, so I guess it’s a candidate for 2.12 (unless it can actually be merged into current testing, in which case please set its base branch to something else than devel?).

#7 Updated by anonym 2017-01-16 10:36:56

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> The branch hasn’t built successfully in Jenkins since a week (and actually was failing since 3 days when this ticket was set as Ready for QA), and it has a merge conflict with current devel.

Sorry! I had failed to click “Submit” in the tab where I had written the above comment for a few days. Apparently things already had gone bad when I noticed this and pressed the button. Next time I’ll make sure to look at Jenkins again.

> Also, it’s based on devel and wasn’t ready in time for the 2.10 freeze, so I guess it’s a candidate for 2.12 (unless it can actually be merged into current testing, in which case please set its base branch to something else than devel?).

Well, we haven’t had issues with merging test/ branches after freezes before, so I still think 2.10 is a good target. I let you decide (so I merged testing instead of devel), taking into account that you probably have higher prio stuff to focus on.

#8 Updated by intrigeri 2017-01-16 11:55:00

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Ready for QA to Info Needed

I’ve skimmed over the log -p and the diff, and it looks good!

One question though: why is the port (/dev/ttyS0) now hard-coded in the script, rather than passed on the command line as it was the case previously?

#9 Updated by anonym 2017-01-25 14:04:03

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> One question though: why is the port (/dev/ttyS0) now hard-coded in the script, rather than passed on the command line as it was the case previously?

See:

9fb70d9c6e Revert back to serial link for the remote shell transport.

I had hardcoded the device used for the Virtio communication, and this was the more convenient way to revert to the serial link. IMHO, since this beast is so extremely Tails-specific, I see no real value in using parameters.

#10 Updated by anonym 2017-01-25 14:04:43

#11 Updated by anonym 2017-01-26 07:32:53

For the record, all Jenkins failures for this branch for the past week are unrelated.

#12 Updated by anonym 2017-01-26 10:46:12

  • Target version changed from Tails_2.12 to Tails_2.11

Its base branch is now stable.

#13 Updated by intrigeri 2017-01-27 13:48:38

  • Assignee changed from intrigeri to anonym
  • % Done changed from 50 to 70
  • QA Check changed from Ready for QA to Info Needed

In elapsed = (Time.now - TIME_AT_START.to_f - 3600).strftime("%H:%M:%S.%9N"), I don’t understand the - 3600 part. Can you please explain?

Otherwise looks good, and Jenkins is pretty happy with it, so feel free to merge yourself with a comment added for the above question :)

#14 Updated by anonym 2017-01-27 19:12:58

  • Assignee changed from anonym to intrigeri
  • % Done changed from 70 to 80
  • QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> In elapsed = (Time.now - TIME_AT_START.to_f - 3600).strftime("%H:%M:%S.%9N"), I don’t understand the - 3600 part. Can you please explain?

Err. Well, I remember that I initially was perplexed why it got offset by +1 hour, but I remember figuring out that it’s the local timezone that is applied, so all we have to do is to force UTC. Fixed now!

> Otherwise looks good, and Jenkins is pretty happy with it, so feel free to merge yourself with a comment added for the above question :)

Well, let’s see if the current change makes enough sense to you. Please review’n’merge!

#15 Updated by intrigeri 2017-01-29 11:19:46

  • Status changed from In Progress to Fix committed
  • % Done changed from 80 to 100

Applied in changeset commit:ef5eb75fc124d1583d69de701fcf685f1567d4f9.

#16 Updated by intrigeri 2017-01-29 11:20:50

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Pass

#17 Updated by anonym 2017-03-09 13:59:13

  • Status changed from Fix committed to Resolved