[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 12/33] qemu: Probe "max" CPU model in TCG




On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> Querying "host" CPU model expansion only makes sense for KVM. QEMU 2.9.0
> introduces a new "max" CPU model which can be used to ask QEMU what the
> best CPU it can provide to a TCG domain is.

But how do we know "max" is available to be used for TCG?  It seems to
me that virQEMUCapsProbeQMPHostCPU now is changed such that there's an
assumption that "max" is available, but the above makes it appear as if
it's a new paremter/attribute [1].

Every time I see TCG I think "Trusted Computing Group" and I certainly
don't think VIR_DOMAIN_TYPE_QEMU. What gets "confusing" eventually is
that virQEMUCapsInitHostCPUModel is called with VIR_DOMAIN_VIRT_KVM and
VIR_DOMAIN_VIRT_QEMU, but qemuCaps->tcgCPUModel is filled in for
VIR_DOMAIN_VIRT_QEMU.

It's not overtly obvious to me from just reading the commit message, but
it seems TCG now becomes a fallback position of sorts which I think is
fine, it's just not described.

> 
> Signed-off-by: Jiri Denemark <jdenemar redhat com>
> ---
> 
> Notes:
>     Version 2:
>     - no change
> 
>  src/qemu/qemu_capabilities.c                       | 151 ++++++++++++-----
>  src/qemu/qemu_capabilities.h                       |   3 +-
>  src/qemu/qemu_capspriv.h                           |   3 +-
>  src/qemu/qemu_command.c                            |   2 +-
>  src/qemu/qemu_process.c                            |   5 +-
>  .../qemucapabilitiesdata/caps_2.8.0.s390x.replies  |   8 +
>  tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml    |   2 +-
>  .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 179 +++++++++++++++++++++
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   | 172 +++++++++++++++++++-
>  tests/qemuxml2argvtest.c                           |   3 +-
>  10 files changed, 480 insertions(+), 48 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 466852d13..2ba82456e 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -400,13 +400,15 @@ struct _virQEMUCaps {
>      size_t ngicCapabilities;
>      virGICCapability *gicCapabilities;
>  
> -    qemuMonitorCPUModelInfoPtr hostCPUModelInfo;
> +    qemuMonitorCPUModelInfoPtr kvmCPUModelInfo;
> +    qemuMonitorCPUModelInfoPtr tcgCPUModelInfo;
>  
>      /* Anything below is not stored in the cache since the values are
>       * re-computed from the other fields or external data sources every
>       * time we probe QEMU or load the results from the cache.
>       */
> -    virCPUDefPtr hostCPUModel;
> +    virCPUDefPtr kvmCPUModel;
> +    virCPUDefPtr tcgCPUModel;
>  };
>  
>  struct virQEMUCapsSearchData {
> @@ -2163,12 +2165,20 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
>              goto error;
>      }
>  
> -    if (qemuCaps->hostCPUModel &&
> -        !(ret->hostCPUModel = virCPUDefCopy(qemuCaps->hostCPUModel)))
> +    if (qemuCaps->kvmCPUModel &&
> +        !(ret->kvmCPUModel = virCPUDefCopy(qemuCaps->kvmCPUModel)))
>          goto error;
>  
> -    if (qemuCaps->hostCPUModelInfo &&
> -        !(ret->hostCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->hostCPUModelInfo)))
> +    if (qemuCaps->tcgCPUModel &&
> +        !(ret->tcgCPUModel = virCPUDefCopy(qemuCaps->tcgCPUModel)))
> +        goto error;
> +
> +    if (qemuCaps->kvmCPUModelInfo &&
> +        !(ret->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->kvmCPUModelInfo)))
> +        goto error;
> +
> +    if (qemuCaps->tcgCPUModelInfo &&
> +        !(ret->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->tcgCPUModelInfo)))
>          goto error;
>  
>      if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
> @@ -2217,8 +2227,10 @@ void virQEMUCapsDispose(void *obj)
>  
>      VIR_FREE(qemuCaps->gicCapabilities);
>  
> -    qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
> -    virCPUDefFree(qemuCaps->hostCPUModel);
> +    qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
> +    qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
> +    virCPUDefFree(qemuCaps->kvmCPUModel);
> +    virCPUDefFree(qemuCaps->tcgCPUModel);
>  }
>  
>  void
> @@ -2435,9 +2447,13 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
>  
>  
>  virCPUDefPtr
> -virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps)
> +virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
> +                        virDomainVirtType type)
>  {
> -    return qemuCaps->hostCPUModel;
> +    if (type == VIR_DOMAIN_VIRT_KVM)
> +        return qemuCaps->kvmCPUModel;
> +    else
> +        return qemuCaps->tcgCPUModel;
>  }
>  
>  
> @@ -2455,7 +2471,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
>                 virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
>  
>      case VIR_CPU_MODE_HOST_MODEL:
> -        return !!qemuCaps->hostCPUModel;
> +        return !!virQEMUCapsGetHostModel(qemuCaps, type);
>  
>      case VIR_CPU_MODE_CUSTOM:
>          if (type == VIR_DOMAIN_VIRT_KVM)
> @@ -2822,14 +2838,24 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
>  
>  static int
>  virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
> -                           qemuMonitorPtr mon)
> +                           qemuMonitorPtr mon,
> +                           bool tcg)
>  {
> -    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) ||
> -        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> +    qemuMonitorCPUModelInfoPtr *modelInfo;
> +    const char *model;
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
>          return 0;

So if no model expansion, then neither is filled in, but there are later
checks which seem to assume one or the other is filled in. I didn't
chase all the callers to check whether they would expect/require
something to be filled in (too many patches, too little time).

>  
> -    return qemuMonitorGetCPUModelExpansion(mon, "static", "host",
> -                                           &qemuCaps->hostCPUModelInfo);
> +    if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {


[1]
It's noot clear to the casual reviewer when tcg and in particular the
"max" parameter was supported.  IOW: The first thought I had was why
isn't there a capability being checked for "max" being available. The
commit message seems to indicate that it's new for 2.9.0, but I don't
see a libvirt capability check for it being available before being used.

The logic changes now such that if !QEMU_CAPS_KVM that model expansion
is called with "max"; whereas, previously a 0 was returned.  Thus if
!tcg and !QEMU_CAPS_KVM, then "max" is used and ModelExpansion is
called. If TCG becomes the "de facto" default -

Just making sure that's expected...

> +        modelInfo = &qemuCaps->tcgCPUModelInfo;
> +        model = "max";
> +    } else {
> +        modelInfo = &qemuCaps->kvmCPUModelInfo;
> +        model = "host";
> +    }
> +
> +    return qemuMonitorGetCPUModelExpansion(mon, "static", model, modelInfo);
>  }
>  
>  struct tpmTypeToCaps {
> @@ -3053,12 +3079,16 @@ virQEMUCapsCPUFilterFeatures(const char *name,
>   */
>  static int
>  virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> +                            virDomainVirtType type,
>                              virCPUDefPtr cpu)
>  {
> -    qemuMonitorCPUModelInfoPtr modelInfo = qemuCaps->hostCPUModelInfo;
> +    qemuMonitorCPUModelInfoPtr modelInfo;
>      size_t i;
>  
> -    if (!modelInfo) {
> +    if (type != VIR_DOMAIN_VIRT_KVM)
> +        return -1;
> +
> +    if (!(modelInfo = qemuCaps->kvmCPUModelInfo)) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("missing host CPU model info from QEMU capabilities "
>                           "for binary %s"),
> @@ -3098,12 +3128,13 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
>   */
>  static int
>  virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
> +                        virDomainVirtType type,
>                          virCPUDefPtr cpu)
>  {
>      int ret = 1;
>  
>      if (ARCH_IS_S390(qemuCaps->arch))
> -        ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpu);
> +        ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpu);
>  
>      if (ret == 0)
>          cpu->fallback = VIR_CPU_FALLBACK_FORBID;
> @@ -3114,7 +3145,8 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
>  
>  void
>  virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> -                            virCapsPtr caps)
> +                            virCapsPtr caps,
> +                            virDomainVirtType type)
>  {
>      virCPUDefPtr cpu = NULL;
>      int rc;
> @@ -3130,7 +3162,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>      cpu->match = VIR_CPU_MATCH_EXACT;
>      cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
>  
> -    if ((rc = virQEMUCapsInitCPUModel(qemuCaps, cpu)) < 0) {
> +    if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu)) < 0) {
>          goto error;
>      } else if (rc == 1) {
>          VIR_DEBUG("No host CPU model info from QEMU; using host capabilities");
> @@ -3143,7 +3175,11 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>              goto error;
>      }
>  
> -    qemuCaps->hostCPUModel = cpu;
> +    if (type == VIR_DOMAIN_VIRT_KVM)
> +        qemuCaps->kvmCPUModel = cpu;
> +    else
> +        qemuCaps->tcgCPUModel = cpu;
> +
>      return;
>  
>   error:
> @@ -3154,7 +3190,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>  
>  static int
>  virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> -                                xmlXPathContextPtr ctxt)
> +                                xmlXPathContextPtr ctxt,
> +                                virDomainVirtType type)
>  {
>      xmlNodePtr hostCPUNode;
>      xmlNodePtr *featureNodes = NULL;
> @@ -3164,7 +3201,12 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>      size_t i;
>      int n;
>  
> -    if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt))) {
> +    if (type == VIR_DOMAIN_VIRT_KVM)
> +        hostCPUNode = virXPathNode("./hostCPU[ type='kvm']", ctxt);
> +    else
> +        hostCPUNode = virXPathNode("./hostCPU[ type='tcg']", ctxt);
> +
> +    if (!hostCPUNode) {
>          ret = 0;
>          goto cleanup;
>      }
> @@ -3232,7 +3274,10 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>          }
>      }
>  
> -    qemuCaps->hostCPUModelInfo = hostCPU;
> +    if (type == VIR_DOMAIN_VIRT_KVM)
> +        qemuCaps->kvmCPUModelInfo = hostCPU;
> +    else
> +        qemuCaps->tcgCPUModelInfo = hostCPU;
>      hostCPU = NULL;
>      ret = 0;
>  
> @@ -3438,7 +3483,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
>      }
>      VIR_FREE(str);
>  
> -    if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt) < 0)
> +    if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> +        virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
>          goto cleanup;
>  
>      if (virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> @@ -3546,7 +3592,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
>      }
>      VIR_FREE(nodes);
>  
> -    virQEMUCapsInitHostCPUModel(qemuCaps, caps);
> +    virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> +    virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
>  
>      ret = 0;
>   cleanup:
> @@ -3560,12 +3607,26 @@ virQEMUCapsLoadCache(virCapsPtr caps,
>  
>  static void
>  virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> -                                  virBufferPtr buf)
> +                                  virBufferPtr buf,
> +                                  virDomainVirtType type)
>  {
> -    qemuMonitorCPUModelInfoPtr model = qemuCaps->hostCPUModelInfo;
> +    qemuMonitorCPUModelInfoPtr model;
> +    const char *typeStr;
>      size_t i;
>  
> -    virBufferAsprintf(buf, "<hostCPU model='%s'>\n", model->name);
> +    if (type == VIR_DOMAIN_VIRT_KVM) {
> +        typeStr = "kvm";
> +        model = qemuCaps->kvmCPUModelInfo;
> +    } else {
> +        typeStr = "tcg";
> +        model = qemuCaps->tcgCPUModelInfo;
> +    }
> +
> +    if (!model)
> +        return;

if !model can happen, how does that affect other places which assume
that are making a similar check to decide which ModelInfo to use?

Also this is "similar" to virQEMUCapsGetHostModel at least for 'model'...

> +
> +    virBufferAsprintf(buf, "<hostCPU type='%s' model='%s'>\n",
> +                      typeStr, model->name);
>      virBufferAdjustIndent(buf, 2);
>  
>      for (i = 0; i < model->nprops; i++) {
> @@ -3670,8 +3731,8 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps,
>      virBufferAsprintf(&buf, "<arch>%s</arch>\n",
>                        virArchToString(qemuCaps->arch));
>  
> -    if (qemuCaps->hostCPUModelInfo)
> -        virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf);
> +    virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
> +    virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
>  
>      virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
>      virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
> @@ -3806,11 +3867,15 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
>      VIR_FREE(qemuCaps->gicCapabilities);
>      qemuCaps->ngicCapabilities = 0;
>  
> -    qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
> -    qemuCaps->hostCPUModelInfo = NULL;
> +    qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
> +    qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
> +    qemuCaps->kvmCPUModelInfo = NULL;
> +    qemuCaps->tcgCPUModelInfo = NULL;
>  
> -    virCPUDefFree(qemuCaps->hostCPUModel);
> -    qemuCaps->hostCPUModel = NULL;
> +    virCPUDefFree(qemuCaps->kvmCPUModel);
> +    virCPUDefFree(qemuCaps->tcgCPUModel);
> +    qemuCaps->kvmCPUModel = NULL;
> +    qemuCaps->tcgCPUModel = NULL;
>  }
>  
>  
> @@ -4368,7 +4433,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA) &&
>          virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
>          goto cleanup;
> -    if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon) < 0)
> +    if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
>          goto cleanup;
>  
>      /* 'intel-iommu' shows up as a device since 2.2.0, but can
> @@ -4410,6 +4475,9 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
>      if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
>          goto cleanup;
>  
> +    if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, true) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>   cleanup:
>      return ret;
> @@ -4754,7 +4822,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
>              virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
>              goto error;
>  
> -        virQEMUCapsInitHostCPUModel(qemuCaps, caps);
> +        virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> +        virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
>      }
>  
>   cleanup:
> @@ -5200,8 +5269,10 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
>          domCaps->cpu.hostPassthrough = true;
>  
>      if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> -                                      VIR_CPU_MODE_HOST_MODEL))
> -        domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->hostCPUModel);
> +                                      VIR_CPU_MODE_HOST_MODEL)) {
> +        virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype);
> +        domCaps->cpu.hostModel = virCPUDefCopy(cpu);
> +    }
>  
>      if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
>                                        VIR_CPU_MODE_CUSTOM)) {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 95bb67d44..3331142ea 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -441,7 +441,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
>                                   virDomainVirtType type,
>                                   char ***names,
>                                   size_t *count);
> -virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps);
> +virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
> +                                     virDomainVirtType type);
>  bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
>                                     virCapsPtr caps,
>                                     virDomainVirtType type,
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index 38b971e0e..75499d462 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -71,5 +71,6 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps,
>  
>  void
>  virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> -                            virCapsPtr caps);
> +                            virCapsPtr caps,
> +                            virDomainVirtType type);
>  #endif
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c00a47a91..4e83dee37 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6782,7 +6782,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
>              if (def->cpu->mode == VIR_CPU_MODE_CUSTOM)
>                  cpuDef = def->cpu;
>              else if (def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
> -                cpuDef = virQEMUCapsGetHostModel(qemuCaps);
> +                cpuDef = virQEMUCapsGetHostModel(qemuCaps, def->virtType);
>  
>              if (cpuDef) {
>                  int svm = virCPUCheckFeature(def->os.arch, cpuDef, "svm");
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 92fa69b3c..b9df01da5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5147,13 +5147,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
>      /* custom CPUs in TCG mode don't need to be compared to host CPU */
>      if (def->virtType != VIR_DOMAIN_VIRT_QEMU ||
>          def->cpu->mode != VIR_CPU_MODE_CUSTOM) {
> -        if (virCPUCompare(caps->host.arch, virQEMUCapsGetHostModel(qemuCaps),
> +        if (virCPUCompare(caps->host.arch,
> +                          virQEMUCapsGetHostModel(qemuCaps, def->virtType),

THis is about where I started wondering about a NULL return here and the
"default" of TCG now...  Although looking at the code around it, this
would seemingly only be called for passthrough or host-model, true?

>                            def->cpu, true) < 0)
>              return -1;
>      }
>  
>      if (virCPUUpdate(def->os.arch, def->cpu,
> -                     virQEMUCapsGetHostModel(qemuCaps)) < 0)
> +                     virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0)
>          goto cleanup;
>  
>      if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType,
> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> index 0405d5d7b..c3cbeee0a 100644
> --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> @@ -13378,3 +13378,11 @@
>    ],
>    "id": "libvirt-2"
>  }
> +
> +{
> +  "id": "libvirt-3",
> +  "error": {
> +    "class": "GenericError",
> +    "desc": "The CPU definition 'max' is unknown."
> +  }
> +}

And this seems to indicate my comments before in
virQEMUCapsProbeQMPHostCPU - if "max" isn't available, but tcg is true
or if !QEMU_CAPS_KVM, then "max" will be tried, which doesn't seem right.

Also does something similar need to be done in caps_2.8.0.x86_64 or does
it not really matter since if it doesn't exist, it wouldn't be defined
or usable.

John

> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> index 1f652bdc2..df7eb18f6 100644
> --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> @@ -133,7 +133,7 @@
>    <kvmVersion>0</kvmVersion>
>    <package></package>
>    <arch>s390x</arch>
> -  <hostCPU model='zEC12.2-base'>
> +  <hostCPU type='kvm' model='zEC12.2-base'>
>      <property name='aefsi' boolean='yes'/>
>      <property name='msa5' boolean='yes'/>
>      <property name='msa4' boolean='yes'/>
> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> index 8d54788df..390f40f9f 100644
> --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> @@ -14708,3 +14708,182 @@
>    ],
>    "id": "libvirt-2"
>  }
> +
> +{
> +  "return": {
> +    "model": {
> +      "name": "base",
> +      "props": {
> +        "cmov": true,
> +        "ia64": false,
> +        "aes": true,
> +        "mmx": true,
> +        "rdpid": false,
> +        "arat": true,
> +        "pause-filter": false,
> +        "xsavec": false,
> +        "osxsave": false,
> +        "kvm-asyncpf": false,
> +        "perfctr-core": false,
> +        "mpx": true,
> +        "pbe": false,
> +        "avx512cd": false,
> +        "decodeassists": false,
> +        "sse4.1": true,
> +        "family": 6,
> +        "avx512f": false,
> +        "msr": true,
> +        "mce": true,
> +        "mca": true,
> +        "xcrypt": false,
> +        "min-level": 13,
> +        "xgetbv1": true,
> +        "cid": false,
> +        "ds": false,
> +        "fxsr": true,
> +        "xsaveopt": true,
> +        "xtpr": false,
> +        "avx512vl": false,
> +        "avx512-vpopcntdq": false,
> +        "phe": false,
> +        "extapic": false,
> +        "3dnowprefetch": false,
> +        "cr8legacy": true,
> +        "xcrypt-en": false,
> +        "pn": false,
> +        "dca": false,
> +        "vendor": "AuthenticAMD",
> +        "pku": true,
> +        "smx": false,
> +        "cmp-legacy": false,
> +        "avx512-4fmaps": false,
> +        "vmcb-clean": false,
> +        "hle": false,
> +        "3dnowext": true,
> +        "npt": false,
> +        "clwb": true,
> +        "lbrv": false,
> +        "adx": true,
> +        "ss": true,
> +        "pni": true,
> +        "svm-lock": false,
> +        "smep": true,
> +        "smap": true,
> +        "pfthreshold": false,
> +        "x2apic": false,
> +        "avx512vbmi": false,
> +        "flushbyasid": false,
> +        "f16c": false,
> +        "ace2-en": false,
> +        "pae": true,
> +        "pat": true,
> +        "sse": true,
> +        "phe-en": false,
> +        "kvm-nopiodelay": false,
> +        "tm": false,
> +        "kvmclock-stable-bit": false,
> +        "hypervisor": true,
> +        "pcommit": true,
> +        "syscall": true,
> +        "avx512dq": false,
> +        "svm": true,
> +        "invtsc": false,
> +        "sse2": true,
> +        "est": false,
> +        "avx512ifma": false,
> +        "tm2": false,
> +        "kvm-pv-eoi": false,
> +        "cx8": true,
> +        "kvm-mmu": false,
> +        "sse4.2": true,
> +        "pge": true,
> +        "pdcm": false,
> +        "model": 6,
> +        "movbe": true,
> +        "nrip-save": false,
> +        "ssse3": true,
> +        "sse4a": true,
> +        "invpcid": false,
> +        "pdpe1gb": true,
> +        "tsc-deadline": false,
> +        "fma": false,
> +        "cx16": true,
> +        "de": true,
> +        "stepping": 3,
> +        "xsave": true,
> +        "clflush": true,
> +        "skinit": false,
> +        "tsc": true,
> +        "tce": false,
> +        "fpu": true,
> +        "ds-cpl": false,
> +        "ibs": false,
> +        "fma4": false,
> +        "la57": true,
> +        "osvw": false,
> +        "apic": true,
> +        "pmm": false,
> +        "tsc-adjust": false,
> +        "kvm-steal-time": false,
> +        "kvmclock": false,
> +        "lwp": false,
> +        "xop": false,
> +        "avx": false,
> +        "ospke": true,
> +        "acpi": true,
> +        "avx512bw": false,
> +        "ace2": false,
> +        "fsgsbase": true,
> +        "ht": false,
> +        "nx": true,
> +        "pclmulqdq": true,
> +        "mmxext": true,
> +        "popcnt": true,
> +        "xsaves": false,
> +        "lm": true,
> +        "umip": false,
> +        "pse": true,
> +        "avx2": false,
> +        "sep": true,
> +        "nodeid-msr": false,
> +        "misalignsse": false,
> +        "min-xlevel": 2147483658,
> +        "bmi1": true,
> +        "bmi2": true,
> +        "kvm-pv-unhalt": false,
> +        "tsc-scale": false,
> +        "topoext": false,
> +        "clflushopt": true,
> +        "monitor": true,
> +        "avx512er": false,
> +        "pmm-en": false,
> +        "pcid": false,
> +        "3dnow": true,
> +        "erms": true,
> +        "lahf-lm": true,
> +        "fxsr-opt": false,
> +        "xstore": false,
> +        "rtm": false,
> +        "lmce": false,
> +        "perfctr-nb": false,
> +        "rdrand": false,
> +        "rdseed": false,
> +        "avx512-4vnniw": false,
> +        "vme": false,
> +        "vmx": false,
> +        "dtes64": false,
> +        "mtrr": true,
> +        "rdtscp": true,
> +        "pse36": true,
> +        "tbm": false,
> +        "wdt": false,
> +        "model-id": "QEMU TCG CPU version 2.5+",
> +        "sha-ni": false,
> +        "abm": true,
> +        "avx512pf": false,
> +        "xstore-en": false
> +      }
> +    }
> +  },
> +  "id": "libvirt-3"
> +}
> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> index 32368e648..520bf80f4 100644
> --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> @@ -205,7 +205,7 @@
>    <kvmVersion>0</kvmVersion>
>    <package> (v2.8.0-877-g38e4b757b4)</package>
>    <arch>x86_64</arch>
> -  <hostCPU model='base'>
> +  <hostCPU type='kvm' model='base'>
>      <property name='cmov' boolean='yes'/>
>      <property name='ia64' boolean='no'/>
>      <property name='aes' boolean='yes'/>
> @@ -375,6 +375,176 @@
>      <property name='avx512pf' boolean='no'/>
>      <property name='xstore-en' boolean='no'/>
>    </hostCPU>
> +  <hostCPU type='tcg' model='base'>
> +    <property name='cmov' boolean='yes'/>
> +    <property name='ia64' boolean='no'/>
> +    <property name='aes' boolean='yes'/>
> +    <property name='mmx' boolean='yes'/>
> +    <property name='rdpid' boolean='no'/>
> +    <property name='arat' boolean='yes'/>
> +    <property name='pause-filter' boolean='no'/>
> +    <property name='xsavec' boolean='no'/>
> +    <property name='osxsave' boolean='no'/>
> +    <property name='kvm-asyncpf' boolean='no'/>
> +    <property name='perfctr-core' boolean='no'/>
> +    <property name='mpx' boolean='yes'/>
> +    <property name='pbe' boolean='no'/>
> +    <property name='avx512cd' boolean='no'/>
> +    <property name='decodeassists' boolean='no'/>
> +    <property name='sse4.1' boolean='yes'/>
> +    <property name='family' ull='6'/>
> +    <property name='avx512f' boolean='no'/>
> +    <property name='msr' boolean='yes'/>
> +    <property name='mce' boolean='yes'/>
> +    <property name='mca' boolean='yes'/>
> +    <property name='xcrypt' boolean='no'/>
> +    <property name='min-level' ull='13'/>
> +    <property name='xgetbv1' boolean='yes'/>
> +    <property name='cid' boolean='no'/>
> +    <property name='ds' boolean='no'/>
> +    <property name='fxsr' boolean='yes'/>
> +    <property name='xsaveopt' boolean='yes'/>
> +    <property name='xtpr' boolean='no'/>
> +    <property name='avx512vl' boolean='no'/>
> +    <property name='avx512-vpopcntdq' boolean='no'/>
> +    <property name='phe' boolean='no'/>
> +    <property name='extapic' boolean='no'/>
> +    <property name='3dnowprefetch' boolean='no'/>
> +    <property name='cr8legacy' boolean='yes'/>
> +    <property name='xcrypt-en' boolean='no'/>
> +    <property name='pn' boolean='no'/>
> +    <property name='dca' boolean='no'/>
> +    <property name='vendor' string='AuthenticAMD'/>
> +    <property name='pku' boolean='yes'/>
> +    <property name='smx' boolean='no'/>
> +    <property name='cmp-legacy' boolean='no'/>
> +    <property name='avx512-4fmaps' boolean='no'/>
> +    <property name='vmcb-clean' boolean='no'/>
> +    <property name='hle' boolean='no'/>
> +    <property name='3dnowext' boolean='yes'/>
> +    <property name='npt' boolean='no'/>
> +    <property name='clwb' boolean='yes'/>
> +    <property name='lbrv' boolean='no'/>
> +    <property name='adx' boolean='yes'/>
> +    <property name='ss' boolean='yes'/>
> +    <property name='pni' boolean='yes'/>
> +    <property name='svm-lock' boolean='no'/>
> +    <property name='smep' boolean='yes'/>
> +    <property name='smap' boolean='yes'/>
> +    <property name='pfthreshold' boolean='no'/>
> +    <property name='x2apic' boolean='no'/>
> +    <property name='avx512vbmi' boolean='no'/>
> +    <property name='flushbyasid' boolean='no'/>
> +    <property name='f16c' boolean='no'/>
> +    <property name='ace2-en' boolean='no'/>
> +    <property name='pae' boolean='yes'/>
> +    <property name='pat' boolean='yes'/>
> +    <property name='sse' boolean='yes'/>
> +    <property name='phe-en' boolean='no'/>
> +    <property name='kvm-nopiodelay' boolean='no'/>
> +    <property name='tm' boolean='no'/>
> +    <property name='kvmclock-stable-bit' boolean='no'/>
> +    <property name='hypervisor' boolean='yes'/>
> +    <property name='pcommit' boolean='yes'/>
> +    <property name='syscall' boolean='yes'/>
> +    <property name='avx512dq' boolean='no'/>
> +    <property name='svm' boolean='yes'/>
> +    <property name='invtsc' boolean='no'/>
> +    <property name='sse2' boolean='yes'/>
> +    <property name='est' boolean='no'/>
> +    <property name='avx512ifma' boolean='no'/>
> +    <property name='tm2' boolean='no'/>
> +    <property name='kvm-pv-eoi' boolean='no'/>
> +    <property name='cx8' boolean='yes'/>
> +    <property name='kvm-mmu' boolean='no'/>
> +    <property name='sse4.2' boolean='yes'/>
> +    <property name='pge' boolean='yes'/>
> +    <property name='pdcm' boolean='no'/>
> +    <property name='model' ull='6'/>
> +    <property name='movbe' boolean='yes'/>
> +    <property name='nrip-save' boolean='no'/>
> +    <property name='ssse3' boolean='yes'/>
> +    <property name='sse4a' boolean='yes'/>
> +    <property name='invpcid' boolean='no'/>
> +    <property name='pdpe1gb' boolean='yes'/>
> +    <property name='tsc-deadline' boolean='no'/>
> +    <property name='fma' boolean='no'/>
> +    <property name='cx16' boolean='yes'/>
> +    <property name='de' boolean='yes'/>
> +    <property name='stepping' ull='3'/>
> +    <property name='xsave' boolean='yes'/>
> +    <property name='clflush' boolean='yes'/>
> +    <property name='skinit' boolean='no'/>
> +    <property name='tsc' boolean='yes'/>
> +    <property name='tce' boolean='no'/>
> +    <property name='fpu' boolean='yes'/>
> +    <property name='ds-cpl' boolean='no'/>
> +    <property name='ibs' boolean='no'/>
> +    <property name='fma4' boolean='no'/>
> +    <property name='la57' boolean='yes'/>
> +    <property name='osvw' boolean='no'/>
> +    <property name='apic' boolean='yes'/>
> +    <property name='pmm' boolean='no'/>
> +    <property name='tsc-adjust' boolean='no'/>
> +    <property name='kvm-steal-time' boolean='no'/>
> +    <property name='kvmclock' boolean='no'/>
> +    <property name='lwp' boolean='no'/>
> +    <property name='xop' boolean='no'/>
> +    <property name='avx' boolean='no'/>
> +    <property name='ospke' boolean='yes'/>
> +    <property name='acpi' boolean='yes'/>
> +    <property name='avx512bw' boolean='no'/>
> +    <property name='ace2' boolean='no'/>
> +    <property name='fsgsbase' boolean='yes'/>
> +    <property name='ht' boolean='no'/>
> +    <property name='nx' boolean='yes'/>
> +    <property name='pclmulqdq' boolean='yes'/>
> +    <property name='mmxext' boolean='yes'/>
> +    <property name='popcnt' boolean='yes'/>
> +    <property name='xsaves' boolean='no'/>
> +    <property name='lm' boolean='yes'/>
> +    <property name='umip' boolean='no'/>
> +    <property name='pse' boolean='yes'/>
> +    <property name='avx2' boolean='no'/>
> +    <property name='sep' boolean='yes'/>
> +    <property name='nodeid-msr' boolean='no'/>
> +    <property name='misalignsse' boolean='no'/>
> +    <property name='min-xlevel' ull='2147483658'/>
> +    <property name='bmi1' boolean='yes'/>
> +    <property name='bmi2' boolean='yes'/>
> +    <property name='kvm-pv-unhalt' boolean='no'/>
> +    <property name='tsc-scale' boolean='no'/>
> +    <property name='topoext' boolean='no'/>
> +    <property name='clflushopt' boolean='yes'/>
> +    <property name='monitor' boolean='yes'/>
> +    <property name='avx512er' boolean='no'/>
> +    <property name='pmm-en' boolean='no'/>
> +    <property name='pcid' boolean='no'/>
> +    <property name='3dnow' boolean='yes'/>
> +    <property name='erms' boolean='yes'/>
> +    <property name='lahf-lm' boolean='yes'/>
> +    <property name='fxsr-opt' boolean='no'/>
> +    <property name='xstore' boolean='no'/>
> +    <property name='rtm' boolean='no'/>
> +    <property name='lmce' boolean='no'/>
> +    <property name='perfctr-nb' boolean='no'/>
> +    <property name='rdrand' boolean='no'/>
> +    <property name='rdseed' boolean='no'/>
> +    <property name='avx512-4vnniw' boolean='no'/>
> +    <property name='vme' boolean='no'/>
> +    <property name='vmx' boolean='no'/>
> +    <property name='dtes64' boolean='no'/>
> +    <property name='mtrr' boolean='yes'/>
> +    <property name='rdtscp' boolean='yes'/>
> +    <property name='pse36' boolean='yes'/>
> +    <property name='tbm' boolean='no'/>
> +    <property name='wdt' boolean='no'/>
> +    <property name='model-id' string='QEMU TCG CPU version 2.5+'/>
> +    <property name='sha-ni' boolean='no'/>
> +    <property name='abm' boolean='yes'/>
> +    <property name='avx512pf' boolean='no'/>
> +    <property name='xstore-en' boolean='no'/>
> +  </hostCPU>
>    <cpu type='kvm' name='max' usable='yes'/>
>    <cpu type='kvm' name='host' usable='yes'/>
>    <cpu type='kvm' name='base' usable='yes'/>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index faa99c64c..5ba2eeab9 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -384,7 +384,8 @@ testUpdateQEMUCaps(const struct testInfo *info,
>      if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0)
>          goto cleanup;
>  
> -    virQEMUCapsInitHostCPUModel(info->qemuCaps, caps);
> +    virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> +    virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
>  
>      virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine);
>  
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]