Richard W.M. Jones
2015-Jun-23 12:25 UTC
[Libguestfs] [PATCH] lib: Add optional 'append' parameter to copy-(device|file)-to-file APIs.
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;
if (!dest_buf) {
reply_with_perror ("malloc");
return -1;
}
- return copy (src, src, dest_buf, dest, DEST_FILE_FLAGS, 0,
+ if ((optargs_bitmask & GUESTFS_COPY_DEVICE_TO_FILE_APPEND_BITMASK)
&&
+ append)
+ wrflags |= O_APPEND;
+ else
+ wrflags |= O_TRUNC;
+
+ return copy (src, src, dest_buf, dest, wrflags, 0666, 0,
srcoffset, destoffset, size, sparse);
}
int
do_copy_file_to_device (const char *src, const char *dest,
int64_t srcoffset, int64_t destoffset, int64_t size,
- int sparse)
+ int sparse, int append)
{
CLEANUP_FREE char *src_buf = sysroot_path (src);
@@ -244,6 +255,12 @@ do_copy_file_to_device (const char *src, const char *dest,
return -1;
}
+ if ((optargs_bitmask & GUESTFS_COPY_FILE_TO_DEVICE_APPEND_BITMASK)
&&
+ append) {
+ reply_with_error ("the append flag cannot be set for this call");
+ return -1;
+ }
+
return copy (src_buf, src, dest, dest, DEST_DEVICE_FLAGS, 0,
srcoffset, destoffset, size, sparse);
}
@@ -251,9 +268,10 @@ do_copy_file_to_device (const char *src, const char *dest,
int
do_copy_file_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 *src_buf = NULL, *dest_buf = NULL;
+ int wrflags = O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC;
src_buf = sysroot_path (src);
if (!src_buf) {
@@ -267,7 +285,13 @@ do_copy_file_to_file (const char *src, const char *dest,
return -1;
}
- return copy (src_buf, src, dest_buf, dest, DEST_FILE_FLAGS,
+ if ((optargs_bitmask & GUESTFS_COPY_FILE_TO_FILE_APPEND_BITMASK)
&&
+ append)
+ wrflags |= O_APPEND;
+ else
+ wrflags |= O_TRUNC;
+
+ return copy (src_buf, src, dest_buf, dest, wrflags, 0666,
COPY_UNLINK_DEST_ON_FAILURE,
srcoffset, destoffset, size, sparse);
}
diff --git a/generator/actions.ml b/generator/actions.ml
index d5e5ccf..11b8652 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -9424,7 +9424,7 @@ See also C<guestfs_part_to_dev>." };
{ defaults with
name = "copy_device_to_device"; added = (1, 13, 25);
- style = RErr, [Device "src"; Device "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"];
+ style = RErr, [Device "src"; Device "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"; OBool "append"];
proc_nr = Some 294;
progress = true;
shortdesc = "copy from source device to destination device";
@@ -9448,6 +9448,11 @@ overlapping regions may not be copied correctly.
If the destination is a file, it is created if required. If
the destination file is not large enough, it is extended.
+If the destination is a file and the C<append> flag is not set,
+then the destination file is truncated. If the C<append> flag is
+set, then the copy appends to the destination file. The C<append>
+flag currently cannot be set for devices.
+
If the C<sparse> flag is true then the call avoids writing
blocks that contain only zeroes, which can help in some situations
where the backing disk is thin-provisioned. Note that unless
@@ -9456,7 +9461,7 @@ in incorrect copying." };
{ defaults with
name = "copy_device_to_file"; added = (1, 13, 25);
- style = RErr, [Device "src"; Pathname "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"];
+ style = RErr, [Device "src"; Pathname "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"; OBool "append"];
proc_nr = Some 295;
progress = true;
shortdesc = "copy from source device to destination file";
@@ -9466,7 +9471,7 @@ of this call." };
{ defaults with
name = "copy_file_to_device"; added = (1, 13, 25);
- style = RErr, [Pathname "src"; Device "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"];
+ style = RErr, [Pathname "src"; Device "dest"], [OInt64
"srcoffset"; OInt64 "destoffset"; OInt64 "size";
OBool "sparse"; OBool "append"];
proc_nr = Some 296;
progress = true;
shortdesc = "copy from source file to destination device";
@@ -9476,24 +9481,32 @@ of this call." };
{ defaults with
name = "copy_file_to_file"; added = (1, 13, 25);
- style = RErr, [Pathname "src"; Pathname "dest"],
[OInt64 "srcoffset"; OInt64 "destoffset"; OInt64
"size"; OBool "sparse"];
+ style = RErr, [Pathname "src"; Pathname "dest"],
[OInt64 "srcoffset"; OInt64 "destoffset"; OInt64
"size"; OBool "sparse"; OBool "append"];
proc_nr = Some 297;
progress = true;
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
[["mkdir"; "/copyff2"];
["fill"; "0"; string_of_int size;
"/copyff2/src"];
["touch"; "/copyff2/dest"];
["truncate_size"; "/copyff2/dest"; string_of_int
size];
- ["copy_file_to_file"; "/copyff2/src";
"/copyff2/dest"; ""; ""; "";
"true"];
- ["is_zero"; "/copyff2/dest"]]), []
+ ["copy_file_to_file"; "/copyff2/src";
"/copyff2/dest"; ""; ""; "";
"true"; "false"];
+ ["is_zero"; "/copyff2/dest"]]), [];
+ InitScratchFS, Always, TestResult (
+ [["mkdir"; "/copyff3"];
+ ["write"; "/copyff3/src"; "hello,
world"];
+ ["copy_file_to_file"; "/copyff3/src";
"/copyff3/dest"; ""; ""; "";
""; "true"];
+ ["copy_file_to_file"; "/copyff3/src";
"/copyff3/dest"; ""; ""; "";
""; "true"];
+ ["copy_file_to_file"; "/copyff3/src";
"/copyff3/dest"; ""; ""; "";
""; "true"];
+ ["read_file"; "/copyff3/dest"]],
+ "compare_buffers (ret, size, \"hello, worldhello, worldhello,
world\", 12*3) == 0"), [];
];
shortdesc = "copy from source file to destination file";
longdesc = "\
--
2.3.1
Pino Toscano
2015-Jun-23 17:40 UTC
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?> > if (!dest_buf) { > reply_with_perror ("malloc"); > return -1; > } > > - return copy (src, src, dest_buf, dest, DEST_FILE_FLAGS, 0, > + if ((optargs_bitmask & GUESTFS_COPY_DEVICE_TO_FILE_APPEND_BITMASK) && > + append) > + wrflags |= O_APPEND; > + else > + wrflags |= O_TRUNC; > + > + return copy (src, src, dest_buf, dest, wrflags, 0666, 0, > srcoffset, destoffset, size, sparse); > } > > int > do_copy_file_to_device (const char *src, const char *dest, > int64_t srcoffset, int64_t destoffset, int64_t size, > - int sparse) > + int sparse, int append) > { > CLEANUP_FREE char *src_buf = sysroot_path (src); > > @@ -244,6 +255,12 @@ do_copy_file_to_device (const char *src, const char *dest, > return -1; > } > > + if ((optargs_bitmask & GUESTFS_COPY_FILE_TO_DEVICE_APPEND_BITMASK) && > + append) { > + reply_with_error ("the append flag cannot be set for this call"); > + return -1; > + } > + > return copy (src_buf, src, dest, dest, DEST_DEVICE_FLAGS, 0, > srcoffset, destoffset, size, sparse); > } > @@ -251,9 +268,10 @@ do_copy_file_to_device (const char *src, const char *dest, > int > do_copy_file_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 *src_buf = NULL, *dest_buf = NULL; > + int wrflags = O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC; > > src_buf = sysroot_path (src); > if (!src_buf) { > @@ -267,7 +285,13 @@ do_copy_file_to_file (const char *src, const char *dest, > return -1; > } > > - return copy (src_buf, src, dest_buf, dest, DEST_FILE_FLAGS, > + if ((optargs_bitmask & GUESTFS_COPY_FILE_TO_FILE_APPEND_BITMASK) && > + append) > + wrflags |= O_APPEND; > + else > + wrflags |= O_TRUNC; > + > + return copy (src_buf, src, dest_buf, dest, wrflags, 0666, > COPY_UNLINK_DEST_ON_FAILURE, > srcoffset, destoffset, size, sparse); > } > diff --git a/generator/actions.ml b/generator/actions.ml > index d5e5ccf..11b8652 100644 > --- a/generator/actions.ml > +++ b/generator/actions.ml > @@ -9424,7 +9424,7 @@ See also C<guestfs_part_to_dev>." }; > > { defaults with > name = "copy_device_to_device"; added = (1, 13, 25); > - style = RErr, [Device "src"; Device "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"]; > + style = RErr, [Device "src"; Device "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"; OBool "append"]; > proc_nr = Some 294; > progress = true; > shortdesc = "copy from source device to destination device"; > @@ -9448,6 +9448,11 @@ overlapping regions may not be copied correctly. > If the destination is a file, it is created if required. If > the destination file is not large enough, it is extended. > > +If the destination is a file and the C<append> flag is not set, > +then the destination file is truncated. If the C<append> flag is > +set, then the copy appends to the destination file. The C<append> > +flag currently cannot be set for devices. > + > If the C<sparse> flag is true then the call avoids writing > blocks that contain only zeroes, which can help in some situations > where the backing disk is thin-provisioned. Note that unless > @@ -9456,7 +9461,7 @@ in incorrect copying." }; > > { defaults with > name = "copy_device_to_file"; added = (1, 13, 25); > - style = RErr, [Device "src"; Pathname "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"]; > + style = RErr, [Device "src"; Pathname "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"; OBool "append"]; > proc_nr = Some 295; > progress = true; > shortdesc = "copy from source device to destination file"; > @@ -9466,7 +9471,7 @@ of this call." }; > > { defaults with > name = "copy_file_to_device"; added = (1, 13, 25); > - style = RErr, [Pathname "src"; Device "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"]; > + style = RErr, [Pathname "src"; Device "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"; OBool "append"]; > proc_nr = Some 296; > progress = true; > shortdesc = "copy from source file to destination device"; > @@ -9476,24 +9481,32 @@ of this call." }; > > { defaults with > name = "copy_file_to_file"; added = (1, 13, 25); > - style = RErr, [Pathname "src"; Pathname "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"]; > + style = RErr, [Pathname "src"; Pathname "dest"], [OInt64 "srcoffset"; OInt64 "destoffset"; OInt64 "size"; OBool "sparse"; OBool "append"]; > proc_nr = Some 297; > progress = true; > 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 inThis 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]; > - ["copy_file_to_file"; "/copyff2/src"; "/copyff2/dest"; ""; ""; ""; "true"]; > - ["is_zero"; "/copyff2/dest"]]), [] > + ["copy_file_to_file"; "/copyff2/src"; "/copyff2/dest"; ""; ""; ""; "true"; "false"]; > + ["is_zero"; "/copyff2/dest"]]), []; > + InitScratchFS, Always, TestResult ( > + [["mkdir"; "/copyff3"]; > + ["write"; "/copyff3/src"; "hello, world"]; > + ["copy_file_to_file"; "/copyff3/src"; "/copyff3/dest"; ""; ""; ""; ""; "true"]; > + ["copy_file_to_file"; "/copyff3/src"; "/copyff3/dest"; ""; ""; ""; ""; "true"]; > + ["copy_file_to_file"; "/copyff3/src"; "/copyff3/dest"; ""; ""; ""; ""; "true"]; > + ["read_file"; "/copyff3/dest"]], > + "compare_buffers (ret, size, \"hello, worldhello, worldhello, world\", 12*3) == 0"), []; > ]; > shortdesc = "copy from source file to destination file"; > longdesc = "\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. -- Pino Toscano
Richard W.M. Jones
2015-Jun-23 20:28 UTC
Re: [Libguestfs] [PATCH] lib: Add optional 'append' parameter to copy-(device|file)-to-file APIs.
On Tue, Jun 23, 2015 at 07:40:57PM +0200, Pino Toscano wrote:> 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?That was my original version. I just questioned whether the code was really any easier to read versus defining the set of O_ flags that we're all pretty familiar with.> > 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's used here ^^ [...]> 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.This is an assumption of the current code, if you look at: https://github.com/libguestfs/libguestfs/blob/master/daemon/copy.c#L39 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Maybe Matching Threads
- [PATCH] lib: Add optional 'append' parameter to copy-(device|file)-to-file APIs.
- [PATCH] resize: fix 'No space left on device' problem when copying to an extended partition (RHBZ#1169015)
- [PATCH 1/2] generator: Simplify the handling of string parameters.
- [PATCH 1/2] generator: Rename java_structs to camel_structs to better reflect their purpose
- [PATCH 08/11] vmci_queue_pair.patch: VMCI queue pair implementation.