Re: [Xen-devel] [PATCH 01/02] Handles broken page occurred before migration

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.10.12 at 12:19, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> @@ -1568,6 +1577,38 @@
>      }
>      break;
>  
> +    case XEN_DOMCTL_set_broken_page_p2m:
> +    {
> +        struct domain *d;
> +        p2m_type_t pt;
> +        unsigned long pfn;
> +        mfn_t mfn;

If you use scope restricted local variables (which I appreciate),
please declare them in the innermost possible scope, ...

> +
> +        d = rcu_lock_domain_by_id(domctl->domain);
> +        if ( d != NULL )
> +        {
> +            pfn = domctl->u.set_broken_page_p2m.pfn;
> +
> +            mfn = get_gfn_query(d, pfn, &pt);

... i.e. here (and then both assignments can become initializers
at once).

> +            if ( !mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt) )
> +            {
> +                put_gfn(d, pfn);
> +                rcu_unlock_domain(d);
> +                ret = -EINVAL;
> +                break;
> +            }
> +
> +            if ( p2m_change_type(d, pfn, pt, p2m_ram_broken) != pt )
> +                ret = -EINVAL;

The two if() conditions can be easily joined (the more that they
both produce -EINVAL).

> +
> +            put_gfn(d, pfn);
> +            rcu_unlock_domain(d);
> +        }
> +        else
> +            ret = -ESRCH;
> +    }
> +    break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, u_domctl);
>          break;



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