[Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

Reformat the qapi-schema TargetType enumeration so that it has just
one target architecture name per line. This allows patches for
adding new targets to just add a single line, rather than having
to reformat most of the list (resulting in a hard-to-check diff).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
d15a9c23 is an example of what you get otherwise.

I would much prefer it if we autogenerated this list so you didn't
need to change this file at all to add a new target, but Anthony
is against that; so this is at least an improvement.

 qapi-schema.json |   30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 199744a..a8d361e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3018,10 +3018,32 @@
 # Since: 1.2.0
 ##
 { 'enum': 'TargetType',
-  'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel',
-            'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie',
-            'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4',
-            'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] }
+  'data': [ 'alpha',
+            'arm',
+            'cris',
+            'i386',
+            'lm32',
+            'm68k',
+            'microblazeel',
+            'microblaze',
+            'mips64el',
+            'mips64',
+            'mipsel',
+            'mips',
+            'moxie',
+            'or32',
+            'ppc64',
+            'ppcemb',
+            'ppc',
+            's390x',
+            'sh4eb',
+            'sh4',
+            'sparc64',
+            'sparc',
+            'unicore32',
+            'x86_64',
+            'xtensaeb',
+            'xtensa' ] }
 
 ##
 # @TargetInfo:
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Paolo Bonzini]

Il 20/05/2013 18:21, Peter Maydell ha scritto:
> Reformat the qapi-schema TargetType enumeration so that it has just
> one target architecture name per line. This allows patches for
> adding new targets to just add a single line, rather than having
> to reformat most of the list (resulting in a hard-to-check diff).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> d15a9c23 is an example of what you get otherwise.
> 
> I would much prefer it if we autogenerated this list so you didn't
> need to change this file at all to add a new target, but Anthony
> is against that; so this is at least an improvement.

I have queued a patch for 1.6 that would change this field to a free
string.  There is no use of this enum, not even for introspection.  You
don't need to know what targets were supported in the version that you
compiled from.  Only one target is supported in this executable anyway.

Paolo

>  qapi-schema.json |   30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 199744a..a8d361e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3018,10 +3018,32 @@
>  # Since: 1.2.0
>  ##
>  { 'enum': 'TargetType',
> -  'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel',
> -            'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie',
> -            'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4',
> -            'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] }
> +  'data': [ 'alpha',
> +            'arm',
> +            'cris',
> +            'i386',
> +            'lm32',
> +            'm68k',
> +            'microblazeel',
> +            'microblaze',
> +            'mips64el',
> +            'mips64',
> +            'mipsel',
> +            'mips',
> +            'moxie',
> +            'or32',
> +            'ppc64',
> +            'ppcemb',
> +            'ppc',
> +            's390x',
> +            'sh4eb',
> +            'sh4',
> +            'sparc64',
> +            'sparc',
> +            'unicore32',
> +            'x86_64',
> +            'xtensaeb',
> +            'xtensa' ] }
>  
>  ##
>  # @TargetInfo:
> 

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

On 20 May 2013 17:35, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 20/05/2013 18:21, Peter Maydell ha scritto:
>> Reformat the qapi-schema TargetType enumeration

> I have queued a patch for 1.6 that would change this field to a free
> string.  There is no use of this enum, not even for introspection.  You
> don't need to know what targets were supported in the version that you
> compiled from.  Only one target is supported in this executable anyway.

That sounds like a better solution. Do you have a pointer to the
patch? I tried a quick list archive search but probably didn't
use the right keywords.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]

On 05/20/2013 10:21 AM, Peter Maydell wrote:
> Reformat the qapi-schema TargetType enumeration so that it has just
> one target architecture name per line. This allows patches for
> adding new targets to just add a single line, rather than having
> to reformat most of the list (resulting in a hard-to-check diff).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> d15a9c23 is an example of what you get otherwise.
> 
> I would much prefer it if we autogenerated this list so you didn't
> need to change this file at all to add a new target, but Anthony
> is against that; so this is at least an improvement.

Yep, that raises (once again) the question of dynamic introspection -
there's a difference between the maximum amount of information known at
compile time, and the subset of enum values that are actually usable at
runtime.  This list is static (all known architectures), depending on
what solutions we come up with for dynamic introspection, maybe that
solution will also have a way for generating enum types.

> 
>  qapi-schema.json |   30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)

At any rate, I see no harm in taking this patch as-is now, while we
still spend time thinking about introspection.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

On 20 May 2013 17:41, Eric Blake <eblake@redhat.com> wrote:
> Yep, that raises (once again) the question of dynamic introspection -
> there's a difference between the maximum amount of information known at
> compile time, and the subset of enum values that are actually usable at
> runtime.  This list is static (all known architectures)

It's not really static though for practical purposes. The user
still has to be able to cope with "I know about architecture foo
but it's not in the list" (if talking to an older qemu) or
"the list contains architecture bar which I don't know about"
(if talking to a newer qemu). I think the only thing we really
need to avoid is changing the name of an existing architecture?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1874 bytes --]

On 05/20/2013 10:47 AM, Peter Maydell wrote:
> On 20 May 2013 17:41, Eric Blake <eblake@redhat.com> wrote:
>> Yep, that raises (once again) the question of dynamic introspection -
>> there's a difference between the maximum amount of information known at
>> compile time, and the subset of enum values that are actually usable at
>> runtime.  This list is static (all known architectures)
> 
> It's not really static though for practical purposes. The user
> still has to be able to cope with "I know about architecture foo
> but it's not in the list" (if talking to an older qemu) or
> "the list contains architecture bar which I don't know about"
> (if talking to a newer qemu). I think the only thing we really
> need to avoid is changing the name of an existing architecture?

I'm not talking about keeping the enum static as in unchanging in all
future qemu releases, so much as static in that "at the time qemu was
compiled, this was the list of architectures qemu knew about".  But what
good does it do to know what other architectures the compile-time qemu
knew about, when what we really care about is what does THIS qemu binary
emulate.  And Paolo raised the point that what we really care about is
static-per-binary - that is, each binary supports exactly one
architecture (unless you are doing a LOT of work to support multi-arch
emulation from a single binary!).

Changing the name of an architecture would be reflected by having a
different qemu-FOO binary name, right?  If that's the case, then
introspection of which architectures are supported is done by listing
all the matches to qemu-* in the directory where binaries are installed,
and we don't need a QMP enum duplicating what the file system already
tells us.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 20/05/2013 18:57, Eric Blake ha scritto:
> Changing the name of an architecture would be reflected by having
> a different qemu-FOO binary name, right?  If that's the case, then 
> introspection of which architectures are supported is done by
> listing all the matches to qemu-* in the directory where binaries
> are installed, and we don't need a QMP enum duplicating what the
> file system already tells us.

Yes, that was exactly my point (plus, those binaries might well come
from different versions of QEMU).

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRmlfnAAoJEBvWZb6bTYbyDq0P/AkeSq6btQY4noFbY7WQwN3k
4+uDUjtwvobsuoqvEKYKMCo6MDyJnfNkgX7S6aNSj+v5+59/tlS0evjgj0N76FGj
GC2FcDmmqu3OSt9+2i+pU6gs3eSk6UFbZ9/nxI9hzJ5aaPq6vCr/0l/lR1Ru5op6
W6705VDwp13jhgx+P14OfwI3qLztIP6/mL8Mc58J+7Y+GQ2SCs1CPJgUH4EW76Qg
uSOZ6FuNan7Hv+aNdUBXeXRteT4AfzwiGeCi2Z5zxvztUJr9YsOx+TKD/6HbO+1l
mLyHsOAEba6Ic9N8t/kZLh8yMK44WMcTX7Xnc583NZOz/njBS3+wJqBQLaggj37N
FX0w0lYkcbflqIei9YKK5X2Pex8tOQxHB7NDoGgcXQiOtMC8s3CJ1J91PaPrEprm
m9s9PXtZ+AXO4gt1cH5sofdPY5NiIChQOjgmp4TFf90NG1+t0DLIqnJaA0BWKEBl
uQbdyV3jUyZOdF0hgGRhrt0S1MuOS2e+cCVj1/qNmoAFMkFKEKevInS3QxH2kSNs
wkjBHZ9g7LU8LO4qyBCpp4mxepsrKh9kgGnJDkx6cVULC3jkjbSqeORFYblgp/lm
4ZWc95r1m8XkLaIrxuA5DPO1yuHT/bfZiZp1KiwMRVD3vI877y31nUeSK8jdv3Rp
iVzfr9t7+uWYWRF3t516
=81Fx
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Paolo Bonzini]

Il 20/05/2013 18:38, Peter Maydell ha scritto:
>> > I have queued a patch for 1.6 that would change this field to a free
>> > string.  There is no use of this enum, not even for introspection.  You
>> > don't need to know what targets were supported in the version that you
>> > compiled from.  Only one target is supported in this executable anyway.
> That sounds like a better solution. Do you have a pointer to the
> patch? I tried a quick list archive search but probably didn't
> use the right keywords.

No, I haven't quite tested it enough.

I'll send an RFC shortly, as soon as compilation finishes.

Paolo

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> Reformat the qapi-schema TargetType enumeration so that it has just
> one target architecture name per line. This allows patches for
> adding new targets to just add a single line, rather than having
> to reformat most of the list (resulting in a hard-to-check diff).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> d15a9c23 is an example of what you get otherwise.
>
> I would much prefer it if we autogenerated this list so you didn't
> need to change this file at all to add a new target, but Anthony
> is against that; so this is at least an improvement.

I don't object to autogenerating it.  I object to autogenerating based
on the selected targets.

The enum should be fixed regardless of what the configure line is.

Regards,

Anthony Liguori

>
>  qapi-schema.json |   30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 199744a..a8d361e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3018,10 +3018,32 @@
>  # Since: 1.2.0
>  ##
>  { 'enum': 'TargetType',
> -  'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel',
> -            'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie',
> -            'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4',
> -            'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] }
> +  'data': [ 'alpha',
> +            'arm',
> +            'cris',
> +            'i386',
> +            'lm32',
> +            'm68k',
> +            'microblazeel',
> +            'microblaze',
> +            'mips64el',
> +            'mips64',
> +            'mipsel',
> +            'mips',
> +            'moxie',
> +            'or32',
> +            'ppc64',
> +            'ppcemb',
> +            'ppc',
> +            's390x',
> +            'sh4eb',
> +            'sh4',
> +            'sparc64',
> +            'sparc',
> +            'unicore32',
> +            'x86_64',
> +            'xtensaeb',
> +            'xtensa' ] }
>  
>  ##
>  # @TargetInfo:
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 20/05/2013 18:21, Peter Maydell ha scritto:
>> Reformat the qapi-schema TargetType enumeration so that it has just
>> one target architecture name per line. This allows patches for
>> adding new targets to just add a single line, rather than having
>> to reformat most of the list (resulting in a hard-to-check diff).
>> 
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> d15a9c23 is an example of what you get otherwise.
>> 
>> I would much prefer it if we autogenerated this list so you didn't
>> need to change this file at all to add a new target, but Anthony
>> is against that; so this is at least an improvement.
>
> I have queued a patch for 1.6 that would change this field to a free
> string.  There is no use of this enum, not even for introspection.

I don't object to this, however..

> You
> don't need to know what targets were supported in the version that you
> compiled from.  Only one target is supported in this executable
> anyway.

It seems useful to me.  One day we may support multiple targets per
executable.

There's no obvious place where all of the possible targets are listed so
from the point of view of someone trying to figure out what the
platforms are while writing a management tool, it seems like a useful
thing to have.

We don't add targets very often...  are we optimizing for an uncommon
scenario here?

Regards,

Anthony Liguori

>
> Paolo
>
>>  qapi-schema.json |   30 ++++++++++++++++++++++++++----
>>  1 file changed, 26 insertions(+), 4 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 199744a..a8d361e 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3018,10 +3018,32 @@
>>  # Since: 1.2.0
>>  ##
>>  { 'enum': 'TargetType',
>> -  'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel',
>> -            'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie',
>> -            'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4',
>> -            'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] }
>> +  'data': [ 'alpha',
>> +            'arm',
>> +            'cris',
>> +            'i386',
>> +            'lm32',
>> +            'm68k',
>> +            'microblazeel',
>> +            'microblaze',
>> +            'mips64el',
>> +            'mips64',
>> +            'mipsel',
>> +            'mips',
>> +            'moxie',
>> +            'or32',
>> +            'ppc64',
>> +            'ppcemb',
>> +            'ppc',
>> +            's390x',
>> +            'sh4eb',
>> +            'sh4',
>> +            'sparc64',
>> +            'sparc',
>> +            'unicore32',
>> +            'x86_64',
>> +            'xtensaeb',
>> +            'xtensa' ] }
>>  
>>  ##
>>  # @TargetInfo:
>> 

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

On 22 May 2013 14:12, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> I would much prefer it if we autogenerated this list so you didn't
>> need to change this file at all to add a new target, but Anthony
>> is against that; so this is at least an improvement.
>
> I don't object to autogenerating it.  I object to autogenerating based
> on the selected targets.

I never suggested that -- what I proposed on IRC was that it
could be autogenerated based on the contents of default-configs/.
But if we're making it not-an-enum anyway the question is moot.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Andreas Färber]

Am 22.05.2013 15:15, schrieb Anthony Liguori:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Il 20/05/2013 18:21, Peter Maydell ha scritto:
>>> Reformat the qapi-schema TargetType enumeration so that it has just
>>> one target architecture name per line. This allows patches for
>>> adding new targets to just add a single line, rather than having
>>> to reformat most of the list (resulting in a hard-to-check diff).
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> d15a9c23 is an example of what you get otherwise.
>>>
>>> I would much prefer it if we autogenerated this list so you didn't
>>> need to change this file at all to add a new target, but Anthony
>>> is against that; so this is at least an improvement.
>>
>> I have queued a patch for 1.6 that would change this field to a free
>> string.  There is no use of this enum, not even for introspection.
> 
> I don't object to this, however..
> 
>> You
>> don't need to know what targets were supported in the version that you
>> compiled from.  Only one target is supported in this executable
>> anyway.
> 
> It seems useful to me.  One day we may support multiple targets per
> executable.
> 
> There's no obvious place where all of the possible targets are listed so
> from the point of view of someone trying to figure out what the
> platforms are while writing a management tool, it seems like a useful
> thing to have.

Does today's API allow for returning multiple enum values? I doubt so...

> We don't add targets very often...  are we optimizing for an uncommon
> scenario here?

Well, lately every second release or so.

More common is however that people start writing a new target and don't
submit it yet (ahem!) while another target gets added, and the current
form of rebreaking this block of enum values causes more conflicts than
with Peter's proposal where the place to add new values is more likely
to differ between targets and becomes more secure to add to.

So if we don't go for Paolo's string version,

Reviewed-by: Andreas Färber <afaerber@suse.de>

One thought that crossed my mind was whether to put [ and ] on lines of
their own to aid adding before alpha and after xtensa, but apart from
aarch64 I couldn't think of such fringe target names. ;)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

On 22 May 2013 14:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>> You
>> don't need to know what targets were supported in the version that you
>> compiled from.  Only one target is supported in this executable
>> anyway.
>
> It seems useful to me.  One day we may support multiple targets per
> executable.

Why would you care about which architectures the executable supports?
What you actually want to know is which machine models are supported;
whether board foo happens to be ARM or PPC isn't really very interesting
IMHO.

-- PMM

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Anthony Liguori]

Andreas Färber <afaerber@suse.de> writes:

> Am 22.05.2013 15:15, schrieb Anthony Liguori:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Il 20/05/2013 18:21, Peter Maydell ha scritto:
>>>> Reformat the qapi-schema TargetType enumeration so that it has just
>>>> one target architecture name per line. This allows patches for
>>>> adding new targets to just add a single line, rather than having
>>>> to reformat most of the list (resulting in a hard-to-check diff).
>>>>
>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>> ---
>>>> d15a9c23 is an example of what you get otherwise.
>>>>
>>>> I would much prefer it if we autogenerated this list so you didn't
>>>> need to change this file at all to add a new target, but Anthony
>>>> is against that; so this is at least an improvement.
>>>
>>> I have queued a patch for 1.6 that would change this field to a free
>>> string.  There is no use of this enum, not even for introspection.
>> 
>> I don't object to this, however..
>> 
>>> You
>>> don't need to know what targets were supported in the version that you
>>> compiled from.  Only one target is supported in this executable
>>> anyway.
>> 
>> It seems useful to me.  One day we may support multiple targets per
>> executable.
>> 
>> There's no obvious place where all of the possible targets are listed so
>> from the point of view of someone trying to figure out what the
>> platforms are while writing a management tool, it seems like a useful
>> thing to have.
>
> Does today's API allow for returning multiple enum values? I doubt
> so...

Nope.

>> We don't add targets very often...  are we optimizing for an uncommon
>> scenario here?
>
> Well, lately every second release or so.
>
> More common is however that people start writing a new target and don't
> submit it yet (ahem!) while another target gets added, and the current
> form of rebreaking this block of enum values causes more conflicts

Sounds like a good reason to submit the target upstream to me...

> than
> with Peter's proposal where the place to add new values is more likely
> to differ between targets and becomes more secure to add to.

There's no need to keep it in sorted order.  We should just add stuff to
the end of the enum.

Heck, if someone wants to send a patch to randomize the sort order,
that'd be fine too :-)

Regards,

Anthony Liguori

> So if we don't go for Paolo's string version,
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
> One thought that crossed my mind was whether to put [ and ] on lines of
> their own to aid adding before alpha and after xtensa, but apart from
> aarch64 I couldn't think of such fringe target names. ;)
>
> Regards,
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 22 May 2013 14:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>> You
>>> don't need to know what targets were supported in the version that you
>>> compiled from.  Only one target is supported in this executable
>>> anyway.
>>
>> It seems useful to me.  One day we may support multiple targets per
>> executable.
>
> Why would you care about which architectures the executable supports?
> What you actually want to know is which machine models are supported;
> whether board foo happens to be ARM or PPC isn't really very interesting
> IMHO.

That's a very good point.  It was the libvirt folks that requested
this.  Perhaps they can shed some light on the logic?

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] New targets (was: [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line)

[Andreas Färber]

Am 22.05.2013 16:28, schrieb Anthony Liguori:
> Andreas Färber <afaerber@suse.de> writes:
>> More common is however that people start writing a new target and don't
>> submit it yet (ahem!) while another target gets added, and the current
>> form of rebreaking this block of enum values causes more conflicts
> 
> Sounds like a good reason to submit the target upstream to me...

So are incompletely implemented targets (wrt instruction set) eligible
for upstream these days? I thought they'd need to be able to run at
least some small image successfully, preferably Linux where possible.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Paolo Bonzini]

Il 22/05/2013 16:29, Anthony Liguori ha scritto:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On 22 May 2013 14:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>> You
>>>> don't need to know what targets were supported in the version that you
>>>> compiled from.  Only one target is supported in this executable
>>>> anyway.
>>>
>>> It seems useful to me.  One day we may support multiple targets per
>>> executable.
>>
>> Why would you care about which architectures the executable supports?
>> What you actually want to know is which machine models are supported;
>> whether board foo happens to be ARM or PPC isn't really very interesting
>> IMHO.
> 
> That's a very good point.  It was the libvirt folks that requested
> this.  Perhaps they can shed some light on the logic?

There is processor-dependent logic in libvirt, for example the CPUID bits.

Paolo

Re: [Qemu-devel] New targets (was: [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line)

[Anthony Liguori]

Andreas Färber <afaerber@suse.de> writes:

> Am 22.05.2013 16:28, schrieb Anthony Liguori:
>> Andreas Färber <afaerber@suse.de> writes:
>>> More common is however that people start writing a new target and don't
>>> submit it yet (ahem!) while another target gets added, and the current
>>> form of rebreaking this block of enum values causes more conflicts
>> 
>> Sounds like a good reason to submit the target upstream to me...
>
> So are incompletely implemented targets (wrt instruction set) eligible
> for upstream these days?

Aren't most of our target incomplete by some standard?

I thought the typical approach for a target was to get linux-user
working first, and then go for softmmu.  As long as linux-user can run
application, I think it's fair game for merging.

> I thought they'd need to be able to run at
> least some small image successfully, preferably Linux where possible.

I think that's far too high of a hurdle.

Regards,

Anthony Liguori

>
> Regards,
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] New targets (was: [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line)

[Peter Maydell]

On 22 May 2013 15:48, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Andreas Färber <afaerber@suse.de> writes:
>> Am 22.05.2013 16:28, schrieb Anthony Liguori:
>> So are incompletely implemented targets (wrt instruction set) eligible
>> for upstream these days?
>
> Aren't most of our target incomplete by some standard?
>
> I thought the typical approach for a target was to get linux-user
> working first, and then go for softmmu.  As long as linux-user can run
> application, I think it's fair game for merging.
>
>> I thought they'd need to be able to run at
>> least some small image successfully, preferably Linux where possible.
>
> I think that's far too high of a hurdle.

I think the bar should probably be at about "either can run some
plausible set of Linux binaries in user mode, or can run some
softmmu image (eg linux with a minimal config", whichever the
target submitter prefers. Basically something plausibly useful
to somebody other than the developer.

thanks
-- PMM

Re: [Qemu-devel] New targets (was: [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line)

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 22 May 2013 15:48, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Andreas Färber <afaerber@suse.de> writes:
>>> Am 22.05.2013 16:28, schrieb Anthony Liguori:
>>> So are incompletely implemented targets (wrt instruction set) eligible
>>> for upstream these days?
>>
>> Aren't most of our target incomplete by some standard?
>>
>> I thought the typical approach for a target was to get linux-user
>> working first, and then go for softmmu.  As long as linux-user can run
>> application, I think it's fair game for merging.
>>
>>> I thought they'd need to be able to run at
>>> least some small image successfully, preferably Linux where possible.
>>
>> I think that's far too high of a hurdle.
>
> I think the bar should probably be at about "either can run some
> plausible set of Linux binaries in user mode, or can run some
> softmmu image (eg linux with a minimal config", whichever the
> target submitter prefers. Basically something plausibly useful
> to somebody other than the developer.

Another way to put this, and this is true for *all* contributions, is
that the bar for acceptance is whether something is useful even if the
submitter immediately stopped contributing after the patch was merged.

Regards,

Anthony Liguori

>
> thanks
> -- PMM

Re: [Qemu-devel] [libvirt] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

On 05/22/2013 08:29 AM, Anthony Liguori wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On 22 May 2013 14:15, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>> You
>>>> don't need to know what targets were supported in the version that you
>>>> compiled from.  Only one target is supported in this executable
>>>> anyway.
>>>
>>> It seems useful to me.  One day we may support multiple targets per
>>> executable.
>>
>> Why would you care about which architectures the executable supports?
>> What you actually want to know is which machine models are supported;
>> whether board foo happens to be ARM or PPC isn't really very interesting
>> IMHO.
> 
> That's a very good point.  It was the libvirt folks that requested
> this.  Perhaps they can shed some light on the logic?

I think knowing the architecture (such as x86 vs. pseries ppc) is used
by libvirt to know what default devices the board supports (for example,
whether usb is present by default).  There's probably still room for
improvement for communication between libvirt and qemu on what exactly
is being supported, and knowing an architecture type may be too broad of
a knob compared to what is really wanted, except that I don't have a
good handle on what is really wanted.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [libvirt] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Peter Maydell]

On 24 May 2013 22:38, Eric Blake <eblake@redhat.com> wrote:
> I think knowing the architecture (such as x86 vs. pseries ppc) is used
> by libvirt to know what default devices the board supports (for example,
> whether usb is present by default).

...but this is a per-board question, since (for instance) some
ARM boards have USB and some do not, some have PCI and some
do not, and so on.

> There's probably still room for
> improvement for communication between libvirt and qemu on what exactly
> is being supported, and knowing an architecture type may be too broad of
> a knob compared to what is really wanted, except that I don't have a
> good handle on what is really wanted.

It does sound like we could use a more specific enquiry mechanism.

-- PMM

Re: [Qemu-devel] [libvirt] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line

[Andreas Färber]

Am 25.05.2013 11:18, schrieb Peter Maydell:
> On 24 May 2013 22:38, Eric Blake <eblake@redhat.com> wrote:
>> I think knowing the architecture (such as x86 vs. pseries ppc) is used
>> by libvirt to know what default devices the board supports (for example,
>> whether usb is present by default).
> 
> ...but this is a per-board question, since (for instance) some
> ARM boards have USB and some do not, some have PCI and some
> do not, and so on.

If we look beyond what we have at the moment, then Anthony has been
promoting the idea of having boards potentially be just a config file
wiring up QOM devices. I.e., the distro or user would be able to have
random boards with a number of devices on them.

In that very general case, much like our boards today, the question of
can-I-attach-a-certain-device-on-a-bus becomes: Can a solver come up
with a path from any device on the board to a device exposing the
required bus.
E.g., if there is either some USB host interface on SysBus or a PCI host
bridge and either a PCI-USB bridge plugged or available to plug, USB
devices can be offered - which would then dynamically rule out
s390-virtio and s390-ccw-virtio without hardcoding anything in libvirt.

Problem is, our devices often have no static way of telling what busses
they will/may provide*, so instantiating would often be the only way to
find out.

* Well, pci-host-bridge as base type might indicate PCIBus to a human,
but ISABus, USBBus and other bridges have non-telling base types.

>> There's probably still room for
>> improvement for communication between libvirt and qemu on what exactly
>> is being supported, and knowing an architecture type may be too broad of
>> a knob compared to what is really wanted, except that I don't have a
>> good handle on what is really wanted.

Knowing the provided architecture would today give us hints what -cpu
options may be supported. That becomes moot in a future
multi-architecture case though, where devices might be constructed from
config or some generic object-create QMP command with qom-set for device
options.

> It does sound like we could use a more specific enquiry mechanism.

Ack, but sadly qemu-system-foo -M bar -S + qom-list + qom-list-types is
the most accurate interface I can think of today.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg