Since commit 65cfecb0f5,
‘guestfs_int_download_to_tmp’ was buggy because it did not deal with
the case where a guest had multiple roots. It cached the downloaded
file under a single name which was not distinguished by which root we
were looking at. As a result, if you inspected (eg.) the icon on such
a guest, then in some circumstances the same icon could be returned
for all roots (incorrectly).
This changes the function in a few ways:
- Don't cache downloaded files. It wasn't a useful feature of the
function in most scenarios. Instead a new name is generated for
every call.
- Use guestfs_int_make_temp_path.
- Allow an extension to be specified.
In the function ‘guestfs_int_make_temp_path’, allow an optional
extension to be specified. For example:
r = guestfs_int_make_temp_path (g, "ls", "txt");
will create paths like ‘<tmpdir>/ls123.txt’.
NULL can also be passed for the extension, which means no extension.
Move the last remaining function ‘guestfs_int_download_to_tmp’ to
lib/inspect-icon.c (the main, but not only user of this function).
Then remove lib/inspect.c entirely.
This is not quite code motion because I updated the comment for the
function to reflect what it does in reality.
We permit the following constructs in libguestfs code:
if (guestfs_some_call (g) == -1) {
fprintf (stderr, "failed: error is %s\n", guestfs_last_error (g));
}
and:
guestfs_push_error_handler (g, NULL, NULL);
guestfs_some_call (g);
guestfs_pop_error_handler (g);
Neither of these would be safe if we allowed the handle to be used
from threads concurrently, since the error string or error handler
could be changed by another thread.
Solve this in approximately the same way that libvirt does: by making
the error, current error handler, and stack of error handlers use
thread-local storage (TLS).
The implementation is not entirely straightforward, mainly because
POSIX doesn't give us useful destructor behaviour, so effectively we
end up creating our own destructor using a linked list.
Note that you have to set the error handler in each thread separately,
which is an API change (eg: if you set the error handler in one
thread, then pass the handle 'g' to another thread, the error handler
in the second thread appears to have reset itself back to the default
error handler). I haven't yet worked out a better way to solve this.
Acquire the per-handle lock on entering each public API function.
The lock is released by a cleanup handler, so we only need to use the
ACQUIRE_LOCK_FOR_CURRENT_SCOPE macro at the top of each function.
Note this means we require __attribute__((cleanup)). On platforms
where this is not supported, the code will probably hang whenever a
libguestfs function is called.
The only definitive list of public APIs is found indirectly in the
generator (in generator/c.ml : globals).
After we move inspection code to the daemon, the library will no
longer have access to ‘struct inspect_fs’, and so we won't be able to
prefix downloads with the "root filesystem number".
Just remove this prefix (it's internal only). However it does mean
that this function can no longer cache downloaded files.
hivex has a function hivex_value_string. We were not calling it under
the mistaken belief that because hivex implements this using iconv,
the function wouldn't work inside the daemon. Instead we
reimplemented the functionality in the library.
This commit deprecates hivex_value_utf8 and removes the library side
code. It replaces it with a plain wrapper around hivex_value_string.
Thanks: Pino Toscano
QEMU >= 2.10 started to do mandatory locking. This checks the QMP
schema to see if we are using that version of qemu. (Note it is
sometimes disabled in downstream builds, and it was also enabled in
upstream prereleases with version 2.9.9x, so we cannot just check the
version number).
Rename the cache files like ‘qemu.stat’ etc so they include the qemu
binary "key" (ie. size and mtime) in the name. This allows a single
user to use multiple qemu binaries in parallel without conflicts.
This adds an extra test using QMP (the QEMU Monitor Protocol). This
allows us to get extra information about the qemu binary beyond what
is available from the version number or ‘qemu -help’.
The previous code duplicated a lot of common code for reading and
writing the cache file per data field. This change simply factors out
that common code. This makes it simpler to add new tests in future.
This is just refactoring, it should have no effect.
Rather unnecessarily this function returned the parsed qemu version.
This complicates further refactoring, so I have changed the function
not to return this, and instead there is a separate function you have
to call to get the version struct (‘guestfs_int_qemu_version’).
Apart from a tiny amount of extra copying this is simply refactoring
of the interface between the direct-mode backend and the qemu query
functions.
Unlike ordinary guestfs_int_cmd_run, the pipe version did not print
the command it was about to run when debugging was enabled.
Fixes commit e98bb86929.
Move the list_filesystems API into the daemon, reimplementing it in
OCaml. Since this API makes many other API calls, it runs a lot
faster in the daemon.
The previously library-side ‘file_architecture’ API is reimplemented
in the daemon, in OCaml.
There are some significant differences compared to the C
implementation:
- The C code used libmagic. That is replaced by calling the ‘file’
command (because that is simpler than using the library).
- The C code had extra cases to deal with compressed files. This is
not necessary since the ‘file’ command supports the ‘-z’ option
which transparently looks inside compressed content (this is a
consequence of the change above).
This commit demonstrates a number of techniques which will be useful
for moving inspection code to the daemon:
- Moving an API from the C library to the OCaml daemon.
- Calling from one OCaml API inside the daemon to another (from
‘Filearch.file_architecture’ to ‘File.file’). This can be done and
is done with C daemon APIs but correct reply_with_error handling is
more difficult in C.
- Use of Str for regular expression matching within the appliance.
We were dropping the add_drive copyonread flag when using the libvirt
backend. This resulted in significant performance degradation (2x-3x
slower) when running virt-v2v against VMware servers.
Thanks: Kun Wei.
This feature allows you to use different image formats for the fixed
appliance. The raw format is used by default.
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
If SIGTERM is blocked in the main program, then it ends up still being
blocked in the subprocess after we fork. This means that we cannot
kill qemu by sending SIGTERM to it. This commit fixes the problem by
unblocking SIGTERM unconditionally after fork.
Thanks: wtfuzz on IRC for reporting and analysis.
If you use the libguestfs tools which open disk images read-only
(eg. virt-df), with formats such as 'vdi', then you will see an error:
error: invalid value for backingformat parameter 'vdi'
This is because opening a disk image read-only will try to create a
qcow2 file with the original image as a backing file. However the
list of permitted backing formats was very restrictive and did not
include 'vdi' (nor many other uncommon formats).
Instead of using a whitelist for backing formats, just validate that
the string is alphanumeric and short.
Thanks: Mike Goodwin for reporting the bug.
PCI devices don't exist/work. You would see errors such as:
qemu-system-s390x: -device virtio-rng-pci,rng=rng0: MSI-X support is mandatory in the S390 architecture
In its current form this is very hard to implement because it requires
us to "unparse" the options, including removing any shell quoting.
It wasn't implemented at all for the libvirt backend.
Also contrary to the documentation, the configure script did not use
these options for testing, but constructed its own set of qemu test
options.
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)
In particular the virt-rescue --scratch option makes it very easy to
add huge numbers of drives. Since the per-backend max_disks limit was
never checked anywhere you could get peculiar failures. Now you'll
get a clear error message:
$ virt-rescue --scratch=256
libguestfs: error: too many drives have been added, the current backend only supports 255 drives
This patch changes appliance search using paths with multiple directories. Now
all appliance checks will be done separately for each directory. For example
if the path LIBGUESTFS_PATH=/a:/b:/c, then all applainces are searched first in
/a, then in /b and then in /c. It allows to flexibly configure the libguestfs
to interact with different appliances.
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
