>
>
> @@ -143,77 +72,75 @@ int monitor_domctl(struct domain *d, struct
> xen_domctl_monitor_op *mop)
>
> case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR:
> {
>
So since we will now have two separate booleans, requested_status and
old_status and then manually verify they are opposite..
> - bool_t status = ad->monitor.mov_to_msr_enabled;
> + bool_t old_status = ad->monitor.mov_to_msr_enabled;
>
...here we should set the field to requested_status, not !old_status. While
they are technically equivalent, the code would read better to other way
around.
>
> - ad->monitor.mov_to_msr_enabled = !status;
> + ad->monitor.mov_to_msr_enabled = !old_status;
domain_unpause(d);
> break;
> }
>
> case XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP:
> {
> - bool_t status = ad->monitor.singlestep_enabled;
> + bool_t old_status = ad->monitor.singlestep_enabled;
>
> - rc = status_check(mop, status);
> - if ( rc )
> - return rc;
> + if ( unlikely(old_status == requested_status) )
> + return -EEXIST;
>
> domain_pause(d);
>
Here as well..
> - ad->monitor.singlestep_enabled = !status;
> + ad->monitor.singlestep_enabled = !old_status;
> domain_unpause(d);
> break;
> }
>
> case XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT:
> {
> - bool_t status = ad->monitor.software_breakpoint_enabled;
> + bool_t old_status = ad->monitor.software_breakpoint_enabled;
>
> - rc = status_check(mop, status);
> - if ( rc )
> - return rc;
> + if ( unlikely(old_status == requested_status) )
> + return -EEXIST;
>
> domain_pause(d);
>
..and here..
> - ad->monitor.software_breakpoint_enabled = !status;
> + ad->monitor.software_breakpoint_enabled = !old_status;
> domain_unpause(d);
> break;
> }
>
> case XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST:
> {
> - bool_t status = ad->monitor.guest_request_enabled;
> + bool_t old_status = ad->monitor.guest_request_enabled;
>
> - rc = status_check(mop, status);
> - if ( rc )
> - return rc;
> + if ( unlikely(old_status == requested_status) )
> + return -EEXIST;
>
> domain_pause(d);
> ad->monitor.guest_request_sync = mop->u.guest_request.sync;
>
..and here.
> - ad->monitor.guest_request_enabled = !status;
> + ad->monitor.guest_request_enabled = !old_status;
> domain_unpause(d);
> break;
> }
>
Otherwise the patch looks good.
Thanks,
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
