All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	muriloo@linux.ibm.com, qemu-devel@nongnu.org
Cc: "Fabiano Rosas" <farosas@linux.ibm.com>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	mopsfelder@gmail.com, qemu-ppc@nongnu.org,
	"Cédric Le Goater" <clg@kaod.org>,
	"Miroslav Rezanina" <mrezanin@redhat.com>
Subject: QEMU with reduced amount of machines in the config (was: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set)
Date: Tue, 10 May 2022 10:40:25 +0200	[thread overview]
Message-ID: <a61ed836-2d31-677c-235d-9fc491e483ac@redhat.com> (raw)
In-Reply-To: <0664ffae-9037-3096-ea0d-6d6732ac0214@ilande.co.uk>

On 05/05/2022 10.19, Mark Cave-Ayland wrote:
> On 05/05/2022 02:24, Murilo Opsfelder Araújo wrote:
> 
>> Hi, Mark.
>>
>> On 5/4/22 11:32, Mark Cave-Ayland wrote:
>>> On 04/05/2022 14:16, Murilo Opsfelder Araújo wrote:
>>>> Hi, Mark.
>>>>
>>>> On 5/4/22 04:10, Mark Cave-Ayland wrote:
>>>>> On 02/05/2022 14:36, Murilo Opsfelder Araújo wrote:
>>>>>
>>>>>> Hi, Mark.
>>>>>>
>>>>>> Thanks for reviewing.  Comments below.
>>>>>>
>>>>>> On 5/2/22 06:43, Mark Cave-Ayland wrote:
>>>>>>> On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote:
[...]
>>>>> Certainly QEMU could do better here, but then if you are already 
>>>>> patching the build to generate a custom configuration as above, you 
>>>>> might as well just patch out the relevant part of hmp-commands-info.hx 
>>>>> at the same time until proper per-device HMP/QMP support is added.
>>>>
>>>> We are not patching the build.  We are just configuring it.
>>>
>>> That's not true though: the spec file linked above contains 20 patches to 
>>> the vanilla QEMU source, including feeding custom device lists into the 
>>> build system via 
>>> https://gitlab.com/redhat/centos-stream/rpms/qemu-kvm/-/blob/c9s/0005-Enable-disable-devices-for-RHEL.patch. 
>>
>> I'm sorry.  I think I wasn't clear enough.
>>
>> The reproducer I sent in the email was *adapted* from CentOS/RHEL 
>> qemu-kvm.spec.
>> Meaning that we configured the devices in the same way that the 
>> CentOS/RHEL package is configuring but used the unmodified QEMU source 
>> tree from upstream.
>>
>> I did that because I wanted to mimic its configuration (devices and 
>> configure options) against the upstream code to determine if the failure 
>> was a downstream or upstream issue.
>> It turns out it's an upstream regression.
> 
> Ah so that's the problem then - you can't guarantee the configuration from a 
> vendor-customised build will work with upstream, particularly when the build 
> system itself has been patched. More explanation below.
> 
>>> Perhaps CONFIG_MOS6522 is missing from ppc64-rh-devices?
>>
>> I don't think so.  Since the CONFIG_MOS6522 is available, one can build 
>> without it and code should cope with that.
> 
> In an upstream build the default boards for each target are listed in the 
> configs/ directory and Kconfig specifies the dependencies such that for ppc 
> and ppc64 CONFIG_MOS6522 is **always** defined. However what happens in the 
> .spec file you linked to is that the device lists **are being overridden** 
> by the provided ppc64-rh-devices file which **doesn't** contain 
> CONFIG_MOS6522. It seems to me that the .spec file can only work with that 
> vendor-specific ppc64-rh-devices file if it also patches the build system to 
> prevent this error occurring.

Well, yes and no. QEMU was quite monolitic for many years (that's where most 
of these downstream patches have their origin from), but the Kconfig stuff 
that was introduced a while ago (which required quite a lot of patches to 
untangle many parts of the build) was indeed meant for giving more 
flexibility here, so that downstream vendors could build QEMU more easily 
with only a subset of the machines (which is not only a desire by Red Hat, 
by the way - do you remember the "nemu" fork a while ago for example?). We 
still have quite a bit on the TODO list for full flexibility (e.g. mips 
config has not been fully switched to Kconfig yet, I think), but for most 
architectures, it should already work that you only select a subset of the 
available machines.

However, as you already noticed the big downside is still that this is not 
tested automatically in the upstream CI environment, so regressions like 
this one here with the mos6522 are expected. In the long run, I think we 
certainly want to test e.g. this cut-down CentOS-stream configuration in the 
CI, too ... it's just a big bunch of work that nobody started to tangle with 
yet.

  Thomas



  reply	other threads:[~2022-05-10  9:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-29 23:31 [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set Murilo Opsfelder Araujo
2022-05-02  9:43 ` Mark Cave-Ayland
2022-05-02 13:36   ` Murilo Opsfelder Araújo
2022-05-03 14:06     ` Fabiano Rosas
2022-05-04  7:16       ` Mark Cave-Ayland
2022-05-04 14:26         ` Fabiano Rosas
2022-05-04 14:48           ` Mark Cave-Ayland
2022-05-10  8:03             ` QEMU 32-bit vs. 64-bit binaries (was: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set) Thomas Huth
2022-05-10  8:26               ` Alistair Francis
2022-05-10  8:54               ` QEMU 32-bit vs. 64-bit binaries Markus Armbruster
2022-05-10  9:01                 ` Thomas Huth
2022-05-10  9:14                   ` Peter Maydell
2022-05-10  9:22                     ` Dr. David Alan Gilbert
2022-05-10  9:31                       ` Thomas Huth
2022-05-10  9:47                         ` Peter Maydell
2022-05-10 10:14                         ` BALATON Zoltan
2022-05-10 12:20                         ` Gerd Hoffmann
2022-05-10 12:25                           ` Peter Maydell
2022-05-04  7:10     ` [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set Mark Cave-Ayland
2022-05-04 13:16       ` Murilo Opsfelder Araújo
2022-05-04 14:32         ` Mark Cave-Ayland
2022-05-05  1:24           ` Murilo Opsfelder Araújo
2022-05-05  8:19             ` Mark Cave-Ayland
2022-05-10  8:40               ` Thomas Huth [this message]
2022-05-06 23:44   ` Murilo Opsfelder Araújo
2022-05-08  9:30     ` Mark Cave-Ayland

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=a61ed836-2d31-677c-235d-9fc491e483ac@redhat.com \
    --to=thuth@redhat.com \
    --cc=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --cc=dgilbert@redhat.com \
    --cc=farosas@linux.ibm.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mopsfelder@gmail.com \
    --cc=mrezanin@redhat.com \
    --cc=muriloo@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.