On Mon, 2018-02-12 at 15:18 -0500, John Ferlan wrote: > > + case VIR_DOMAIN_FEATURE_CAPABILITIES: > > + if (src->features[i] != dst->features[i]) { > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > + _("State of feature '%s:%s' differs: " > > + "source: '%s', destination: '%s'"), > > + featureName, "policy", > > OK - based on what I saw Peter ACK for Michal's patches related to his > similar usage, fine. Still looks strange especially since we're talking > about a named XML attribute which we document as "policy". I just hope > there isn't some language variant that alters the meaning. It does look > strange to me "State of feature 'capabilities:policy' differs: "... I > almost wonder if "State of feature 'capabilities' attribute 'policy' > differs: " would help (or really matter). Hm. Maybe it could look like virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%s' differs: " "source: '%s,%s=%s', destination: '%s,%s=%s'"), featureName, virTristateSwitchTypeToString(src->features[i]), "version", virGICVersionTypeToString(src->gic_version), virTristateSwitchTypeToString(dst->features[i]), "version", virGICVersionTypeToString(dst->gic_version)); so that the state of the feature itself and that of its various attributes, when present, would be displayed without ambiguity. > Also, based on the tests/domainschemadata/domain-caps-features.xml: > > <capabilities policy="deny"> > <mknod state="on"/> > </capabilities> > > /me wonders how many more yaks might get shaved if we needed to check > differences w/r/t the caps_features array? > > IOW: If src->features[i] != VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT, then > should we be checking each of the VIR_DOMAIN_CAPS_FEATURE_LAST to ensure > that they're not different too. > > Not something required for this patch, but perhaps a follow-up... For some reason, I thought caps_features were handled later on along with kvm_features and hyperv_features, but I see now that's not the case. I agree that's something that should be addressed. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list