Subject: Re: bug#36117: qemu-binfmt with non-native chroot
From: Maxim Cournoyer <[email-masked]>
To: Ludovic Courtès <[email-masked]>
Cc: Vagrant Cascadian <[email-masked]>, [email-masked]
Date: Mon, 15 Mar 2021 18:07:27 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

Maxim Cournoyer <[email-masked]> writes:

> Hello again,
>
> Ludovic Courtès <[email-masked]> writes:
>
> [...]
>
>>>  ;; Platforms that QEMU can emulate.
>>> -(define-record-type <qemu-platform>
>>> -  (qemu-platform name family magic mask)
>>> +(define-record-type* <qemu-platform>
>>
>> Since this is for internal consumption, I’m in favor of keeping plain
>> ‘define-record-type’.  Also, I don’t think the “F” flag belongs here,
>> it’s mostly orthogonal.
>
> Even though it's still internal for now, the intent here was to allow
> the advanced users (those who would use (@@ ...) to access what they
> need) to configure the qemu-platform objects should they have special
> needs for it.
>
> I also don't think the flags belong to a qemu-platform object, but the
> same can be said for the magic field.  The most elegant thing would be
> to have a binfmt_misc service which we would then extend for QEMU, but
> that's more work and the use cases appear to be rare outside of QEMU (I
> can only think of WINE).  If you feel strongly about it I can revert
> those hunks and hard-coded the F flag.
>
>>>  (define qemu-binfmt-service-type
>>>    ;; TODO: Make a separate binfmt_misc service out of this?
>>>    (service-type (name 'qemu-binfmt)
>>> @@ -800,9 +833,7 @@ given QEMU package."
>>>                                            (const
>>>                                             (list %binary-format-file-system)))
>>>                         (service-extension shepherd-root-service-type
>>> -                                          qemu-binfmt-shepherd-services)
>>> -                       (service-extension guix-service-type
>>> -                                          qemu-binfmt-guix-chroot)))
>>> +                                          qemu-binfmt-shepherd-services)))
>>
>> As discussed on IRC, the downside of this approach is increased disk and
>> memory footprint (those big binaries have to be loaded in memory).
>
> The 'big' binaries are not that bigger than what we already have.  For a
> typical one:
>
> $ du -h {/gnu/store/7w04gv6m92n40dainn4s6xr3l20r90xw-qemu-5.1.0,\
>   /gnu/store/wqh2dyskzkl4vjn6harclyl317h4vfaf-qemu-5.1.0-static}/bin/qemu-arm
>
> 4.6M    /gnu/store/7w04gv6m92n40dainn4s6xr3l20r90xw-qemu-5.1.0/bin/qemu-arm
> 6.4M    /gnu/store/wqh2dyskzkl4vjn6harclyl317h4vfaf-qemu-5.1.0-static/bin/qemu-arm
>
> Only the registered QEMU architectures would be preloaded; so if you
> enable 5, the increase it at worst ~ 30 MiB.  Nothing too worrying on
> machines that should be quite capable for transparent emulation
> purposes, in my opinion.
>
>> One possibility would be to add an option to choose between this and the
>> current approach, but maybe it’s not worth the maintenance trouble.
>
> I would rather K.I.S.S., with this only case that works everywhere and
> thus doesn't surprise anyone.
>
>> Thanks for fixing this issue!
>
> Thanks for the review!
>
> Maxim

I went ahead and pushed this change to master alongside the update to
QEMU 5.2.0.

If there are improvements/changes to be made, we can pick it up from
there!

Closing,

Maxim