Run this command across the source:
perl -pi.bak -e 's/(20[012][0-9])-20[12][012]/$1-2023/g' `git ls-files`
and remove changes to po{,-docs}/*.po{,t} (these will be regenerated
later when we run 'make dist').
We have traditionally used custom printf formatters %Q and %R, where
%Q replaces the argument with a shell-quoted string, and %R replaces
the argument with a sysroot-prefixed shell-quoted string. They are
actually pretty useful, but unfortunately only supported by glibc.
We only used them in about a dozen places in the daemon (much code
having been replaced by OCaml which does not need them).
In every remaining case we were constructing a command using code like
this:
asprintf_nowarn (&cmd,
"cd %Q && find -print0 | %s -0 -o -H %s --quiet", ...);
We can replace this with:
char *cmd;
size_t cmd_size;
fp = open_memstream (&cmd, &cmd_size);
fprintf (fp, "cd ");
shell_quote (dir, fp);
fprintf (fp, " && find -print0 | %s -0 -o -H %s --quiet", ...);
fclose (fp);
As part of our efforts to clean up and simplify libguestfs, removing
gnulib deletes a large dependency that we mostly no longer use and
causes problems for new users trying to build the library from source.
A few modules from gnulib are still used (under a compatible license)
and these are copied into gnulib/lib/
Linux from around 5.6 now enumerates individual disks in any order
(whereas previously it enumerated only drivers in parallel). This
means that /dev/sdX ordering is no longer stable - in particular we
cannot be sure that /dev/sda inside the guest is the first disk that
was attached to the appliance, /dev/sdb the second disk and so on.
However we can still use SCSI PCI device numbering as found in
/dev/disk/by-path. Use this to translate device names in and out of
the appliance.
Thanks: Vitaly Kuznetsov, Paolo Bonzini, Dan Berrangé.
When the daemon starts up it creates a fresh (empty) LVM configuration
and starts up lvmetad (which depends on the LVM configuration).
However this appears to cause problems: Some types of PV seem to
require lvmetad and don't work without it
(https://bugzilla.redhat.com/show_bug.cgi?id=1581810). If we don't
start lvmetad earlier, the device nodes are not created.
Therefore move the whole initialization step into appliance/init.
Two further changes had to be made:
Now we are using lvmetad all the time, using vgchange is incorrect.
With lvmetad activated early we must use ‘pvscan --cache --activate ay’
to scan all disks for PVs and activate any VGs on them (although the
documentation is complex, confusing and contradictory so I'm not
completely sure about this).
The ‘lvm_system_dir’ local variable in ‘daemon/lvm-filter.c’
previously contained the path of the directory above $LVM_SYSTEM_DIR
(eg. $LVM_SYSTEM_DIR = "/etc/lvm", lvm_system_dir = "/etc"). As this
was highly confusing, I have changed it so the local variable and the
environment variable have identical contents. This involved removing
the ‘lvm/’ component from a couple of paths since it is now included
in the local variable.
This change allows parts of the daemon to be written in the OCaml
programming language. I am using the ‘Main Program in C’ method along
with ‘-output-obj’ to create an object file from the OCaml code /
runtime, as described here:
https://caml.inria.fr/pub/docs/manual-ocaml/intfc.html
Furthermore, change the generator to allow individual APIs to be
implemented in OCaml. This is picked by setting:
impl = OCaml <ocaml_function>;
The generator creates ‘do_function’ (the same one you would have to
write by hand in C), with the function calling the named
‘ocaml_function’ and dealing with marshalling/unmarshalling the OCaml
parameters.
LVM is fine with a completely empty configuration file (meaning "all
defaults"), so start with one instead of copying the system
configuration file.
Also this means we can very easily implement lvm_set_filter
functionality without using Augeas, since we no longer have to worry
about existing filters being present.
Thanks: Alasdair Kergon, Zdenek Kabelac.
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.
After the previous refactoring, we are able to link the daemon to
common/utils, and also remove some of the "duplicate" functions that
the daemon carried ("duplicate" in quotes because they were often not
exact duplicates).
Also this removes the duplicate reimplementation of (most) cleanup
functions in the daemon, since those are provided by libutils now.
It also allows us in future (but not in this commit) to move utility
functions from the daemon into libutils.
Remove much of the text detailing how device name translation
happened. Since we removed support for virtio-blk
(commit 9e0294f88f) and deprecated the
‘iface’ parameter, only /dev/sdX device names should be visible
through the public APIs, both in parameters and in return values from
calls like guestfs_list_devices and guestfs_list_partitions.
Note the above is in fact not true for the UML backend, but UML is
broken in the kernel and in any case this will be fixed later.
(cherry picked from commit 2727e589db216bf0731385966889a4f66dbfe225)
Add udev_settle_file() to run 'udevadm settle' with --exit-if-exists option. It
will slightly reduce the waiting-time for pending events if we need to wait
for events related to a particular device/file.
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
RWMJ:
- Use local variable for MAX_ARGS.
- Use commandv instead of commandrv, fix checking of return code.
It will be useful also for APIs different than tar-out, so move it to
guestfsd.c, and add it a parameter to specify the function name that
invoked it.
This is mostly code motion.
Run the following command over the source:
perl -pi.bak -e 's/(20[01][0-9])-2016/$1-2017/g' `git ls-files`
(Thanks Rich for the perl snippet, as used in past years.)
Currently lvmetad is started in init, and thus using the system
(= appliance) configuration of lvm. Later on, in the daemon, a local
copy of the lvm configuration is setup, and set it for use using the
LVM_SYSTEM_DIR environment variable: this means only the programmes
executed by the daemon will use the local lvm configuration, and not
lvmetad.
Thus manually start lvmetad from the daemon, right after having setup
the local lvm configuration, and still without failing if it cannot be
executed.
Additionally, since lvmetad now respects the right configuration, make
sure to update its cache when rescanning the VGs by passing --cache to
vgscan.
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).
If add_string_nodup fails free the passed string instead of leaking it,
as that string would have been owned by the stringbuf.
Adapt few places to this behaviour.
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`
GCC has two warnings related to large stack frames. We were already
using the -Wframe-larger-than warning, but this reduces the threshold
from 10000 to 5000 bytes.
However that warning only covers the static part of frames (not
alloca). So this change also enables -Wstack-usage=10000 which covers
both the static and dynamic usage (alloca and variable length arrays).
Multiple changes are made throughout the code to reduce frames to fit
within these new limits.
Note that stack allocation of large strings can be a security issue.
For example, we had code like:
size_t len = strlen (fs->windows_systemroot) + 64;
char software[len];
snprintf (software, len, "%s/system32/config/software",
fs->windows_systemroot);
where fs->windows_systemroot is guest controlled. It's not clear what
the effects might be of allowing the guest to allocate potentially
very large stack frames, but at best it allows the guest to cause
libguestfs to segfault. It turns out we are very lucky that
fs->windows_systemroot cannot be set arbitrarily large (see checks in
is_systemroot).
This commit changes those to large heap allocations instead.
It's a bitmask, so unsigned is the right choice. eg. We can more
easily print it using %x.
This patch changes the size of the fd mask and the values of the
COMMAND_FLAG_* constants, but since these are internal definitions
that doesn't matter.
- add a flag to request chroot for the process, which is done only as
very last (before chdir) operation before exec'ing the process in the
child: this avoids using CHROOT_IN & CHROOT_OUT around command*
invocations, and reduces the code spent in chroot mode
- add failure checks for dup2, open, and chdir done in child, not
proceeding to executing the process if they fail
- open /dev/null without O_CLOEXEC, so it stays available for the
exec'ed process, and thus we don't need to provide an own fd for stdin
Followup of commit fd2f175ee7, thanks also
to the notes and hints provided by Mateusz Guzik.
There is GUESTFSD_EXT_CMD defining a string for udevadm (so it is marked
as "used tool" in the appliance), but it is not actually used when
starting udevadm.
There should be no behaviour change.
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.
Instead of parsing /proc/cmdline from the daemon, move all of that
parsing into the init script, and pass the argument via the daemon
command line.
For example, previously the daemon and init script both looked for
guestfs_network=1 in /proc/cmdline. Now the init script still looks
for it, and if found it runs `guestfsd --network'.
