Re: [PATCH v3] dm_op: Add xendevicemodel_modified_memory_bulk.

["Jan Beulich" <JBeulich@suse.com>]

>>> On 29.03.17 at 16:35, <Jennifer.Herbert@citrix.com> wrote:
> On 29/03/17 11:38, Jan Beulich wrote:
>>>>> On 28.03.17 at 15:18, <jennifer.herbert@citrix.com> wrote:
>>> @@ -441,13 +481,8 @@ static int dm_op(domid_t domid,
>>>           struct xen_dm_op_modified_memory *data =
>>>               &op.u.modified_memory;
>>>   
>>> -        const_op = false;
>>> -
>>> -        rc = -EINVAL;
>>> -        if ( data->pad )
>>> -            break;
>>> -
>>> -        rc = modified_memory(d, data);
>>> +        rc = modified_memory(d, data, &bufs[1]);
>>> +        const_op = (rc != 0);
>> Isn't this wrong now, i.e. don't you need to copy back the
>> header now in all cases?
> 
> I only define what I'll set nr_extents to in case of error, and of 
> course opaque
> is opaque.

Well, but you do need the opaque value for the continuation,
don't you? In which case you need to also write back on
-ERESTART. And as you say you need to write back in case
of error. So I'd expect

       const_op = !rc;

> By only writing back on error, I hoped to improve efficiency for the 
> common case,
> (especially for existing use with calls of one extent).  (I know its 
> only a small difference)
> If you want me to write back - what do you want me to write back for 
> success?

Right, avoiding to write something useless is sensible. If anything,
the original value of nr_extents would make sense to be written
back, but that value was long lost by that time.

Jan


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