monitor: disable "info kvm" if !KVM

[PATCH] monitor: disable "info kvm" if !KVM

Posted by Bin Guo 1 month ago

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 hmp-commands-info.hx | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 41674dcbe1..1927649493 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -294,6 +294,7 @@ SRST
     being coalesced.
 ERST
 
+#if defined(CONFIG_KVM)
     {
         .name       = "kvm",
         .args_type  = "",
@@ -301,6 +302,7 @@ ERST
         .help       = "show KVM information",
         .cmd        = hmp_info_kvm,
     },
+#endif
 
 SRST
   ``info kvm``
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Daniel P. Berrangé 1 month ago

On Mon, Jan 05, 2026 at 11:43:09AM +0800, Bin Guo wrote:

Missing commit message explaining why we should do this ?

The 'hmp_info_kvm' implementation is unconditionally built and this patch
isn't changing that, so this result in building unused code.

Conceptually even if KVM is not built, it makes sense to have an
'info kvm' command to tell the user that KVM is not available.

> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  hmp-commands-info.hx | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 41674dcbe1..1927649493 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -294,6 +294,7 @@ SRST
>      being coalesced.
>  ERST
>  
> +#if defined(CONFIG_KVM)
>      {
>          .name       = "kvm",
>          .args_type  = "",
> @@ -301,6 +302,7 @@ ERST
>          .help       = "show KVM information",
>          .cmd        = hmp_info_kvm,
>      },
> +#endif
>  
>  SRST
>    ``info kvm``
> -- 
> 2.50.1 (Apple Git-155)
> 

With 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: Re: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Bin Guo 1 month ago

Daniel P. Berrangé wrote on Mon, 5 Jan 2026 10:37:16 +0000:

> Missing commit message explaining why we should do this ?

The reason for submitting this patch is that the newly supported 
'info accelerators' command can provide a more comprehensive view
of the accelerator's status.

> 
> The 'hmp_info_kvm' implementation is unconditionally built and this patch
> isn't changing that, so this result in building unused code.
> 
> Conceptually even if KVM is not built, it makes sense to have an
> 'info kvm' command to tell the user that KVM is not available.

I agree with you, we can skip this patch.

With regards,
Bin Guo

Re: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Markus Armbruster 1 month ago

Bin Guo <guobin@linux.alibaba.com> writes:

> Daniel P. Berrangé wrote on Mon, 5 Jan 2026 10:37:16 +0000:
>
>> Missing commit message explaining why we should do this ?
>
> The reason for submitting this patch is that the newly supported 
> 'info accelerators' command can provide a more comprehensive view
> of the accelerator's status.

Should we deprecate "info kvm"?

[...]

Re: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Daniel P. Berrangé 1 month ago

On Thu, Jan 08, 2026 at 08:57:59AM +0100, Markus Armbruster wrote:
> Bin Guo <guobin@linux.alibaba.com> writes:
> 
> > Daniel P. Berrangé wrote on Mon, 5 Jan 2026 10:37:16 +0000:
> >
> >> Missing commit message explaining why we should do this ?
> >
> > The reason for submitting this patch is that the newly supported 
> > 'info accelerators' command can provide a more comprehensive view
> > of the accelerator's status.
> 
> Should we deprecate "info kvm"?

Conceptually it is certainly redundant, and for HMP we offer no long
term stability promise. Is the benefiting of deprecating and then
removing it, worth the inconvenience we'll cause ?

Perhaps the more important Q first is whether we should deprecate
query-kvm in QMP ?

With 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: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Markus Armbruster 4 weeks, 1 day ago

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Jan 08, 2026 at 08:57:59AM +0100, Markus Armbruster wrote:
>> Bin Guo <guobin@linux.alibaba.com> writes:
>> 
>> > Daniel P. Berrangé wrote on Mon, 5 Jan 2026 10:37:16 +0000:
>> >
>> >> Missing commit message explaining why we should do this ?
>> >
>> > The reason for submitting this patch is that the newly supported 
>> > 'info accelerators' command can provide a more comprehensive view
>> > of the accelerator's status.
>> 
>> Should we deprecate "info kvm"?
>
> Conceptually it is certainly redundant, and for HMP we offer no long
> term stability promise. Is the benefiting of deprecating and then
> removing it, worth the inconvenience we'll cause ?
>
> Perhaps the more important Q first is whether we should deprecate
> query-kvm in QMP ?

Yes.

"info kvm" is a thin wrapper around query-kvm.  We should either keep
both or neither.

What should management applications use, query-kvm or
query-accelerators?

If our answer is query-accelerators, we should guide them by deprecating
query-kvm now.  This doesn't mean we must delete it as soon as the
deprecation grace period ends.

If our answer is "we don't care", we can keep query-kvm.

If our answer is query-kvm, adding query-accelerators was a mistake.
But I don't think it is.

Re: [PATCH] monitor: disable "info kvm" if !KVM

Posted by Daniel P. Berrangé 4 weeks, 1 day ago

On Thu, Jan 08, 2026 at 12:35:07PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Thu, Jan 08, 2026 at 08:57:59AM +0100, Markus Armbruster wrote:
> >> Bin Guo <guobin@linux.alibaba.com> writes:
> >> 
> >> > Daniel P. Berrangé wrote on Mon, 5 Jan 2026 10:37:16 +0000:
> >> >
> >> >> Missing commit message explaining why we should do this ?
> >> >
> >> > The reason for submitting this patch is that the newly supported 
> >> > 'info accelerators' command can provide a more comprehensive view
> >> > of the accelerator's status.
> >> 
> >> Should we deprecate "info kvm"?
> >
> > Conceptually it is certainly redundant, and for HMP we offer no long
> > term stability promise. Is the benefiting of deprecating and then
> > removing it, worth the inconvenience we'll cause ?
> >
> > Perhaps the more important Q first is whether we should deprecate
> > query-kvm in QMP ?
> 
> Yes.
> 
> "info kvm" is a thin wrapper around query-kvm.  We should either keep
> both or neither.
> 
> What should management applications use, query-kvm or
> query-accelerators?

Functionally it doesn't matter which they use if they care about KVM
state.

Conceptually we want them to prefer the latter since it means their
code is more portable to other accelerators QEMU has.

> 
> If our answer is query-accelerators, we should guide them by deprecating
> query-kvm now.  This doesn't mean we must delete it as soon as the
> deprecation grace period ends.
> 
> If our answer is "we don't care", we can keep query-kvm.
> 
> If our answer is query-kvm, adding query-accelerators was a mistake.
> But I don't think it is.

Yeah, feels like we want the query-kvm deprecation.

With 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 :|