xapi-project / vhd-tool

Command-line tools for streaming and manipulating vhd format data
Other
8 stars 26 forks source link

CA-306395 Convert sendfile to direct_copy #85

Closed TimSmithCtx closed 5 years ago

TimSmithCtx commented 5 years ago

In the general case it is better to use read() and write() with O_DIRECT for large data transfers than to unconditionally send everything via sendfile(), particularly when the output is not a pipe or a socket.

This converts the previous sendfile wrapper into a more general direct_copy wrapper. Later, if we decide we need to get more performance between certain types of filedescriptors, we can make use of the stub_init() function which currently allocates the buffer record an engine selector in the direct_copy_handle based on information obtained from fstat() on the two filedescriptors.

Incorporates changes from Rob Hoes rob.hoes@citrix.com: Remove unused reference to sendfile binding Add init and cleanup stubs for sendfile replacement Rename sendfile64_stubs.c to direct_copy_stubs.c

Signed-off-by: Tim Smith tim.smith@citrix.com Signed-off-by: Rob Hoes rob.hoes@citrix.com

TimSmithCtx commented 5 years ago

There's a question if we want the forcing-on of O_DIRECT in the wrapper or should it be in the caller? Good taste would prefer the latter in which case I'll amend.

lindig commented 5 years ago

Careful review by @edwintorok. I agree that a pointer must be marked as abstract before it is passed back to OCaml.

robhoes commented 5 years ago

I would have preferred to keep the rename in a separate commit, so that we can more easily see the actual diff.

robhoes commented 5 years ago

I agree that adding the Abstract tag plus setting the pointer to NULL is a bit safer, so we should do that. But then we should also check for NULL handles in the actual copy function, and raise an appropriate exception.

TimSmithCtx commented 5 years ago

I think this should address most things. Note I haven't tested this yet as it's stacked up behind another long running test. I will comment again when the tests are done.