Re: [Xen-devel] [PATCH v2 1/3] x86/hvm: give HVMOP_set_param and HVMOP_get_param their own functions

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:jbeulich@xxxxxxxx]
> Sent: 01 May 2015 20:19
> To: Andrew Cooper; Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
> Subject: Re: [PATCH v2 1/3] x86/hvm: give HVMOP_set_param and
> HVMOP_get_param their own functions
> 
> >>> Paul Durrant <paul.durrant@xxxxxxxxxx> 05/01/15 4:05 PM >>>
> >--- a/xen/arch/x86/hvm/hvm.c
> >+++ b/xen/arch/x86/hvm/hvm.c
> >@@ -5638,6 +5638,299 @@ static int hvmop_set_evtchn_upcall_vector(
> >return 0;
> >}
>  >
> >+static int hvmop_set_param(
> >+    XEN_GUEST_HANDLE_PARAM(xen_hvm_param_t) arg)
> >+{
> >+    struct domain *curr_d = current->domain;
> >+    struct xen_hvm_param a;
> >+    struct domain *d;
> >+    struct vcpu *v;
> >+    int rc = 0;
> 
> Iirc Andrew indicated that Coverity would complain about dead initializers 
> like
> this.

Yes, it may. I'll ditch it in v3.

> 
> >+    if ( copy_from_guest(&a, arg, 1) )
> >+        return -EFAULT;
> >+
> >+    if ( a.index >= HVM_NR_PARAMS )
> >+        return -EINVAL;
> >+
> >+    d = rcu_lock_domain_by_any_id(a.domid);
> >+    if ( d == NULL )
> >+        return -ESRCH;
> >+
> >+    rc = -EINVAL;
> 
> (Not used anywhere up from here.)
> 
> >+    if ( is_pvh_domain(d)
> >+         && (a.index != HVM_PARAM_CALLBACK_IRQ) )
> >+        goto out;
> 
> It would have been nice if you had corrected style issues like the misplaced
> &&
> as you go; I'll try to remember to do so while committing (together with a
> few more
> and the adjustment for the issue above).

I'm happy to do it. Just missed it.

> 
> >+    case HVM_PARAM_IOREQ_PFN:
> >+    case HVM_PARAM_BUFIOREQ_PFN:
> >+    case HVM_PARAM_BUFIOREQ_EVTCHN:
> >+    {
> >+        domid_t domid;
> >+
> >+        /* May need to create server */
> >+        domid = d->arch.hvm_domain.params[HVM_PARAM_DM_DOMAIN];
> >+        rc = hvm_create_ioreq_server(d, domid, 1, 1, NULL);
> >+        if ( rc != 0 && rc != -EEXIST )
> >+            goto out;
> >+        /*FALLTHRU*/
> >+    }
> >+    default:
> 
> Andrew - will Coverity be happy with the fall-through comment being
> followed
> by a closing brace?
> 

It looks wrong anyway. I'll move to after the brace.

  Paul

> Jan


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