Re: [PATCH v15 15/15] hw/vmapple/vmapple: Add vmapple machine type

[Philippe Mathieu-Daudé]

On 23/12/24 23:17, Phil Dennis-Jordan wrote:

     > Do you have other changes in your v16? I'm quite happy to
    integrate this
     > v15.

    The ones proposed by Akihiko are the only ones. I’ve already
    implemented them, so I’m happy to post v16.

OK, rebasing on the upstream master branch has revealed it's not quite so simple: there are a bunch of clashes with the recent sysemu->system rename and the removal of DEFINE_PROP_END_OF_LIST(). They amount to

Yep :/

pretty trivial changes, but almost all commits needed adjusting, so pulling v15 would be painful. I've just posted my v16 (including all the changes discussed here).

Thanks for taking an interest in this patch set!

     >> Adapting this giant macro for each machine type seems rather
    error-prone and the kind of thing a computer does better than a
    human writing the code. I can't help but wonder if we could define a
    generic version in boards.h and only implement the
    DEFINE_*_MACHINE{_LATEST,}() wrappers specifically for each machine
    type. I've created an issue for this on GitLab: https://gitlab.com/
    qemu-project/qemu/-/issues/2744 <https://gitlab.com/qemu-project/
    qemu/-/issues/2744> <https:// gitlab.com/qemu-project/qemu/-/
    issues/2744 <http://gitlab.com/qemu-project/qemu/-/issues/2744>> I
    might attack that once I've cut down my backlog of unmerged patches.
     >
     > Do we really want the vmapple machines to be versioned? I see 3
    options:
     > 1/ No (simplest)
     > 2/ Not yet, adding versioning when we see the needs
     > 3/ Yes
     >
     > Personally I prefer/recommend 1/ or 2/ ;)

    I didn’t realise these were an option. :-) I was just inheriting
    Alex’s code here and assumed it was the standard thing to do.

    I could imagine we might change a few things to get better guest
    version support and perhaps the guest iCloud support that was added
    to the Virtualization.framework in macOS 15. But yeah it seems
    unlikely that any of this would cause regressions.

    What change would you propose to remove the versioning? Is there a
    specific machine type I should use as reference? Or do you just want
    to change that yourself, if I push my v16 code to a public repository?

OK, grepping through the code base reveals there are quite a number of the "smaller" machine types are unversioned. So I guess that's a no- brainer then. I've worked out I essentially just need to move the compat properties registration to the main init, the "desc" field on the machine class needs to be set, and the type becomes non-abstract.

Correct.

Everything still appears to work when I add that back in after removing all the macro definitions and instantiations. Change included in v16, but I've still got the reverse of the removal commit around, so if anyone has good arguments why we need it, I can easily add versioning back in.

Great :)

Thanks,

Phil.