On Mon, Jul 29, 2013 at 05:31:19PM -0600, Don Dugger wrote:
>
> Currently the virConnectBaselineCPU API does not expose the CPU features
> that are part of the CPU's model. This patch adds a new flag,
> VIR_CONNECT_BASELINE_SHOW_MODEL, that causes the API to explictly
> list all features that are part of that model.
Nit-pick: I'd prefer the constant to be named
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE
so that it is more precisely describing what it is doing,
and includes the word "CPU" there to match the API name.
> +static void
> +x86AddFeatures(virCPUDefPtr cpu,
> + struct x86_map *map)
> +{
> + const struct x86_model *candidate;
> + const struct x86_feature *feature = map->features;
> +
> + candidate = map->models;
> + while (candidate != NULL) {
> + if (STREQ(cpu->model, candidate->name))
> + break;
> + candidate = candidate->next;
> + }
> + if (!candidate) {
> + VIR_WARN("Odd, %s not a known CPU model\n", cpu->model);
VIR_WARN shouldn't be used for errors which can occur in public API call
paths. It should virReportError() and return an error code '-1' to the
caller of the API
> + return;
> + }
> + while (feature != NULL) {
> + if (x86DataIsSubset(candidate->data, feature->data)) {
> + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0) {
> + VIR_WARN("CPU model %s, no room for feature %s", cpu->model, feature->name);
> + return;
Again this should be reporting an error & propagating it back to the
caller
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list