Re: [Xen-devel] [PATCH] xen: fail gnttab_grow_table() in case of missing allocations

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Sep 29, 2017 at 05:39:22AM +0000, Juergen Gross wrote:
> In case gnttab_grow_table() is being called without grant_table_init()
> having been called for the domain, e.g. in case of a toolstack error,
> fail the function instead of crashing the system.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  xen/common/grant_table.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 71706f5cba..f2598a902f 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1669,7 +1669,11 @@ gnttab_grow_table(struct domain *d, unsigned int 
> req_nr_frames)
>      struct grant_table *gt = d->grant_table;
>      unsigned int i, j;
>  
> -    ASSERT(gt->active);
> +    if ( unlikely(!gt->active) )
> +    {
> +        gdprintk(XENLOG_WARNING, "grant_table_set_limits() call missing.\n");

In the commit message you mention 'grant_table_init', yet the error
message here says grant_table_set_limits. Shouldn't both mention the
same precursor function?

Also I think this might be better as gprintk instead of gdprintk.

> +        return 0;

This return 0 has confused me, I was going to ask to return ENODEV,
but then I saw this is actually treated like a boolean. Might be good
to make gnttab_grow_table return bool, or either make it return proper
error codes.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel