[PATCH] Image API: add FAT32 support

  • Done
  • quality assurance status badge
Details
3 participants
  • Tobias Geerinckx-Rice
  • Mathieu Othacehe
  • Pavel Shlyak
Owner
unassigned
Submitted by
Pavel Shlyak
Severity
normal
P
P
Pavel Shlyak wrote on 26 May 2022 20:02
(address . guix-patches@gnu.org)
3EE7299C-64ED-4062-BA31-ADF3A6AD6A37@pantherx.org
I believe it is an important change as Raspberry PI (and I suppose some other boards) cannot boot with fat16 boot partitions.
With this patch, "vfat" is treated as fat16 partition not to break backward-compatibility. "fat32", on the other hand, creates a fat32 partition.
Attachment: 0001-Image-API-support-FAT32-partitions.patch
T
T
Tobias Geerinckx-Rice wrote on 26 May 2022 20:07
(name . Pavel Shlyak)(address . p.shlyak@pantherx.org)
87bkvk6tp0@nckx
Hi Pavel,

Pavel Shlyak ???
Toggle quote (4 lines)
> I believe it is an important change as Raspberry PI (and I
> suppose some other boards) cannot boot with fat16 boot
> partitions.

LGTM in principle.

The FS_BITS argument should be a regular number. You can coerce
it with number->string in the command line.

On the subject of being explicit is good: we should add "fat16"
type and treat "vfat" as a softly deprecated alias. But that can
be done in a later patch.

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYo/FKw0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15v/oA/0mmvcJbt4k2Qt186TpOF7ig4VrZm+zMDIoOyjLV
yC58AP4+iXSPi6tjXd8+Ya6adpJVdFx2Hz9xbpxJ9pv3M7F+BA==
=jX82
-----END PGP SIGNATURE-----

P
P
Pavel Shlyak wrote on 26 May 2022 22:54
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
DE6446E5-48C6-49A3-8372-7E1FC7DD7941@pantherx.org
Thank you for a quick response!
I have changed it according to your recommendations. I hope it’s better now.
Attachment: 0001-Image-API-support-FAT32-partitions.patch
Toggle quote (16 lines)
> 26 ??? 2022 ?., ? 21:07, Tobias Geerinckx-Rice <me@tobias.gr> ???????(?):
>
> Hi Pavel,
>
> Pavel Shlyak ???
>> I believe it is an important change as Raspberry PI (and I suppose some other boards) cannot boot with fat16 boot partitions.
>
> LGTM in principle.
>
> The FS_BITS argument should be a regular number. You can coerce it with number->string in the command line.
>
> On the subject of being explicit is good: we should add "fat16" type and treat "vfat" as a softly deprecated alias. But that can be done in a later patch.
>
> Kind regards,
>
> T G-R
P
P
Pavel Shlyak wrote on 29 May 2022 13:46
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
6C6B28B2-B753-4A55-9CCD-05AB16EDB733@pantherx.org
Please, do not merge that until further notice. It looks like there’s a problem with the code.
P
P
Pavel Shlyak wrote on 29 May 2022 16:35
[PATCH] Image API: add FAT32 support
(address . 55663@debbugs.gnu.org)
4054081F-9D51-49E0-958F-C9B1BFA7FB24@pantherx.org
I have updated the patch so now it automatically sets block size for non-esp partitions. The previous behavior (for esp) was retained not to break things I’m not quite aware of.
Now I finally got correct/bootable partition layout with Raspberry PI4.
Attachment: 0001-Image-API-support-FAT32-partitions.patch
M
M
Mathieu Othacehe wrote on 30 May 2022 09:01
(name . Pavel Shlyak)(address . p.shlyak@pantherx.org)(address . 55663@debbugs.gnu.org)
874k17ld1d.fsf_-_@gnu.org
Hello Pavel,

Toggle quote (3 lines)
> gnu/build/image.scm | 24 ++++++++++++++----------
> gnu/system/image.scm | 9 +++++++--

Please write a commit message following the guidelines available here:

Toggle quote (2 lines)
> +(define* (make-vfat-image partition target root fs_bits)

s/fs_bits/fs-bits/

Toggle quote (2 lines)
> + ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")

This line is longer than 78 characters you can break it between the two
strings comparisons.

Toggle quote (3 lines)
> + (
> + (or (string=? file-system "vfat")

Merge those two lines.

Toggle quote (3 lines)
> + (string=? file-system "fat32")
> + ) "F")

Ditto.

Can you please send a v2?

Thanks for your contribution,

Mathieu
P
P
Pavel Shlyak wrote on 30 May 2022 11:01
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 55663@debbugs.gnu.org)
CABF5CFF-255F-4515-AF1D-6B8A8CE5210F@pantherx.org
Attachment: 0001-guix-Image-API-support-FAT32-partitions.patch
Toggle quote (35 lines)
> 30 ??? 2022 ?., ? 10:01, Mathieu Othacehe <othacehe@gnu.org> ???????(?):
>
>
> Hello Pavel,
>
>> gnu/build/image.scm | 24 ++++++++++++++----------
>> gnu/system/image.scm | 9 +++++++--
>
> Please write a commit message following the guidelines available here:
> https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
>
>> +(define* (make-vfat-image partition target root fs_bits)
>
> s/fs_bits/fs-bits/
>
>> + ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")
>
> This line is longer than 78 characters you can break it between the two
> strings comparisons.
>
>> + (
>> + (or (string=? file-system "vfat")
>
> Merge those two lines.
>
>> + (string=? file-system "fat32")
>> + ) "F")
>
> Ditto.
>
> Can you please send a v2?
>
> Thanks for your contribution,
>
> Mathieu
M
M
Mathieu Othacehe wrote on 31 May 2022 15:07
(name . Pavel Shlyak)(address . p.shlyak@pantherx.org)(address . 55663-done@debbugs.gnu.org)
87a6axj1en.fsf_-_@gnu.org
Hello Pavel,

Thanks for the v2, pushed on master.

Mathieu
Closed
?