| Message ID | 20250418172908.25147-2-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | single-binary: Make hw/arm/ common | expand |
+Daniel/Markus on this single one. On 18/4/25 19:28, Philippe Mathieu-Daudé wrote: > The QAPI-generated 'TargetInfo' structure name is only used > in a single file. We want to heavily use another structure > similarly named. Rename the QAPI one, since structure names > are not part of the public API. > > Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > qapi/machine.json | 12 ++++++------ > hw/core/machine-qmp-cmds.c | 4 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/qapi/machine.json b/qapi/machine.json > index a6b8795b09e..3246212f048 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -275,15 +275,15 @@ > { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } > > ## > -# @TargetInfo: > +# @BinaryTargetInfo: > # > -# Information describing the QEMU target. > +# Information describing the QEMU binary target. > # > -# @arch: the target architecture > +# @arch: the binary target architecture > # > # Since: 1.2 > ## > -{ 'struct': 'TargetInfo', > +{ 'struct': 'BinaryTargetInfo', > 'data': { 'arch': 'SysEmuTarget' } } > > ## > @@ -291,11 +291,11 @@ > # > # Return information about the target for this QEMU > # > -# Returns: TargetInfo > +# Returns: BinaryTargetInfo > # > # Since: 1.2 > ## > -{ 'command': 'query-target', 'returns': 'TargetInfo' } > +{ 'command': 'query-target', 'returns': 'BinaryTargetInfo' } > > ## > # @UuidInfo: > diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c > index 3130c5cd456..408994b67d7 100644 > --- a/hw/core/machine-qmp-cmds.c > +++ b/hw/core/machine-qmp-cmds.c > @@ -132,9 +132,9 @@ CurrentMachineParams *qmp_query_current_machine(Error **errp) > return params; > } > > -TargetInfo *qmp_query_target(Error **errp) > +BinaryTargetInfo *qmp_query_target(Error **errp) > { > - TargetInfo *info = g_malloc0(sizeof(*info)); > + BinaryTargetInfo *info = g_malloc0(sizeof(*info)); > > info->arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, > &error_abort);
On 4/18/25 10:28, Philippe Mathieu-Daudé wrote: > The QAPI-generated 'TargetInfo' structure name is only used > in a single file. We want to heavily use another structure > similarly named. Rename the QAPI one, since structure names > are not part of the public API. > > Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > qapi/machine.json | 12 ++++++------ > hw/core/machine-qmp-cmds.c | 4 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > The QAPI-generated 'TargetInfo' structure name is only used > in a single file. We want to heavily use another structure > similarly named. Rename the QAPI one, since structure names > are not part of the public API. > > Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > qapi/machine.json | 12 ++++++------ > hw/core/machine-qmp-cmds.c | 4 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/qapi/machine.json b/qapi/machine.json > index a6b8795b09e..3246212f048 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -275,15 +275,15 @@ > { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } > > ## > -# @TargetInfo: > +# @BinaryTargetInfo: > # > -# Information describing the QEMU target. > +# Information describing the QEMU binary target. What's "the QEMU binary target"? The QEMU binary's target? From the QMP user's point of view, perhaps "the QEMU process's target" would make more sense. > # > -# @arch: the target architecture > +# @arch: the binary target architecture Are there non-binary target architectures? > # > # Since: 1.2 > ## > -{ 'struct': 'TargetInfo', > +{ 'struct': 'BinaryTargetInfo', > 'data': { 'arch': 'SysEmuTarget' } } > > ## > @@ -291,11 +291,11 @@ > # > # Return information about the target for this QEMU > # > -# Returns: TargetInfo > +# Returns: BinaryTargetInfo > # > # Since: 1.2 > ## > -{ 'command': 'query-target', 'returns': 'TargetInfo' } > +{ 'command': 'query-target', 'returns': 'BinaryTargetInfo' } > > ## > # @UuidInfo: > diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c > index 3130c5cd456..408994b67d7 100644 > --- a/hw/core/machine-qmp-cmds.c > +++ b/hw/core/machine-qmp-cmds.c > @@ -132,9 +132,9 @@ CurrentMachineParams *qmp_query_current_machine(Error **errp) > return params; > } > > -TargetInfo *qmp_query_target(Error **errp) > +BinaryTargetInfo *qmp_query_target(Error **errp) > { > - TargetInfo *info = g_malloc0(sizeof(*info)); > + BinaryTargetInfo *info = g_malloc0(sizeof(*info)); > > info->arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, > &error_abort);
On 22/4/25 07:55, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> The QAPI-generated 'TargetInfo' structure name is only used >> in a single file. We want to heavily use another structure >> similarly named. Rename the QAPI one, since structure names >> are not part of the public API. >> >> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> qapi/machine.json | 12 ++++++------ >> hw/core/machine-qmp-cmds.c | 4 ++-- >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/qapi/machine.json b/qapi/machine.json >> index a6b8795b09e..3246212f048 100644 >> --- a/qapi/machine.json >> +++ b/qapi/machine.json >> @@ -275,15 +275,15 @@ >> { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } >> >> ## >> -# @TargetInfo: >> +# @BinaryTargetInfo: >> # >> -# Information describing the QEMU target. >> +# Information describing the QEMU binary target. > > What's "the QEMU binary target"? The QEMU binary's target? For me 'qemu-system-aarch64' is a QEMU binary, but for Pierrick and Richard it is the QEMU target, so I merged both names ¯\_(ツ)_/¯ This structure describes the static target configuration built into a binary, i.e. TARGET_NAME=aarch64, TARGET_BIG_ENDIAN=false. For the forthcoming single/heterogeneous binary, we don't have a particular restricted configuration in the binary. What about "Information describing the QEMU target configuration built in a binary."? > > From the QMP user's point of view, perhaps "the QEMU process's target" > would make more sense. So maybe ProcessTargetInfo is a better structure name. For heterogeneous target I suppose we'll return SYS_EMU_TARGET_HETERO and we'll provide new QMP commands, possibly returning array of ProcessTargetInfo. > >> # >> -# @arch: the target architecture >> +# @arch: the binary target architecture > > Are there non-binary target architectures? :) I won't update this line. > >> # >> # Since: 1.2 >> ## >> -{ 'struct': 'TargetInfo', >> +{ 'struct': 'BinaryTargetInfo', >> 'data': { 'arch': 'SysEmuTarget' } } >> >> ## >> @@ -291,11 +291,11 @@ >> # >> # Return information about the target for this QEMU >> # >> -# Returns: TargetInfo >> +# Returns: BinaryTargetInfo >> # >> # Since: 1.2 >> ## >> -{ 'command': 'query-target', 'returns': 'TargetInfo' } >> +{ 'command': 'query-target', 'returns': 'BinaryTargetInfo' } >> >> ## >> # @UuidInfo: >> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c >> index 3130c5cd456..408994b67d7 100644 >> --- a/hw/core/machine-qmp-cmds.c >> +++ b/hw/core/machine-qmp-cmds.c >> @@ -132,9 +132,9 @@ CurrentMachineParams *qmp_query_current_machine(Error **errp) >> return params; >> } >> >> -TargetInfo *qmp_query_target(Error **errp) >> +BinaryTargetInfo *qmp_query_target(Error **errp) >> { >> - TargetInfo *info = g_malloc0(sizeof(*info)); >> + BinaryTargetInfo *info = g_malloc0(sizeof(*info)); >> >> info->arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, >> &error_abort); >
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 22/4/25 07:55, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >> >>> The QAPI-generated 'TargetInfo' structure name is only used >>> in a single file. We want to heavily use another structure >>> similarly named. Rename the QAPI one, since structure names >>> are not part of the public API. >>> >>> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>> --- >>> qapi/machine.json | 12 ++++++------ >>> hw/core/machine-qmp-cmds.c | 4 ++-- >>> 2 files changed, 8 insertions(+), 8 deletions(-) >>> >>> diff --git a/qapi/machine.json b/qapi/machine.json >>> index a6b8795b09e..3246212f048 100644 >>> --- a/qapi/machine.json >>> +++ b/qapi/machine.json >>> @@ -275,15 +275,15 @@ >>> { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } >>> >>> ## >>> -# @TargetInfo: >>> +# @BinaryTargetInfo: >>> # >>> -# Information describing the QEMU target. >>> +# Information describing the QEMU binary target. >> >> What's "the QEMU binary target"? The QEMU binary's target? > > For me 'qemu-system-aarch64' is a QEMU binary, Makes sense to me. > but for Pierrick and > Richard it is the QEMU target, Make about as much sense to me as calling my /usr/bin/gcc "the GNU C compiler target", i.e. none. It's the GNU C compiler targeting x86_64. > so I merged both names ¯\_(ツ)_/¯ If it gets your patch merged... > This structure describes the static target configuration built into > a binary, i.e. TARGET_NAME=aarch64, TARGET_BIG_ENDIAN=false. > > For the forthcoming single/heterogeneous binary, we don't have a > particular restricted configuration in the binary. > > What about "Information describing the QEMU target configuration > built in a binary."? That's better. Here's my try: "Information on the target configuration built into the QEMU binary." >> From the QMP user's point of view, perhaps "the QEMU process's target" >> would make more sense. > > So maybe ProcessTargetInfo is a better structure name. QemuTargetInfo would sidestep binary vs. process. > For heterogeneous target I suppose we'll return SYS_EMU_TARGET_HETERO > and we'll provide new QMP commands, possibly returning array of > ProcessTargetInfo. [...]
On 22/4/25 11:10, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> On 22/4/25 07:55, Markus Armbruster wrote: >>> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >>> >>>> The QAPI-generated 'TargetInfo' structure name is only used >>>> in a single file. We want to heavily use another structure >>>> similarly named. Rename the QAPI one, since structure names >>>> are not part of the public API. >>>> >>>> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> --- >>>> qapi/machine.json | 12 ++++++------ >>>> hw/core/machine-qmp-cmds.c | 4 ++-- >>>> 2 files changed, 8 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/qapi/machine.json b/qapi/machine.json >>>> index a6b8795b09e..3246212f048 100644 >>>> --- a/qapi/machine.json >>>> +++ b/qapi/machine.json >>>> @@ -275,15 +275,15 @@ >>>> { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } >>>> >>>> ## >>>> -# @TargetInfo: >>>> +# @BinaryTargetInfo: >>>> # >>>> -# Information describing the QEMU target. >>>> +# Information describing the QEMU binary target. >>> >>> What's "the QEMU binary target"? The QEMU binary's target? >> >> For me 'qemu-system-aarch64' is a QEMU binary, > > Makes sense to me. > >> but for Pierrick and >> Richard it is the QEMU target, > > Make about as much sense to me as calling my /usr/bin/gcc "the GNU C > compiler target", i.e. none. It's the GNU C compiler targeting x86_64. > >> so I merged both names ¯\_(ツ)_/¯ > > If it gets your patch merged... > >> This structure describes the static target configuration built into >> a binary, i.e. TARGET_NAME=aarch64, TARGET_BIG_ENDIAN=false. >> >> For the forthcoming single/heterogeneous binary, we don't have a >> particular restricted configuration in the binary. >> >> What about "Information describing the QEMU target configuration >> built in a binary."? > > That's better. Here's my try: "Information on the target configuration > built into the QEMU binary." > >>> From the QMP user's point of view, perhaps "the QEMU process's target" >>> would make more sense. >> >> So maybe ProcessTargetInfo is a better structure name. > > QemuTargetInfo would sidestep binary vs. process. Thank you! > >> For heterogeneous target I suppose we'll return SYS_EMU_TARGET_HETERO >> and we'll provide new QMP commands, possibly returning array of >> ProcessTargetInfo. > > [...] >
On Tue, 22 Apr 2025, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: >> On 22/4/25 07:55, Markus Armbruster wrote: >>> Philippe Mathieu-Daudé <philmd@linaro.org> writes: >>> >>>> The QAPI-generated 'TargetInfo' structure name is only used >>>> in a single file. We want to heavily use another structure >>>> similarly named. Rename the QAPI one, since structure names >>>> are not part of the public API. >>>> >>>> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>>> --- >>>> qapi/machine.json | 12 ++++++------ >>>> hw/core/machine-qmp-cmds.c | 4 ++-- >>>> 2 files changed, 8 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/qapi/machine.json b/qapi/machine.json >>>> index a6b8795b09e..3246212f048 100644 >>>> --- a/qapi/machine.json >>>> +++ b/qapi/machine.json >>>> @@ -275,15 +275,15 @@ >>>> { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } >>>> >>>> ## >>>> -# @TargetInfo: >>>> +# @BinaryTargetInfo: >>>> # >>>> -# Information describing the QEMU target. >>>> +# Information describing the QEMU binary target. >>> >>> What's "the QEMU binary target"? The QEMU binary's target? >> >> For me 'qemu-system-aarch64' is a QEMU binary, > > Makes sense to me. > >> but for Pierrick and >> Richard it is the QEMU target, > > Make about as much sense to me as calling my /usr/bin/gcc "the GNU C > compiler target", i.e. none. It's the GNU C compiler targeting x86_64. There's some explanation on different usages of target in https://www.qemu.org/docs/master/devel/tcg-ops.html maybe that explains different views. Regards, BALATON Zoltan >> so I merged both names ¯\_(ツ)_/¯ > > If it gets your patch merged... > >> This structure describes the static target configuration built into >> a binary, i.e. TARGET_NAME=aarch64, TARGET_BIG_ENDIAN=false. >> >> For the forthcoming single/heterogeneous binary, we don't have a >> particular restricted configuration in the binary. >> >> What about "Information describing the QEMU target configuration >> built in a binary."? > > That's better. Here's my try: "Information on the target configuration > built into the QEMU binary." > >>> From the QMP user's point of view, perhaps "the QEMU process's target" >>> would make more sense. >> >> So maybe ProcessTargetInfo is a better structure name. > > QemuTargetInfo would sidestep binary vs. process. > >> For heterogeneous target I suppose we'll return SYS_EMU_TARGET_HETERO >> and we'll provide new QMP commands, possibly returning array of >> ProcessTargetInfo. > > [...] > > >
diff --git a/qapi/machine.json b/qapi/machine.json index a6b8795b09e..3246212f048 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -275,15 +275,15 @@ { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' } ## -# @TargetInfo: +# @BinaryTargetInfo: # -# Information describing the QEMU target. +# Information describing the QEMU binary target. # -# @arch: the target architecture +# @arch: the binary target architecture # # Since: 1.2 ## -{ 'struct': 'TargetInfo', +{ 'struct': 'BinaryTargetInfo', 'data': { 'arch': 'SysEmuTarget' } } ## @@ -291,11 +291,11 @@ # # Return information about the target for this QEMU # -# Returns: TargetInfo +# Returns: BinaryTargetInfo # # Since: 1.2 ## -{ 'command': 'query-target', 'returns': 'TargetInfo' } +{ 'command': 'query-target', 'returns': 'BinaryTargetInfo' } ## # @UuidInfo: diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index 3130c5cd456..408994b67d7 100644 --- a/hw/core/machine-qmp-cmds.c +++ b/hw/core/machine-qmp-cmds.c @@ -132,9 +132,9 @@ CurrentMachineParams *qmp_query_current_machine(Error **errp) return params; } -TargetInfo *qmp_query_target(Error **errp) +BinaryTargetInfo *qmp_query_target(Error **errp) { - TargetInfo *info = g_malloc0(sizeof(*info)); + BinaryTargetInfo *info = g_malloc0(sizeof(*info)); info->arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, &error_abort);