Re: [Xen-devel] Re: paging_domctl() missing break statements?

[Patrick Colp <pjcolp@xxxxxxxxx>]

The problem with the patch is that with the break statements in the
"else" cases of XEN_DOMCTL_SHADOW_OP_ENABLE and
XEN_DOMCTL_SHADOW_OP_OFF it currently falls through. Simply sticking
break in at the bottom changes these control flow paths.

So a (more) proper patch should replicate the code of
XEN_DOMCTL_SHADOW_OP_OFF (and of OP_CLEAN and OP_PEEK):

        if ( paging_mode_log_dirty(d) )
            if ( (rc = paging_log_dirty_disable(d)) != 0 )
                return rc;

and

         return paging_log_dirty_op(d, sc);

before the break statement. Same with the XEN_DOMCTL_SHADOW_OP_OFF
case statement... it should have:

         return paging_log_dirty_op(d, sc);

after the initial if statement as its "else" case.

I can whip up a patch to do that, although it's not clear to me that
this is entirely cleaner. Or that XEN_DOMCTL_SHADOW_OP_ENABLE needs
the XEN_DOMCTL_SHADOW_OP_OFF code. This would have to be analysed more
carefully (unless somebody knows the answer off the top of their
head). It could just be that in the "else" case for
XEN_DOMCTL_SHADOW_OP_ENABLE it should just do "return
paging_log_dirty_op(d, sc);". Any thoughts?


Patrick


On 23 June 2010 05:51, Tim Deegan <Tim.Deegan@xxxxxxxxxx> wrote:
> At 13:27 +0100 on 23 Jun (1277299651), Paolo Bonzini wrote:
>> This was never applied.
>
> It was applied and then reverted because it caused Xen crashes:
> http://xenbits.xensource.com/xen-unstable.hg?rev/c4301c2c727d
> and nobody's got round to properly reworking it.
>
> Cheers,
>
> Tim.
>
> --
> Tim Deegan <Tim.Deegan@xxxxxxxxxx>
> Principal Software Engineer, XenServer Engineering
> Citrix Systems UK Ltd. Â(Company #02937203, SL9 0BG)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel