[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

Debbugs page

Pavel Shlyak wrote 3 years ago
(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
Tobias Geerinckx-Rice wrote 3 years ago
(name . Pavel Shlyak)(address . p.shlyak@pantherx.org)(address . guix-patches@gnu.org)(address . 55663@debbugs.gnu.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-----

Pavel Shlyak wrote 3 years ago
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . guix-patches@gnu.org)(address . 55663@debbugs.gnu.org)
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
Pavel Shlyak wrote 3 years ago
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . guix-patches@gnu.org)(address . 55663@debbugs.gnu.org)
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.
Pavel Shlyak wrote 3 years ago
[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
Mathieu Othacehe wrote 3 years ago
(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
Pavel Shlyak wrote 3 years ago
(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
Mathieu Othacehe wrote 3 years ago
(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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 55663@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 55663
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help