Re: [Xen-devel] [PATCH v2 2/3] x86/hvm: introduce functions for HVMOP_get/set_param allowance checks

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:jbeulich@xxxxxxxx]
> Sent: 01 May 2015 20:28
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
> Subject: Re: [PATCH v2 2/3] x86/hvm: introduce functions for
> HVMOP_get/set_param allowance checks
> 
> >>> Paul Durrant <paul.durrant@xxxxxxxxxx> 05/01/15 4:05 PM >>>
> >+    /* The following parameters cannot be set by the guest. */
> 
> Didn't you agree that "cannot" isn't the right term here?
> 

Yes, sorry I must have missed that.

> >+    /* The following parameters can only be changed once. */
> >+    switch ( a->index )
> >+    {
> >+    case HVM_PARAM_VIRIDIAN:
> >+    case HVM_PARAM_IOREQ_SERVER_PFN:
> >+    case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
> >+        if (value != 0 && a->value != value)
> 
> There are still blanks missing here.
> 

True.

> >case HVM_PARAM_IDENT_PT:
> >-        /* Not reflexive, as we must domain_pause(). */
> 
> And I continue to think that retaining these comments would be useful for
> documentation purposes (even when switching to white lists - they should
> then remain explicit case labels grouped with the "default" one). And taking
> into consideration the comment change requested at the very top (which
> changes again in the 3rd patch), there could then perhaps be a distinction
> between "can't", "shouldn't", and "mustn't" (again solely serving doc
> purposes).
> 

Ok, if you don't mind have the extra functionally redundant case statements.

  Paul

> Jan


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