Syslinux - Feb 2015 - isohybrid and ISO images whose size is not a multiple of 2048 bytes vs. VirtualBox

Hi,
> Now, I have a question:  what about the "offset" parameter? How
it
> modifies our assumptions and calculations if the "offset"
parameter is
> not zero?
The offset influences only the start address of the
partition. The alignment goal is about the end address.
So these concepts are nearly independent.

Of course, partition start must be smaller than partition
end. But a mountable ISO cannot be smaller than
18 * 2048 bytes = 72 in units of -o. And -o is limited
to 64. (Not so clear why ...)


A non-zero offset makes the MBR ISO partition unmountable
unless the ISO is specially prepared by a second superblock
and directory tree.
So isohybrid option -o is for experts, anyway.
 
The ISO partition covers the block range at least up to
the end of the ISO filesystem and includes the padding.
Even if it is not mountable, it still protects the ISO
filesystem from being regarded as free space by partition
editors.

> Perhaps the code will 
> be more clear to me when I'll see it included in isohybrid.c
This is a patch against
  http://git.kernel.org/cgit/boot/syslinux/syslinux.git/plain/utils/isohybrid.c

It roughly compiles here. (Up to missing uuid_generate.)
------------------------------------------------------------------------

--- isohybrid.c 2015-02-20 10:43:49.000000000 +0100
+++ isohybrid_test.c    2015-02-20 11:01:33.000000000 +0100
@@ -947,7 +947,7 @@ main(int argc, char *argv[])
     int i = 0;
     FILE *fp = NULL;
     uint8_t *buf = NULL, *bufz = NULL;
-    int cylsize = 0, frac = 0;
+    int cylsize = 0, frac = 0, align_factor, align_cylsize;
     size_t orig_gpt_size, free_space, gpt_size;
     struct iso_primary_descriptor descriptor;
 
@@ -1058,9 +1058,20 @@ main(int argc, char *argv[])
     isosize = lendian_int(descriptor.size) *
lendian_short(descriptor.block_size);
     free_space = isostat.st_size - isosize;
 
+    /* Making sure that the resulting image size is divisible by 2048.
+       (Can waste nearly 32 MB. Should be done on user request only.)
+    */
+    if ((head * sector) % 4 == 0)
+        align_factor = 1;
+    else if ((head * sector) % 2 == 0)
+        align_factor = 2;
+    else
+        align_factor = 4;
+
     cylsize = head * sector * 512;
-    frac = isostat.st_size % cylsize;
-    padding = (frac > 0) ? cylsize - frac : 0;
+    align_cylsize = cylsize * align_factor;
+    frac = isostat.st_size % align_cylsize;
+    padding = (frac > 0) ? align_cylsize - frac : 0;
 
     if (mode & VERBOSE)
         printf("imgsize: %zu, padding: %d\n",
(size_t)isostat.st_size, padding);

------------------------------------------------------------------------

I wrapped the essential code into a standalone program for
playing with a few numbers:

-h 255 -s 63 , ISO size 1085739008 :
  align_factor= 4 , padding= 32899072 , image size= 136.000000 cylinders

-h 252 -s 63 , ISO size 1085739008 :
  align_factor= 1 , padding= 3481600 , image size= 134.000000 cylinders

-h 128 -s 32  , ISO size 1085739008 :
  align_factor= 1 , padding= 585728 , image size= 518.000000 cylinders

"padding" is the actual waste.
Well, ISO size 1085739008 was designed to let -h 255 -s 63 look bad.
Let's choose some random size around 1.5 GB: 2048 * 723452 

-h 255 -s 63 , ISO size 1481629696 :
  align_factor= 4 , padding= 31821824 , image size= 184.000000 cylinders

-h 252 -s 63 , ISO size 1481629696 :
  align_factor= 1 , padding= 5888000 , image size= 183.000000 cylinders

-h 128 -s 32 , ISO size 1481629696 :
  align_factor= 1 , padding= 1056768 , image size= 707.000000 cylinders

Bad luck again.
I swear i just toggled a 7 and then blindly 5 digits
for the "random" number.
More tries

-h 255 -s 63 , ISO size 1478760448 :
  align_factor= 4 , padding= 1789952 , image size= 180.000000 cylinders

-h 255 -s 63 , ISO size 1630908416 :
  align_factor= 4 , padding= 14147584 , image size= 200.000000 cylinders

I expect the average waste to be a bit less than 16 MB.


Have a nice day :)

Thomas

> 
> I wrapped the essential code into a standalone program for
> playing with a few numbers:
 
What about '-h 255 -s 63 , ISO size 1085736960' ?
 
> I expect the average waste to be a bit less than 16 MB.
 
Yes, but slightly misleading. For example, the "16MB average" is 
relevant for 255/63.

Some user reading the code might not know the whole reasoning for:
 
> +       (Can waste nearly 32 MB. Should be done on user request only.)
 
I would suggest instead:

+       (Could waste up to a maximum of nearly 32 MB in certain cases.)

Note that I also removed the second phrase, "Should be done on user 
request only", as I disagree.

IMO, users should not be left with incompatible isohybrid images. For 
example, the reason for an image to fail in some VM should not need to 
be searched in this Syslinux Mailing List (a task that is not so easy 
to do). Beta testers of ISO images should not need to report to their 
respective distros that the image is failing under some situation. 
Maintainers should not need to burn their brains about the reasons. 
Users should not blame the distro (without even reporting the problem).

We are talking about an average padding of less than 2% for realistic 
cases, and that's just for the minimal size (around 1GiB). If users are 
required to download images of 3GB, adding 16MiB (on average) is less 
than 1%.

Additionally, we are talking about an average of 16MiB of padding for 
"un-padded" images. Current (as of 6.03) isohybrid images are already 
padded (with a different algorithm), so the additional (differential) 
"waste" produced by this patch is (much) less than 32MiB, and less
than
16MiB on average.

In other words, I would suggest using a more accurate wording, and 
implementing the correction, instead of leaving this issue on the air. 
Realistically, I would not expect distro maintainers to edit 
isohybrid.c so to make use of the patch; they will use what is already 
there by default. Let's see this issue solved.

Finally, let's not forget that for an image size smaller than 1GiB, the 
default geometry of 64/32 will not produce any additional waste 
comparing to the current version.

BTW, it would be nice to see TAILS testing this patch and reporting 
some feedback.

Thank you,
Ady.
 
> _______________________________________________
> Syslinux mailing list
> Submissions to Syslinux at zytor.com
> Unsubscribe or set options at:
> http://www.zytor.com/mailman/listinfo/syslinux
>

Hi,

On 20/02/2015 11:29, Thomas Schmitt via Syslinux wrote:
> Hi,
>
>> Now, I have a question:  what about the "offset" parameter?
How it
>> modifies our assumptions and calculations if the "offset"
parameter is
>> not zero?
>
> The offset influences only the start address of the
> partition. The alignment goal is about the end address.
> So these concepts are nearly independent.
>
> Of course, partition start must be smaller than partition
> end. But a mountable ISO cannot be smaller than
> 18 * 2048 bytes = 72 in units of -o. And -o is limited
> to 64. (Not so clear why ...)
>
>
> A non-zero offset makes the MBR ISO partition unmountable
> unless the ISO is specially prepared by a second superblock
> and directory tree.
> So isohybrid option -o is for experts, anyway.
>
> The ISO partition covers the block range at least up to
> the end of the ISO filesystem and includes the padding.
> Even if it is not mountable, it still protects the ISO
> filesystem from being regarded as free space by partition
> editors.
>
>
>> Perhaps the code will
>> be more clear to me when I'll see it included in isohybrid.c
>
> This is a patch against
>   
http://git.kernel.org/cgit/boot/syslinux/syslinux.git/plain/utils/isohybrid.c
Once testing will have been performed, could a patch be provided
against the code in release 6.03 (I don't know if it differs
from the one in git), and also against the code in stable releases
4.06 and 4.07?[1]

[1]Unless it be applied in a maintenance release, of course.

Thanks in advance and best regards,

Didier

Hi,
> What about '-h 255 -s 63 , ISO size 1085736960' ?
-h 255 -s 63 , ISO size 1085736960 :
  align_factor= 4 , padding= 0 , image size= 132.000000 cylinders

-----------------------------------------------------------------
Code of isohybrid_test_mockup.c :
-----------------------------------------------------------------

#include <stdlib.h>
#include <stdio.h>

int main(int argc, char **argv)
{
 int cylsize = 0, frac = 0, align_factor, align_cylsize;
 int padding;
 unsigned long img_size;
 int head, sector;

 sscanf(argv[1], "%d", &head);
 sscanf(argv[2], "%d", &sector);
 sscanf(argv[3], "%lu", &img_size);

 /* Begin of isohybrid.c code */

    /* Making sure that the resulting image size is divisible by 2048.
       (Can waste nearly 32 MB. Should be done on user request only.)
    */
    if ((head * sector) % 4 == 0)
        align_factor = 1;
    else if ((head * sector) % 2 == 0)
        align_factor = 2;
    else
        align_factor = 4;

    cylsize = head * sector * 512;
    align_cylsize = cylsize * align_factor;
    frac = img_size % align_cylsize;
    padding = (frac > 0) ? align_cylsize - frac : 0;

 /* End of isohybrid.c code */

 printf("-h %d -s %d , ISO size %lu :\n", head, sector, img_size);
 printf("  align_factor= %d , padding= %d , image size= %f
cylinders\n",
        align_factor, padding,
        (double) (img_size + padding) / (double) cylsize);

 exit(0);
}

-----------------------------------------------------------------
End of code
-----------------------------------------------------------------

Compile by

  cc -g -Wall -o isohybrid_test_mockup isohybrid_test_mockup.c

Run as e.g.

  ./isohybrid_test_mockup 255 63 1085736960

> > I expect the average waste to be a bit less than 16 MB.
> Yes, but slightly misleading. For example, the "16MB average" is 
> relevant for 255/63.
We advise two geometries: -h 63 -s 32 and -h 255 -s 63.
isohybrid behavior with the first one will not be changed.
With the second one it would waste 16 MB in average.

> Some user reading the code might not know the whole reasoning for:
This is not yet a proposal for changing isohybrid.c.
It is for testing the consequences of the idea on
real ISOs. If anybody is interested to do so ...

> Note that I also removed the second phrase, "Should be done on user 
> request only", as I disagree.
Whoever decides about committing a final patch will also
decide whether this behavior should be hardcoded, or be
default, or need to be enabled explicitely by the user.

I'd vote for the latter.

> I would not expect distro maintainers to edit 
> isohybrid.c so to make use of the patch;
Of course a final patch would offer an option to enable
the feature. Something like --big-align alias -B.


Have a nice day :)

Thomas

Hi,

Didier Spaier wrote:
> Once testing will have been performed, could a patch be provided
> against the code in release 6.03 [...] ?
Shall we count this as a vote in favor of hardcoded
--big-align ?

If you have a source code installation of SYSLINUX,
then try to apply the patch and test.
It is small enough to be done by hand resp. copy+paste,
if necessary.
The code snippet where it gets inserted looks similar than
the one in the old Perl script. So it should be to find
in all versions.


Have a nice day :)

Thomas