@@ -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