8:27 p.m.
Quoting Collin Walling (2018-07-11 18:25:00)
Not sure if I agree with the renaming here.
From what I understand, "if a cpu property is not migratable, then the
cpu model is also not migratable".
So I think the original naming scheme is fine.
Very much welcome the feedback here from folks with experience...
Got to say though as a newbie I spent a crazy amount of time trying to sort out
what migratability in qemuMonitorCPUModelInfo and migratable in
qemuMonitorCPUProperty actually mean.
qemuMonitorCPUModelInfo looks like it should just be a libvirt structure
encapsulating the CPUModelInfo in QMP messages.
But the CPUModelInfo in QMP messages doesn't contain migratability or
migratable.
Turns out migratability and (prop) migratable are something libvirt overloads in
CPUModelInfo with that's only used when storing host cpu capabilities in
virQEMUCapsHostCPUData.
Moreover, the mechanism that virQEMUCapsProbeQMPHostCPU uses to get QMP to
enumerate both migratable and non-migratable Host CPU properties only seems to
work for some archs like X86. Other archs will only enumerate the migratable
host properties.
So as best I can tell the "migratablity" variable in the qemuMonitorCPUModelInfo
structure only means we were able to enumerate both migratable and
non-migratable properties of the Host CPU (only when stored in
virQEMUCapsHostCPUData) on a limited set of arch like X86.
In fact if Host CPU's "migratability" is set to true it likely means the
Host
CPU's CPUModelInfo contains non-migratable properties in addition to migratable
properties.
So that is what I was trying to get at renaming 'migratability' to
'props_migratable_valid'. Trying to clarify that in the HostCPU case we were
able to enumerate some non-migratable features and correspondingly indicate what
properties are and are not migratable in the Host CPU.
With that said I would be glad to defer to those with libvirt experience here to
tell me if I missed something, we should not change the name, or some other
additional cleanup is in order.
On 07/09/2018 11:56 PM, 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_capabilities.c b/src/qemu/qemu_capabilities.c
> index c7da916f9a..3d78e2e29b 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2410,7 +2410,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
> }
> }
>
> - modelInfo->migratability = true;
> + modelInfo->props_migratable_valid = true;
> }
>
> VIR_STEAL_PTR(cpuData->info, modelInfo);
> @@ -2465,7 +2465,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCapsPtr qemuCaps,
> }
>
> VIR_STEAL_PTR(*features, list);
> - if (migratable && !data->info->migratability)
> + if (migratable && !data->info->props_migratable_valid)
> ret = 1;
> else
> ret = 0;
> @@ -2864,7 +2864,7 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
> virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type);
> int ret = 1;
>
> - if (migratable && cpuData->info &&
!cpuData->info->migratability)
> + if (migratable && cpuData->info &&
!cpuData->info->props_migratable_valid)
> return 1;
>
> if (ARCH_IS_S390(qemuCaps->arch)) {
> @@ -3047,7 +3047,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> _("invalid migratability value for host CPU
model"));
> goto cleanup;
> }
> - hostCPU->migratability = val == VIR_TRISTATE_BOOL_YES;
> + hostCPU->props_migratable_valid = val == VIR_TRISTATE_BOOL_YES;
> VIR_FREE(str);
>
> ctxt->node = hostCPUNode;
> @@ -3540,7 +3540,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> virBufferAsprintf(buf,
> "<hostCPU type='%s' model='%s'
migratability='%s'>\n",
> typeStr, model->name,
> - model->migratability ? "yes" : "no");
> + model->props_migratable_valid ? "yes" :
"no");
> virBufferAdjustIndent(buf, 2);
>
> for (i = 0; i < model->nprops; i++) {
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index a3278c018e..371aaa15da 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3671,7 +3671,7 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo
*orig)
> if (VIR_STRDUP(copy->name, orig->name) < 0)
> goto error;
>
> - copy->migratability = orig->migratability;
> + copy->props_migratable_valid = orig->props_migratable_valid;
> copy->nprops = orig->nprops;
>
> for (i = 0; i < orig->nprops; i++) {
> 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;
> };
>
> typedef enum {
>
--
Respectfully,
- Collin Walling