Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 6:51 PM, Tamas K Lengyel wrote:



On Mon, Feb 15, 2016 at 9:44 AM, Jan Beulich > wrote:


>>> On 15.02.16 at 17:28, > wrote:
> On 2/15/2016 4:08 PM, Jan Beulich wrote:
>>
>>> After changing 1 to 1U though, I don't understand why we
should also
>>> range-check mop->event.
>>> I'm imagining when (mop->event > 31):
>>> * (1U << mop->event) = 0 or >= (0x1 + 0x) (?)
>> No, it's plain undefined.
>
> Weirdo C, didn't know that!
> I've just read
http://www.danielvik.com/2010/05/c-language-quirks.html .
> That's crazy, I can't believe such 'quirks' exist and that I
never knew
> of them.
> So then, would this do:
>
> /* sanity check - avoid '<<' operator undefined behavior */
> if ( unlikely(mop->event > 31) )
>  return -EINVAL;
> if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U <<
mop->event))) )
>  return -EOPNOTSUPP;

I'd say -EOPNOTSUPP in both cases, but if the maintainers like
-EINVAL better I wouldn't insist on my preference.


The best approach of course would be if we had __MAX values defined 
for such enums to check against but that doesn't seem to be part of 
Xen's coding practice. So in this case I would say leave it as -EINVAL 
as it's more descriptive of the problem and may even signal to the 
caller some inherent bug on their side, not just that the requested 
option is not supported.


Thanks,
Tamas



Good points, noted.

Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Tamas K Lengyel]

On Mon, Feb 15, 2016 at 9:44 AM, Jan Beulich  wrote:

> >>> On 15.02.16 at 17:28,  wrote:
> > On 2/15/2016 4:08 PM, Jan Beulich wrote:
> >>
> >>> After changing 1 to 1U though, I don't understand why we should also
> >>> range-check mop->event.
> >>> I'm imagining when (mop->event > 31):
> >>> * (1U << mop->event) = 0 or >= (0x1 + 0x) (?)
> >> No, it's plain undefined.
> >
> > Weirdo C, didn't know that!
> > I've just read http://www.danielvik.com/2010/05/c-language-quirks.html .
> > That's crazy, I can't believe such 'quirks' exist and that I never knew
> > of them.
> > So then, would this do:
> >
> > /* sanity check - avoid '<<' operator undefined behavior */
> > if ( unlikely(mop->event > 31) )
> >  return -EINVAL;
> > if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
> >  return -EOPNOTSUPP;
>
> I'd say -EOPNOTSUPP in both cases, but if the maintainers like
> -EINVAL better I wouldn't insist on my preference.
>

The best approach of course would be if we had __MAX values defined for
such enums to check against but that doesn't seem to be part of Xen's
coding practice. So in this case I would say leave it as -EINVAL as it's
more descriptive of the problem and may even signal to the caller some
inherent bug on their side, not just that the requested option is not
supported.

Thanks,
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Jan Beulich]

>>> On 15.02.16 at 17:28,  wrote:
> On 2/15/2016 4:08 PM, Jan Beulich wrote:
>>
>>> After changing 1 to 1U though, I don't understand why we should also
>>> range-check mop->event.
>>> I'm imagining when (mop->event > 31):
>>> * (1U << mop->event) = 0 or >= (0x1 + 0x) (?)
>> No, it's plain undefined.
> 
> Weirdo C, didn't know that!
> I've just read http://www.danielvik.com/2010/05/c-language-quirks.html .
> That's crazy, I can't believe such 'quirks' exist and that I never knew 
> of them.
> So then, would this do:
> 
> /* sanity check - avoid '<<' operator undefined behavior */
> if ( unlikely(mop->event > 31) )
>  return -EINVAL;
> if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
>  return -EOPNOTSUPP;

I'd say -EOPNOTSUPP in both cases, but if the maintainers like
-EINVAL better I wouldn't insist on my preference.

Jan


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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 4:08 PM, Jan Beulich wrote:



After changing 1 to 1U though, I don't understand why we should also
range-check mop->event.
I'm imagining when (mop->event > 31):
* (1U << mop->event) = 0 or >= (0x1 + 0x) (?)

No, it's plain undefined.


Weirdo C, didn't know that!
I've just read http://www.danielvik.com/2010/05/c-language-quirks.html .
That's crazy, I can't believe such 'quirks' exist and that I never knew 
of them.

So then, would this do:

/* sanity check - avoid '<<' operator undefined behavior */
if ( unlikely(mop->event > 31) )
return -EINVAL;
if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << mop->event))) )
return -EOPNOTSUPP;


?

Thanks,
Corneliu.

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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Tamas K Lengyel]

On Mon, Feb 15, 2016 at 7:08 AM, Jan Beulich  wrote:

> >>> On 15.02.16 at 14:29,  wrote:
> > On 2/15/2016 2:44 PM, Jan Beulich wrote:
> >>
> >>>   switch ( mop->op )
> >>>   {
> >>>   case XEN_DOMCTL_MONITOR_OP_ENABLE:
> >>>   case XEN_DOMCTL_MONITOR_OP_DISABLE:
> >>>   /* Check if event type is available. */
> >>>   if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 <<
> mop->event))) )
> >>>   return -EOPNOTSUPP;
> >>>   /* Arch-side handles enable/disable ops. */
> >>>   return arch_monitor_domctl_event(d, mop);
> >> Ah, I see now that I've mis-read the default: code further below,
> >> which actually calls arch_monitor_domctl_op(), not ..._event().
> >> However, there's an "undefined behavior" issue with the code
> >> above then when mop->event >= 31 - I think you want to left
> >> shift 1U instead of plain 1, and you need to range check
> >> mop->event first.
> >>
> > Never looked @ that part before, used it the way it was.
> > I suppose that's because "according to the C specification, the result
> > of a bit shift
> > operation on a signed argument gives implementation-defined results, so
> > in/theory/|1U << i|is
> > more portable than|1 << i|" (taken from a stackoverflow post).
>
> Yes.
>
> > After changing 1 to 1U though, I don't understand why we should also
> > range-check mop->event.
> > I'm imagining when (mop->event > 31):
> > * (1U << mop->event) = 0 or >= (0x1 + 0x) (?)
>
> No, it's plain undefined.
>
> > * in both cases arch_monitor_get_capabilities(d) & (1U << mop->event)
> > would be = 0
> > * in which case we would return -EOPNOTSUPP
> > , no?
>
> And even that would be true only today, and would break once
> bit 31 gets a meaning. Whenever possible we should avoid
> introducing such latent issues.
>

Ah yes, good catch, +1 for doing this extra check.

Thanks,
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Jan Beulich]

>>> On 15.02.16 at 14:29,  wrote:
> On 2/15/2016 2:44 PM, Jan Beulich wrote:
>>
>>>   switch ( mop->op )
>>>   {
>>>   case XEN_DOMCTL_MONITOR_OP_ENABLE:
>>>   case XEN_DOMCTL_MONITOR_OP_DISABLE:
>>>   /* Check if event type is available. */
>>>   if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 << 
>>> mop->event))) )
>>>   return -EOPNOTSUPP;
>>>   /* Arch-side handles enable/disable ops. */
>>>   return arch_monitor_domctl_event(d, mop);
>> Ah, I see now that I've mis-read the default: code further below,
>> which actually calls arch_monitor_domctl_op(), not ..._event().
>> However, there's an "undefined behavior" issue with the code
>> above then when mop->event >= 31 - I think you want to left
>> shift 1U instead of plain 1, and you need to range check
>> mop->event first.
>>
> Never looked @ that part before, used it the way it was.
> I suppose that's because "according to the C specification, the result 
> of a bit shift
> operation on a signed argument gives implementation-defined results, so 
> in/theory/|1U << i|is
> more portable than|1 << i|" (taken from a stackoverflow post).

Yes.

> After changing 1 to 1U though, I don't understand why we should also 
> range-check mop->event.
> I'm imagining when (mop->event > 31):
> * (1U << mop->event) = 0 or >= (0x1 + 0x) (?)

No, it's plain undefined.

> * in both cases arch_monitor_get_capabilities(d) & (1U << mop->event) 
> would be = 0
> * in which case we would return -EOPNOTSUPP
> , no?

And even that would be true only today, and would break once
bit 31 gets a meaning. Whenever possible we should avoid
introducing such latent issues.

Jan


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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 2:44 PM, Jan Beulich wrote:



  switch ( mop->op )
  {
  case XEN_DOMCTL_MONITOR_OP_ENABLE:
  case XEN_DOMCTL_MONITOR_OP_DISABLE:
  /* Check if event type is available. */
  if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 << 
mop->event))) )
  return -EOPNOTSUPP;
  /* Arch-side handles enable/disable ops. */
  return arch_monitor_domctl_event(d, mop);

Ah, I see now that I've mis-read the default: code further below,
which actually calls arch_monitor_domctl_op(), not ..._event().
However, there's an "undefined behavior" issue with the code
above then when mop->event >= 31 - I think you want to left
shift 1U instead of plain 1, and you need to range check
mop->event first.

Jan


Never looked @ that part before, used it the way it was.
I suppose that's because "according to the C specification, the result 
of a bit shift
operation on a signed argument gives implementation-defined results, so 
in/theory/|1U << i|is

more portable than|1 << i|" (taken from a stackoverflow post).

After changing 1 to 1U though, I don't understand why we should also 
range-check mop->event.

I'm imagining when (mop->event > 31):
* (1U << mop->event) = 0 or >= (0x1 + 0x) (?)
* in both cases arch_monitor_get_capabilities(d) & (1U << mop->event) 
would be = 0

* in which case we would return -EOPNOTSUPP
, no?

Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Jan Beulich]

>>> On 15.02.16 at 13:14,  wrote:
> On 2/15/2016 1:41 PM, Jan Beulich wrote:
> On 15.02.16 at 07:37,  wrote:
>>>   default:
>>> -return -EOPNOTSUPP;
>>> +/*
>>> + * Should not be reached unless arch_monitor_get_capabilities() is 
>>> not
>>> + * properly implemented. In that case, since reaching this point 
>>> does
>>> + * not really break anything, don't crash the hypervisor, issue a
>>> + * warning instead of BUG().
>>> + */
>>> +printk(XENLOG_WARNING
>>> +"WARNING, BUG: arch_monitor_get_capabilities() not 
>>> implemented"
>>> +"properly.\n");
>>>   
>>> -};
>>> +return -EOPNOTSUPP;
>>> +}
>> I disagree with the issuing of a message here. At the very least this
>> should be a dprintk(). Perhaps an ASSERT_UNREACHABLE() would be
>> the way to go?
> 
> IDK, but I agree that it doesn't look so elegant like that.
> Won't ASSERT_UNREACHABLE crash the hypervisor?

In a debug build yes. In a release build no.

>> What's worse though is that I can't see the checking
>> which would make true the "should not be reached" statement above
>> (not that you must not rely on the caller of the hypercall to be well
>> behaved).
> 
> The reasoning is as follows.
>  From this part in monitor_domctl:
> 
>  switch ( mop->op )
>  {
>  case XEN_DOMCTL_MONITOR_OP_ENABLE:
>  case XEN_DOMCTL_MONITOR_OP_DISABLE:
>  /* Check if event type is available. */
>  if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 << 
> mop->event))) )
>  return -EOPNOTSUPP;
>  /* Arch-side handles enable/disable ops. */
>  return arch_monitor_domctl_event(d, mop);
> 
> we can see that arch_monitor_domctl_event gets to be called only when 
> arch_monitor_get_capabilities reports
> an 'available' mop->event.
> But if then in arch_monitor_domctl_event the default case is reached, it 
> would mean
> that arch_monitor_get_capabilities reported a mop->event as being 
> available/supported
> when is *not actually handled* anywhere.

Ah, I see now that I've mis-read the default: code further below,
which actually calls arch_monitor_domctl_op(), not ..._event().
However, there's an "undefined behavior" issue with the code
above then when mop->event >= 31 - I think you want to left
shift 1U instead of plain 1, and you need to range check
mop->event first.

Jan


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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 2:25 PM, Stefano Stabellini wrote:

On Mon, 15 Feb 2016, Jan Beulich wrote:

On 15.02.16 at 07:37,  wrote:

  default:
-return -EOPNOTSUPP;
+/*
+ * Should not be reached unless arch_monitor_get_capabilities() is not
+ * properly implemented. In that case, since reaching this point does
+ * not really break anything, don't crash the hypervisor, issue a
+ * warning instead of BUG().
+ */
+printk(XENLOG_WARNING
+"WARNING, BUG: arch_monitor_get_capabilities() not implemented"
+"properly.\n");
  
-};

+return -EOPNOTSUPP;
+}

I disagree with the issuing of a message here. At the very least this
should be a dprintk(). Perhaps an ASSERT_UNREACHABLE() would be
the way to go? What's worse though is that I can't see the checking
which would make true the "should not be reached" statement above
(not that you must not rely on the caller of the hypercall to be well
behaved).

ASSERT_UNREACHABLE() is appropriate here



Noted.

Corneliu.

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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Stefano Stabellini]

On Mon, 15 Feb 2016, Jan Beulich wrote:
> >>> On 15.02.16 at 07:37,  wrote:
> >  default:
> > -return -EOPNOTSUPP;
> > +/*
> > + * Should not be reached unless arch_monitor_get_capabilities() is 
> > not
> > + * properly implemented. In that case, since reaching this point 
> > does
> > + * not really break anything, don't crash the hypervisor, issue a
> > + * warning instead of BUG().
> > + */
> > +printk(XENLOG_WARNING
> > +"WARNING, BUG: arch_monitor_get_capabilities() not 
> > implemented"
> > +"properly.\n");
> >  
> > -};
> > +return -EOPNOTSUPP;
> > +}
> 
> I disagree with the issuing of a message here. At the very least this
> should be a dprintk(). Perhaps an ASSERT_UNREACHABLE() would be
> the way to go? What's worse though is that I can't see the checking
> which would make true the "should not be reached" statement above
> (not that you must not rely on the caller of the hypercall to be well
> behaved).

ASSERT_UNREACHABLE() is appropriate here

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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 1:41 PM, Jan Beulich wrote:

+++ b/xen/include/xen/monitor.h
@@ -0,0 +1,30 @@
+/*
+ * include/xen/monitor.h
+ *
+ * Common monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see .
+ */
+
+#ifndef __MONITOR_H__
+#define __MONITOR_H__

__XEN_MONITOR_H__ please.


Hadn't noticed this comment, also noted.

Thanks,
Corneliu.

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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 1:41 PM, Jan Beulich wrote:

On 15.02.16 at 07:37,  wrote:

  default:
-return -EOPNOTSUPP;
+/*
+ * Should not be reached unless arch_monitor_get_capabilities() is not
+ * properly implemented. In that case, since reaching this point does
+ * not really break anything, don't crash the hypervisor, issue a
+ * warning instead of BUG().
+ */
+printk(XENLOG_WARNING
+"WARNING, BUG: arch_monitor_get_capabilities() not implemented"
+"properly.\n");
  
-};

+return -EOPNOTSUPP;
+}

I disagree with the issuing of a message here. At the very least this
should be a dprintk(). Perhaps an ASSERT_UNREACHABLE() would be
the way to go?


IDK, but I agree that it doesn't look so elegant like that.
Won't ASSERT_UNREACHABLE crash the hypervisor?


What's worse though is that I can't see the checking
which would make true the "should not be reached" statement above
(not that you must not rely on the caller of the hypercall to be well
behaved).


The reasoning is as follows.
From this part in monitor_domctl:

switch ( mop->op )
{
case XEN_DOMCTL_MONITOR_OP_ENABLE:
case XEN_DOMCTL_MONITOR_OP_DISABLE:
/* Check if event type is available. */
if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 << 
mop->event))) )

return -EOPNOTSUPP;
/* Arch-side handles enable/disable ops. */
return arch_monitor_domctl_event(d, mop);

we can see that arch_monitor_domctl_event gets to be called only when 
arch_monitor_get_capabilities reports

an 'available' mop->event.
But if then in arch_monitor_domctl_event the default case is reached, it 
would mean
that arch_monitor_get_capabilities reported a mop->event as being 
available/supported

when is *not actually handled* anywhere.




--- /dev/null
+++ b/xen/include/xen/monitor.h
@@ -0,0 +1,30 @@
+/*
+ * include/xen/monitor.h
+ *
+ * Common monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see .
+ */
+
+#ifndef __MONITOR_H__
+#define __MONITOR_H__

__XEN_MONITOR_H__ please.


+#include 
+#include 
+
+int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);

Neither of the two includes seem necessary for this prototype, all
you need are forward declarations of the two involved structures.

Jan


Noted.

Corneliu.

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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Jan Beulich]

>>> On 15.02.16 at 07:37,  wrote:
>  default:
> -return -EOPNOTSUPP;
> +/*
> + * Should not be reached unless arch_monitor_get_capabilities() is 
> not
> + * properly implemented. In that case, since reaching this point does
> + * not really break anything, don't crash the hypervisor, issue a
> + * warning instead of BUG().
> + */
> +printk(XENLOG_WARNING
> +"WARNING, BUG: arch_monitor_get_capabilities() not 
> implemented"
> +"properly.\n");
>  
> -};
> +return -EOPNOTSUPP;
> +}

I disagree with the issuing of a message here. At the very least this
should be a dprintk(). Perhaps an ASSERT_UNREACHABLE() would be
the way to go? What's worse though is that I can't see the checking
which would make true the "should not be reached" statement above
(not that you must not rely on the caller of the hypercall to be well
behaved).

> --- /dev/null
> +++ b/xen/include/xen/monitor.h
> @@ -0,0 +1,30 @@
> +/*
> + * include/xen/monitor.h
> + *
> + * Common monitor_op domctl handler.
> + *
> + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
> + * Copyright (c) 2016, Bitdefender S.R.L.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see 
> .
> + */
> +
> +#ifndef __MONITOR_H__
> +#define __MONITOR_H__

__XEN_MONITOR_H__ please.

> +#include 
> +#include 
> +
> +int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *op);

Neither of the two includes seem necessary for this prototype, all
you need are forward declarations of the two involved structures.

Jan


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

Re: [Xen-devel] [PATCH v3 2/2] xen/vm-events: Move parts of monitor_domctl code to common-side.

[Corneliu ZUZU]

On 2/15/2016 8:37 AM, Corneliu ZUZU wrote:

diff --git a/xen/common/monitor.c b/xen/common/monitor.c
new file mode 100644
index 000..b708cab
--- /dev/null
+++ b/xen/common/monitor.c
+int rc;
+bool_t requested_status = 0;
+
+if ( unlikely(current->domain == d) ) /* no domain_pause() */
+return -EPERM;
+
+rc = xsm_vm_event_control(XSM_PRIV, d, mop->op, mop->event);
+if ( unlikely(rc) )
+return rc;
+
+switch ( mop->op )
+{
+case XEN_DOMCTL_MONITOR_OP_ENABLE:
+requested_status = 1;
+/* fallthrough */
+case XEN_DOMCTL_MONITOR_OP_DISABLE:
+/* Check if event type is available. */
+if ( unlikely(!(arch_monitor_get_capabilities(d) & (1 << mop->event))) 
)
+return -EOPNOTSUPP;
+/* Arch-side handles enable/disable ops. */
+return arch_monitor_domctl_event(d, mop);



Only noticed now, requested_status now became unused in this function.
Will remove in v4.

Corneliu.

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