Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value.

[konrad wilk <konrad.wilk@oracle.com>]

On 4/15/2013 12:50 PM, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
>> Konrad Rzeszutek Wilk writes ("[PATCH 7/7] xl: Fix 'free_memory' to include outstanding_claims value."):
>>> Updating to make it clear that free_memory reported by 'xl info'
>>> is influenced by the outstanding claim value. [...]
>>>       if (libxl_get_physinfo(ctx, &info) != 0) {
>>>           fprintf(stderr, "libxl_physinfo failed.\n");
>>>           return;
>>>       }
>>> -
>>> +    /*
>>> +     * Don't bother checking "claim_mode" as user might have turned it off
>>> +     * and we have outstanding claims.
>>> +     */
>>> +    if ((claims = libxl_get_claiminfo(ctx)) < 0){
>>> +        fprintf(stderr, "libxl_get_claiminfo failed. errno: %d (%s)\n",
>>> +                errno, strerror(errno));
>>> +        return;
>>> +    }
>> ...
>>> +        printf("free_memory            : %"PRIu64"\n", (info.free_pages - claims) / i);
>> This has a race, I think.
> I have checked this and the race is in the hypercall API.  The
> hypercall API has already been checked in.  So, under the
> circumstances, for this patch:
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>

Great! Thanks.
> However, there is a release-critical bug here.  The following things
> need to be changed:
>
>   * We need a race-free version of the hypercall API.
>   * We need a race-free version of the xc API.
>   * We need a race-free version of the libxl API.
>
> I think this is a release-critical bug because fixing it involves an
> API change at all these 3 levels.  We don't want to release 4.3 with
> a broken API as that will complicate fixing this bug.

Right. Let me enumerate some ideas for fixing this on a different thread 
(if we have the same race in mind that you spotted).
>
> George, can you please add this to your tracking list ?
>
> Having said all that, George, am I OK from a freeze POV to pull
> Konrad's series into staging ?

Would you like me to rebase it once more with the s/def_bool/int/ and 
the resurrection of a line change? I can repost just against the 
offending patch?

I might be a bit late with doing it (Tuesday night) since I am traveling 
today.
>
> Ian.