From: Claudio Fontana <cfontana@suse.de>
To: Paolo Bonzini <pbonzini@redhat.com>,
Gerd Hoffmann <kraxel@redhat.com>,
Peter Maydell <peter.maydell@linaro.org>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"QEMU Developers" <qemu-devel@nongnu.org>,
"Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PULL 00/10] Modules 20200702 patches
Date: Mon, 6 Jul 2020 16:30:22 +0200 [thread overview]
Message-ID: <b6d38863-1427-8721-5743-cd73e743bf40@suse.de> (raw)
In-Reply-To: <d9e28e08-e8d3-edd4-dcf0-af207a5ad3b8@redhat.com>
On 7/3/20 2:05 PM, Paolo Bonzini wrote:
> On 03/07/20 12:39, Gerd Hoffmann wrote:
>> On Fri, Jul 03, 2020 at 09:54:13AM +0100, Peter Maydell wrote:
>>> On Thu, 2 Jul 2020 at 13:23, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>>
>>>> The following changes since commit fc1bff958998910ec8d25db86cd2f53ff125f7ab:
>>>>
>>>> hw/misc/pca9552: Add missing TypeInfo::class_size field (2020-06-29 21:16:10 +0100)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>> git://git.kraxel.org/qemu tags/modules-20200702-pull-request
>>>>
>>>> for you to fetch changes up to 474a5d66036d18ee5ccaa88364660d05bf32127b:
>>>>
>>>> chardev: enable modules, use for braille (2020-07-01 21:08:11 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> qom: add support for qom objects in modules.
>>>> build some devices (qxl, virtio-gpu, ccid, usb-redir) as modules.
>>>> build braille chardev as module.
>>>>
>>>> note: qemu doesn't rebuild objects on cflags changes (specifically
>>>> -fPIC being added when code is switched from builtin to module).
>>>> Workaround for resulting build errors: "make clean", rebuild.
>>>>
>>>> ----------------------------------------------------------------
>>>>
>>>> Gerd Hoffmann (10):
>>>> module: qom module support
>>>> object: qom module support
>>>> qdev: device module support
>>>> build: fix device module builds
>>>> ccid: build smartcard as module
>>>> usb: build usb-redir as module
>>>> vga: build qxl as module
>>>> vga: build virtio-gpu only once
>>>> vga: build virtio-gpu as module
>>>> chardev: enable modules, use for braille
>>>
>>> No code review at all? :-(
>>
>> Well, there have been 5 revisions on the list, partly due to bugs being
>> fixed, partly with changes as response to review comments. So it got
>> some review (not much though) even though the v5 series (posted on June
>> 22th, so there was more than a week time) didn't got any acks so far.
>>
>>> In particular the "build: fix device module
>>> builds" commit (as you note in your commit message) does not look at
>>> all right.
>>
>> I think this stop-gap will do fine as long as we don't have any
>> target-specific modules.
>
> Yeah, it's hackish but target-specific modules would be a huge
> complication so I don't think we'd be having them anytime soon. With
> Meson removing the unnest-vars logic, the hack would also go away on its
> own. So I guess it's okay.
>
> Paolo
>
>
Hi Paolo,
well, since it is one of my main objectives behind lots of the refactoring work I have in progress,
and have been announcing this to the list since May:
https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg04628.html
I would disagree with the fact that target-specific modules are a huge complication in by themselves, as I got some initial promising results in building them.
Will work on the series and contribute it as soon as the basic requisite initial refactoring series are in,
so that the usefulness of target-specific modules can be demonstrated.
In my view modules could have been implemented differently, instead of the current design, in a way that would have made them easier to extend.
Probably meson is an interesting tool, I think however that good build designs will still be necessary,
and that bad ones will still result in difficult to extend build features.
Thanks & ciao,
Claudio
next prev parent reply other threads:[~2020-07-06 14:31 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-02 12:20 [PULL 00/10] Modules 20200702 patches Gerd Hoffmann
2020-07-02 12:20 ` [PULL 01/10] module: qom module support Gerd Hoffmann
2020-07-02 12:20 ` [PULL 02/10] object: " Gerd Hoffmann
2020-07-02 12:20 ` [PULL 03/10] qdev: device " Gerd Hoffmann
2020-07-02 12:20 ` [PULL 04/10] build: fix device module builds Gerd Hoffmann
2020-07-03 9:01 ` Claudio Fontana
2020-07-03 10:25 ` Gerd Hoffmann
2020-07-03 13:00 ` Claudio Fontana
2020-07-06 13:14 ` Gerd Hoffmann
2020-07-02 12:20 ` [PULL 05/10] ccid: build smartcard as module Gerd Hoffmann
2020-07-02 12:20 ` [PULL 06/10] usb: build usb-redir " Gerd Hoffmann
2020-07-02 12:20 ` [PULL 07/10] vga: build qxl " Gerd Hoffmann
2020-07-02 12:20 ` [PULL 08/10] vga: build virtio-gpu only once Gerd Hoffmann
2020-07-02 12:20 ` [PULL 09/10] vga: build virtio-gpu as module Gerd Hoffmann
2020-07-02 12:20 ` [PULL 10/10] chardev: enable modules, use for braille Gerd Hoffmann
2020-07-03 8:54 ` [PULL 00/10] Modules 20200702 patches Peter Maydell
2020-07-03 10:39 ` Gerd Hoffmann
2020-07-03 12:05 ` Paolo Bonzini
2020-07-06 13:20 ` Gerd Hoffmann
2020-07-06 14:30 ` Claudio Fontana [this message]
2020-07-03 9:14 ` Claudio Fontana
2020-07-03 10:29 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b6d38863-1427-8721-5743-cd73e743bf40@suse.de \
--to=cfontana@suse.de \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=kraxel@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).