Bug #5571

Test suite: filesystem shares vs. snapshots

Added by Tails 2013-07-18 07:42:57 . Updated 2017-01-24 20:47:22 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Start date:
2013-08-08
Due date:
2015-07-15
% Done:

100%

Feature Branch:
test/5571-no-more-filesystem-shares
Type of work:
Code
Blueprint:

Starter:
0
Affected tool:
Deliverable for:

Description

Filesystem shares cannot (due to QEMU limitations) be added to an active VM, and cannot (due to QEMU limitations) be active (i.e. mounted) during a snapshot save. Hence unmounting before save and remounting after restore them seems like a good idea.

However, the 9p* modules seem to get into a broken state during a save + restore (the "tags" used as mount source cannot be found), so unloading before save and reload after restore comes to mind. But loading 9pnet_virtio fails after a restore with probe of virtio2 failed with error -2 (in dmesg) and the shares remain unmountable.

We should research this further, and determine whether we’re just doing something wrong, or if this is an upstream bug.

Given we don’t use 9p shares at all, do we care more than what’s needed to just quickly report this to Debian and be done with it?

Actually we do: git grep "I setup a filesystem share". This bug is what forces us to "drop" the background snapshot in the MAT can clean a PDF file scenario. But it’s perhaps not a big issue. I’m fine with closing this bug and living with this as a limitation, I just want someone else’s oppinion. After all, we have more important stuff to do.


Files


Subtasks

Feature #6224: Reproduce the 9p reload bug with a recent kernel Resolved

100

Feature #6225: Report the 9p kernel bug Rejected

0

Feature #6226: Ask about FS shares vs. snapshots on libvirt list Resolved

100

Bug #10214: Get 9p live migration support into QEMU Rejected

0


History

#1 Updated by intrigeri 2013-08-08 13:58:25

  • Starter set to No

The August dev meeting said that this is no practical issue right now, since we only use FS shares for things that don’t care about these bugs:

  • MAT: should be moved to a dedicated feature anyway
  • usb_install: doesn’t use background snapshots

… but a limitation in what we can do in the future, when we want to
have libvirt fs shares work even when scenarios are using
a background snapshot.

So, we want to report this bug where it should be.

#2 Updated by intrigeri 2013-08-08 13:58:42

  • Type of work changed from Discuss to Code

#3 Updated by intrigeri 2013-12-27 05:17:27

  • Category set to Test suite

#4 Updated by BitingBird 2014-06-09 10:58:33

  • Subject changed from test suite: fs shares vs snapshots to Test suite: fs shares vs snapshots

#5 Updated by intrigeri 2014-07-19 20:09:17

  • Subject changed from Test suite: fs shares vs snapshots to Test suite: filesystem shares vs. background snapshots

#6 Updated by intrigeri 2014-07-19 20:09:31

  • Tracker changed from Feature to Bug

#7 Updated by anonym 2015-01-10 06:09:34

  • Target version set to Tails_1.4.1

#8 Updated by anonym 2015-01-10 17:17:33

  • blocks #8668 added

#9 Updated by anonym 2015-01-10 18:26:54

  • Assignee set to anonym

#10 Updated by kytv 2015-05-15 22:03:38

Edit: wrong ticket.

#11 Updated by intrigeri 2015-06-29 00:48:44

  • Target version changed from Tails_1.4.1 to Tails_1.5

#13 Updated by anonym 2015-08-03 11:01:39

  • Target version changed from Tails_1.5 to Tails_1.6

#14 Updated by intrigeri 2015-08-26 05:54:43

  • Deliverable for set to 266

#15 Updated by anonym 2015-09-13 11:48:08

I have some pretty good news! Live migration of (p9) shared filesystems can be fixed with this patch series ([Qemu-devel] [PATCH 0/2] Virtio-9p live migration patchset). So, yeah, this is a QEMU problem, not a libvirt one. It is pretty obvious to me now, years later when I have a much better understanding of our virtualization stack, but I am still apparently brain-dead enough to not reconsider what I have been reading in these tickets since then. Oh well…

So, the patches apply cleanly on Jessie’s QEMU, 2.1+dfsg-12+deb8u2, and I have attached a Debian source patch that applies against that version. With the patched QEMU, the filesystem shares work after restoring a snapshot without any extra work (e.g. no unmounting + remounting). Woo! For us to use this we’d need to rethink how we use the shares completely, but that’s a later question.

So how do we proceed? It seems to me that the current children tickets are irrelevant. New ones should be created about communicating our need for this on qemu-devel@, and possibly try to refresh the patches for QEMU’s current master (on patch doesn’t apply cleanly any more). What do you think?

#18 Updated by intrigeri 2015-09-15 11:49:51

> So how do we proceed? It seems to me that the current children tickets are irrelevant. New ones should be created about communicating our need for this on qemu-devel@, and possibly try to refresh the patches for QEMU’s current master (on patch doesn’t apply cleanly any more). What do you think?

I say close the obsolete tickets and create new ones.

Any estimate wrt. how close to be applied upstream these patches are?

#19 Updated by anonym 2015-09-16 16:38:23

  • Type of work changed from Code to Wait

intrigeri wrote:
> > So how do we proceed? It seems to me that the current children tickets are irrelevant. New ones should be created about communicating our need for this on qemu-devel@, and possibly try to refresh the patches for QEMU’s current master (on patch doesn’t apply cleanly any more). What do you think?
>
> I say close the obsolete tickets and create new ones.

Done.

> Any estimate wrt. how close to be applied upstream these patches are?

Nope. The last incarnation of the patch series was sent almost a year ago, and apparently it dropped one of the original patches that fixed a “theoretical potential bug”. I’ve emailed both the original and second submitter to ask if any of them are interested in pushing for it again. => Wait

#20 Updated by intrigeri 2015-09-16 23:34:15

> Type of work changed from Code to Wait

I think one subtask should have “Type of work = Wait”, but not this parent ticket.

#21 Updated by anonym 2015-09-17 07:13:34

  • Type of work changed from Wait to Code

intrigeri wrote:
> > Type of work changed from Code to Wait
>
> I think one subtask should have “Type of work = Wait”, but not this parent ticket.

Done.

#22 Updated by anonym 2015-09-22 15:03:09

  • Target version changed from Tails_1.6 to Tails_1.7

#23 Updated by sajolida 2015-10-01 07:19:59

Note that Tails 1.7 is passed the milestone for SponsorS (October 15).

#24 Updated by sajolida 2015-10-01 07:20:58

Sorry, actually this was due on July 15 :)

#25 Updated by intrigeri 2015-10-01 09:04:49

  • Deliverable for deleted (266)

This is about an internal commitment. What was promised internally has been completed already. The new subtasks are new, bonus stuff. I don’t know how to translate that into Redmine and have no time to look into it further right now, just to try and lower a bit the level of stress in passing.

#26 Updated by anonym 2015-10-05 06:29:31

Implementation detail: We might want to use some variation of the procedure we use for Bug #10288 (see its description).

#27 Updated by anonym 2015-10-14 08:22:05

Hm. I’ve just seen that one can share folders via SPICE but it requires spice-webdavd which is not packaged in Debian. I couldn’t get it to compile inside Tails (neither on Wheezy (too old libsoup) or Jessie (some weird linking error)). Any way, it’s an alternative I guess…

#28 Updated by anonym 2015-10-17 06:30:32

  • Target version changed from Tails_1.7 to Tails_1.8

#29 Updated by anonym 2015-10-17 06:31:17

  • Type of work changed from Code to Wait

#30 Updated by intrigeri 2015-10-20 03:26:54

  • blocked by deleted (#8668)

#31 Updated by intrigeri 2015-10-20 03:27:50

(Encoding the fact that what was promised has been done already.)

#32 Updated by intrigeri 2015-10-20 03:28:31

  • Type of work changed from Wait to Code

> Type of work changed from Code to Wait

Reverting again, see Bug #5571#note-20.

#33 Updated by intrigeri 2015-12-05 14:09:39

  • Target version changed from Tails_1.8 to Tails_2.2

Postponing to after January, since times will be a bit crazy until then.

#34 Updated by anonym 2016-02-24 18:47:42

  • Target version changed from Tails_2.2 to Tails_2.3

I’ll give this one some more attention in March.

#35 Updated by anonym 2016-04-20 10:56:10

  • Target version changed from Tails_2.3 to Tails_2.4

#36 Updated by anonym 2016-06-08 01:34:50

  • Target version changed from Tails_2.4 to Tails_2.5

#37 Updated by intrigeri 2016-07-13 12:26:56

  • Target version changed from Tails_2.5 to Tails_2.7

#38 Updated by anonym 2016-10-07 15:10:01

  • Subject changed from Test suite: filesystem shares vs. background snapshots to Test suite: filesystem shares vs. snapshots

Since the hallowed Feature #6094 was merged, we have no “background” snapshots any more.

#39 Updated by anonym 2016-11-04 13:29:04

  • Assignee changed from anonym to intrigeri
  • QA Check set to Info Needed

Given the complexity of making filesystem shares working with snapshots, I think we should just stop using filesystem shares. We just need a fast channel to transfer data from the host to the guest filesystem (e.g. SPICE, or the remote shell if we solve Feature #11887 + Feature #11888) and then I think we can replace steps like this:

Given I setup a filesystem share containing the Tails ISO


with

Given I plug a USB drive containing the Tails ISO


The only drawbacks (compared to a filesystem share) I can see is that this will need additional space and time for these transferred files. Whatever. At the moment we reboot when we need this, which at least takes a lot more time, and I think we can leave the peak disk usage as-is by just reordering features depending on this (for large files, e.g. the ISO) to run early.

What do you think?

#40 Updated by intrigeri 2016-11-04 18:44:18

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Info Needed to Dev Needed

> What do you think?

Sounds good.

#41 Updated by anonym 2016-11-04 19:35:26

I just realized that we don’t need the SPICE/remote shell, but can use guestfs to copy in the files to the virtual disk. So this is not blocked at all by anything. Woo!

#42 Updated by anonym 2016-11-08 19:20:52

  • Status changed from Confirmed to In Progress
  • Feature Branch set to test/5571-no-more-filesystem-shares

anonym wrote:
> I just realized that we don’t need the SPICE/remote shell, but can use guestfs to copy in the files to the virtual disk. So this is not blocked at all by anything. Woo!

Implemented! Let’s see what Jenkins thinks about it.

#43 Updated by bertagaz 2016-11-17 17:38:33

  • Target version changed from Tails_2.7 to Tails_2.9.1

#44 Updated by anonym 2016-11-29 19:54:02

Had to push some fixes. What say you, Jenkins?

#45 Updated by anonym 2016-12-09 12:37:49

Had to push another fix. Jenkins?

#46 Updated by anonym 2016-12-09 12:38:02

  • Target version changed from Tails_2.9.1 to Tails 2.10

#47 Updated by anonym 2016-12-11 19:58:45

And again. Jenkins?

#48 Updated by anonym 2016-12-27 18:40:22

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

Jenkins finally seems to have accepted this branch. Please review and merge!

PS: For this branch I tried out a workflow where I only relied on Jenkins’ test suite runs in order to get an idea of how easy it would be to contribute test suite work without being able to run it locally. As I expected, anything beside trivial image bumps ends up with many back-and-fourths making this an awkward experience, and the long feedback loop makes this useless for learning how to write tests.

#49 Updated by intrigeri 2016-12-28 08:33:50

  • Status changed from In Progress to Fix committed

Applied in changeset commit:05e8bebcaa726745c632b83e39d428151cec8e88.

#50 Updated by intrigeri 2016-12-28 08:40:50

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

I’ve pushed a few fixes on top of the branch and merged into stable and devel. Then I’ve merged into feature/stretch, and resolved conflicts (including one caused by having dogtail’ified the same code both on feature/stretch and on this branch, but in different ways; meh) the best I could. Cool!

#51 Updated by anonym 2017-01-24 20:47:22

  • Status changed from Fix committed to Resolved