[Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig

Dr. David Alan Gilbert (git) posted 7 patches 7 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Dr. David Alan Gilbert (git) 7 years, 8 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow a bunch of the info commands to be used in preconfig.

version, chardev, name, uuid,memdev, iothreads
  Were enabled in QMP in the previous patch from Igor

status, hotpluggable_cpus
  Was enabled in the original allow-preconfig series

history
  is HMP specific

usbhost, qom-tree, numa
  Don't have a QMP equivalent

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands-info.hx | 12 ++++++++++++
 hmp-commands.hx      |  1 +
 2 files changed, 13 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ddfcd5adcc..f4d0d7d2d0 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -19,6 +19,7 @@ ETEXI
         .params     = "",
         .help       = "show the version of QEMU",
         .cmd        = hmp_info_version,
+        .flags      = "p",
     },
 
 STEXI
@@ -47,6 +48,7 @@ ETEXI
         .params     = "",
         .help       = "show the character devices",
         .cmd        = hmp_info_chardev,
+        .flags      = "p",
     },
 
 STEXI
@@ -165,6 +167,7 @@ ETEXI
         .params     = "",
         .help       = "show the command line history",
         .cmd        = hmp_info_history,
+        .flags      = "p",
     },
 
 STEXI
@@ -315,6 +318,7 @@ ETEXI
         .params     = "",
         .help       = "show NUMA information",
         .cmd        = hmp_info_numa,
+        .flags      = "p",
     },
 
 STEXI
@@ -343,6 +347,7 @@ ETEXI
         .params     = "",
         .help       = "show host USB devices",
         .cmd        = hmp_info_usbhost,
+        .flags      = "p",
     },
 
 STEXI
@@ -399,6 +404,7 @@ ETEXI
         .params     = "",
         .help       = "show the current VM status (running|paused)",
         .cmd        = hmp_info_status,
+        .flags      = "p",
     },
 
 STEXI
@@ -457,6 +463,7 @@ ETEXI
         .params     = "",
         .help       = "show the current VM name",
         .cmd        = hmp_info_name,
+        .flags      = "p",
     },
 
 STEXI
@@ -471,6 +478,7 @@ ETEXI
         .params     = "",
         .help       = "show the current VM UUID",
         .cmd        = hmp_info_uuid,
+        .flags      = "p",
     },
 
 STEXI
@@ -613,6 +621,7 @@ ETEXI
         .params     = "[path]",
         .help       = "show QOM composition tree",
         .cmd        = hmp_info_qom_tree,
+        .flags      = "p",
     },
 
 STEXI
@@ -671,6 +680,7 @@ ETEXI
         .params     = "",
         .help       = "show memory backends",
         .cmd        = hmp_info_memdev,
+        .flags      = "p",
     },
 
 STEXI
@@ -699,6 +709,7 @@ ETEXI
         .params     = "",
         .help       = "show iothreads",
         .cmd        = hmp_info_iothreads,
+        .flags      = "p",
     },
 
 STEXI
@@ -829,6 +840,7 @@ ETEXI
         .params     = "",
         .help       = "Show information about hotpluggable CPUs",
         .cmd        = hmp_hotpluggable_cpus,
+        .flags      = "p",
     },
 
 STEXI
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8bf590ae4b..dc82ed526f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1856,6 +1856,7 @@ ETEXI
         .help       = "show various information about the system state",
         .cmd        = hmp_info_help,
         .sub_table  = info_cmds,
+        .flags      = "p",
     },
 
 STEXI
-- 
2.17.0


Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Markus Armbruster 7 years, 8 months ago
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Allow a bunch of the info commands to be used in preconfig.
>
> version, chardev, name, uuid,memdev, iothreads
>   Were enabled in QMP in the previous patch from Igor

Yes, these are okay together with PATCH 4.

> status, hotpluggable_cpus
>   Was enabled in the original allow-preconfig series

query-status looks okay to me.

> history
>   is HMP specific

Yes.

> usbhost, qom-tree, numa
>   Don't have a QMP equivalent

HMP commands without a QMP equivalent are okay if their functionality
makes no sense in QMP, or is of use only for human users.

Example for "makes no sense in QMP": setting the current CPU, because a
QMP monitor doesn't have a current CPU.

Examples for "is of use only for human users": HMP command "help", the
integrated pocket calculator.

Now let's review the three commands:

* Gerd, why does "info usbhost" have no QMP equivalent?

* Eduardo, why does "info numa" have no QMP equivalent?

* "info qom-tree" is a recursive variant of qom-list that skips anything
  but children.  This convenience command exists so you don't have to
  filter and string together output from many qom-list.

  I think it stands to reason that if providing "info qom-tree" makes
  sense, then so does qom-list (HMP and QMP).  If qom-list, then
  qom-list-types, qom-list-properties, qom-get, and probably even
  qom-set (I've always been suspicious of qom-set, but that has nothing
  to do with preconfig state).

  It might make sense to split off the whole QOM shebang into a separate
  patch.

> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Dr. David Alan Gilbert 7 years, 8 months ago
* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Allow a bunch of the info commands to be used in preconfig.
> >
> > version, chardev, name, uuid,memdev, iothreads
> >   Were enabled in QMP in the previous patch from Igor
> 
> Yes, these are okay together with PATCH 4.
> 
> > status, hotpluggable_cpus
> >   Was enabled in the original allow-preconfig series
> 
> query-status looks okay to me.
> 
> > history
> >   is HMP specific
> 
> Yes.
> 
> > usbhost, qom-tree, numa
> >   Don't have a QMP equivalent
> 
> HMP commands without a QMP equivalent are okay if their functionality
> makes no sense in QMP, or is of use only for human users.
> 
> Example for "makes no sense in QMP": setting the current CPU, because a
> QMP monitor doesn't have a current CPU.
> 
> Examples for "is of use only for human users": HMP command "help", the
> integrated pocket calculator.

Right, but they do already exist; it's possible we may want to fix/add
QMP versions - but this series isn't about going through and fixing
existing stuff up.

> Now let's review the three commands:
> 
> * Gerd, why does "info usbhost" have no QMP equivalent?
> 
> * Eduardo, why does "info numa" have no QMP equivalent?
> 
> * "info qom-tree" is a recursive variant of qom-list that skips anything
>   but children.  This convenience command exists so you don't have to
>   filter and string together output from many qom-list.
> 
>   I think it stands to reason that if providing "info qom-tree" makes
>   sense, then so does qom-list (HMP and QMP).  If qom-list, then
>   qom-list-types, qom-list-properties, qom-get, and probably even
>   qom-set (I've always been suspicious of qom-set, but that has nothing
>   to do with preconfig state).
> 
>   It might make sense to split off the whole QOM shebang into a separate
>   patch.

People have been trying to add qom-get etc for quite a while (I tried a
couple of years ago); it gets stuck in type display issues.  I've not
directly seen a need for those other variants, but qom-get is something
I'd love to have, still that's a job for another patch.

'info qom-tree' is very very useful when debugging qemu to see what the
basic state we're building is; it's primarily for debugging.

Dave

> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Gerd Hoffmann 7 years, 8 months ago
  Hi,

> > Now let's review the three commands:
> > 
> > * Gerd, why does "info usbhost" have no QMP equivalent?

Works only when running qemu directly, in the libvirt sandbox qemu
hasn't the permissions needed to scan the host usb bus so that would be
rather pointless ...

cheers,
  Gerd


Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Markus Armbruster 7 years, 8 months ago
Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> > Now let's review the three commands:
>> > 
>> > * Gerd, why does "info usbhost" have no QMP equivalent?
>
> Works only when running qemu directly, in the libvirt sandbox qemu
> hasn't the permissions needed to scan the host usb bus so that would be
> rather pointless ...

I don't think this meets either of the two criteria:

* I meets "makes no sense in QMP" only if QMP implies "can't scan host
  USB bus".  Libvirt implies it, but QMP doesn't imply libvirt; it's its
  most important, not its sole user.

* It meets "of use only for human users" only if we're convinced it's of
  no use to programs.

  To avoid speculation and endless arguments about what could or could
  not be of use, we've always stuck to "when in doubt, assume it could
  be of use".

  "Libvirt can't use it" falls short.

  "Any management application worth anything would deny QEMU the
  capability to scan the USB host bus, and thus wouldn't be able to use
  it" is exactly the argument we intended to avoid.

QMP falling short of completeness in relatively unimportant ways like
this one isn't exactly terrible.  The most serious effect is probably
serving as a bad example that leads to further arguments like this one.
These are well worth avoiding, though.

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Dr. David Alan Gilbert 7 years, 8 months ago
* Markus Armbruster (armbru@redhat.com) wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> >   Hi,
> >
> >> > Now let's review the three commands:
> >> > 
> >> > * Gerd, why does "info usbhost" have no QMP equivalent?
> >
> > Works only when running qemu directly, in the libvirt sandbox qemu
> > hasn't the permissions needed to scan the host usb bus so that would be
> > rather pointless ...
> 
> I don't think this meets either of the two criteria:
> 
> * I meets "makes no sense in QMP" only if QMP implies "can't scan host
>   USB bus".  Libvirt implies it, but QMP doesn't imply libvirt; it's its
>   most important, not its sole user.
> 
> * It meets "of use only for human users" only if we're convinced it's of
>   no use to programs.
> 
>   To avoid speculation and endless arguments about what could or could
>   not be of use, we've always stuck to "when in doubt, assume it could
>   be of use".
> 
>   "Libvirt can't use it" falls short.
> 
>   "Any management application worth anything would deny QEMU the
>   capability to scan the USB host bus, and thus wouldn't be able to use
>   it" is exactly the argument we intended to avoid.
> 
> QMP falling short of completeness in relatively unimportant ways like
> this one isn't exactly terrible.  The most serious effect is probably
> serving as a bad example that leads to further arguments like this one.
> These are well worth avoiding, though.

Markus:
  a) This is a separate discussion; info usbhost has been there for many
years;  this patch set doesn't change that.
  b) From HMP, if someone wants to add a command like 'info usbhost' to
make their debugging of USB easy, then HMP is all for that and there's
no way I'm going to require QMP implementations for a debug command.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Markus Armbruster 7 years, 8 months ago
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
>> 
>> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >
>> > Allow a bunch of the info commands to be used in preconfig.
>> >
>> > version, chardev, name, uuid,memdev, iothreads
>> >   Were enabled in QMP in the previous patch from Igor
>> 
>> Yes, these are okay together with PATCH 4.
>> 
>> > status, hotpluggable_cpus
>> >   Was enabled in the original allow-preconfig series
>> 
>> query-status looks okay to me.
>> 
>> > history
>> >   is HMP specific
>> 
>> Yes.
>> 
>> > usbhost, qom-tree, numa
>> >   Don't have a QMP equivalent
>> 
>> HMP commands without a QMP equivalent are okay if their functionality
>> makes no sense in QMP, or is of use only for human users.
>> 
>> Example for "makes no sense in QMP": setting the current CPU, because a
>> QMP monitor doesn't have a current CPU.
>> 
>> Examples for "is of use only for human users": HMP command "help", the
>> integrated pocket calculator.
>
> Right, but they do already exist; it's possible we may want to fix/add
> QMP versions - but this series isn't about going through and fixing
> existing stuff up.
>
>> Now let's review the three commands:
>> 
>> * Gerd, why does "info usbhost" have no QMP equivalent?
>> 
>> * Eduardo, why does "info numa" have no QMP equivalent?
>> 
>> * "info qom-tree" is a recursive variant of qom-list that skips anything
>>   but children.  This convenience command exists so you don't have to
>>   filter and string together output from many qom-list.
>> 
>>   I think it stands to reason that if providing "info qom-tree" makes
>>   sense, then so does qom-list (HMP and QMP).  If qom-list, then
>>   qom-list-types, qom-list-properties, qom-get, and probably even
>>   qom-set (I've always been suspicious of qom-set, but that has nothing
>>   to do with preconfig state).
>> 
>>   It might make sense to split off the whole QOM shebang into a separate
>>   patch.
>
> People have been trying to add qom-get etc for quite a while (I tried a
> couple of years ago); it gets stuck in type display issues.  I've not
> directly seen a need for those other variants, but qom-get is something
> I'd love to have, still that's a job for another patch.

Yes.

> 'info qom-tree' is very very useful when debugging qemu to see what the
> basic state we're building is; it's primarily for debugging.

I'm not at all opposed to enabling qom-tree, but I want its QMP building
blocks enabled as well then.  I think enabling their HMP buddies as well
would only make sense.

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Dr. David Alan Gilbert 7 years, 8 months ago
* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
> >> 
> >> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >
> >> > Allow a bunch of the info commands to be used in preconfig.
> >> >
> >> > version, chardev, name, uuid,memdev, iothreads
> >> >   Were enabled in QMP in the previous patch from Igor
> >> 
> >> Yes, these are okay together with PATCH 4.
> >> 
> >> > status, hotpluggable_cpus
> >> >   Was enabled in the original allow-preconfig series
> >> 
> >> query-status looks okay to me.
> >> 
> >> > history
> >> >   is HMP specific
> >> 
> >> Yes.
> >> 
> >> > usbhost, qom-tree, numa
> >> >   Don't have a QMP equivalent
> >> 
> >> HMP commands without a QMP equivalent are okay if their functionality
> >> makes no sense in QMP, or is of use only for human users.
> >> 
> >> Example for "makes no sense in QMP": setting the current CPU, because a
> >> QMP monitor doesn't have a current CPU.
> >> 
> >> Examples for "is of use only for human users": HMP command "help", the
> >> integrated pocket calculator.
> >
> > Right, but they do already exist; it's possible we may want to fix/add
> > QMP versions - but this series isn't about going through and fixing
> > existing stuff up.
> >
> >> Now let's review the three commands:
> >> 
> >> * Gerd, why does "info usbhost" have no QMP equivalent?
> >> 
> >> * Eduardo, why does "info numa" have no QMP equivalent?
> >> 
> >> * "info qom-tree" is a recursive variant of qom-list that skips anything
> >>   but children.  This convenience command exists so you don't have to
> >>   filter and string together output from many qom-list.
> >> 
> >>   I think it stands to reason that if providing "info qom-tree" makes
> >>   sense, then so does qom-list (HMP and QMP).  If qom-list, then
> >>   qom-list-types, qom-list-properties, qom-get, and probably even
> >>   qom-set (I've always been suspicious of qom-set, but that has nothing
> >>   to do with preconfig state).
> >> 
> >>   It might make sense to split off the whole QOM shebang into a separate
> >>   patch.
> >
> > People have been trying to add qom-get etc for quite a while (I tried a
> > couple of years ago); it gets stuck in type display issues.  I've not
> > directly seen a need for those other variants, but qom-get is something
> > I'd love to have, still that's a job for another patch.
> 
> Yes.
> 
> > 'info qom-tree' is very very useful when debugging qemu to see what the
> > basic state we're building is; it's primarily for debugging.
> 
> I'm not at all opposed to enabling qom-tree, but I want its QMP building
> blocks enabled as well then.  I think enabling their HMP buddies as well
> would only make sense.

Hmm; OK, enabling qom-list, qom-get, qom-set, qom-list-types,
qom-list-properties for qmp.
qom-list and qom-set enabled in HMP.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Eduardo Habkost 7 years, 8 months ago
On Tue, Jun 12, 2018 at 09:49:14AM +0100, Dr. David Alan Gilbert wrote:
[...]
> > > People have been trying to add qom-get etc for quite a while (I tried a
> > > couple of years ago); it gets stuck in type display issues.  I've not
> > > directly seen a need for those other variants, but qom-get is something
> > > I'd love to have, still that's a job for another patch.
> > 
> > Yes.
> > 
> > > 'info qom-tree' is very very useful when debugging qemu to see what the
> > > basic state we're building is; it's primarily for debugging.
> > 
> > I'm not at all opposed to enabling qom-tree, but I want its QMP building
> > blocks enabled as well then.  I think enabling their HMP buddies as well
> > would only make sense.
> 
> Hmm; OK, enabling qom-list, qom-get, qom-set, qom-list-types,
> qom-list-properties for qmp.
> qom-list and qom-set enabled in HMP.

I would prefer to avoid enabling qom-set in preconfig mode unless
we have a good reason for that.  I don't trust all existing
property setters to not crash or break if the machine is not
initialized yet.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Daniel P. Berrangé 7 years, 8 months ago
On Wed, Jun 13, 2018 at 10:47:45AM -0300, Eduardo Habkost wrote:
> On Tue, Jun 12, 2018 at 09:49:14AM +0100, Dr. David Alan Gilbert wrote:
> [...]
> > > > People have been trying to add qom-get etc for quite a while (I tried a
> > > > couple of years ago); it gets stuck in type display issues.  I've not
> > > > directly seen a need for those other variants, but qom-get is something
> > > > I'd love to have, still that's a job for another patch.
> > > 
> > > Yes.
> > > 
> > > > 'info qom-tree' is very very useful when debugging qemu to see what the
> > > > basic state we're building is; it's primarily for debugging.
> > > 
> > > I'm not at all opposed to enabling qom-tree, but I want its QMP building
> > > blocks enabled as well then.  I think enabling their HMP buddies as well
> > > would only make sense.
> > 
> > Hmm; OK, enabling qom-list, qom-get, qom-set, qom-list-types,
> > qom-list-properties for qmp.
> > qom-list and qom-set enabled in HMP.
> 
> I would prefer to avoid enabling qom-set in preconfig mode unless
> we have a good reason for that.  I don't trust all existing
> property setters to not crash or break if the machine is not
> initialized yet.

If we're in early startup, the impact of a crash is pretty minor - QEMU
will simply exit, which is not significantly differnt from if a setter
handled it "correctly" by setting an Error **errp. A mgmt app that uses
this and finds a setter which crashes will detect the problem pretty
quickly & report bugs.

QEMU is complex enough that we're unlikely to ever find broken setters
by code inspection. So if we don't allow it in preconfig mode, then
I doubt we'll ever find & fix the problems.

So, IMHO, we should allow qom-set and just fix problems as they come
to light, as we would for any other part of QEMU which has plenty of
scope for crashers in rarely tested codepaths.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Eduardo Habkost 7 years, 8 months ago
On Wed, Jun 13, 2018 at 02:53:37PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 13, 2018 at 10:47:45AM -0300, Eduardo Habkost wrote:
> > On Tue, Jun 12, 2018 at 09:49:14AM +0100, Dr. David Alan Gilbert wrote:
> > [...]
> > > > > People have been trying to add qom-get etc for quite a while (I tried a
> > > > > couple of years ago); it gets stuck in type display issues.  I've not
> > > > > directly seen a need for those other variants, but qom-get is something
> > > > > I'd love to have, still that's a job for another patch.
> > > > 
> > > > Yes.
> > > > 
> > > > > 'info qom-tree' is very very useful when debugging qemu to see what the
> > > > > basic state we're building is; it's primarily for debugging.
> > > > 
> > > > I'm not at all opposed to enabling qom-tree, but I want its QMP building
> > > > blocks enabled as well then.  I think enabling their HMP buddies as well
> > > > would only make sense.
> > > 
> > > Hmm; OK, enabling qom-list, qom-get, qom-set, qom-list-types,
> > > qom-list-properties for qmp.
> > > qom-list and qom-set enabled in HMP.
> > 
> > I would prefer to avoid enabling qom-set in preconfig mode unless
> > we have a good reason for that.  I don't trust all existing
> > property setters to not crash or break if the machine is not
> > initialized yet.
> 
> If we're in early startup, the impact of a crash is pretty minor - QEMU
> will simply exit, which is not significantly differnt from if a setter
> handled it "correctly" by setting an Error **errp. A mgmt app that uses
> this and finds a setter which crashes will detect the problem pretty
> quickly & report bugs.
> 
> QEMU is complex enough that we're unlikely to ever find broken setters
> by code inspection. So if we don't allow it in preconfig mode, then
> I doubt we'll ever find & fix the problems.
> 
> So, IMHO, we should allow qom-set and just fix problems as they come
> to light, as we would for any other part of QEMU which has plenty of
> scope for crashers in rarely tested codepaths.

Good points.

I'm still worried about making qom-set confused with a supported
configuration API, and have applications starting to rely on it.

But I guess this problem could be solved by properly documenting
qom-set as a debugging aid, not a supported API.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Eduardo Habkost 7 years, 8 months ago
On Mon, Jun 11, 2018 at 02:01:52PM +0200, Markus Armbruster wrote:
> * Eduardo, why does "info numa" have no QMP equivalent?

Nobody ever asked for one, which seems to qualify as "only for
human users".

Should we add an equivalent QMP command even if we don't expect
anybody to use it?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Igor Mammedov 7 years, 8 months ago
On Mon, 11 Jun 2018 15:40:16 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Jun 11, 2018 at 02:01:52PM +0200, Markus Armbruster wrote:
> > * Eduardo, why does "info numa" have no QMP equivalent?
> 
> Nobody ever asked for one, which seems to qualify as "only for
> human users".
> 
> Should we add an equivalent QMP command even if we don't expect
> anybody to use it?

we inderectly can fetch numa info via QMP, using 
  query-hotpluggable-cpus
for CPU mapping and
  query-memory-devices
for (NV|PC)-dimm devices, however there is no QMP way for getting
for numa mapping of initial RAM nor configured numa nodes
(not counting querying CLI options).

So perhaps we need info 'numa' equivalent for QMP which would give
the same amount of information as HMP in one query.

Maybe libvirt side as actual users know better if it's really needed (CCed)

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Markus Armbruster 7 years, 8 months ago
Igor Mammedov <imammedo@redhat.com> writes:

> On Mon, 11 Jun 2018 15:40:16 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
>> On Mon, Jun 11, 2018 at 02:01:52PM +0200, Markus Armbruster wrote:
>> > * Eduardo, why does "info numa" have no QMP equivalent?
>> 
>> Nobody ever asked for one, which seems to qualify as "only for
>> human users".
>> 
>> Should we add an equivalent QMP command even if we don't expect
>> anybody to use it?

The hard requirement for QMP from day one was "provide everything
machine clients need".  To avoid speculation and endless arguments about
what might be needed / not needed, we resolved to approximate this by
"provide everything, except stuff that's *clearly* of no use to
machines".

When you think a command is such an exception, you should explain why in
its commit message.

Note that HMP need not provide the functionality in the exact same
packaging.  For instance, it's fine to have building blocks in QMP, and
just a high-level command in HMP.  However, the latter must be
implemented with the building blocks to make it obvious that QMP
provides the same functionality.

For additional references, see
Message-ID: <87d0x1p6go.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg02176.html

> we inderectly can fetch numa info via QMP, using 
>   query-hotpluggable-cpus
> for CPU mapping and
>   query-memory-devices
> for (NV|PC)-dimm devices, however there is no QMP way for getting
> for numa mapping of initial RAM nor configured numa nodes
> (not counting querying CLI options).

Sounds like most of the building blocks are already there.  The
"obviousness" isn't.

> So perhaps we need info 'numa' equivalent for QMP which would give
> the same amount of information as HMP in one query.

I'd appreciate patches to get us to "QMP has the building blocks, and
HMP is implemented with them".

> Maybe libvirt side as actual users know better if it's really needed (CCed)

Always welcome.

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Eduardo Habkost 7 years, 8 months ago
On Tue, Jun 12, 2018 at 09:00:20AM +0200, Markus Armbruster wrote:
> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Mon, 11 Jun 2018 15:40:16 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> >> On Mon, Jun 11, 2018 at 02:01:52PM +0200, Markus Armbruster wrote:
> >> > * Eduardo, why does "info numa" have no QMP equivalent?
> >> 
> >> Nobody ever asked for one, which seems to qualify as "only for
> >> human users".
> >> 
> >> Should we add an equivalent QMP command even if we don't expect
> >> anybody to use it?
> 
> The hard requirement for QMP from day one was "provide everything
> machine clients need".  To avoid speculation and endless arguments about
> what might be needed / not needed, we resolved to approximate this by
> "provide everything, except stuff that's *clearly* of no use to
> machines".
> 
> When you think a command is such an exception, you should explain why in
> its commit message.

It's not an exception nor it needs to be one.  I was just not
aware of the above approximation.


> 
> Note that HMP need not provide the functionality in the exact same
> packaging.  For instance, it's fine to have building blocks in QMP, and
> just a high-level command in HMP.  However, the latter must be
> implemented with the building blocks to make it obvious that QMP
> provides the same functionality.
> 
> For additional references, see
> Message-ID: <87d0x1p6go.fsf@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg02176.html
> 
> > we inderectly can fetch numa info via QMP, using 
> >   query-hotpluggable-cpus
> > for CPU mapping and
> >   query-memory-devices
> > for (NV|PC)-dimm devices, however there is no QMP way for getting
> > for numa mapping of initial RAM nor configured numa nodes
> > (not counting querying CLI options).
> 
> Sounds like most of the building blocks are already there.  The
> "obviousness" isn't.
> 
> > So perhaps we need info 'numa' equivalent for QMP which would give
> > the same amount of information as HMP in one query.
> 
> I'd appreciate patches to get us to "QMP has the building blocks, and
> HMP is implemented with them".

Agreed.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 5/7] hmp: Add info commands for preconfig
Posted by Daniel P. Berrangé 7 years, 8 months ago
On Mon, Jun 11, 2018 at 11:33:42PM +0200, Igor Mammedov wrote:
> On Mon, 11 Jun 2018 15:40:16 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Mon, Jun 11, 2018 at 02:01:52PM +0200, Markus Armbruster wrote:
> > > * Eduardo, why does "info numa" have no QMP equivalent?
> > 
> > Nobody ever asked for one, which seems to qualify as "only for
> > human users".
> > 
> > Should we add an equivalent QMP command even if we don't expect
> > anybody to use it?
> 
> we inderectly can fetch numa info via QMP, using 
>   query-hotpluggable-cpus
> for CPU mapping and
>   query-memory-devices
> for (NV|PC)-dimm devices, however there is no QMP way for getting
> for numa mapping of initial RAM nor configured numa nodes
> (not counting querying CLI options).
> 
> So perhaps we need info 'numa' equivalent for QMP which would give
> the same amount of information as HMP in one query.
> 
> Maybe libvirt side as actual users know better if it's really needed (CCed)

Libvirt knows the numa config it used to spawn the guest with, so there's
no immediate compelling reason to query it back, as QEMU doesn't change
it later.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|