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

Re: [libvirt] [PATCH] Add flag to BaselineCPU API to return detailed CPU features



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 :|


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