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

Re: [libvirt] [PATCH 1/4] qemu: Enable configuration of HPT resizing for pSeries guests



On Mon, 2017-11-13 at 10:36 -0500, John Ferlan wrote:
> > @@ -4776,6 +4777,13 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> >      if (qemuCaps->version >= 2006000)
> >          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT);
> >  
> > +    /* HPT resizing is supported since QEMU 2.10 on ppc64; unfortunately
> > +     * there's no sane way to probe for it */
> > +    if (qemuCaps->version >= 2010000 &&
> > +        ARCH_IS_PPC64(qemuCaps->arch)) {
> 
> This check differs slightly from qemuDomainDefVerifyFeatures

QEMU capabilities are per-binary, so even though the resize-hpt
property is only supported by the pseries machine type we have to
store it globally. Plus at this point we just have no idea what
machine type the user is eventually going to choose. The capability
name should make it clear that it's pseries only.

> > @@ -7526,6 +7526,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
> >              }
> >          }
> >  
> > +        if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) {
> > +            const char *str;
> > +
> > +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
> > +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +                               _("HTP resizing is not supported by this "
> > +                                 "QEMU binary"));
> > +                goto cleanup;
> > +            }
> > +
> > +            str = virDomainHPTResizingTypeToString(def->hpt_resizing);
> > +            if (!str) {
> > +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +                               _("Invalid setting for HPT resizing"));
> > +                goto cleanup;
> > +            }
> > +
> > +            virBufferAsprintf(&buf, ",resize-hpt=%s", str);
> 
> This one only cares about the capability...

Because by the time we get to build the QEMU command line we will
have made sure the setting is only used along with a compatible
machine type in qemuDomainDefVerifyFeatures() below, no need to
duplicate the check and risk it getting out of sync eventually.

> > @@ -3142,6 +3142,14 @@ qemuDomainDefVerifyFeatures(const virDomainDef *def)
> >          return -1;
> >      }
> >  
> > +    if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON &&
> > +        !qemuDomainIsPSeries(def)) {
> 
> But this one adds the def->machine.os check...  I know it's post parse
> so it should thus cause failure before building the command line occurs,
> so the question is should capability use "&& qemuDomainIsPSeries"?  Your
> call...

As explained above, it can't :)

-- 
Andrea Bolognani / Red Hat / Virtualization


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