Re: [Xen-devel] [PATCH v7 10/15] argo: implement the notify op

[Christopher Clark <christopher.w.clark@xxxxxxxxx>]

On Mon, Feb 4, 2019 at 7:12 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 31.01.19 at 05:28, <christopher.w.clark@xxxxxxxxx> wrote:
> > @@ -1802,6 +2157,21 @@ do_argo_op(unsigned int cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg1,
> >          break;
> >      }
> >
> > +    case XEN_ARGO_OP_notify:
> > +    {
> > +        XEN_GUEST_HANDLE_PARAM(xen_argo_ring_data_t) ring_data_hnd =
> > +                   guest_handle_cast(arg1, xen_argo_ring_data_t);
> > +
> > +        if ( unlikely((!guest_handle_is_null(arg2)) || arg3 || arg4) )
> > +        {
> > +            rc = -EINVAL;
> > +            break;
> > +        }
> > +
> > +        rc = notify(currd, ring_data_hnd);
> > +        break;
> > +    }
> > +
> >      default:
> >          rc = -EOPNOTSUPP;
> >          break;
> > @@ -1912,6 +2282,21 @@ compat_argo_op(unsigned int cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg1,
> >          break;
> >      }
> >
> > +    case XEN_ARGO_OP_notify:
> > +    {
> > +        XEN_GUEST_HANDLE_PARAM(xen_argo_ring_data_t) ring_data_hnd =
> > +                   guest_handle_cast(arg1, xen_argo_ring_data_t);
> > +
> > +        if ( unlikely((!guest_handle_is_null(arg2)) || arg3 || arg4) )
> > +        {
> > +            rc = -EINVAL;
> > +            break;
> > +        }
> > +
> > +        rc = notify(currd, ring_data_hnd);
> > +        break;
> > +    }
>
> At the example of this (likely applies to earlier patches as much): Aren't
> you afraid of this recurring duplication? It's quite easy, especially when
> the functions here grow a little further, for someone to forget updating
> one (more likely the compat one obviously). Did you consider forwarding
> all operations not needing translation straight into do_argo_op(), and
> handling only the sendv one here?

Ack, ok.

Christopher

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel