Tails

#1 Updated by anonym 2015-02-11 21:27:19

• related to Feature #8510: Distribute a hybrid ISO image added

#2 Updated by intrigeri 2015-02-11 21:57:38

• Subject changed from Virtualbox cannot use our isohybrid images to Virtualbox cannot use our hybrid ISO images

#3 Updated by anonym 2015-02-12 01:41:29

• Status changed from Confirmed to Rejected

This is embarrassing but good news. I just realized that I had (for testing purposes) manually downgraded to an older version of Virtualbox a few weeks ago. After re-installing the up-to-date 4.3.18, the issue isn’t there any more.

#5 Updated by intrigeri 2015-02-12 11:54:13

• Subject changed from Virtualbox cannot use our hybrid ISO images to Some versions of VirtualBox cannot use our hybrid ISO images
• Status changed from Rejected to Confirmed
• Assignee set to anonym
• QA Check set to Info Needed

#7 Updated by anonym 2015-02-12 12:11:41

• Subject changed from Some versions of VirtualBox cannot use our hybrid ISO images to VirtualBox cannot use our hybrid ISO images when larger than 1 GB

The “larger than 1 GB” part is a bit of a guess, but it’s better than the old title.

#9 Updated by anonym 2015-02-16 12:05:31

• Subject changed from VirtualBox cannot use our hybrid ISO images when larger than 1 GB to VirtualBox cannot use our ISO images when its size is not a multiple of 2048 bytes
• Assignee changed from anonym to intrigeri

intrigeri wrote:
> > but it will discourage testing of our nightly builds for users that prefer Virtualbox.
>
> Why? Our nightly builds are compressed just like our releases.

Ah, bad assumption. Sorry!

> Now, the Tails/Jessie nightly builds are dangerously close to 1GB, and maybe bigger (but that’s unrelated to the release vs. nightly topic). Do they work in VirtualBox?

I downloaded tails-i386-feature_jessie-1.3-20150216T0511Z-9375b40.iso which is 1109866496 bytes in size, and it does not trigger the failure. For comparison, an image that failed for me was 1266693120 bytes in size.

I dug around in the VirtualBox sources and found the check that failed in src/VBox/Storage/RAW.cpp:388:

if (cbFile > 32768 && !(cbFile % 2048))
{
    *penmType = VDTYPE_DVD;
[...]

where cbFile is the size in bytes of the image. So, according to VirtualBox, a DVD image’s size (in bytes) must be a multiple 2048. As you can see, my bad image isn’t, but 1.3~rc1 and the feature/jessie build I downloaded are. I don’t think we do anything that guarantees that the images we care for (stable/RC releases, nightly builds) end up with a sizes as multiples of 2048, so this is something we actually may need to ensure.

VirtualBox is quite sensible here since the data part of each sector of an ISO image is 2048 bytes (see e.g. the Wikipedia page for ISO Image, so it seems isohybrid indeed can create hybrid ISO images that fail this requirement. I suppose isohybrid already is operating in the realm of non-standards, but filing a bug may be worth it. Still, since trailing data shouldn’t be of any consequence, we can fix our images by padding them with zeros so their sizes are multiples of 2048 bytes with:

--- a/auto/build
+++ b/auto/build
@@ -153,6 +153,7 @@ if [ -e "${BUILD_FILENAME}.${BUILD_FILENAME_EXT}" ]; then
          echo "Hybriding it..."
          isohybrid $AMNESIA_ISOHYBRID_OPTS \
                    "${BUILD_FILENAME}.${BUILD_FILENAME_EXT}"
+         truncate -s %2048 "${BUILD_FILENAME}.${BUILD_FILENAME_EXT}"
       fi
    else
       echo "Warning: image created, but lb build exited with code $RET"

What do you think? (And yes, I’ve tested that running truncate on any of my problematic images fixes them.)

#11 Updated by intrigeri 2015-02-17 11:09:04

• Assignee changed from intrigeri to anonym

#12 Updated by anonym 2015-02-17 16:02:28

• Assignee deleted (anonym)
• QA Check changed from Info Needed to Ready for QA
• Feature Branch set to bugfix/8891-iso-image-padding

intrigeri wrote:
> > What do you think? (And yes, I’ve tested that running truncate on any of my problematic images fixes them.)
>
> I say we should do both:
>
> * pad our own images as you suggest: either for 1.3.1, or straight for 1.3 if the 1.3 ISO’s size isn’t a multiple of 2048

Done, see the feature branch. To avoid “randomly” (or whatever) ending up with a problematic Tails 1.3 ISO image it’d be great to have this in that release.

So, what about side effects? As far as my understanding goes, padding is completely harmless w.r.t. functionality, so we shouldn’t see any BIOS/DVD-reader compatibility issues or anything like that. However, I guess some burning programs might detect and strip the padding so that one cannot verify the ISO image’s signature again /dev/sr0 or whetever. Not sure we care.

Anything else?

> * raise the issue on the syslinux mailing-list
>
> I’m already on the syslinux mailing-list, so I can do the latter. anonym, wanna take care of the former?

I’m happy with this plan.

#13 Updated by intrigeri 2015-02-17 16:27:34

• Assignee set to intrigeri
• % Done changed from 0 to 50

#14 Updated by intrigeri 2015-02-17 18:25:28

• Type of work changed from Research to Code

intrigeri wrote:
> * pad our own images as you suggest: either for 1.3.1, or straight for 1.3 if the 1.3 ISO’s size isn’t a multiple of 2048

I’ll review (and quite likely, merge) this. Thanks for the branch!

> * raise the issue on the syslinux mailing-list

Done, Cc’d anonym as he’s the one who masters the gory details.

#15 Updated by intrigeri 2015-02-17 18:57:24

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

anonym, it seems you forgot to push that branch. Please reassign to me for QA once it’s done.

#17 Updated by Tails 2015-02-20 09:17:51

• Status changed from Confirmed to In Progress

Applied in changeset commit:9e7301f3d5bdd238dd13e193e2bd5fdc2f168f41.

#18 Updated by anonym 2015-02-20 09:28:03

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

intrigeri wrote:
> anonym, it seems you forgot to push that branch. Please reassign to me for QA once it’s done.

Pushed, however…

> Already two answers on the thread :)

Maybe we should post-pone dealing with this, preferably until it’s fixed in the most sensible way possible in isohybrid itself? It seems like I’m the only one that do development builds of Tails with gzip-compression and run them in Virtualbox occasionally => I’m the only one suffering from this issue. Or perhaps sajolida too? Any way, in that situation the image can be manually fixed with the truncate command which is simple enough for me (and sajolida). I’d hate for the padding to introduce some arcane issue that we and the nice people on the syslinux list haven’t thought about.

#20 Updated by intrigeri 2015-02-20 10:10:08

• Priority changed from Elevated to Normal
• Target version changed from Tails_1.3 to Tails_2.0
• QA Check deleted (Info Needed)
• Type of work changed from Code to Wait

Next step is to wait for isohybrid to fix that. See the aforementionned thread on the syslinux mailing-list.

#22 Updated by BitingBird 2015-05-04 10:36:20

• Target version changed from Tails_2.0 to Tails_1.4

Users are unable to use 1.4rc1 because of this issue, let’s fix that for the real one :)

#24 Updated by BitingBird 2015-05-05 22:26:17

• has duplicate Bug #9348: VirtualBox fails to load Tails 1.4~rc1 added

#26 Updated by intrigeri 2015-05-07 19:09:07

• Assignee changed from intrigeri to anonym
• QA Check set to Ready for QA
• Type of work changed from Wait to Code

> => I’m going to try booting in various contexts an ISO built from testing + the proposed branch merged in, and if I experience no regression then I’ll ask for a review’n’merge.

All current libvirt+kvm tests have been run on current Debian sid.

Note that the tested ISO was built with the crazy-slow XZ options, and I’ve verified that the call to truncate did make it larger: 962242560 bytes before hybrid’ing, 962357760 bytes after hybrid’ing, 962359296 bytes after truncate.

Boots fine:

• libvirt + kvm, ISO passed as a virtual DVD
• libvirt + kvm, ISO installed to a virtual USB drive with dd
• libvirt + kvm, ISO installed to a virtual USB drive with Tails Installer

=> please review’n’merge into testing and devel.

#27 Updated by anonym 2015-05-08 15:11:34

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

From commit:936d444:

+# Clone all output, from this point on, to the log file
+exec >  >(tee -a "$BUILD_LOG")
+trap "kill -9 $! 2>/dev/null" EXIT HUP INT QUIT TERM
+exec 2> >(tee -a "$BUILD_LOG" >&2)
+trap "kill -9 $! 2>/dev/null" EXIT HUP INT QUIT TERM

Do we really have to kill with SIGKILL? We’ve had some bad experiences with such abrupt killing lately in the test suite, so I’m worrying about similar things (subprocesses/zombies beaing left, other stuff not being cleaned up, etc.). Did you try with SIGTERM?

Otherwise it looks good (didn’t test yet).

#28 Updated by intrigeri 2015-05-08 15:47:56

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

> Do we really have to kill with SIGKILL? We’ve had some bad experiences with such abrupt killing lately in the test suite, so I’m worrying about similar things (subprocesses/zombies beaing left, other stuff not being cleaned up, etc.).

Thanks for raising this topic!

The specific problem you’re referring too only applies to killing processes that fork child processes (and then cannot clean them up when killed with SIGKILL). This doesn’t apply here, since tee does not fork any child process AFAICT.

In the specific case at hand, I think it’s safer to kill tee with SIGKILL, to increase the chances that tee is indeed killed, and hence to lower the chances that the build process leaves dangling processes behind it. Note that tee’s output to file is not buffered (POSIX 1003.1), and IIRC shell redirection is line-buffered in the worst case, so killing tee with SIGKILL should not lead to lost messages. So, I’m inclined to keep the code as-is (SIGKILL) and revisit if it causes problems later on.

> Did you try with SIGTERM?

I did not.

#29 Updated by anonym 2015-05-08 19:44:36

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

Applied in changeset commit:5b642ad26d83632ed72631b886e4cb4827270e51.

#30 Updated by anonym 2015-05-08 19:45:51

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

Fair enough. Let’s revisit if we notice lost messages. Merged!

#31 Updated by BitingBird 2015-05-12 18:44:07

• Status changed from Fix committed to Resolved