For Device parameters we expect a block device name. However we were
only testing for "/dev/..." and so chardevs (from the appliance) could
be passed here, resulting in strange effects. This adds a function
is_device_parameter which tests for a valid block device name.
For Dev_or_Path parameters much the same, except we can also use the
is_device_parameter function elsewhere in the daemon to distinguish if
we were called with a device or path parameter. Previously we used a
simple test if the path begins with "/dev/...".
Reported by Mathieu Tarral.
GUESTFSD_EXT_CMD was used by OpenSUSE to track which external commands
are run by the daemon and package those commands into the appliance.
It is no longer used by recent SUSE builds, so remove it.
Thanks: Pino Toscano, Olaf Hering.
In every instance where we used the ‘cancel_stmt’ parameter of
these macros:
ABS_PATH
NEED_ROOT
the value was only ever ‘cancel_receive ()’ or empty. We only use
‘cancel_receive’ for FileIn functions, so replace it with a simple
flag for whether the current function is a FileIn function.
This also removes some incorrect comments about macros that cannot be
used with FileIn functions when in fact they can.
Declare most of the stringsbuf as CLEANUP_FREE_STRINGSBUF, so they are
freed completely on stack unwind: use take_stringsbuf() in return
places to take away from the stringsbuf its content, and remove all the
manual calls to free_stringslen (no more needed now).
This requires to not use free_stringslen anymore on failure in the
helper functions of stringsbuf, which now leave the content as-is (might
be still useful even on error).
This allows us to simplify the memory management of stringsbuf's, which
are not properly fully freed, fixing memory leaks in some error paths
(which were not calling free_stringslen).
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`
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.
This is also called implicitly from internal_autosync, ensuring that
exit won't fail because of an open handle.
libguestfs: error: internal_autosync: umount: /sysroot: umount: /sysroot: target is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
A Mountable is passed from the library to the daemon as a string. The daemon
stub parses it into a mountable_t, which it passes to the implementation.
Update all implementations which now take a mountable_t.
Replace selected calls to 'perror (filename)' with:
fprintf (stderr, "syscall: %s: %m\n", filename);
so that more information is available about precisely which syscall
failed.
Note this is *not* reply_with_perror. These messages are only printed
in verbose output, for the benefit of debugging.
guestfsd calls many different tools. Keeping track of all of them is
error prone. This patch introduces a new helper macro to put the command
string into its own ELF section:
GUESTFSD_EXT_CMD(C_variable, command_name);
This syntax makes it still possible to grep for used command names.
The actual usage of the collected list could be like this:
objcopy -j .guestfsd_ext_cmds -O binary daemon/guestfsd /dev/stdout |
tr '\0' '\n' | sort -u
The resulting output will be used to tell mkinitrd which programs to
copy into the initrd.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
RWMJ:
- Move str_vgchange at request of author.
- Fix snprintf call in daemon/debug.c
Use Dev_or_Path instead of String.
Remove the RESOLVE_DEVICE since Dev_or_Path will generate
REQUIRE_ROOT_OR_RESOLVE_DEVICE instead.
RWMJ:
Note a change in semantics: this now requires root. However this is
OK and still works with mkmountpoint and friends because
'is_root_mounted' works even if something is mounted below the root.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Previously a lot of daemon code used three variables (a string list,
'int size' and 'int alloc') to track growable strings buffers. This
commit implements a simple struct containing the same variables, but
using size_t instead of int:
struct stringsbuf {
char **argv;
size_t size;
size_t alloc;
};
Use it like this:
DECLARE_STRINGSBUF (ret);
//...
if (add_string (&ret, str) == -1)
return NULL;
//...
if (end_stringsbuf (&ret) == -1)
return NULL;
return ret.argv;
Reimplement these so they read /proc/mounts instead of trying to parse
the output of the 'mount' external command.
One consequence of this is that these commands now work again for
ntfs-3g filesystems.
The particular issue is that ntfs-3g (or FUSE?) no longer appears
to update /etc/mtab, which meant that umount-all was not unmounting
these partitions. But parsing /proc/mounts is simpler and more
robust in any case.
As a previous, incorrect attempt to fix RHBZ#576879 we tried to
prevent the daemon from sending an error reply if the daemon had
cancelled the transfer. This is wrong: the daemon should send an
error reply in these cases.
A simple test case is this:
guestfish -N fs -m /dev/sda1 upload big-file /
(This fails because the target "/" is a directory, not a file.)
Prior to this commit, libguestfs would hang instead of printing an
error. With this commit, libguestfs prints an error.
What is happening is:
(1) Library is uploading
a file (2) In the middle of the long
upload, daemon detects an error.
Daemon cancels.
(3) Library detects cancel,
sends cancel chunk, then waits
for the error reply from the
daemon. (4) Daemon is supposed to send
an error reply message.
Because step (4) wasn't happening, uploads that failed like this would
hang in the library (waiting for the error message, while the daemon
was waiting for the next request).
This also adds a regression test.
This temporarily breaks the "both ends cancel" case (RHBZ#576879c5).
Therefore the test for that is disabled, and this is fixed in the next
patch in the series.
This partially reverts commit dc706a639e.
Since Fedora util-linux 2.19, the %post script does:
rm -f /etc/mtab
ln -s /proc/mounts /etc/mtab
We are no longer running %post scripts, so this means that /etc/mtab
is a plain file in the appliance. Usual 'mount' still updates it, but
for some reason mount.ntfs does *not* update it in Fedora 15, meaning
that you couldn't mount and then operate on NTFS partitions.
It seems better to always parse /proc/mounts (ie. what the kernel
thinks is mounted) unconditionally, rather than relying on the
capriciousness of the external mount command.
Therefore, parse /proc/mounts instead of /etc/mtab, but add a note
saying that in future we should really be parsing
/proc/self/mountinfo, but that needs a custom parser, and the format
is rather tricky:
http://lxr.linux.no/#linux+v2.6.37/Documentation/filesystems/proc.txt#L1462
We used to maintain a global flag 'root_mounted' which tells us if the
user has mounted something on root (ie. on the sysroot directory).
This flag caused a lot of trouble (eg. RHBZ#599503) because it's hard
to keep the flag updated correctly when the user can do arbitrary
mounts and also use mkmountpoint.
Remove this flag and replace it with a test to see if something is
mounted on *or under* the sysroot. (It has to be *or under* because
of mkmountpoint and friends).
This also replaces a rather convoluted "have we mounted root yet"
check in the mount* APIs with a simpler check to see if the mountpoint
exists and is an ordinary directory.
This reverts commit ad2abf89c3.
Ubuntu still has errors even with the addition of udev_settle
after umount-all. Therefore this was just masking the problem.
During a FileIn command (eg. upload, tar-in) if both sides
experience errors, then both sides could send cancel messages,
the result being lost synchronization.
The reason for the lost synch was because the daemon was ignoring
this case and sending an error message back which the library side
(which had cancelled) was not expecting.
Fix this by checking in the daemon for the case where the library
also cancels during daemon cancellation, and not sending an error
messages.
This also includes an enhanced regression test which checks for this
case.
This extends the original fix in
commit 5922d7084d.
More details can be found here:
https://bugzilla.redhat.com/show_bug.cgi?id=576879#c5
Modify the generator so that it can correctly handle early
cancellation for Pathname|Device|.. parameters. This fixes
the upload command, but consequently we need to fix the
parameters for tar_in and t?z_in commands. This should also
mean that 'win:' can now be used as the second argument of
tar_in and t?z_in commands in guestfish, whereas previously
this wouldn't have worked.
Adds a regression test for the original problem.
The RPC stubs already prefix the command name to error messages.
The daemon doesn't have to do this. As a (small) benefit this also
makes the daemon slightly smaller.
Code in the daemon such as:
if (argv[0] == NULL) {
reply_with_error ("passed an empty list");
return NULL;
}
now results in error messages like this:
><fs> command ""
libguestfs: error: command: passed an empty list
(whereas previously you would have seen ..command: command:..)
Nearly every file-related function in daemons/*.c is affected:
Remove this pair of statements from each affected do_* function:
- NEED_ROOT (return -1);
- ABS_PATH (dir, return -1);
and change the type of the corresponding parameter to "const char *".
* src/generator.ml: Emit NEED_ROOT just once, even when there are two or
more Pathname args.
