Improve the error messages produced by C-based tools in case of issues
with the command line options:
- explicitly mention to use -a/-d (and -A/-D in virt-diff)
- when extra arguments are found, mention the correct way to pass
options to certain command line switches (like --format)
- in virt-inspector, give a cleaner error message when neither -i nor
any -m is specified
In all the cases, keep the extra notice to use 'TOOL --help' to get more
help with it.
Like with the previous commit, this replaces instances of:
if (something_bad) {
fprintf (stderr, "%s: error message\n", guestfs_int_program_name);
exit (EXIT_FAILURE);
}
with:
if (something_bad)
error (EXIT_FAILURE, 0, "error message");
(except in a few cases were errno was incorrectly being ignored, in
which case I have fixed that).
It's slightly more complex than the previous commit because we must be
careful to:
- Remove the program name (since error(3) prints it).
- Remove any trailing \n character from the message.
Candidates for replacement were found using:
pcregrep --buffer-size 10M -M '\bfprintf\b.*\n.*\bexit\b' `git ls-files`
Wherever we had code which did:
if (something_bad) {
perror (...);
exit (EXIT_FAILURE);
}
replace this with use of the error(3) function:
if (something_bad)
error (EXIT_FAILURE, errno, ...);
The error(3) function is supplied by glibc, or by gnulib on platforms
which don't have it, and is much more flexible than perror(3). Since
we already use error(3), there seems to be no downside to mandating it
everywhere.
Note there is one nasty catch with error(3): error (EXIT_SUCCESS, ...)
does *not* exit! This is also the reason why error(3) cannot be
marked as __attribute__((noreturn)).
Because the examples can't use gnulib, I did not change them.
To search for multiline patterns of the above form, pcregrep -M turns
out to be very useful:
pcregrep --buffer-size 10M -M '\bperror\b.*\n.*\bexit\b' `git ls-files`
Remove man pages and other pages which 'make clean' did not remove
before.
To evaluate which pages could be removed, I did a full build and
check, and then ran 'make clean' followed by 'git clean -xdf'. By
examining the output of the git clean command I could see which files
were being missed.
Files that are _not_ removed by make clean or make distclean:
- generator-built files
- Makefile, Makefile.in, .deps, .depend
- any ./configure output files (maybe they should be?)
Move the random set of HTML files we build from html/ into
the website/ directory.
Also in the website/ directory, put the index.html file from
http://libguestfs.org, which was previously not under version control.
It is generated from index.html.in so we can automatically add the
current version and release date.
Also in the website/ directory, put various CSS file, images, etc.
which are required by the website and were also previously not under
version control.
Change the 'make website' rule to 'make maintainer-upload-website'.
As the name suggests, it is only useful for the maintainer, and will
fail with an error for anyone else.
Create a new top-level directory called test-data, which will carry
all the test data which is large and/or shared between multiple tests.
There are actually several new subdirectories created:
test-data/binaries: The pre-built binary and library files for random
architectures that we use to test various architecture detection
features (was part of tests/data).
test-data/blank-disks: The blank disks which are used for disk format
detection (was part of tests/data).
test-data/files: Other miscellaneous test files from tests/data that
are not included in the above.
test-data/phony-guests: The phony guests (was tests/guests).
test-data: The top-level directory builds the 'test.iso' image file
that is used for testing the C API and in miscellaneous other tests.
Because of previous automated commits, such as changing 'guestfs___'
-> 'guestfs_int_', several function calls no longer lined up with
their parameters, and some lines were too long.
The bulk of this commit was done using emacs batch mode and the
technique described here:
http://www.cslab.pepperdine.edu/warford/BatchIndentationEmacs.html
The changes suggested by emacs were then reviewed by hand.
Updating gnulib has caused -Wformat-signedness to be enabled. This
has revealed many problems in C format strings. The fixes here fall
into the following main categories:
- Using %d with an unsigned parameter.
- %x and %o expect an unsigned argument.
- uid_t and gid_t are unsigned on Linux. The safe way to print these
is to cast them to uintmax_t and then print them using the %ju
modifier (see http://stackoverflow.com/a/1401581).
- Using %d to print an enum. Since enums may be either char or int,
I fixed this by casting the enum to int.
- strtol_error & lzma_ret are both unsigned types.
Done using a sequence of regular expressions like this:
perl -pi.bak -e 's{C</}{F</}g' `git ls-files \*.pod` generator/actions.ml
perl -pi.bak -e 's{C<C:\\}{F<C:\\}g' `git ls-files \*.pod` generator/actions.ml
[etc]
and then tediously checking every change by hand.
libguestfs has used double and triple underscores in identifiers.
These aren't valid for global names in C++.
The first step is to replace all guestfs___* (3 underscores) with
guestfs_int_*. We've used guestfs_int_* elsewhere already as a prefix
for internal identifiers.
This is an entirely mechanical change done using:
git ls-files | xargs perl -pi.bak -e 's/guestfs___/guestfs_int_/g'
Reference: http://stackoverflow.com/a/228797
The gnulib 'error' module uses 'program_name'. On some platforms --
but not Linux / glibc -- it references it as:
extern char *program_name;
This means when you compile libguestfs on non-glibc (eg. Mac OS X)
gnulib requires 'program_name' as an external string reference, which
we don't provide.
This change doesn't define this string reference for gnulib, but it
does change the name of the macro we use to avoid conflicts if we
eventually need to export 'program_name' as a string.
Thanks: Margaret Lewicka
Just like --long-options, it makes it possible to know which short
options are supported by each tool; this can help improving the bash
completion, for example.
The existing APIs guestfs_stat, guestfs_lstat and guestfs_lstatlist
return a stat structure that contains atime, mtime and ctime fields
that store only the timestamp in seconds.
Modern filesystems can store timestamps down to nanosecond
granularity, and the ordinary glibc stat(2) wrapper will return these
in "hidden" stat fields:
struct timespec st_atim; /* Time of last access. */
struct timespec st_mtim; /* Time of last modification. */
struct timespec st_ctim; /* Time of last status change. */
with the following macros defined for backwards compatibility:
#define st_atime st_atim.tv_sec
#define st_mtime st_mtim.tv_sec
#define st_ctime st_ctim.tv_sec
It is not possible to redefine guestfs_stat to return a longer struct
guestfs_stat with room for the extra nanosecond fields, because that
would break the ABI of guestfs_lstatlist as it returns an array
containing consecutive stat structs (not pointers). Changing the
return type of guestfs_stat would break API. Changing the generator
to support symbol versioning is judged to be too intrusive.
Therefore this adds a new struct (guestfs_statns) and new APIs:
guestfs_statns
guestfs_lstatns
guestfs_lstatnslist
which return the new struct (or array of structs in the last case).
The old APIs may of course still be used, forever, but are deprecated
and shouldn't be used in new programs.
Because virt tools are compiled with -DGUESTFS_WARN_DEPRECATED=1, I
have updated all the places calling the deprecated functions. This
has revealed some areas for improvement: in particular virt-diff and
virt-ls could be changed to print the nanosecond fields.
FUSE now returns nanoseconds in stat calls where available, fixing
https://bugzilla.redhat.com/show_bug.cgi?id=1144891
Notes about the implementation:
- guestfs_internal_lstatlist has been removed and replaced by
guestfs_internal_lstatnslist. As the former was an internal API no
one should have been calling it, or indeed can call it unless they
start defining their own header files.
- guestfs_stat and guestfs_lstat have been changed into library-side
functions. They, along with guestfs_lstatlist, are now implemented
as wrappers around the new functions which just throw away the
nanosecond fields.
All tests run under the ./run binary. For a long time the ./run
binary has set the $PATH environment variable to contain all of the
directories with binaries in them.
Therefore there is no reason to use ../fish/guestfish instead of just
plain guestfish (and the same applies to other built binaries).
In most C tools, virt-sysprep and virt-customize, you have to put the
--format parameter before the corresponding -a parameter. ie. The
following is correct:
guestfish --format qcow2 -a disk1 -a disk2
But the following is incorrect. The --format parameter is dangling
and prior to this commit would have been silently ignored:
guestfish -a disk1 -a disk2 --format qcow2
After this change, dangling --format parameters now lead to an error:
guestfish: --format parameter must appear before -a parameter
In virt-customize, also check that --attach-format parameter appears
before --attach parameter.
Thanks: Lingfei Kong
This changes podwrapper so that the input (POD) files should not
contain an =encoding directive. However they must be UTF-8.
Podwrapper then adds the '=encoding utf8' directive back during final
generation.
This in particular avoids problems with nested =encoding directives in
fragments. These break POD, and are undesirable anyway.
