On 29/03/17 11:38, Jan Beulich wrote:
On 28.03.17 at 15:18, <jennifer.herb...@citrix.com> wrote:
Perhaps drop "already"? Personally I also wouldn't mind you dropping
the variable altogether and using header->opaque directly, but I
guess that's too "opaque" for your taste?
It would make the code too opaque for my taste, so I'll just drop the
'already' bit.
@@ -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.
If I where to write back, I'd be writing back 0 to nr_extents - which
wouldn’t really
mean anything since I’m not defining the order for which I’m processing
them.
In fact the only thing it tells you is that extent 0 is the last one
processed, which
I don't think its all that useful.
Ideally I'd prefer to leave it untouched on success, but the original
value is lost on
continuation, this would be more involved.
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?
Cheers,
-jenny
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
