[Libguestfs] Proposed new file apis

[Richard W.M. Jones]

On Mon, Aug 23, 2010 at 03:11:27PM +0100, Matthew Booth wrote:
> diff --git a/src/generator.ml b/src/generator.ml
> index e18fa38..1f4d294 100755
> --- a/src/generator.ml
> +++ b/src/generator.ml
> @@ -5336,6 +5336,112 @@ filesystem can be found.
>  
>  To find the label of a filesystem, use C<guestfs_vfs_label>.");
>  
> +  ("open_file", (RInt "handle", [Pathname "path"]), 267, [],

I think this call should be hopen_file.

> +   "open a file",
> +   "\
> +This command opens a file in the guest and returns a handle
> +to it. The returned handle is an opaque int which can be passed
> +to guestfs_h* calls.

You should say:

... passed to C<guestfs_h*> calls.

The generator knows how to convert that to the correct language-
specific syntax, eg. C<$g->h*> for Perl.

BTW you need two spaces after full stops.

> +open_file returns -1 on error.");

You don't need to say this, or anything about errors passim.  The
generator will produce the correct documentation.  In languages other
than C, errors are turned into exceptions, and so it's wrong to say
that -1 is an error in those languages.

Have a look at the documentation that the generator makes, to check it
matches your expectations:

  $ ./src/generator.ml
  $ less src/guestfs.3
  $ less fish/guestfish.1

> +  ("open_device", (RInt "handle", [Device "device"]), 268, [],
> +   [],
> +   "open a block device",
> +   "\
> +This command opens a block device and returns a handle to it.
> +It is able to open any block device with a node under /dev,
> +including LVM logical volumes.

You don't need to say the bit about block devices under /dev.  The use
of Device in the argument type implies that.

> +  ("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
> +open_*.");

You should say:

... opened with C<guestfs_hopen_*>.

> +  ("hread", (RBufferOut "content", [Int "handle"; Int64 "size"]), 271,
> +  [ProtocolLimitWarning], [],
> +   "read data from an open handle",
> +   "\
> +This command reads up to C<size> bytes from C<handle> and returns
> +the data in C<content>. It will attempt to read exactly C<size>
> +bytes, but may return less. Returned C<content> with size 0
> +indicates EOF.
>
> +hread returns a NULL C<content> on error.");
> +
> +  ("hpread", (RBufferOut "content", [Int "handle"; Int64 "size";
> +                                     Int64 "offset"]), 272,
> +  [ProtocolLimitWarning], [],
> +   "read data from an open handle at a specific offset",
> +   "\
> +This command reads up to C<size> bytes from C<handle> starting
> +at C<offset> bytes from the beginning of the file. It returns
> +the data in C<content>. It will attempt to read exactly C<size>
> +bytes, but may return less. Returned C<content> with size 0
> +indicates EOF.
> +
> +hpread returns a NULL C<content> on error.");
> +
> +  ("hwrite", (RErr, [Int "handle"; BufferIn "content"]), 273,
> +   [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.");

So we're definitely disallowing partial writes, even though in some
future version we might allow writes to non-file handles?  This API
won't allow partial writes.

> +  ("hpwrite", (RErr, [Int "handle"; BufferIn "content"]), 274,
> +   [ProtocolLimitWarning], [],
> +   "write data to an open handle at a specific offset",
> +   "\
> +This command writes all the data in C<content> to C<handle>,
> +starting at C<offset> bytes from the beginning of the file. It
> +returns an error if this fails.");

The offset is missing from the API.

> +  ("hseek", (RInt64 "newoffset", [Int "handle"; Int64 "offset";
> +                                  Int "whence"]), 275, [],
> +   [],
> +   "seek on an open handle",
> +   "\
> +This command updates the 'current position' in C<handle>. This

"current position" doesn't need those extra quotes.

> +affects the hread and hwrite calls. See L<lseek(3p)> for details

If you're referring to another call, write:

... C<guestfs_hread> and C<guestfs_hwrite> ...

The generator will rewrite this correctly.

> +of C<offset> and C<whence>.

C<whence> is presumably 0/1/2?  You should document this, or use a
String if you want an enumerated type.

> +On success, hseek returns the new offset from the beginning of
> +the file. It returns an offset of -1 on failure.");
> +
> +  ("htruncate", (RErr, [Int "handle"; Int64 "size"]), 276, [],
> +   [],
> +   "truncate a file",
> +   "\
> +This command sets the size of the file referred to by C<handle>
> +to C<size>. If the file was previously larger than C<size>, the
> +extra data will be lost. If the file was previously smaller than
> +C<size>, the file will be zero-filled. The latter case will
> +typically create a sparse file.");

Since we already have truncate and allocate calls, I wonder what the
benefit is of having handle versions.  I mean, these are extremely
infrequent operations compared to hread/hwrite, so I doubt there's any
optimization benefit in adding these, and if we add these, why not
other very infrequent ops.

> +  ("hallocate", (RErr, [Int "handle"; Int64 "offset"; Int64 "length"]), 277, [],
> +   [],
> +   "reserve space for a file",
> +   "\
> +This command ensures that C<length> bytes of disk space have been
> +allocated, starting at an offset of C<offset> bytes from the
> +beginning of the file, for the file referred to by C<handle>.
> +This will increase the size of the file if necessary. Any newly
> +allocated space will be zero-filled.");
> +
>  ]
>  
>  let all_functions = non_daemon_functions @ daemon_functions

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v