Bug #6538

Tails Installer tries to install to too small devices

Added by intrigeri 2013-12-26 13:34:47 . Updated 2015-03-31 19:02:21 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Installation
Target version:
Start date:
2013-12-26
Due date:
% Done:

100%

Feature Branch:
bugfix/8691-and-6538
Type of work:
Code
Blueprint:

Starter:
1
Affected tool:
Installer
Deliverable for:

Description

As reported in https://mailman.boum.org/pipermail/tails-dev/2013-September/003627.html, and confirmed later with our automated test suite, when the destination device is too small (e.g. currently 2 GiB is too small), but already has a MBR partition table (in case in matters), Tails Installer still tries to partition it, and obviously fails. And returns “Error: Can’t have a partition outside the disk!”.

Tails Installer should be better at detecting destination devices that are too small.

Ideally, these devices would simply be filtered out and not proposed as valid destinations when run with the -P (aka. “Clone and Install”, aka. self.parent.opts.partition is true in liveusb/gui.py. Ideally, a warning would be displayed in the logging text area for each ignored device, so that users don’t wonder why their stick does not appear.


Subtasks


Related issues

Related to Tails - Feature #9129: Write regression tests for installing Tails to too small devices Resolved 2015-03-30
Has duplicate Tails - Bug #8408: Tails Installer should say when a USB stick is too small Duplicate 2014-12-08

History

#1 Updated by kurono 2014-04-24 20:02:02

  • Status changed from Confirmed to In Progress
  • Assignee set to kurono

#2 Updated by intrigeri 2014-07-12 10:35:02

What’s the status on this?

#3 Updated by sajolida 2014-09-09 13:25:44

  • Description updated

#4 Updated by BitingBird 2014-12-09 13:20:00

  • has duplicate Bug #8408: Tails Installer should say when a USB stick is too small added

#5 Updated by kurono 2014-12-09 20:06:32

  • Assignee deleted (kurono)
  • Target version set to Tails_1.2.2
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices

#6 Updated by intrigeri 2014-12-11 17:15:29

  • Assignee set to kurono
  • QA Check changed from Ready for QA to Dev Needed

The limit needs to be raised to ~4GB, see tails-dev@.

#7 Updated by kurono 2014-12-11 19:10:21

  • Assignee deleted (kurono)
  • QA Check changed from Dev Needed to Ready for QA

#8 Updated by anonym 2014-12-12 16:41:54

  • Target version changed from Tails_1.2.2 to Tails_1.2.3

#9 Updated by alant 2014-12-23 17:27:50

  • Assignee set to kurono
  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch changed from bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices to liveusb-creator:bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices

First, thanks for your contribution.

I created a branch containing your patch in liveusb-creator repository. Next time, please don’t specify a feature branch if it doesn’t exist yet, or provide the full URL of the repository we should pull from to get it.

About your patch:

  • there’s already a variable system_partition_size = 2500 # MiB at creator.py:72. I think it’s confusing to have two unrelated size variables defined in two different files. What it we increase system_partition_size above the new min_device_size at some point? Please use system_partition_size. If needed you can create a min_persistence_parition_size variable.
  • there’s already an (unused!) is_device_big_enough method at creator.py:280. Why not to use it instead of writing a new test? You’ll probably need to modify it before though.

#10 Updated by BitingBird 2015-01-04 19:04:36

  • Affected tool set to Installer

#11 Updated by kurono 2015-01-06 21:25:08

  • Assignee deleted (kurono)
  • QA Check changed from Dev Needed to Ready for QA

Thanks for your feedback :)

I have made the required changes. Now the branch bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices
does exist in https://git-tails.immerda.ch/kurono/liveusb-creator

Regarding the fix, I created two new variables in the config.py file, so it gets easier to update the required sizes when they change (which is likely to be often).

Besides I am checking the size not only for the device but also the parent device. This is to avoid situations where the device is only the system partition (2500 MB) which causes a crash in the installation process.

#12 Updated by intrigeri 2015-01-07 10:20:41

  • Assignee set to alant
  • Feature Branch changed from liveusb-creator:bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices to kurono:bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices

Alan, will you take care of the review again?

#13 Updated by intrigeri 2015-01-13 12:52:58

  • Assignee changed from alant to anonym

#14 Updated by anonym 2015-01-13 14:12:05

  • Assignee changed from anonym to intrigeri

Delegating to intrigeri.

#15 Updated by intrigeri 2015-01-13 15:02:39

  • Assignee changed from intrigeri to kurono
  • Target version changed from Tails_1.2.3 to Tails_1.3
  • QA Check changed from Ready for QA to Dev Needed

Thanks!

Here’s a review:

  • I’m not happy with plugging the “skip too small devices” logics into a totally unrelated block of code that’s about gathering data about the parent device. The resulting code flow feels wrong. I’d rather see this new logic put in a block similar to the “Skip things without a size” one, after we’ve gathered the data we need.
  • I’m not convinced that a 4000 MiB hard limit is sound: I suspect that there are media labelled as “4 GB”, that would be a little bit smaller. Tails will install and work perfectly fine on a 3.5 GiB drive, so I suggest setting min_persistence_partition_size to 1000, which should avoid the aforementioned problem.
  • What’s the intended behavior on “Clone and upgrade”? I guess we should not apply the full device size filter in this case. Or do we want to force users who previously installed Tails on a 2GiB stick (with a 1.5 GiB system partition, as we created in older Tails Installer) to reinstall?
  • What’s the intended behavior on “Upgrade from ISO”? Same as above.
  • A very long line (if (self.system_partition_size + [...]) is introduced in this branch. Let’s avoid this, please :)

#16 Updated by kurono 2015-02-09 20:57:21

  • Assignee deleted (kurono)
  • QA Check changed from Dev Needed to Ready for QA

#17 Updated by intrigeri 2015-02-10 09:07:06

  • Assignee set to kurono
  • QA Check changed from Ready for QA to Info Needed

intrigeri wrote:
> * What’s the intended behavior on “Clone and upgrade”? I guess we should not apply the full device size filter in this case. Or do we want to force users who previously installed Tails on a 2GiB stick (with a 1.5 GiB system partition, as we created in older Tails Installer) to reinstall?
> * What’s the intended behavior on “Upgrade from ISO”? Same as above.

Ping?

#18 Updated by kurono 2015-02-10 17:43:38

  • Assignee deleted (kurono)
  • QA Check changed from Info Needed to Ready for QA

intrigeri wrote:
> intrigeri wrote:
> > * What’s the intended behavior on “Clone and upgrade”? I guess we should not apply the full device size filter in this case. Or do we want to force users who previously installed Tails on a 2GiB stick (with a 1.5 GiB system partition, as we created in older Tails Installer) to reinstall?
> > * What’s the intended behavior on “Upgrade from ISO”? Same as above.
>
> Ping?

According to the ticket description:

“Ideally, these devices would simply be filtered out and not proposed as valid destinations when run with the -P (aka. ”Clone and Install“, aka. self.parent.opts.partition is true in liveusb/gui.py. Ideally, a warning would be displayed in the logging text area for each ignored device, so that users don’t wonder why their stick does not appear.”

So basically the filter only is applied on “Clone and Install”, because its only when the mbr is modified and the error is triggered.

Besides I fixed a minor typo:
https://git-tails.immerda.ch/kurono/liveusb-creator/log/?h=bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices

#19 Updated by intrigeri 2015-02-11 11:12:13

  • Assignee set to intrigeri
  • Target version changed from Tails_1.3 to Tails_1.3.2
  • % Done changed from 0 to 50

No time to review’n’merge this before the freeze, I’ll take care of it for 1.3.1.

#20 Updated by intrigeri 2015-03-09 13:48:37

  • Assignee changed from intrigeri to kytv
  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch changed from kurono:bugfix/6538-Tails-Installer-tries-to-install-to-too-small-devices to bugfix/8691-and-6538

I’ve merged kurono’s branch for this ticket and Bug #8691 into the bugfix/8691-and-6538 branch in our liveusb-creator Git repo, packaged it in the debian_bugfix-8691-and-6538 branch, build and uploaded the package to the APT suite for bugfix/8691-and-6538, and pushed to bugfix/8691-and-6538 some draft, untested, incomplete regression tests for these fixes. Now handing it to Kill Your TV who’ll write these tests, and then reassign to me for the final review and merge.

#21 Updated by anonym 2015-03-30 08:45:38

  • Assignee changed from kytv to anonym

intrigeri wrote:
> Now handing it to Kill Your TV who’ll write these tests, and then reassign to me for the final review and merge.

It seems this won’t happen for Tails 1.3.2, so I’ve split that part to Feature #9129.

I’ll manually test kurono’s patch (via intrigeri’s branch) as a last-minute effort to get this into Tails 1.3.2. To be continued…

#22 Updated by intrigeri 2015-03-30 08:55:50

  • related to Feature #9129: Write regression tests for installing Tails to too small devices added

#23 Updated by anonym 2015-03-30 11:32:05

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

anonym wrote:
> I’ll manually test kurono’s patch (via intrigeri’s branch) as a last-minute effort to get this into Tails 1.3.2. To be continued…

It works fine. I’ve merged this, congrats, kurono!

However, given the silent nature of how too small devices are dropped, I’m afraid users now will start to overwhelm our front desk with: “Tails Installer doesn’t detect my USB drive”. Opened Bug #9130 for that.

#24 Updated by anonym 2015-03-30 11:37:34

Forgot to say: I didn’t merge bugfix/8691-and-6538 in Tails’ Git, since it only contains a WIP of the automated tests. The fix for this branch is merged in liveuse-creator’s master branch, and will end up in Tails 1.3.2 when I start preparing the image.

#25 Updated by anonym 2015-03-31 19:02:21

  • Status changed from Fix committed to Resolved