On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> The element will be generalized in the following commit.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>
> Notes:
> Version 2:
> - no change
>
> src/qemu/qemu_capabilities.c | 14 +-
> tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 30 +--
> tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 322 +++++++++++------------
> 3 files changed, 183 insertions(+), 183 deletions(-)
>
This is essentially assuming the capabilities cache is being re-created
and not re-read... Hopefully that works correctly... It was the change
of an element name without some sort of pseudonym added or failure
returned if "feature" was found. I didn't check callers - but if
something here failed would it *ensure* that the capabilities cache was
re-created?
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 688d19504..aab336954 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -3180,7 +3180,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
>
> ctxt->node = hostCPUNode;
>
> - if ((n = virXPathNodeSet("./feature", ctxt, &featureNodes)) > 0) {
> + if ((n = virXPathNodeSet("./property", ctxt, &featureNodes)) > 0) {
featureNodes should be renamed to propertyNodes too...
> if (VIR_ALLOC_N(hostCPU->props, n) < 0)
> goto cleanup;
>
> @@ -3191,14 +3191,14 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> if (!hostCPU->props[i].name) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("missing 'name' attribute for a host CPU"
> - " model feature in QEMU capabilities cache"));
> + " model property in QEMU capabilities cache"));
> goto cleanup;
> }
>
> - if (!(str = virXMLPropString(featureNodes[i], "supported"))) {
> + if (!(str = virXMLPropString(featureNodes[i], "boolean"))) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("missing 'supported' attribute for a host CPU"
> - " model feature in QEMU capabilities cache"));
> + _("missing 'boolean' attribute for a host CPU"
> + " model property in QEMU capabilities cache"));
> goto cleanup;
> }
> if (STREQ(str, "yes")) {
> @@ -3207,7 +3207,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> hostCPU->props[i].supported = false;
> } else {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("invalid supported value: '%s'"), str);
> + _("invalid boolean value: '%s'"), str);
boolean's are typically true/false - so if "boolean='no'", then does
that mean it's something else ;-)
I'm not sure boolean is the best name, but I see from the next patch
it's used because of the "type" being read from monitor/json.
Perhaps rather than "boolean" go with "value"... That way when you
implement more types in the next patch you get:
property name='%s' value='yes|no'
property name='%s' type='string' value='%s'
property name='%s' type='ull' value=%ull
If "type" isn't found, then value is assumed to be a boolean... In the
following patch qemuMonitorCPUPropertyType alters BOOLEAN to DEFAULT.
Not a fan of 'ull' either - I think it should just be 'number' and the
implementation details is that the number is a ULL.
John
> goto cleanup;
> }
> VIR_FREE(str);
> @@ -3552,7 +3552,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> virBufferAdjustIndent(buf, 2);
>
> for (i = 0; i < model->nprops; i++) {
> - virBufferAsprintf(buf, "<feature name='%s' supported='%s'/>\n",
> + virBufferAsprintf(buf, "<property name='%s' boolean='%s'/>\n",
> model->props[i].name,
> model->props[i].supported ? "yes" : "no");
> }
naturally adjustments here...
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
