Ludovic Courtès <ludo@gnu.org> writes:

>> @@ -141,7 +141,7 @@ the #:references-graphs parameter of 'derivation'."
>>    (size        partition-size)
>>    (file-system partition-file-system (default "ext4"))
>>    (label       partition-label (default #f))
>> -  (bootable?   partition-bootable? (default #f))
>> +  (flags       partition-flags (default '()))
>>    (initializer partition-initializer (default (const #t))))
>
> So ‘flags’ must be a list of strings, and each string is something
> Parted recognizes, right?

Correct.

> It would be slightly nicer to make it a list of symbols.

Good idea.

>> +  (define (find-root-partition partitions)
>> +    "Return the first partition found with the boot flag set."
>> +    ;; FIXME: This probably does not work. What's the best way to do this?
>> +    (find (match-lambda
>> +            (($ <partition> _ _ _ _ flags)
>> +             (member "boot" flags)))
>> +          partitions))
>
> Why wouldn’t it work?  LGTM.
>
> BTW, in this case, I’d recommend using ‘partition-flags’ instead of
> ‘match’ with the 4 wildcards; probably safer.  :-)
>
> That said, it’s probably best to keep
>
>   (define (partition-bootable? partition)
>     (member "boot" (partition-flags partition)))
>
>>    (let* ((partitions (initialize-partition-table device partitions))
>> -         (root       (find partition-bootable? partitions))
>> +         (root       (find-root-partition partitions))
>
> … and keep this line unchanged (‘find-foo’ procedures are an
> anti-pattern in a way.)

That's much better indeed. Thanks!

>> From 9db90ea41a94ecbe42bba88de1c2e3ac607d5ea4 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@fastmail.com>
>> Date: Tue, 11 Apr 2017 10:55:22 +0200
>> Subject: [PATCH 2/4] vm: Unconditionally add a small ESP partition.
>>
>> * gnu/system/vm.scm (qemu-image): Append 20MB FAT32 partition.
>
> [...]
>
>>                    (partitions (list (partition
>>                                       (size #$(- disk-image-size
>> -                                                (* 10 (expt 2 20))))
>> +                                                (* 30 (expt 2 20))))
>>                                       (label #$file-system-label)
>>                                       (file-system #$file-system-type)
>>                                       (flags '("boot"))
>> -                                     (initializer initialize)))))
>> +                                     (initializer initialize))
>> +                                    (partition
>> +                                     ;; Append a small FAT32 partition for
>> +                                     ;; use with UEFI bootloaders.
>> +                                     (size (* 20 (expt 2 20)))
>> +                                     (label "gnu-esp")
>> +                                     (file-system "vfat")
>> +                                     (flags '("esp"))))))
>
> Should we do it conditionally, only when targeting UEFI?

Probably, but I'm not sure which condition (maybe 'uefi-disk-image'?).
Will revisit this once it works :)

> (BTW, what’s “esp”?)

ESP is short for 'EFI System Partition' (I'll mention this in the code
somewhere). UEFI firmwares scans each connected I/O device looking for
this partition, which must be flagged as such and FAT-formatted.

Thanks for the feedback! I'll keep hammering at this and should
hopefully have something usable within a few weeks. Currently, need to
figure out why the qemu builder can't find the ISO8859-1 kernel module.