qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: "Eric Blake" <eblake@redhat.com>, "Ján Tomko" <jtomko@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Michael Roth" <mdroth@linux.vnet.ibm.com>,
	"Anthony Liguori" <aliguori@amazon.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] Should we have a 2.0-rc3 ?
Date: Thu, 10 Apr 2014 17:27:07 +0200	[thread overview]
Message-ID: <5346B84B.4030506@suse.de> (raw)
In-Reply-To: <5346B2A3.1080900@redhat.com>


On 10.04.14 17:02, Eric Blake wrote:
> On 04/10/2014 07:45 AM, Alexander Graf wrote:
>
>>>>> Is this something that can be quickly fixed (perhaps by reverting the
>>>>> PPC patch until a more complete solution is ready), and if so, is it
>>>>> worth doing for 2.0 proper, rather than waiting for 2.0.1?
>>>> Which way works better for you? I'd be perfectly fine with reverting
>>>> the patch. Libvirt is the only reason that path is there in the first
>>>> place.
>>>>
>>> If I read the git history correctly, there were two patches changing
>>> pci bus
>>> names for ppc in this release, not just one:
>> The main difference is that the g3beige and mac99 targets are not
>> supported by libvirt FWIW :).
>>
>> But I agree that this is messy. And a pretty intrusive change pretty
>> late in the game. Eric, how hard would a special case for this be in
>> libvirt code? Are we talking about a 2 line patch?
> Here's the current libvirt patch proposal:
>
> https://www.redhat.com/archives/libvir-list/2014-April/msg00444.html
>
> a bit more than a 2-line patch:
>
>   src/qemu/qemu_capabilities.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
>
> We already have to special case on machine type for all qemu older than
> the point where we introduce sane names; but it would be nicer if that
> were the ONLY special casing (rather than having the _additional_
> special casing that for 2.0, ppc, but not other machines, behave
> differently).  The IDEAL situation is to have a QMP command that can
> query which naming convention is in use for a given machine; even if
> such command is not introduced until 2.1, the logic will look something
> like:
>
> if (probe exists)
>    use results of probe to set QEMU_CAPS_PCI_MULTIBUS
> else if (machine with sane handling)
>    assume QEMU_CAPS_PCI_MULTIBUS
> else
>    assume no QEMU_CAPS_PCI_MULTIBUS
>
> and is completely independent of version checks, which means it is
> portable even to downstream backports where the version number is not as
> large as upstream, without any modification when backporting this hunk.
>
> Without a QMP command to probe it, but with all machines switched to
> sane naming in the same version of qemu, the logic looks more like:
>
> if (x86 or 686)
>    assume QEMU_CAPS_PCI_MULTIBUS
> else if (version check) // evil for downstream backports
>    set QEMU_CAPS_PCI_MULTIBUS if new enough
>
> which looks shorter, but plays havoc with downstream ports, which now
> have to patch the version check to play nicely with downstream.
>
> Furthermore, if qemu 2.0 is released with PPC being a special case, the
> logic expands:
>
> if (x86 or 686)
>    assume QEMU_CAPS_PCI_MULTIBUS
> else if (PPC)
>    if (version check for 2.0) // evil for downstream
>      set QEMU_CAPS_PCI_MULTIBUS
> else if (version check for 2.1) // evil for downstream
>    set QEMU_CAPS_PCI_MULTIBUS
>
> and now there are two version checks instead of one that downstream has
> to worry about.

Hrm, so what if we just ditch pre-2.0 support for PPC in libvirt? Then 
it'd become

if (machine_type == pc || machine_type == pseries || machine_type == 
ppce500)
   assume QEMU_CAPS_PCI_MULTIBUS
else ...

and everyone is happy, no? :)

(note that I prefer to base machine specific bits on machine types, but 
if you like to commonalize all PPC machines I'm fine with that too)


Alex

  reply	other threads:[~2014-04-10 15:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-10 11:17 [Qemu-devel] Should we have a 2.0-rc3 ? Peter Maydell
2014-04-10 11:24 ` Alexander Graf
2014-04-10 15:22   ` Michael S. Tsirkin
2014-04-10 11:49 ` Kevin Wolf
2014-04-10 12:44 ` Eric Blake
2014-04-10 12:46   ` Alexander Graf
2014-04-10 12:51     ` Eric Blake
2014-04-10 12:56       ` Alexander Graf
2014-04-10 13:41     ` Ján Tomko
2014-04-10 13:45       ` Alexander Graf
2014-04-10 15:02         ` Eric Blake
2014-04-10 15:27           ` Alexander Graf [this message]
2014-04-10 15:38             ` Eric Blake
2014-04-10 15:42               ` Alexander Graf
2014-04-11  8:01           ` Markus Armbruster
2014-04-11  8:37             ` Daniel P. Berrange
2014-04-10 15:26 ` Michael S. Tsirkin
2014-04-10 18:55 ` Cole Robinson
2014-04-10 21:30 ` Peter Maydell
2014-04-11 17:37 ` Peter Maydell
2014-04-11 22:55   ` Alexander Graf
2014-04-12  1:49   ` Paolo Bonzini
2014-04-12  8:48   ` Michael Tokarev

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=5346B84B.4030506@suse.de \
    --to=agraf@suse.de \
    --cc=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=eblake@redhat.com \
    --cc=jtomko@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@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).