[libvirt] [PATCHv2 03/11] qemu_monitor: Indicate when CPUModelInfo props report migratablity

[Chris Venteicher]

Quoting Jiri Denemark (2018-07-12 06:59:18)
> On Mon, Jul 09, 2018 at 22:56:47 -0500, Chris Venteicher wrote:
> > Renamed variable in CPUModelInfo such that
> > props_migratable_valid is true when properties in CPUModelInfo
> > have been updated to accurately indicate if property is / isn't
> > migratable.
> > 
> > Property migratability is not returned directly in QMP messages but
> > rather is sometimes calculated within Libvirt by other means and then
> > stored in CPUModelInfo properties by Libvirt. props_migratable_valid is
> > set to true when this calculation has been done by Libvirt.
> > ---
> >  src/qemu/qemu_capabilities.c | 10 +++++-----
> >  src/qemu/qemu_monitor.c      |  2 +-
> >  src/qemu/qemu_monitor.h      |  2 +-
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> ...
> > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> > index 18b59be985..208a7f5d21 100644
> > --- a/src/qemu/qemu_monitor.h
> > +++ b/src/qemu/qemu_monitor.h
> > @@ -1005,7 +1005,7 @@ struct _qemuMonitorCPUModelInfo {
> >      char *name;
> >      size_t nprops;
> >      qemuMonitorCPUPropertyPtr props;
> > -    bool migratability;
> > +    bool props_migratable_valid;
> >  };
> 
> I don't see a reason for renaming the variable. The new name is uglier
> and may be confusing in exactly the same way you found migratability to
> be confusing. Just add a comment which would explain that the
> migratability tells whether we can use the prop->migratable value to
> check if a particular feature is migratable.
> 

I still have some concerns about the use of the migratability 
(props_migratability_valid) variable.

There seems to be a very narrow case where the CPUModelInfo is non-migratable 
(contains non-migratable props) ... arch:X86 + name:host.

In all other cases, specific model names (ex IvyBridge) or other Archs, the 
CPUModelInfo is by default migratable yet the variable migratability 
(props_migratable_valid) is false.

There seems to be multiple cases in existing code base where the CPUModelInfo is 
actually migratable but is treated as non-migratable because the migratability 
(props_migratability_valid) is false. 

Not sure if this is really a problem or if I am just missing something yet.

Seems like the most efficient thing is to try to sort it out in a stand alone 
patch that's easy to give feedback on.

Regardless, I will try to keep the naming consistent (migratability) and use a 
comment if needed.

Chris

> Jirka