[PATCH] New APIs: hopen-device hopen-file hread hwrite hseek hclose hclose-all
These APIs provide handle-based read/write streaming and random access to files and block devices. --- daemon/.gitignore | 1 + daemon/Makefile.am | 1 + daemon/hfile.c | 196 +++++++++++++++++++++++++++++++++++++++++++++ daemon/m4/gnulib-cache.m4 | 3 +- po/POTFILES.in | 2 + regressions/Makefile.am | 1 + regressions/test-hfile.pl | 102 +++++++++++++++++++++++ src/MAX_PROC_NR | 2 +- src/generator.ml | 114 ++++++++++++++++++++++++++ 9 files changed, 420 insertions(+), 2 deletions(-) create mode 100644 daemon/hfile.c create mode 100755 regressions/test-hfile.pl diff --git a/daemon/.gitignore b/daemon/.gitignore index e06c684..c9837be 100644 --- a/daemon/.gitignore +++ b/daemon/.gitignore @@ -50,6 +50,7 @@ m4/gettime.m4 m4/gettimeofday.m4 m4/getpagesize.m4 m4/getugroups.m4 +m4/gl_list.m4 m4/glibc21.m4 m4/glob.m4 m4/gnulib-common.m4 diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 0c8be08..c243bb6 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -94,6 +94,7 @@ guestfsd_SOURCES = \ guestfsd.c \ headtail.c \ hexdump.c \ + hfile.c \ htonl.c \ initrd.c \ inotify.c \ diff --git a/daemon/hfile.c b/daemon/hfile.c new file mode 100644 index 0000000..fedb502 --- /dev/null +++ b/daemon/hfile.c @@ -0,0 +1,196 @@ +/* libguestfs - the guestfsd daemon + * Copyright (C) 2010 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include <config.h> + +#include <errno.h> +#include <fcntl.h> +#include <unistd.h> + +#include "gl_array_list.h" + +#include "daemon.h" +#include "actions.h" + +gl_list_t handles; + +static void init_handle_list (void) __attribute__((constructor)); +void +init_handle_list (void) +{ + handles = gl_list_nx_create_empty(GL_ARRAY_LIST, NULL, NULL, NULL, false); +} + +static int +hopen_check(int fd, const char *path) +{ + if (fd < 0) { + reply_with_perror_errno(errno, "open %s failed", path); + return -1; + } + + gl_list_node_t node = gl_list_nx_add_last(handles, (void *)(long)fd); + if (NULL == node) { + close(fd); + + reply_with_error("gl_list_nx_add_last"); + return -1; + } + + return fd; +} + +int /* RInt */ +do_hopen_file (const char *path, int flags) +{ + int fd; + + CHROOT_IN; + if (flags && O_CREAT) { + fd = open(path, flags, 0777); + } else { + fd = open(path, flags); + } + CHROOT_OUT; + + return hopen_check(fd, path); +} + +int /* RInt */ +do_hopen_device (const char *device, int flags) +{ + int fd; + + fd = open(device, flags); + + return hopen_check(fd, device); +} + +int /* RErr */ +do_hclose (int handle) +{ + gl_list_node_t node = gl_list_search(handles, (void *)(long)handle); + if (NULL == node) { + reply_with_error("%i does not refer to an open handle", handle); + return -1; + } + gl_list_remove_node(handles, node); + + if (close(handle) < 0) { + reply_with_perror_errno(errno, "close handle %i failed", handle); + return -1; + } + + return 0; +} + +int /* RErr */ +do_hclose_all (void) +{ + const void *eltp; + gl_list_node_t node; + gl_list_iterator_t i = gl_list_iterator(handles); + + char *msg = NULL; + size_t msglen = 0; + + bool failed = false; + while (gl_list_iterator_next(&i, &eltp, &node)) { + gl_list_remove_node(handles, node); + + int fd = (long)eltp; + if (close(fd) < 0) { + failed = true; + + char *error = NULL; + int len = asprintf(&error, "close handle %i failed: %m ", fd); + if (len < 0) { + reply_with_error("asprintf"); + } + + msg = realloc(msg, msglen + len); + if (NULL == msg) { + reply_with_error("realloc"); + return -1; + } + + memcpy(msg + msglen, error, len); + msglen += len; + free(error); + } + } + + if (failed) { + msg[msglen] = '\0'; + reply_with_error("%s", msg); + return -1; + } + + return 0; +} + +char * /* RBufferOut */ +do_hread (int handle, int64_t size, size_t *size_r) +{ + char *buf = malloc(size); + if (NULL == buf) { + reply_with_error("malloc"); + return NULL; + } + + ssize_t in = read(handle, buf, size); + if (in < 0) { + reply_with_perror_errno(errno, "error reading from handle %i", handle); + goto error; + } + + *size_r = in; + return buf; +error: + free(buf); + return NULL; +} + +int /* RErr */ +do_hwrite (int handle, const char *content, size_t content_size) +{ + size_t pos = 0; + while (pos < content_size) { + ssize_t out = write(handle, content + pos, content_size - pos); + + if (out < 0) { + reply_with_perror_errno(errno, "error writing to handle %i", handle); + return -1; + } + pos += out; + } + + return 0; +} + +int64_t /* RInt64 */ +do_hseek (int handle, int64_t offset, int whence) +{ + int64_t ret = lseek(handle, offset, whence); + if (ret < 0) { + reply_with_perror_errno(errno, "error seeking in handle %i", handle); + return -1; + } + + return ret; +} diff --git a/daemon/m4/gnulib-cache.m4 b/daemon/m4/gnulib-cache.m4 index f026cb3..25ffd27 100644 --- a/daemon/m4/gnulib-cache.m4 +++ b/daemon/m4/gnulib-cache.m4 @@ -15,11 +15,12 @@ # Specification in the form of a command-line invocation: -# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings +# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl array-list byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings # Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) gl_MODULES([ + array-list byteswap c-ctype connect diff --git a/po/POTFILES.in b/po/POTFILES.in index e5fb857..c846e05 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -29,6 +29,7 @@ daemon/grub.c daemon/guestfsd.c daemon/headtail.c daemon/hexdump.c +daemon/hfile.c daemon/htonl.c daemon/initrd.c daemon/inotify.c @@ -99,6 +100,7 @@ perl/lib/Sys/Guestfs.pm perl/lib/Sys/Guestfs/Lib.pm python/guestfs-py.c regressions/rhbz501893.c +regressions/test-hfile.pl regressions/test-lvm-mapping.pl regressions/test-noexec-stack.pl ruby/ext/guestfs/_guestfs.c diff --git a/regressions/Makefile.am b/regressions/Makefile.am index ca4292a..f7d9992 100644 --- a/regressions/Makefile.am +++ b/regressions/Makefile.am @@ -35,6 +35,7 @@ TESTS = \ test-cancellation-download-librarycancels.sh \ test-cancellation-upload-daemoncancels.sh \ test-find0.sh \ + test-hfile.pl \ test-luks.sh \ test-lvm-filtering.sh \ test-lvm-mapping.pl \ diff --git a/regressions/test-hfile.pl b/regressions/test-hfile.pl new file mode 100755 index 0000000..4c23722 --- /dev/null +++ b/regressions/test-hfile.pl @@ -0,0 +1,102 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Temp qw(tempfile); +use Sys::Guestfs; + +# Create a new guestfs handle +my $g = Sys::Guestfs->new(); + +# Create a 10M image file and add it to the handle +my ($fh, $path) = tempfile(); +truncate($fh, 1024*1024*10); +close($fh) or die("close $path failed: $!"); +$g->add_drive($path); + +$g->launch(); + +# As qemu is now running we can unlink the image +unlink($path) or die("unlink $path failed: $!"); + +my $string = "abcd"; + +# Test read and writing directly to a block device +my $h = $g->hopen_device("/dev/vda", 2); +$g->hwrite($h, $string); +$g->hseek($h, 0, 0); +my $in = $g->hread($h, length($string)); + +die("device: hread returned $in") unless($in eq $string); +$g->hclose($h); + +# Check $h is no longer valid +eval { + $g->hseek($h, 0, 0); +}; +die("device: handle wasn't closed") unless($@); + +# Make an ext2 filesystem and mount it +$g->mkfs("ext2", "/dev/vda"); +$g->mount("/dev/vda", "/"); + +# Open should fail read-only +eval { + $h = $g->hopen_file("/test", 0); +}; +die("file: read-only open non-existant file") unless($@); + +# Open should fail write-only without O_CREAT +eval { + $h = $g->hopen_file("/test", 1); +}; +die("file: write-only open non-existant file") unless($@); + +# Open should fail read-write without O_CREAT +eval { + $h = $g->hopen_file("/test", 2); +}; +die("file: read-write open non-existant file") unless($@); + +# Open the file write only, and create it +$h = $g->hopen_file("/test", 1 | 64); + +# Write data to it +$g->hwrite($h, $string); + +# Verify we can't read from it +$g->hseek($h, 0, 0); +eval { + $in = $g->hread($h, length($string)); +}; +die("file: read of write-only file") unless($@); + +# close and check the contents with cat +$g->hclose($h); +$in = $g->cat("/test"); +die("file: hwrite data test") unless($in eq $string); + +# Open the file read-only +$h = $g->hopen_file("/test", 0); + +# Read the file in 1 byte chunks until the end +$in = ""; +my $chunk; +do { + $chunk = $g->hread($h, 1); + $in .= $chunk; +} until(length($chunk) == 0); +die("file: hread in chunks") unless($in eq $string); + +# Read past EOF returns a short read +$g->hseek($h, 0, 0); +$in = $g->hread($h, length($string) + 1); +die("file: read past EOF") unless($in eq $string); + +# Test close-all +$g->hclose_all(); +eval { + $g->hseek($h, 0, 0); +}; +die("file: close-all") unless($@); diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c1d1ffb..305aa98 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -266 +273 diff --git a/src/generator.ml b/src/generator.ml index 00caa6a..48010dc 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -5355,6 +5355,120 @@ filesystem can be found. To find the label of a filesystem, use C<guestfs_vfs_label>."); + ("hopen_file", (RInt "handle", [Pathname "path"; Int "flags"]), 267, [], + [], + "open a file", + "\ +This command opens C<path> in the guest and returns a handle +to it. The returned handle is an opaque int which can be passed +to C<guestfs_h*> calls. + +C<flags> determines how the file is opened. It is a bitwise or of: + +=over + +=item 0 + +(O_RDONLY) Open the file read only. + +=item 1 + +(O_WRONLY) Open the file write only. + +=item 2 + +(O_RDWR) Open the file read-write. + +=item 64 + +(O_CREAT) Create the file if it does not already exist. The file +will be created with a mode of 0777, modified by the daemon's +umask. See L<UMASK>. + +=item 512 + +(O_TRUNC) Truncate a file opened for write to zero length when +opening. + +=item 1024 + +(O_APPEND) Always write to the end of the file. + +=item 262144 + +(O_NOATIME) Don't update the access time when reading from a file. + +=back"); + + ("hopen_device", (RInt "handle", [Device "device"; Int "flags"]), 268, [], + [], + "open a block device", + "\ +This command opens a block device and returns a handle to it. +The returned handle is an opaque int which can be passed to +C<guestfs_h*> calls. + +See L<guestfs_hopen_file> for a description of C<flags>."); + + ("hclose", (RErr, [Int "handle"]), 269, [], + [], + "close a file handle", + "\ +This command closes C<handle> and releases all resources associated +with it."); + + ("hclose_all", (RErr, []), 270, [], + [], + "close all open file handles", + "\ +This command closes all file handles which have been opened with +C<guestfs_hopen_*>."); + + ("hread", (RBufferOut "content", [Int "handle"; Int64 "length"]), 271, + [ProtocolLimitWarning], [], + "read data from an open handle", + "\ +This command reads up to C<length> bytes from C<handle> and returns +the data in C<content>. It will attempt to read exactly C<length> +bytes, but may return less. Returned C<content> with size 0 +indicates EOF."); + + ("hwrite", (RErr, [Int "handle"; BufferIn "content"]), 272, + [ProtocolLimitWarning], [], + "write data to an open handle", + "\ +This command writes all the data in C<content> to C<handle>. It +returns an error if this fails."); + + ("hseek", (RInt64 "newoffset", [Int "handle"; Int64 "offset"; + Int "whence"]), 273, [], + [], + "seek on an open handle", + "\ +This command updates the current position in C<handle>. This +affects the C<guestfs_hread> and C<guestfs_hwrite> calls. + +C<whence> determines the point of reference for C<offset>. It +must be one of: + +=over + +=item 0 + +C<offset> is relative to the beginning of the file. + +=item 1 + +C<offset> is relative to the current position. + +=item 2 + +C<offset> is relative to the end of the file. + +=back + +hseek returns the new offset from the beginning of the file."); + ] let all_functions = non_daemon_functions @ daemon_functions -- 1.7.2.2
On Thu, Aug 26, 2010 at 11:45:17AM +0100, Matthew Booth wrote:
+ if (fd < 0) {
I believe that these error checks should be == -1 instead of < 0. (There are several more below).
+ reply_with_perror_errno(errno, "open %s failed", path);
Just use: reply_with_perror ("open failed: %s", path). It's a macro which expands to the same as above. There are several more cases where the same thing is done.
+ char *error = NULL; + int len = asprintf(&error, "close handle %i failed: %m ", fd); + if (len < 0) { + reply_with_error("asprintf"); + }
Missing return statement.
+ msg = realloc(msg, msglen + len); + if (NULL == msg) { + reply_with_error("realloc"); + return -1; + } + + memcpy(msg + msglen, error, len); + msglen += len; + free(error); + } + } + + if (failed) { + msg[msglen] = '\0'; + reply_with_error("%s", msg); + return -1; + }
Is there a way to do this without dynamic memory allocations? How about putting a fixed buffer on the stack which is the same length as the maximum error message size (64K, so not too large for the stack): char errors[GUESTFS_ERROR_LEN]; char *error_p = errors; size_t error_n = sizeof errors - 1; /* when you get an error ... */ if (error_n > 0) { strncpy (error_p, strerror (errno), error_n); size_t n = strlen (errors); error_p = &errors[n]; error_n = errors[sizeof errors] - error_p - 1; } Or you could just return the first error ...
+ return 0; +} + +char * /* RBufferOut */ +do_hread (int handle, int64_t size, size_t *size_r) +{
Need to check that the handle is a member of the list. You should also check that size < GUESTFS_MESSAGE_MAX, just to avoid huge-but-not-fatal memory allocations at the next line. See the implementation of do_pread in daemon/file.c for an example.
+ char *buf = malloc(size); + if (NULL == buf) { + reply_with_error("malloc");
reply_with_perror.
+ goto error;
+error: + free(buf); + return NULL;
But of a mouthful, just when we want to free a single value. I think the goto is unnecessary here.
+int /* RErr */ +do_hwrite (int handle, const char *content, size_t content_size) +{ + size_t pos = 0; + while (pos < content_size) { + ssize_t out = write(handle, content + pos, content_size - pos); + + if (out < 0) { + reply_with_perror_errno(errno, "error writing to handle %i", handle); + return -1; + } + pos += out; + } + + return 0; +}
... and you were going to change do_pwrite as well.
+# Create a 10M image file and add it to the handle +my ($fh, $path) = tempfile(); +truncate($fh, 1024*1024*10); +close($fh) or die("close $path failed: $!");
In the other scripts we create a file in the current directory called "test.img". See regressions/test-lvm-mapping.pl for an example.
+# As qemu is now running we can unlink the image +unlink($path) or die("unlink $path failed: $!");
... but unlinking the image early is a good idea.
+my $string = "abcd"; + +# Test read and writing directly to a block device +my $h = $g->hopen_device("/dev/vda", 2); +$g->hwrite($h, $string); +$g->hseek($h, 0, 0); +my $in = $g->hread($h, length($string)); + +die("device: hread returned $in") unless($in eq $string); +$g->hclose($h); + +# Check $h is no longer valid +eval { + $g->hseek($h, 0, 0); +}; +die("device: handle wasn't closed") unless($@);
It's not really much of a test, writing and reading 4 bytes. Aligned vs unaligned? Seeking and writing in a large sparse file? Writing past the end of a large file? Writing past the end of a device? Reading and writing large buffers (just below 2MB and over 4MB, the latter should fail gracefully).
+=over
=over 4 for consistency at least?
+See L<guestfs_hopen_file> for a description of C<flags>.");
You should write: See C<guestfs_hopen_file> etc
+This command writes all the data in C<content> to C<handle>. It +returns an error if this fails.");
You don't need to say it returns an error if it fails. It would be a serious bug if it _didn't_ do this :=) and in any case the surrounding boilerplate will tell the caller how to detect errors from the call. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On 26/08/10 13:52, Richard W.M. Jones wrote:
It's not really much of a test, writing and reading 4 bytes.
It's supposed to exercise the libguestfs code, rather than the implementations of open, read, write and close. Just making the string longer doesn't really add anything to the existing coverage.
Aligned vs unaligned? Seeking and writing in a large sparse file? Writing past the end of a large file?
I don't think the above are interesting to test as their behaviours are well defined elsewhere.
Writing past the end of a device?
That's a potentially interesting test of error handling, though.
Reading and writing large buffers (just below 2MB and over 4MB, the latter should fail gracefully).
I ought to be testing that. I'll add it. I agree with all the rest of the review comments. Patch coming up at some point. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
These APIs provide handle-based read/write streaming and random access to files and block devices. --- daemon/.gitignore | 1 + daemon/Makefile.am | 1 + daemon/hfile.c | 224 +++++++++++++++++++++++++++++++++++++++++++++ daemon/m4/gnulib-cache.m4 | 3 +- po/POTFILES.in | 2 + regressions/Makefile.am | 1 + regressions/test-hfile.pl | 194 +++++++++++++++++++++++++++++++++++++++ src/MAX_PROC_NR | 2 +- src/generator.ml | 113 +++++++++++++++++++++++ 9 files changed, 539 insertions(+), 2 deletions(-) create mode 100644 daemon/hfile.c create mode 100755 regressions/test-hfile.pl diff --git a/daemon/.gitignore b/daemon/.gitignore index e06c684..c9837be 100644 --- a/daemon/.gitignore +++ b/daemon/.gitignore @@ -50,6 +50,7 @@ m4/gettime.m4 m4/gettimeofday.m4 m4/getpagesize.m4 m4/getugroups.m4 +m4/gl_list.m4 m4/glibc21.m4 m4/glob.m4 m4/gnulib-common.m4 diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 0c8be08..c243bb6 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -94,6 +94,7 @@ guestfsd_SOURCES = \ guestfsd.c \ headtail.c \ hexdump.c \ + hfile.c \ htonl.c \ initrd.c \ inotify.c \ diff --git a/daemon/hfile.c b/daemon/hfile.c new file mode 100644 index 0000000..7dc43fa --- /dev/null +++ b/daemon/hfile.c @@ -0,0 +1,224 @@ +/* libguestfs - the guestfsd daemon + * Copyright (C) 2010 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include <config.h> + +#include <errno.h> +#include <fcntl.h> +#include <unistd.h> + +#include "gl_array_list.h" + +#include "daemon.h" +#include "actions.h" + +gl_list_t handles; + +static void init_handle_list (void) __attribute__((constructor)); +void +init_handle_list (void) +{ + handles = gl_list_nx_create_empty(GL_ARRAY_LIST, NULL, NULL, NULL, false); +} + +static gl_list_node_t +get_handle (int handle) +{ + gl_list_node_t node = gl_list_search(handles, (void *)(long)handle); + if (NULL == node) { + reply_with_error("%i does not refer to an open handle", handle); + return NULL; + } + + return node; +} + +static int +hopen_check(int fd, const char *path) +{ + if (fd == -1) { + reply_with_perror("open %s failed", path); + return -1; + } + + gl_list_node_t node = gl_list_nx_add_last(handles, (void *)(long)fd); + if (NULL == node) { + close(fd); + + reply_with_error("gl_list_nx_add_last"); + return -1; + } + + return fd; +} + +int /* RInt */ +do_hopen_file (const char *path, int flags) +{ + int fd; + + CHROOT_IN; + if (flags && O_CREAT) { + fd = open(path, flags, 0777); + } else { + fd = open(path, flags); + } + CHROOT_OUT; + + return hopen_check(fd, path); +} + +int /* RInt */ +do_hopen_device (const char *device, int flags) +{ + int fd; + + fd = open(device, flags); + + return hopen_check(fd, device); +} + +int /* RErr */ +do_hclose (int handle) +{ + gl_list_node_t node = get_handle(handle); + if (NULL == node) { + return -1; + } + + gl_list_remove_node(handles, node); + + if (close(handle) < 0) { + reply_with_perror("close handle %i failed", handle); + return -1; + } + + return 0; +} + +int /* RErr */ +do_hclose_all (void) +{ + const void *eltp; + gl_list_node_t node; + gl_list_iterator_t i = gl_list_iterator(handles); + + char errors[GUESTFS_ERROR_LEN]; + char *error_p = errors; + size_t error_n = sizeof(errors); + + bool failed = false; + while (gl_list_iterator_next(&i, &eltp, &node)) { + gl_list_remove_node(handles, node); + + int fd = (long)eltp; + if (close(fd) < 0) { + failed = true; + + if (error_n != 0) { + size_t len = snprintf(error_p, error_n, + "close handle %i failed: %m ", fd); + if (len < error_n) { + error_p += len; + error_n -= len; + } else { + error_n = 0; + error_p = &errors[GUESTFS_ERROR_LEN]; + } + } + } + } + + if (failed) { + /* Overwrite the trailing space with a NULL terminator */ + *(error_p - 1) = '\0'; + reply_with_error("%s", errors); + return -1; + } + + return 0; +} + +char * /* RBufferOut */ +do_hread (int handle, int64_t size, size_t *size_r) +{ + if (get_handle(handle) == NULL) { + return NULL; + } + +#define HREAD_MESSAGE_MAX (GUESTFS_MESSAGE_MAX - \ + sizeof(guestfs_message_header) - \ + sizeof(guestfs_hread_args)) + if (size > (int64_t) HREAD_MESSAGE_MAX) { + size = HREAD_MESSAGE_MAX; + } +#undef HREAD_MESSAGE_MAX + + char *buf = malloc(size); + if (NULL == buf) { + reply_with_perror("malloc"); + return NULL; + } + + ssize_t in = read(handle, buf, size); + if (in < 0) { + reply_with_perror("error reading from handle %i", handle); + free(buf); + return NULL; + } + + *size_r = in; + return buf; +} + +int /* RErr */ +do_hwrite (int handle, const char *content, size_t content_size) +{ + if (get_handle(handle) == NULL) { + return -1; + } + + size_t pos = 0; + while (pos < content_size) { + ssize_t out = write(handle, content + pos, content_size - pos); + + if (out < 0) { + reply_with_perror("error writing to handle %i", handle); + return -1; + } + pos += out; + } + + return 0; +} + +int64_t /* RInt64 */ +do_hseek (int handle, int64_t offset, int whence) +{ + if (get_handle(handle) == NULL) { + return -1; + } + + int64_t ret = lseek(handle, offset, whence); + if (ret < 0) { + reply_with_perror("error seeking in handle %i", handle); + return -1; + } + + return ret; +} diff --git a/daemon/m4/gnulib-cache.m4 b/daemon/m4/gnulib-cache.m4 index f026cb3..25ffd27 100644 --- a/daemon/m4/gnulib-cache.m4 +++ b/daemon/m4/gnulib-cache.m4 @@ -15,11 +15,12 @@ # Specification in the form of a command-line invocation: -# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings +# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl array-list byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings # Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) gl_MODULES([ + array-list byteswap c-ctype connect diff --git a/po/POTFILES.in b/po/POTFILES.in index e5fb857..c846e05 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -29,6 +29,7 @@ daemon/grub.c daemon/guestfsd.c daemon/headtail.c daemon/hexdump.c +daemon/hfile.c daemon/htonl.c daemon/initrd.c daemon/inotify.c @@ -99,6 +100,7 @@ perl/lib/Sys/Guestfs.pm perl/lib/Sys/Guestfs/Lib.pm python/guestfs-py.c regressions/rhbz501893.c +regressions/test-hfile.pl regressions/test-lvm-mapping.pl regressions/test-noexec-stack.pl ruby/ext/guestfs/_guestfs.c diff --git a/regressions/Makefile.am b/regressions/Makefile.am index ca4292a..f7d9992 100644 --- a/regressions/Makefile.am +++ b/regressions/Makefile.am @@ -35,6 +35,7 @@ TESTS = \ test-cancellation-download-librarycancels.sh \ test-cancellation-upload-daemoncancels.sh \ test-find0.sh \ + test-hfile.pl \ test-luks.sh \ test-lvm-filtering.sh \ test-lvm-mapping.pl \ diff --git a/regressions/test-hfile.pl b/regressions/test-hfile.pl new file mode 100755 index 0000000..502f303 --- /dev/null +++ b/regressions/test-hfile.pl @@ -0,0 +1,194 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Temp qw(tempfile); +use Sys::Guestfs; + +# Defined in src/generator.ml +use constant GUESTFS_MESSAGE_MAX => 4 * 1024 * 1024; + +# Create a new guestfs handle +my $g = Sys::Guestfs->new(); + +# Initialise the guestfs handle with a 10M image file +{ + my $testimg = "test.img"; + unlink($testimg); + + my $fh; + open($fh, '>', $testimg) or die("Open $testimg: $!"); + truncate($fh, 1024*1024*10) or die("truncate $testimg: $!"); + close($fh) or die("close $testimg: $!"); + + $g->add_drive($testimg); + $g->launch(); + + # As qemu is now running we can unlink the image + unlink($testimg) or die("unlink $testimg failed: $!"); +} + +my $string = "abcd"; + +# Check we can't read or write handles we haven't opened +{ + # Read from guestfsd's STDIN + eval { my $in = $g->hread(0, 4); }; + die("read guestfsd's STDIN") unless ($@); + + # Write to guestfsd's STDOUT + eval { $g->hwrite(1, "test"); }; + die("write guestfsd's STDOUT") unless ($@); +} + +{ + # Test read and writing directly to a block device + my $h = $g->hopen_device("/dev/vda", 2); + + $g->hwrite($h, $string); + $g->hseek($h, 0, 0); + my $in = $g->hread($h, length($string)); + die("device: hread returned $in") unless ($in eq $string); + + # Check we can't seek past the end of the file + eval { $g->hseek($h, 1024*1024*11, 0); }; + die("device: seek past EOD") unless ($@); + + # Check we can't write past the end of the file + $g->hseek($h, 0, 2); + eval { $g->hwrite($h, "test"); }; + die("device: write past EOD") unless ($@); + + $g->hclose($h); + + # Check $h is no longer valid + eval { + $g->hseek($h, 0, 0); + }; + die("device: handle wasn't closed") unless($@); +} + +# Make an ext2 filesystem and mount it +$g->mkfs("ext2", "/dev/vda"); +$g->mount("/dev/vda", "/"); + +# Open should fail read-only +eval { + my $h = $g->hopen_file("/test", 0); +}; +die("file: read-only open non-existant file") unless($@); + +# Open should fail write-only without O_CREAT +eval { + my $h = $g->hopen_file("/test", 1); +}; +die("file: write-only open non-existant file") unless($@); + +# Open should fail read-write without O_CREAT +eval { + my $h = $g->hopen_file("/test", 2); +}; +die("file: read-write open non-existant file") unless($@); + +{ + # Open the file write only, and create it + my $h = $g->hopen_file("/test", 1 | 64); + my $in; + + # Write data to it + $g->hwrite($h, $string); + + # Verify we can't read from it + $g->hseek($h, 0, 0); + eval { + $in = $g->hread($h, length($string)); + }; + die("file: read of write-only file") unless($@); + + # close and check the contents with cat + $g->hclose($h); + $in = $g->cat("/test"); + die("file: hwrite data test") unless($in eq $string); +} + +{ + # Open the file read-only + my $h = $g->hopen_file("/test", 0); + + # Read the file in 1 byte chunks until the end + my $in = ""; + my $chunk; + do { + $chunk = $g->hread($h, 1); + $in .= $chunk; + } until(length($chunk) == 0); + die("file: hread in chunks") unless($in eq $string); + + # Read past EOF returns a short read + $g->hseek($h, 0, 0); + $in = $g->hread($h, length($string) + 1); + die("file: read past EOF") unless($in eq $string); + + # Test close-all + $g->hclose_all(); + eval { + $g->hseek($h, 0, 0); + }; + die("file: close-all") unless($@); +} + +{ + # Open the file read-write + my $h = $g->hopen_file("/test", 2); + + # Create a list of the first 2M ints + my @ints; + for (my $i = 0; $i < 1024 * 1024 * 2; $i++) { + push(@ints, $i); + } + + # Write 2M of data (512k ints) + $g->hwrite($h, pack("L".512 * 1024, @ints)); + + # Check the file size + my $size = $g->filesize("/test"); + die("file: 2M write wrote $size") unless($size == 1024 * 1024 * 2); + + # Write another 3M of data (768k ints) for the next test + $g->hwrite($h, pack("L".768 * 1024, @ints[512*1024..(512+768)*1024])); + + # Check the file size is now 5M for good measure + $size = $g->filesize("/test"); + die("file: 2M + 3M append wrote $size") unless($size == 1024 * 1024 * 5); + + my $bytes; + my @data; + + # Check that read 2M works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*2); + @data = unpack("L*", $bytes); + die("file: read 2M") unless(scalar(@data) == 1024 * 512 && + $data[$#data] == $#data); + + # Check that read 4M-1 works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*4-1); + @data = unpack("L*", $bytes); + die("file: read 4M-1") unless($data[$#data] == $#data); + + # Check that read 4M works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*4); + @data = unpack("L*", $bytes); + die("file: read 4M") unless($data[$#data] == $#data); + + # Check that we can read 5M in 2 3M max chunks + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*3-1); # Misalign the data for kicks + $bytes .= $g->hread($h, 1024*1024*3-1); + @data = unpack("L*", $bytes); + die("file: chunked read") unless($#data == (512+768)*1024-1 && + $data[$#data] == $#data); +} diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c1d1ffb..305aa98 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -266 +273 diff --git a/src/generator.ml b/src/generator.ml index c25c871..dc072ef 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -5355,6 +5355,119 @@ filesystem can be found. To find the label of a filesystem, use C<guestfs_vfs_label>."); + ("hopen_file", (RInt "handle", [Pathname "path"; Int "flags"]), 267, [], + [], + "open a file", + "\ +This command opens C<path> in the guest and returns a handle +to it. The returned handle is an opaque int which can be passed +to C<guestfs_h*> calls. + +C<flags> determines how the file is opened. It is a bitwise or of: + +=over 4 + +=item 0 + +(O_RDONLY) Open the file read only. + +=item 1 + +(O_WRONLY) Open the file write only. + +=item 2 + +(O_RDWR) Open the file read-write. + +=item 64 + +(O_CREAT) Create the file if it does not already exist. The file +will be created with a mode of 0777, modified by the daemon's +umask. See L<UMASK>. + +=item 512 + +(O_TRUNC) Truncate a file opened for write to zero length when +opening. + +=item 1024 + +(O_APPEND) Always write to the end of the file. + +=item 262144 + +(O_NOATIME) Don't update the access time when reading from a file. + +=back 4"); + + ("hopen_device", (RInt "handle", [Device "device"; Int "flags"]), 268, [], + [], + "open a block device", + "\ +This command opens a block device and returns a handle to it. +The returned handle is an opaque int which can be passed to +C<guestfs_h*> calls. + +See C<guestfs_hopen_file> for a description of C<flags>."); + + ("hclose", (RErr, [Int "handle"]), 269, [], + [], + "close a file handle", + "\ +This command closes C<handle> and releases all resources associated +with it."); + + ("hclose_all", (RErr, []), 270, [], + [], + "close all open file handles", + "\ +This command closes all file handles which have been opened with +C<guestfs_hopen_*>."); + + ("hread", (RBufferOut "content", [Int "handle"; Int64 "length"]), 271, + [ProtocolLimitWarning], [], + "read data from an open handle", + "\ +This command reads up to C<length> bytes from C<handle> and returns +the data in C<content>. It will attempt to read exactly C<length> +bytes, but may return less. Returned C<content> with size 0 +indicates EOF."); + + ("hwrite", (RErr, [Int "handle"; BufferIn "content"]), 272, + [ProtocolLimitWarning], [], + "write data to an open handle", + "\ +This command writes all the data in C<content> to C<handle>."); + + ("hseek", (RInt64 "newoffset", [Int "handle"; Int64 "offset"; + Int "whence"]), 273, [], + [], + "seek on an open handle", + "\ +This command updates the current position in C<handle>. This +affects the C<guestfs_hread> and C<guestfs_hwrite> calls. + +C<whence> determines the point of reference for C<offset>. It +must be one of: + +=over 4 + +=item 0 + +C<offset> is relative to the beginning of the file. + +=item 1 + +C<offset> is relative to the current position. + +=item 2 + +C<offset> is relative to the end of the file. + +=back 4 + +hseek returns the new offset from the beginning of the file."); + ] let all_functions = non_daemon_functions @ daemon_functions -- 1.7.2.2
This updated patch uses xwrite to write a whole buffer, and uses O_CLOEXEC when opening files and devices.
These APIs provide handle-based read/write streaming and random access to files and block devices. --- daemon/.gitignore | 1 + daemon/Makefile.am | 1 + daemon/hfile.c | 228 +++++++++++++++++++++++++++++++++++++++++++++ daemon/m4/gnulib-cache.m4 | 3 +- po/POTFILES.in | 2 + regressions/Makefile.am | 1 + regressions/test-hfile.pl | 194 ++++++++++++++++++++++++++++++++++++++ src/MAX_PROC_NR | 2 +- src/generator.ml | 113 ++++++++++++++++++++++ 9 files changed, 543 insertions(+), 2 deletions(-) create mode 100644 daemon/hfile.c create mode 100755 regressions/test-hfile.pl diff --git a/daemon/.gitignore b/daemon/.gitignore index e06c684..c9837be 100644 --- a/daemon/.gitignore +++ b/daemon/.gitignore @@ -50,6 +50,7 @@ m4/gettime.m4 m4/gettimeofday.m4 m4/getpagesize.m4 m4/getugroups.m4 +m4/gl_list.m4 m4/glibc21.m4 m4/glob.m4 m4/gnulib-common.m4 diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 0c8be08..c243bb6 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -94,6 +94,7 @@ guestfsd_SOURCES = \ guestfsd.c \ headtail.c \ hexdump.c \ + hfile.c \ htonl.c \ initrd.c \ inotify.c \ diff --git a/daemon/hfile.c b/daemon/hfile.c new file mode 100644 index 0000000..7c424e6 --- /dev/null +++ b/daemon/hfile.c @@ -0,0 +1,228 @@ +/* libguestfs - the guestfsd daemon + * Copyright (C) 2010 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include <config.h> + +#include <errno.h> +#include <fcntl.h> +#include <unistd.h> + +#include "gl_array_list.h" + +#include "daemon.h" +#include "actions.h" + +#ifndef O_CLOEXEC + #define O_CLOEXEC 0 +#endif + +gl_list_t handles; + +static void init_handle_list (void) __attribute__((constructor)); +void +init_handle_list (void) +{ + handles = gl_list_nx_create_empty(GL_ARRAY_LIST, NULL, NULL, NULL, false); +} + +static gl_list_node_t +get_handle (int handle) +{ + gl_list_node_t node = gl_list_search(handles, (void *)(long)handle); + if (NULL == node) { + reply_with_error("%i does not refer to an open handle", handle); + return NULL; + } + + return node; +} + +static int +hopen_check(int fd, const char *path) +{ + if (fd == -1) { + reply_with_perror("open %s failed", path); + return -1; + } + + gl_list_node_t node = gl_list_nx_add_last(handles, (void *)(long)fd); + if (NULL == node) { + close(fd); + + reply_with_error("gl_list_nx_add_last"); + return -1; + } + + return fd; +} + +int /* RInt */ +do_hopen_file (const char *path, int flags) +{ + int fd; + + /* Don't leak descriptors to other child processes */ + flags |= O_CLOEXEC; + + CHROOT_IN; + if (flags && O_CREAT) { + fd = open(path, flags, 0777); + } else { + fd = open(path, flags); + } + CHROOT_OUT; + + return hopen_check(fd, path); +} + +int /* RInt */ +do_hopen_device (const char *device, int flags) +{ + int fd; + + /* Don't leak descriptors to other child processes */ + flags |= O_CLOEXEC; + + fd = open(device, flags); + + return hopen_check(fd, device); +} + +int /* RErr */ +do_hclose (int handle) +{ + gl_list_node_t node = get_handle(handle); + if (NULL == node) { + return -1; + } + + gl_list_remove_node(handles, node); + + if (close(handle) < 0) { + reply_with_perror("close handle %i failed", handle); + return -1; + } + + return 0; +} + +int /* RErr */ +do_hclose_all (void) +{ + const void *eltp; + gl_list_node_t node; + gl_list_iterator_t i = gl_list_iterator(handles); + + char errors[GUESTFS_ERROR_LEN]; + char *error_p = errors; + size_t error_n = sizeof(errors); + + bool failed = false; + while (gl_list_iterator_next(&i, &eltp, &node)) { + gl_list_remove_node(handles, node); + + int fd = (long)eltp; + if (close(fd) < 0) { + failed = true; + + if (error_n != 0) { + size_t len = snprintf(error_p, error_n, + "close handle %i failed: %m ", fd); + if (len < error_n) { + error_p += len; + error_n -= len; + } else { + error_n = 0; + error_p = &errors[GUESTFS_ERROR_LEN]; + } + } + } + } + + if (failed) { + /* Overwrite the trailing space with a NULL terminator */ + *(error_p - 1) = '\0'; + reply_with_error("%s", errors); + return -1; + } + + return 0; +} + +char * /* RBufferOut */ +do_hread (int handle, int64_t size, size_t *size_r) +{ + if (get_handle(handle) == NULL) { + return NULL; + } + +#define HREAD_MESSAGE_MAX (GUESTFS_MESSAGE_MAX - \ + sizeof(guestfs_message_header) - \ + sizeof(guestfs_hread_args)) + if (size > (int64_t) HREAD_MESSAGE_MAX) { + size = HREAD_MESSAGE_MAX; + } +#undef HREAD_MESSAGE_MAX + + char *buf = malloc(size); + if (NULL == buf) { + reply_with_perror("malloc"); + return NULL; + } + + ssize_t in = read(handle, buf, size); + if (in < 0) { + reply_with_perror("error reading from handle %i", handle); + free(buf); + return NULL; + } + + *size_r = in; + return buf; +} + +int /* RErr */ +do_hwrite (int handle, const char *content, size_t content_size) +{ + if (get_handle(handle) == NULL) { + return -1; + } + + if (xwrite(handle, content, content_size) == -1) { + reply_with_perror("error writing to handle %i", handle); + return -1; + } + + return 0; +} + +int64_t /* RInt64 */ +do_hseek (int handle, int64_t offset, int whence) +{ + if (get_handle(handle) == NULL) { + return -1; + } + + int64_t ret = lseek(handle, offset, whence); + if (ret < 0) { + reply_with_perror("error seeking in handle %i", handle); + return -1; + } + + return ret; +} diff --git a/daemon/m4/gnulib-cache.m4 b/daemon/m4/gnulib-cache.m4 index f026cb3..25ffd27 100644 --- a/daemon/m4/gnulib-cache.m4 +++ b/daemon/m4/gnulib-cache.m4 @@ -15,11 +15,12 @@ # Specification in the form of a command-line invocation: -# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings +# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl array-list byteswap c-ctype connect error fsusage futimens getaddrinfo getline glob hash ignore-value manywarnings mkdtemp netdb openat perror pread read-file readlink select sleep socket strchrnul strndup symlinkat sys_select sys_wait vasprintf warnings # Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) gl_MODULES([ + array-list byteswap c-ctype connect diff --git a/po/POTFILES.in b/po/POTFILES.in index e5fb857..c846e05 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -29,6 +29,7 @@ daemon/grub.c daemon/guestfsd.c daemon/headtail.c daemon/hexdump.c +daemon/hfile.c daemon/htonl.c daemon/initrd.c daemon/inotify.c @@ -99,6 +100,7 @@ perl/lib/Sys/Guestfs.pm perl/lib/Sys/Guestfs/Lib.pm python/guestfs-py.c regressions/rhbz501893.c +regressions/test-hfile.pl regressions/test-lvm-mapping.pl regressions/test-noexec-stack.pl ruby/ext/guestfs/_guestfs.c diff --git a/regressions/Makefile.am b/regressions/Makefile.am index ca4292a..f7d9992 100644 --- a/regressions/Makefile.am +++ b/regressions/Makefile.am @@ -35,6 +35,7 @@ TESTS = \ test-cancellation-download-librarycancels.sh \ test-cancellation-upload-daemoncancels.sh \ test-find0.sh \ + test-hfile.pl \ test-luks.sh \ test-lvm-filtering.sh \ test-lvm-mapping.pl \ diff --git a/regressions/test-hfile.pl b/regressions/test-hfile.pl new file mode 100755 index 0000000..502f303 --- /dev/null +++ b/regressions/test-hfile.pl @@ -0,0 +1,194 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Temp qw(tempfile); +use Sys::Guestfs; + +# Defined in src/generator.ml +use constant GUESTFS_MESSAGE_MAX => 4 * 1024 * 1024; + +# Create a new guestfs handle +my $g = Sys::Guestfs->new(); + +# Initialise the guestfs handle with a 10M image file +{ + my $testimg = "test.img"; + unlink($testimg); + + my $fh; + open($fh, '>', $testimg) or die("Open $testimg: $!"); + truncate($fh, 1024*1024*10) or die("truncate $testimg: $!"); + close($fh) or die("close $testimg: $!"); + + $g->add_drive($testimg); + $g->launch(); + + # As qemu is now running we can unlink the image + unlink($testimg) or die("unlink $testimg failed: $!"); +} + +my $string = "abcd"; + +# Check we can't read or write handles we haven't opened +{ + # Read from guestfsd's STDIN + eval { my $in = $g->hread(0, 4); }; + die("read guestfsd's STDIN") unless ($@); + + # Write to guestfsd's STDOUT + eval { $g->hwrite(1, "test"); }; + die("write guestfsd's STDOUT") unless ($@); +} + +{ + # Test read and writing directly to a block device + my $h = $g->hopen_device("/dev/vda", 2); + + $g->hwrite($h, $string); + $g->hseek($h, 0, 0); + my $in = $g->hread($h, length($string)); + die("device: hread returned $in") unless ($in eq $string); + + # Check we can't seek past the end of the file + eval { $g->hseek($h, 1024*1024*11, 0); }; + die("device: seek past EOD") unless ($@); + + # Check we can't write past the end of the file + $g->hseek($h, 0, 2); + eval { $g->hwrite($h, "test"); }; + die("device: write past EOD") unless ($@); + + $g->hclose($h); + + # Check $h is no longer valid + eval { + $g->hseek($h, 0, 0); + }; + die("device: handle wasn't closed") unless($@); +} + +# Make an ext2 filesystem and mount it +$g->mkfs("ext2", "/dev/vda"); +$g->mount("/dev/vda", "/"); + +# Open should fail read-only +eval { + my $h = $g->hopen_file("/test", 0); +}; +die("file: read-only open non-existant file") unless($@); + +# Open should fail write-only without O_CREAT +eval { + my $h = $g->hopen_file("/test", 1); +}; +die("file: write-only open non-existant file") unless($@); + +# Open should fail read-write without O_CREAT +eval { + my $h = $g->hopen_file("/test", 2); +}; +die("file: read-write open non-existant file") unless($@); + +{ + # Open the file write only, and create it + my $h = $g->hopen_file("/test", 1 | 64); + my $in; + + # Write data to it + $g->hwrite($h, $string); + + # Verify we can't read from it + $g->hseek($h, 0, 0); + eval { + $in = $g->hread($h, length($string)); + }; + die("file: read of write-only file") unless($@); + + # close and check the contents with cat + $g->hclose($h); + $in = $g->cat("/test"); + die("file: hwrite data test") unless($in eq $string); +} + +{ + # Open the file read-only + my $h = $g->hopen_file("/test", 0); + + # Read the file in 1 byte chunks until the end + my $in = ""; + my $chunk; + do { + $chunk = $g->hread($h, 1); + $in .= $chunk; + } until(length($chunk) == 0); + die("file: hread in chunks") unless($in eq $string); + + # Read past EOF returns a short read + $g->hseek($h, 0, 0); + $in = $g->hread($h, length($string) + 1); + die("file: read past EOF") unless($in eq $string); + + # Test close-all + $g->hclose_all(); + eval { + $g->hseek($h, 0, 0); + }; + die("file: close-all") unless($@); +} + +{ + # Open the file read-write + my $h = $g->hopen_file("/test", 2); + + # Create a list of the first 2M ints + my @ints; + for (my $i = 0; $i < 1024 * 1024 * 2; $i++) { + push(@ints, $i); + } + + # Write 2M of data (512k ints) + $g->hwrite($h, pack("L".512 * 1024, @ints)); + + # Check the file size + my $size = $g->filesize("/test"); + die("file: 2M write wrote $size") unless($size == 1024 * 1024 * 2); + + # Write another 3M of data (768k ints) for the next test + $g->hwrite($h, pack("L".768 * 1024, @ints[512*1024..(512+768)*1024])); + + # Check the file size is now 5M for good measure + $size = $g->filesize("/test"); + die("file: 2M + 3M append wrote $size") unless($size == 1024 * 1024 * 5); + + my $bytes; + my @data; + + # Check that read 2M works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*2); + @data = unpack("L*", $bytes); + die("file: read 2M") unless(scalar(@data) == 1024 * 512 && + $data[$#data] == $#data); + + # Check that read 4M-1 works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*4-1); + @data = unpack("L*", $bytes); + die("file: read 4M-1") unless($data[$#data] == $#data); + + # Check that read 4M works + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*4); + @data = unpack("L*", $bytes); + die("file: read 4M") unless($data[$#data] == $#data); + + # Check that we can read 5M in 2 3M max chunks + $g->hseek($h, 0, 0); + $bytes = $g->hread($h, 1024*1024*3-1); # Misalign the data for kicks + $bytes .= $g->hread($h, 1024*1024*3-1); + @data = unpack("L*", $bytes); + die("file: chunked read") unless($#data == (512+768)*1024-1 && + $data[$#data] == $#data); +} diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c1d1ffb..305aa98 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -266 +273 diff --git a/src/generator.ml b/src/generator.ml index c25c871..dc072ef 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -5355,6 +5355,119 @@ filesystem can be found. To find the label of a filesystem, use C<guestfs_vfs_label>."); + ("hopen_file", (RInt "handle", [Pathname "path"; Int "flags"]), 267, [], + [], + "open a file", + "\ +This command opens C<path> in the guest and returns a handle +to it. The returned handle is an opaque int which can be passed +to C<guestfs_h*> calls. + +C<flags> determines how the file is opened. It is a bitwise or of: + +=over 4 + +=item 0 + +(O_RDONLY) Open the file read only. + +=item 1 + +(O_WRONLY) Open the file write only. + +=item 2 + +(O_RDWR) Open the file read-write. + +=item 64 + +(O_CREAT) Create the file if it does not already exist. The file +will be created with a mode of 0777, modified by the daemon's +umask. See L<UMASK>. + +=item 512 + +(O_TRUNC) Truncate a file opened for write to zero length when +opening. + +=item 1024 + +(O_APPEND) Always write to the end of the file. + +=item 262144 + +(O_NOATIME) Don't update the access time when reading from a file. + +=back 4"); + + ("hopen_device", (RInt "handle", [Device "device"; Int "flags"]), 268, [], + [], + "open a block device", + "\ +This command opens a block device and returns a handle to it. +The returned handle is an opaque int which can be passed to +C<guestfs_h*> calls. + +See C<guestfs_hopen_file> for a description of C<flags>."); + + ("hclose", (RErr, [Int "handle"]), 269, [], + [], + "close a file handle", + "\ +This command closes C<handle> and releases all resources associated +with it."); + + ("hclose_all", (RErr, []), 270, [], + [], + "close all open file handles", + "\ +This command closes all file handles which have been opened with +C<guestfs_hopen_*>."); + + ("hread", (RBufferOut "content", [Int "handle"; Int64 "length"]), 271, + [ProtocolLimitWarning], [], + "read data from an open handle", + "\ +This command reads up to C<length> bytes from C<handle> and returns +the data in C<content>. It will attempt to read exactly C<length> +bytes, but may return less. Returned C<content> with size 0 +indicates EOF."); + + ("hwrite", (RErr, [Int "handle"; BufferIn "content"]), 272, + [ProtocolLimitWarning], [], + "write data to an open handle", + "\ +This command writes all the data in C<content> to C<handle>."); + + ("hseek", (RInt64 "newoffset", [Int "handle"; Int64 "offset"; + Int "whence"]), 273, [], + [], + "seek on an open handle", + "\ +This command updates the current position in C<handle>. This +affects the C<guestfs_hread> and C<guestfs_hwrite> calls. + +C<whence> determines the point of reference for C<offset>. It +must be one of: + +=over 4 + +=item 0 + +C<offset> is relative to the beginning of the file. + +=item 1 + +C<offset> is relative to the current position. + +=item 2 + +C<offset> is relative to the end of the file. + +=back 4 + +hseek returns the new offset from the beginning of the file."); + ] let all_functions = non_daemon_functions @ daemon_functions -- 1.7.2.2
Matt, Can you rebase these two patches to the git HEAD: https://www.redhat.com/archives/libguestfs/2010-August/msg00156.html "New APIs: hopen-device hopen-file hread hwrite hseek hclose hclose-all" https://www.redhat.com/archives/libguestfs/2010-August/msg00157.html "Update pwrite to write a full buffer" Also attach them (instead of inline) since git am can't apply inline patches, or else something along the way is mangling them. I'm assuming those are the latest versions, but if not, then send the latest versions instead. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Fri, Aug 27, 2010 at 04:27:23PM +0100, Matthew Booth wrote:
+#define HREAD_MESSAGE_MAX (GUESTFS_MESSAGE_MAX - \ + sizeof(guestfs_message_header) - \ + sizeof(guestfs_hread_args)) + if (size > (int64_t) HREAD_MESSAGE_MAX) { + size = HREAD_MESSAGE_MAX; + } +#undef HREAD_MESSAGE_MAX
This upper limit isn't correct because sizeof C structure has no relationship with XDR serialization. Use GUESTFS_MESSAGE_MAX here to prevent huge mallocs, and allow ordinary message serialization to divine the true upper limit. See how pread does it here: http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/file.c;h=476f44569a...
# Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) gl_MODULES([ + array-list
I wonder if using a hash is better? It should be faster for large numbers of handles, although that's an unlikely scenario. But also we already use hash in a few places. However the API of Gnulib hashtables is fearsomely complicated.
--- /dev/null +++ b/regressions/test-hfile.pl
The tests are much better now. Can we test that it works if you have two handles open? Use guestfs_fill_pattern to fill a couple of files with test pattern data and check that reads from each are handled independently.
+ ("hwrite", (RErr, [Int "handle"; BufferIn "content"]), 272, + [ProtocolLimitWarning], [], + "write data to an open handle", + "\ +This command writes all the data in C<content> to C<handle>.");
Although this is fine for files, it ties us to this "full write" behaviour even if we decided to implement pipes, say. In general the API is fine and I'd like to apply it. See my previous email too about problems with the patch format. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
On Fri, Aug 27, 2010 at 04:27:23PM +0100, Matthew Booth wrote:
+umask. See L<UMASK>.
This needs to be: See L<guestfs(3)/UMASK>. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
participants (2)
-
Matthew Booth -
Richard W.M. Jones