The existing code had a bug which you can demonstrate by doing:
$ guestfish -N fs:btrfs:10G -m /dev/sda1 \
btrfs-subvolume-create /sub :
btrfs-subvolume-snapshot /sub /snap1 : \
btrfs-subvolume-snapshot /sub /snap123 : \
btrfs-subvolume-snapshot /sub /snap123456 : \
btrfs-subvolume-show /sub
...
libguestfs: error: appliance closed the connection unexpectedly.
This usually means the libguestfs appliance crashed.
As the code for parsing the output and creating the comma-separated
list of snapshots was unncessarily complicated in the first place,
simplify it. This also fixes the bug.
This also adds a regression test.
Thanks: Arye Yurkovsky
Link: https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/thread/QV5VDHIH7WRUNAE54K6OEOKJMWL6M7EM/
The old btrfs-fsck API used "btrfs check" which appears to be broken
or deprecated. The real tool you should use is "btrfs scrub". We
have already implemented that API, but it is very awkward to use from
libguestfs. In particular there's no existing way to run the scrub
and wait for it to finish.
Fix this by deprecating btrfs-fsck. Implement a new API
btrfs-scrub-full which runs btrfs scrub in the foreground, waits for
it to finish, and handles errors. It's much more like fsck tools in
other filesystems.
Thanks: Eric Sandeen
Fixes: https://issues.redhat.com/browse/RHEL-91936
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').
This was deprecated in btrfs 4.14.1 and recently removed (see
btrfs-progs commit 4bd94dba8a "btrfs-progs: mkfs: remove alloc start
options and docs"). If the option is set simply ignore it.
Remove two wrong error handling situations in btrfs_balance_status:
- if the calloc for 'ret' fails, the 'goto error' would try to
dereference the resulting null pointer (to free the items of the
return struct); hence, just return null directly, instead of jumping
to 'error'
- if the strdup() for 'btrfsbalance_status' fails, then the directly
return would leak 'ret'
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.
An old version of btrfs-progs (3.17 in this case) has a different
version string, so adapt our filtering to include this line as well.
Related to/updates commit 839ae5bcd5.
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.
Versions older than 4.3.1 output also the version string after the
output. Ignore such line, since it isn't relevant to what
btrfs_filesystem_show needs.
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.
I enabled the -Wshadow warning temporarily in order to do these fixes,
but had to disable it again afterwards. The reason is that this warns
about shadowing globals, which is sort of a good thing, but because we
have a global called "verbose" just about everywhere, and at the same
time we baked a function argument called "verbose" into several
unchangable APIs, well, we're stuck without being able to use this
warning.
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.
Modify the function from a fixed delimiter to a variable. So,
it can be used in more APIs later. Also modified the existing
callers
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
If we are not interested in output of a command being run, then there's
no need to still capture it in a (automatically freed, but still)
variable.
If any of those outputs is needed, a variable for it can be easily added
back.
