Project

General

Profile

Feature #12707

Feature #12705: Update the size of the system partition to >= 4 GiB

Bump the size of the system partition created by Tails Installer

Added by intrigeri about 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority:
Elevated
Assignee:
-
Category:
Installation
Target version:
Start date:
06/15/2017
Due date:
% Done:

100%

Feature Branch:
feature/12705-bump-system-partition-size, installer:feature/12705-bump-system-partition-size
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Installer

Description

This is about:

  • creating a bigger system partition, whose size depends on the size of the target device;
  • handling nicely the fact that "Clone and Upgrade" will become impossible in some cases;
  • refusing installing a new Tails on a device smaller than 8 GiB.

See the parent ticket for detailed specifications, and see #11627 for UI messages that sajolida drafted already (I expect they'll be worked on again but that should be enough to get started with the coding).


Related issues

Blocks Tails - Feature #13234: Core work 2017Q3: Foundations Team Resolved 06/29/2017

Associated revisions

Revision 8c2cd427 (diff)
Added by intrigeri almost 2 years ago

Test suite: take into account that Tails Installer will soon refuse installing Tails to devices smaller than 8 GiB (refs: #12707).

It'll still allow upgrading such sticks though.

Revision c867dd3d (diff)
Added by intrigeri almost 2 years ago

Test suite: use 7200 MiB virtual USB drives when we really mean 8 GiB (refs: #12707).

In the real world, USB sticks labeled "8 GB" can be much smaller,
so Tails Installer will accept anything that's at least 7200 MiB.

This commit makes us exercise something closer to what happens in the real
world, and incidentally it'll save storage space on our isotesters
and improve test suite performance a bit :)

Revision 85854ef0
Added by anonym almost 2 years ago

Merge remote-tracking branch 'origin/feature/12705-bump-system-partition-size' into devel

Fix-committed: #12436, #12707

Revision 4e57ec00 (diff)
Added by intrigeri almost 2 years ago

Test suite: use 7200 MiB virtual USB drives for snapshots as well when we really mean 8 GiB (refs: #12707).

Fixup on commit c867dd3da6bfed2c671af74602cbb246a097a76c.

History

#1 Updated by intrigeri about 2 years ago

kurono, do you want to take care of this? Ideally we would have a PoC branch by the end of July, so we have time to refine the UI messages, write automated tests, identify & fix bugs. Otherwise no big deal, I will (perhaps later than 3.2 though).

#2 Updated by intrigeri about 2 years ago

#3 Updated by intrigeri almost 2 years ago

  • Priority changed from Normal to Elevated

(This is one of our top UX issues currently AFAICT.)

#4 Updated by kurono almost 2 years ago

intrigeri wrote:

kurono, do you want to take care of this? Ideally we would have a PoC branch by the end of July, so we have time to refine the UI messages, write automated tests, identify & fix bugs. Otherwise no big deal, I will (perhaps later than 3.2 though).

I completely missed this one :/ Let me know if I still have time to work on it, or if you prefer to take care.

#5 Updated by intrigeri almost 2 years ago

I completely missed this one :/ Let me know if I still have time to work on it, or if you prefer to take care.

I probably won't have time to work on this by the end of August, so please go ahead :)

#6 Updated by kurono almost 2 years ago

  • Assignee changed from intrigeri to kurono
  • Target version changed from Tails_3.2 to Tails_3.3

#7 Updated by kurono almost 2 years ago

  • Assignee changed from kurono to intrigeri
  • QA Check set to Info Needed
  • Feature Branch set to feature/Bump-size-system-partition-installer

intrigeri wrote:

This is about:

I have implemented a first iteration, and have some questions:

  • creating a bigger system partition, whose size depends on the size of the target device;

I have done this according to the parent ticket:
"bump the default system partition size for newly installed Tails to 4 GiB on 8 GiB sticks, and to 8 GiB on sticks of 16 GiB or larger"
So I don't know if 8GiB is the top size, or we want even bigger partitions?

  • handling nicely the fact that "Clone and Upgrade" will become impossible in some cases;

I think this is already been done by an exception.
Do we want to manage that in a different way?
I have changed the message, according to this:
https://labs.riseup.net/code/issues/11628
But the message is not clear on how to apply it to the installer instead of the upgrader.

  • refusing installing a new Tails on a device smaller than 8 GiB.

Done.

See the parent ticket for detailed specifications, and see #11627 for UI messages that sajolida drafted already (I expect they'll be worked on again but that should be enough to get started with the coding).

#8 Updated by intrigeri almost 2 years ago

Target version changed from Tails_3.2 to Tails_3.3

Note that 3.3 is a bugfix release, so if this does not go into 3.2, it's unlikely we can ship it before 3.4 (January).
Let's try to aim for 3.2, if feasible?

#9 Updated by intrigeri almost 2 years ago

  • Target version changed from Tails_3.3 to Tails_3.2
  • Feature Branch changed from feature/Bump-size-system-partition-installer to kurono/liveusb-creator:feature/Bump-size-system-partition-installer

#10 Updated by intrigeri almost 2 years ago

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

Hi!

I have done this according to the parent ticket:

\o/

"bump the default system partition size for newly installed Tails to 4 GiB on 8 GiB sticks, and to 8 GiB on sticks of 16 GiB or larger"
So I don't know if 8GiB is the top size, or we want even bigger partitions?

With "8 GiB on sticks of 16 GiB or larger", we meant that any stick that's 16GiB or larger will get a 8 GiB system partition. Is this what you implemented?

  • handling nicely the fact that "Clone and Upgrade" will become impossible in some cases;

I think this is already been done by an exception.

Indeed. Can you please double-check that this is checked before Tails Installer starts applying the upgrade (i.e. copying files)?

I have changed the message, according to this:
https://labs.riseup.net/code/issues/11628
But the message is not clear on how to apply it to the installer instead of the upgrader.

Right, #11628 is about the Upgrader, and here we need different text: it makes no sense to point to the manual upgrade instructions in a situation when a manual upgrade is failing. The parent ticket will tell you about how we decided to handle this (see "provide useful messages when upgrade by cloning is impossible"). But AFAICT no actual strings were drafted yet, so I'm hereby sending this ticket to sajolida's plate.

Meanwhile, you (kurono) could improve/clarify the corresponding code. I'm not quite sure what "free space" means when check_free_space is run in this context: IIRC the installer will first delete all (expected?) files on the target FS, and then copy files there from the source device. So what we care about is the size of the target FS minus unexpected files (that won't be deleted). I'm a little bit confused. If the code is correct, please add comments so I won't ask the same question next time.

#11 Updated by BitingBird almost 2 years ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10

#12 Updated by kurono almost 2 years ago

intrigeri wrote:

Hi!

I have done this according to the parent ticket:

\o/

"bump the default system partition size for newly installed Tails to 4 GiB on 8 GiB sticks, and to 8 GiB on sticks of 16 GiB or larger"
So I don't know if 8GiB is the top size, or we want even bigger partitions?

With "8 GiB on sticks of 16 GiB or larger", we meant that any stick that's 16GiB or larger will get a 8 GiB system partition. Is this what you implemented?

Yes this is what I have done. Now that I think about it, what about the size of the persistent partition?
I have configured it as a minimum of 4000 MiB, since the min. usb stick size required now is 8Gb.

  • handling nicely the fact that "Clone and Upgrade" will become impossible in some cases;

I think this is already been done by an exception.

Indeed. Can you please double-check that this is checked before Tails Installer starts applying the upgrade (i.e. copying files)?

This applies to the later comment: It seems like the installer, when doing the upgrade process:
  • deletes all the known files in the system partition.
  • check for the free available space (gui.py:self.live.check_free_space())
  • then extracts the ISO/copies source to the stick.

So yes, there could be the case that the installer deletes the files from a very old installation, and it is unable to copy a newer and bigger ISO/source installation. Maybe we can add an extra check before the files are deleted.

I have changed the message, according to this:
https://labs.riseup.net/code/issues/11628
But the message is not clear on how to apply it to the installer instead of the upgrader.

Right, #11628 is about the Upgrader, and here we need different text: it makes no sense to point to the manual upgrade instructions in a situation when a manual upgrade is failing. The parent ticket will tell you about how we decided to handle this (see "provide useful messages when upgrade by cloning is impossible"). But AFAICT no actual strings were drafted yet, so I'm hereby sending this ticket to sajolida's plate.

Meanwhile, you (kurono) could improve/clarify the corresponding code. I'm not quite sure what "free space" means when check_free_space is run in this context: IIRC the installer will first delete all (expected?) files on the target FS, and then copy files there from the source device. So what we care about is the size of the target FS minus unexpected files (that won't be deleted). I'm a little bit confused. If the code is correct, please add comments so I won't ask the same question next time.

#13 Updated by intrigeri almost 2 years ago

hi!

Now that I think about it, what about the size of the persistent partition?
I have configured it as a minimum of 4000 MiB, since the min. usb stick size required now is 8Gb.

I don't get how it's relevant for Tails Installer, since we don't manage that partition there. Can you please clarify?

  • handling nicely the fact that "Clone and Upgrade" will become impossible in some cases;

I think this is already been done by an exception.

Indeed. Can you please double-check that this is checked before Tails Installer starts applying the upgrade (i.e. copying files)?

This applies to the later comment: It seems like the installer, when doing the upgrade process:
  • deletes all the known files in the system partition.
  • check for the free available space (gui.py:self.live.check_free_space())
  • then extracts the ISO/copies source to the stick.

So yes, there could be the case that the installer deletes the files from a very old installation, and it is unable to copy a newer and bigger ISO/source installation.

Wow, that sounds bad. If I'm not mistaken this is already the case today, but the chances this actually happens will grow once devices that fully use their 4-8GiB system partition and then users might be stuck with persistent data that's hard to recover.

Maybe we can add an extra check before the files are deleted.

This would be sweet. I'm not sure if it's a blocker though: sajolida?

Meanwhile, please ensure we point users to https://tails.boum.org/doc/first_steps/persistence/copy/ whenever this happens (to ease a bit their migration to a bigger device).

#14 Updated by kurono almost 2 years ago

intrigeri wrote:

hi!

Now that I think about it, what about the size of the persistent partition?
I have configured it as a minimum of 4000 MiB, since the min. usb stick size required now is 8Gb.

I don't get how it's relevant for Tails Installer, since we don't manage that partition there. Can you please clarify?

Currently we use creator.py:is_device_big_enough() to check the size of the USB stick before presenting it to the user.
The device is "big enough" if it is bigger than self.system_partition_size + self.min_persistence_partition_size. So that value is used just for checking that there is enough space for a system partition plus a persistent partition.

Take a look of https://labs.riseup.net/code/issues/6538

#15 Updated by intrigeri almost 2 years ago

Hi!

Meta: I would very much like to get this done in 3.2, but the freeze for Tails 3.2 is on September 14. Please let me know ASAP if this crazy timeline works for you, or if I'd better take over this ticket (in part or fully).

kurono wrote:

intrigeri wrote:

Now that I think about it, what about the size of the persistent partition?
I have configured it as a minimum of 4000 MiB, since the min. usb stick size required now is 8Gb.

I don't get how it's relevant for Tails Installer, since we don't manage that partition there. Can you please clarify?

Currently we use creator.py:is_device_big_enough() to check the size of the USB stick before presenting it to the user.
The device is "big enough" if it is bigger than self.system_partition_size + self.min_persistence_partition_size. So that value is used just for checking that there is enough space for a system partition plus a persistent partition.

Take a look of https://labs.riseup.net/code/issues/6538

I see, thanks!

On the one hand, so far it did make sense to express the is_device_big_enough condition in terms of system_partition_size + min_persistence_partition_size: that expressed our intent better than a hard-coded minimum device size. But OTOH, the idea behind the new algorithm we want to use (and that this ticket is about) is a bit different: when we say "bump the default system partition size for newly installed Tails […] to 8 GiB on sticks of 16 GiB or larger", IMO we pick 8 GiB, and not more, because 8 GiB will be enough for applying tons of incremental upgrades, and not because for some reason we would think that a persistent partition on a 16+ GiB device must be at least 8 GiB large.

So I don't think we should stick to the system_partition_size + min_persistence_partition_size paradigm, that doesn't express the intent behind this code anymore and actually makes things more complicated than they could be. At this point I think we should (mixing on-topic stuff with refactoring I think your changes need in the same area) is:

  1. drop min_persistence_partition_size entirely
  2. replace the system_partition_size variable with a function or method (called exactly the same) that computes the desired value according to our new algorithm
  3. use the new system_partition_size method in is_device_big_enough and in any other place where we need it
  4. stop modifying the system_partition_size attribute in is_device_big_enough: so far that method was a predicate, as indicated by its name, and a predicate must not have side effects
  5. iff. it's too costly to do the udev dance every time we need system_partition_size, teach system_partition_size to cache/memoize its result, keeping in mind that it might have to be recomputed when the user selects a different target device

What do you think?

#16 Updated by kurono almost 2 years ago

intrigeri wrote:

Hi!

Meta: I would very much like to get this done in 3.2, but the freeze for Tails 3.2 is on September 14. Please let me know ASAP if this crazy timeline works for you, or if I'd better take over this ticket (in part or fully).

wow that is Thursday, so even if I implemented the changes Today, we would still go trough reviews and iterations, so even doe I would like to help that to happen, it would be better if you take it from here.

#17 Updated by intrigeri almost 2 years ago

wow that is Thursday, so even if I implemented the changes Today, we would still go trough reviews and iterations, so even doe I would like to help that to happen, it would be better if you take it from here.

OK, I will. Sorry for my own latency on this ticket previously, that has been one of the causes for this crazy timing (the rescheduling of 3.2 one week earlier than planned didn't help though).

#18 Updated by intrigeri almost 2 years ago

  • Assignee changed from sajolida to intrigeri
  • Feature Branch changed from kurono/liveusb-creator:feature/Bump-size-system-partition-installer to kurono/liveusb-creator:feature/Bump-size-system-partition-installer,feature/12705-bump-system-partition-size

(In the end sajolida and I concluded that we don't need to update strings as the situation when they are displayed is unlikely to happen soon, so this will be handled on #14622.)

#19 Updated by intrigeri almost 2 years ago

  • % Done changed from 10 to 20
  • Feature Branch changed from kurono/liveusb-creator:feature/Bump-size-system-partition-installer,feature/12705-bump-system-partition-size to feature/12705-bump-system-partition-size, installer:feature/12705-bump-system-partition-size

#20 Updated by intrigeri almost 2 years ago

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

#21 Updated by anonym almost 2 years ago

  • Status changed from In Progress to Fix committed
  • Assignee deleted (anonym)
  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

#22 Updated by intrigeri almost 2 years ago

  • Status changed from Fix committed to In Progress

#23 Updated by intrigeri almost 2 years ago

  • Status changed from In Progress to Fix committed

#24 Updated by anonym almost 2 years ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF