9 a.m.
I've attached a patch to generator.ml for the proposed new file apis.
Note that hread, hpread, hwrite and hpwrite are slightly different to
the apis I proposed previously. I've also added hallocate for good measure.
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
9:11 a.m.
On 23/08/10 15:00, Matthew Booth wrote:
I've attached a patch to generator.ml for the proposed new file
apis.
Note that hread, hpread, hwrite and hpwrite are slightly different to
the apis I proposed previously. I've also added hallocate for good measure.
I forgot hclose and hclose_all.
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
10:23 a.m.
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
10:58 a.m.
On 23/08/10 16:23, Richard W.M. Jones wrote:
BTW you need two spaces after full stops.
Out of curiosity is that a technical or a stylistic requirement?
Stylistically, double spaces after a full stop have always seemed
anachronistic to me.
> > +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.
Yes. I think reads and writes are quite different in this respect. You
always know exactly what you want to write, but you may not know what
can be read. If this assumption is wrong, now's the time to disagree.
> > + ("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.
Oops :)
> > + ("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.
Are you saying that a String can be magically turned into an enumerated
type somehow? I was trying to work out how to use 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.
htruncate is required when streaming a file with sparse sections. i.e.
h = hopen_file("/foo");
hwrite(h, "foo");
htruncate(h, hseek(h, 1024, CUR));
Technically you only need this for a sparse section at the end of a
file. You could do it by mixing paths and file handles, but that would
be a bit jarring.
hallocate is similarly there to avoid a jarring mix of paths and file
handles when operating on a single file. I could also add fchmod and
fchown, I guess. Any more?
> > + ("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
Thanks,
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
11:04 a.m.
On Mon, Aug 23, 2010 at 04:58:38PM +0100, Matthew Booth wrote:
On 23/08/10 16:23, Richard W.M. Jones wrote:
>BTW you need two spaces after full stops.
Out of curiosity is that a technical or a stylistic requirement?
Stylistically, double spaces after a full stop have always seemed
anachronistic to me.
It's a stylistic one.
>>> +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.
Yes. I think reads and writes are quite different in this respect.
You always know exactly what you want to write, but you may not know
what can be read. If this assumption is wrong, now's the time to
disagree.
I think we should change pwrite in that case. pwrite only works on
files, and AFAICT there is never a situation where a partial write is
possible for a file, so we should turn pwrite into a full write and
change the documentation accordingly.
>>> + ("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.
Oops :)
>>> + ("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.
Are you saying that a String can be magically turned into an
enumerated type somehow? I was trying to work out how to use an
enumerated type.
Sadly not. However we have a long history of using strings instead of
enumerated types throughout the API. In any case, either 0/1/2 needs
to be documented, or this should be a string. It should not be an
undocumented int.
>>> +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.
htruncate is required when streaming a file with sparse sections. i.e.
h = hopen_file("/foo");
hwrite(h, "foo");
htruncate(h, hseek(h, 1024, CUR));
Technically you only need this for a sparse section at the end of a
file. You could do it by mixing paths and file handles, but that
would be a bit jarring.
I don't think I understand this. If you're writing to a file, you can
truncate it once (before any writing) to set its size:
truncate-size "/output" 100M;
hopen-file "/output";
hwrite ...;
hwrite ...;
hwrite ...;
hclose fd;
Why is htruncate needed during writes?
hallocate is similarly there to avoid a jarring mix of paths and
file handles when operating on a single file. I could also add
fchmod and fchown, I guess. Any more?
My point was we don't need to duplicate all of these.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
11:29 a.m.
On 23/08/10 17:04, Richard W.M. Jones wrote:
On Mon, Aug 23, 2010 at 04:58:38PM +0100, Matthew Booth wrote:
> On 23/08/10 16:23, Richard W.M. Jones wrote:
>> BTW you need two spaces after full stops.
>
> Out of curiosity is that a technical or a stylistic requirement?
> Stylistically, double spaces after a full stop have always seemed
> anachronistic to me.
It's a stylistic one.
Meh. Done anyway.
>>>> +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.
>
> Yes. I think reads and writes are quite different in this respect.
> You always know exactly what you want to write, but you may not know
> what can be read. If this assumption is wrong, now's the time to
> disagree.
I think we should change pwrite in that case. pwrite only works on
files, and AFAICT there is never a situation where a partial write is
possible for a file, so we should turn pwrite into a full write and
change the documentation accordingly.
Sure.
> Are you saying that a String can be magically turned into an
> enumerated type somehow? I was trying to work out how to use an
> enumerated type.
Sadly not. However we have a long history of using strings instead of
enumerated types throughout the API. In any case, either 0/1/2 needs
to be documented, or this should be a string. It should not be an
undocumented int.
Ok.
>>>> +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.
>
> htruncate is required when streaming a file with sparse sections. i.e.
>
> h = hopen_file("/foo");
> hwrite(h, "foo");
> htruncate(h, hseek(h, 1024, CUR));
>
> Technically you only need this for a sparse section at the end of a
> file. You could do it by mixing paths and file handles, but that
> would be a bit jarring.
I don't think I understand this. If you're writing to a file, you can
truncate it once (before any writing) to set its size:
truncate-size "/output" 100M;
hopen-file "/output";
hwrite ...;
hwrite ...;
hwrite ...;
hclose fd;
Why is htruncate needed during writes?
You assume above that you know in advance how large the file you're
writing will be. As I said, you could move that truncate-size to the
end, but mixing paths and handles is nasty and occasionally error-prone.
Consider:
h = hopen_file("/foo");
hwrite ...;
hwrite ...;
mv("/foo", "/bar");
hwrite ...;
hwrite ...;
truncate("/foo", x);
You've now created 2 files rather than 1. If you use htruncate rather
than truncate here it's robust to any changes in paths.
> hallocate is similarly there to avoid a jarring mix of paths and
> file handles when operating on a single file. I could also add
> fchmod and fchown, I guess. Any more?
My point was we don't need to duplicate all of these.
hallocate has the same argument as above: it's a non-path based
alternative. The argument for it is the same as the argument for all the
posix f* calls which take file descriptors instead of path names. There
may be a performance benefit to it, but principally it's harder to make
a class of stupid mistake.
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
11:37 a.m.
On Mon, Aug 23, 2010 at 05:29:28PM +0100, Matthew Booth wrote:
You assume above that you know in advance how large the file
you're
writing will be. As I said, you could move that truncate-size to the
end, but mixing paths and handles is nasty and occasionally
error-prone.
Consider:
h = hopen_file("/foo");
hwrite ...;
hwrite ...;
mv("/foo", "/bar");
hwrite ...;
hwrite ...;
truncate("/foo", x);
You've now created 2 files rather than 1. If you use htruncate
rather than truncate here it's robust to any changes in paths.
OK, so you can poke your own eye out ...
This is all being accessed from a single program. I think you just
should take care not to do that.
Nevertheless we can start without these APIs and add them later if
there is a pressing need. But I thought the original point of handles
was for optimization, and we don't need to optimize something which is
done exactly once.
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
11:34 a.m.
On Mon, Aug 23, 2010 at 05:04:57PM +0100, Richard W.M. Jones wrote:
hopen-file "/output";
On a separate point, do we not need to say whether the file
is being opened for read or write?
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
5186
days inactive
5186
days old
7 comments
2 participants
participants (2)
-
Matthew Booth -
Richard W.M. Jones