Re: [Libguestfs] [PATCH] lib: Add optional 'append' parameter to copy-(device|file)-to-file APIs.

In data martedì 23 giugno 2015 13:25:28, Richard W.M. Jones ha scritto: > This allows you to append one file to another: > > copy-file-to-file /input.txt /output.txt append:true > > will append the contents of /input.txt to /output.txt. > --- > daemon/copy.c | 38 +++++++++++++++++++++++++++++++------- > generator/actions.ml | 29 +++++++++++++++++++++-------- > 2 files changed, 52 insertions(+), 15 deletions(-) > > diff --git a/daemon/copy.c b/daemon/copy.c > index bab00fe..6f3e844 100644 > --- a/daemon/copy.c > +++ b/daemon/copy.c > @@ -30,7 +30,6 @@ > #include "actions.h" > > /* wrflags */ > -#define DEST_FILE_FLAGS O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0666 > #define DEST_DEVICE_FLAGS O_WRONLY|O_CLOEXEC, 0 > > /* flags */ > @@ -210,8 +209,13 @@ copy (const char *src, const char *src_display, > int > do_copy_device_to_device (const char *src, const char *dest, > int64_t srcoffset, int64_t destoffset, int64_t size, > - int sparse) > + int sparse, int append) > { > + if ((optargs_bitmask & GUESTFS_COPY_DEVICE_TO_DEVICE_APPEND_BITMASK) && > + append) { > + reply_with_error ("the append flag cannot be set for this call"); > + return -1; > + } > return copy (src, src, dest, dest, DEST_DEVICE_FLAGS, 0, > srcoffset, destoffset, size, sparse); > } > @@ -219,23 +223,30 @@ do_copy_device_to_device (const char *src, const char *dest, > int > do_copy_device_to_file (const char *src, const char *dest, > int64_t srcoffset, int64_t destoffset, int64_t size, > - int sparse) > + int sparse, int append) > { > CLEANUP_FREE char *dest_buf = sysroot_path (dest); > + int wrflags = O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC; Cannot DEST_FILE_FLAGS (without O_TRUNC, of course) be used here (and below) to avoid copying&pasting these attributes?

> tests = [ > InitScratchFS, Always, TestResult ( > [["mkdir"; "/copyff"]; > ["write"; "/copyff/src"; "hello, world"]; > - ["copy_file_to_file"; "/copyff/src"; "/copyff/dest"; ""; ""; ""; ""]; > + ["copy_file_to_file"; "/copyff/src"; "/copyff/dest"; ""; ""; ""; ""; "false"]; > ["read_file"; "/copyff/dest"]], > "compare_buffers (ret, size, \"hello, world\", 12) == 0"), []; > - let size = 1024 * 1024 in > InitScratchFS, Always, TestResultTrue ( > + let size = 1024 * 1024 in This seems unused? Or is it supposed to be used and its usage has been forgotten? > [["mkdir"; "/copyff2"]; > ["fill"; "0"; string_of_int size; "/copyff2/src"]; > ["touch"; "/copyff2/dest"]; > ["truncate_size"; "/copyff2/dest"; string_of_int size];

It would seem okay for me, although maybe we should decouple the enums for flags, so there's no need to add non-functional append flags to the *_to_device variants.