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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list