[PATCH] copy-chunk / FSRVP (remote snapshots) changes

[David Disseldorp]

Hi Jeremy,

On Fri, 5 Oct 2012 17:27:28 -0700
Jeremy Allison <jra at samba.org> wrote:

...
> FYI. I haven't forgotten about this. I'm currently doing
> some cleanup work on the second patch :
> 
> s3-rpc: convert process_complete_pdu and callers async
> 
> In lots of error cases you're doing things like:
> 
>                         if (data_left > 0) {
>                                 DEBUG(0, ("Unacceptable, more than one PDU\n"));
>                                 state->data_processed = -1;
>                                 tevent_req_done(req);
>                                 return tevent_req_post(req, ev);
>                         }
> 
> and I think this should look like:
> 
>                         if (data_left > 0) {
>                                 DEBUG(0, ("Unacceptable, more than one PDU\n"));
>                                 state->data_processed = -1;
>                                 tevent_req_error(req, EINVAL);
>                                 return tevent_req_post(req, ev);
>                         }
> 
> as the:
> 
> 	tevent_req_done(req);
> 	return tevent_req_post(req, ev);
> 
> tends to be done in success codepaths (IMHO).

write_to_internal_pipe() previously returned errors through a negative
data_processed return. In conversion to async I kept the same logic,
such that write_to_internal_pipe_recv() is checked for a negative return
and NT_STATUS_UNEXPECTED_IO_ERROR is set on the parent req accordingly.

I agree it would be nicer if the error were propagated via the ntstatus
embedded in the subreq. I'll clean this up and push a new branch.

Thanks for the feedback :)

Cheers, David