Re: [Xen-devel] [PATCH v2] common/vm_event: Initialize vm_event lists on domain creation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.08.17 at 17:17, <aisaila@xxxxxxxxxxxxxxx> wrote:
> On Jo, 2017-08-24 at 07:24 -0600, Jan Beulich wrote:
>> > @@ -500,6 +519,9 @@ bool_t vm_event_check_ring(struct
>> > vm_event_domain *ved)
>> >  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain
>> > *ved,
>> >                            bool_t allow_sleep)
>> >  {
>> > +    if ( !ved )
>> > +        return -ENOSYS;
>> I don't think ENOSYS is correct here.
> Can you tell me what is the preferred return value here?

-EOPNOTSUPP is what we commonly use in such cases.

>> > @@ -599,7 +621,6 @@ int vm_event_domctl(struct domain *d,
>> > xen_domctl_vm_event_op_t *vec,
>> >  #ifdef CONFIG_HAS_MEM_PAGING
>> >      case XEN_DOMCTL_VM_EVENT_OP_PAGING:
>> >      {
>> > -        struct vm_event_domain *ved = &d->vm_event->paging;
>> Dropping this local variable (and similar ones below) pointlessly
>> increases the size of this patch.
> I have dropped the local var because in case of a XEN_VM_EVENT_ENABLE
> d->vm_event_... is allocated in the vm_event_enable function below so
> it will allocate mem for the local variable.

I don't see how that renders the local variable useless. But anyway,
its the maintainers of that code to judge.

Also - please trim your replies.

Jan


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