Hi, Maxim Cournoyer <[email protected]> writes:
> Hello again, > > Ludovic Courtès <[email protected]> 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
