Re: [PATCH] treewide: replace qemu_hw_version() with QEMU_HW_VERSION

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Mar 24, 2026 at 06:11:04PM +0100, Paolo Bonzini wrote:
> On Tue, Mar 24, 2026 at 6:02 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Mar 24, 2026 at 05:57:05PM +0100, Paolo Bonzini wrote:
> > > The version is never set on 2.5+ machine types, so qemu_hw_version() and
> > > qemu_set_hw_version() are not needed anymore.
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > >  include/hw/core/boards.h  |  6 ------
> > >  include/qemu/hw-version.h | 15 ---------------
> > >  hw/ide/core.c             |  2 +-
> > >  hw/scsi/megasas.c         |  2 +-
> > >  hw/scsi/scsi-bus.c        |  2 +-
> > >  hw/scsi/scsi-disk.c       |  2 +-
> > >  system/vl.c               |  4 ----
> > >  target/s390x/cpu_models.c |  2 +-
> > >  util/osdep.c              | 12 ------------
> > >  9 files changed, 5 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/include/hw/core/boards.h b/include/hw/core/boards.h
> > > index f85f31bd90d..b8dad0a1074 100644
> > > --- a/include/hw/core/boards.h
> > > +++ b/include/hw/core/boards.h
> > > @@ -197,11 +197,6 @@ typedef struct {
> > >   *    used to provide @cpu_index to socket number mapping, allowing
> > >   *    a machine to group CPU threads belonging to the same socket/package
> > >   *    Returns: socket number given cpu_index belongs to.
> > > - * @hw_version:
> > > - *    Value of QEMU_VERSION when the machine was added to QEMU.
> > > - *    Set only by old machines because they need to keep
> > > - *    compatibility on code that exposed QEMU_VERSION to guests in
> > > - *    the past (and now use qemu_hw_version()).
> > >   * @possible_cpu_arch_ids:
> > >   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
> > >   *    which includes CPU IDs for present and possible to hotplug CPUs.
> > > @@ -297,7 +292,6 @@ struct MachineClass {
> > >      const char *default_display;
> > >      const char *default_nic;
> > >      GPtrArray *compat_props;
> > > -    const char *hw_version;
> > >      ram_addr_t default_ram_size;
> > >      const char *default_cpu_type;
> > >      bool default_kernel_irqchip_split;
> > > diff --git a/include/qemu/hw-version.h b/include/qemu/hw-version.h
> > > index 730a8c904d9..a40a6c204a6 100644
> > > --- a/include/qemu/hw-version.h
> > > +++ b/include/qemu/hw-version.h
> > > @@ -7,21 +7,6 @@
> > >  #ifndef QEMU_HW_VERSION_H
> > >  #define QEMU_HW_VERSION_H
> > >
> > > -/*
> > > - * Starting on QEMU 2.5, qemu_hw_version() returns "2.5+" by default
> > > - * instead of QEMU_VERSION, so setting hw_version on MachineClass
> > > - * is no longer mandatory.
> > > - *
> > > - * Do NOT change this string, or it will break compatibility on all
> > > - * machine classes that don't set hw_version.
> > > - */
> >
> > I feel like we should be keeping a comment statement that
> > this must never be changed due to ABI compat.
> >
> > >  #define QEMU_HW_VERSION "2.5+"
> 
> Ok, what about this:
> 
>  * Starting on QEMU 2.5, devices with a version string in their
>  * identification data return "2.5+" instead of QEMU_VERSION.  Do
>  * NOT change this string as it is visible to guests.

Yep, with that comment

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> > Also, is there any better place this can live now that this
> > header file has a single #define and nothing else ?
> 
> I could put it in a random qdev header that everyone includes already
> - but I agree with Peter that it's better that it stays isolated, so
> that it does not grow more uses.

Yep, ok.


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|