February 2018 - Libguestfs - libguestfs List Archives

[PATCH v2] launch: add support for autodetection of appliance image format
by Pavel Butsykin 06 Feb '20

06 Feb '20

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(a)virtuozzo.com> --- lib/launch-direct.c | 2 ++ lib/launch-libvirt.c | 19 ++++++++++++------- m4/guestfs_appliance.m4 | 11 +++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/launch-direct.c b/lib/launch-direct.c index 0be662e25..b9b54857a 100644 --- a/lib/launch-direct.c +++ b/lib/launch-direct.c @@ -592,7 +592,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) append_list ("id=appliance"); append_list ("cache=unsafe"); append_list ("if=none"); +#ifndef APPLIANCE_FMT_AUTO append_list ("format=raw"); +#endif } end_list (); start_list ("-device") { append_list ("scsi-hd"); diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c index 4adb2cfb3..030ea6911 100644 --- a/lib/launch-libvirt.c +++ b/lib/launch-libvirt.c @@ -212,9 +212,10 @@ get_source_format_or_autodetect (guestfs_h *g, struct drive *drv) /** * Create a qcow2 format overlay, with the given C<backing_drive> - * (file). The C<format> parameter, which must be non-NULL, is the - * backing file format. This is used to create the appliance overlay, - * and also for read-only drives. + * (file). The C<format> parameter is the backing file format. + * The C<format> parameter can be NULL, in this case the backing + * format will be determined automatically. This is used to create + * the appliance overlay, and also for read-only drives. */ static char * make_qcow2_overlay (guestfs_h *g, const char *backing_drive, @@ -223,8 +224,6 @@ make_qcow2_overlay (guestfs_h *g, const char *backing_drive, char *overlay; struct guestfs_disk_create_argv optargs; - assert (format != NULL); - if (guestfs_int_lazy_make_tmpdir (g) == -1) return NULL; @@ -232,8 +231,10 @@ make_qcow2_overlay (guestfs_h *g, const char *backing_drive, optargs.bitmask = GUESTFS_DISK_CREATE_BACKINGFILE_BITMASK; optargs.backingfile = backing_drive; - optargs.bitmask |= GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK; - optargs.backingformat = format; + if (format) { + optargs.bitmask |= GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK; + optargs.backingformat = format; + } if (guestfs_disk_create_argv (g, overlay, "qcow2", -1, &optargs) == -1) { free (overlay); @@ -461,7 +462,11 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) /* Note that appliance can be NULL if using the old-style appliance. */ if (appliance) { +#ifdef APPLIANCE_FMT_AUTO + params.appliance_overlay = make_qcow2_overlay (g, appliance, NULL); +#else params.appliance_overlay = make_qcow2_overlay (g, appliance, "raw"); +#endif if (!params.appliance_overlay) goto cleanup; } diff --git a/m4/guestfs_appliance.m4 b/m4/guestfs_appliance.m4 index 81c43879f..4e1ec8135 100644 --- a/m4/guestfs_appliance.m4 +++ b/m4/guestfs_appliance.m4 @@ -139,3 +139,14 @@ AC_SUBST([GUESTFS_DEFAULT_PATH]) AC_DEFINE_UNQUOTED([GUESTFS_DEFAULT_PATH], ["$GUESTFS_DEFAULT_PATH"], [Define guestfs default path.]) + +AC_ARG_ENABLE([appliance-fmt-auto], + [AS_HELP_STRING([--enable-appliance-fmt-auto], + [enable autodetection of appliance image format @<:@default=no@:>@])], + [ENABLE_APPLIANCE_FMT_AUTO="$enableval"], + [ENABLE_APPLIANCE_FMT_AUTO=no]) + +if test "x$ENABLE_APPLIANCE_FMT_AUTO" = "xyes"; then + AC_DEFINE([APPLIANCE_FMT_AUTO], [1], + [Define to 1 if enabled autodetection of appliance image format.]) +fi -- 2.13.0

2 4

1.39 proposal: Let's split up the libguestfs git repo and tarballs
by Richard W.M. Jones 29 Nov '19

29 Nov '19

My contention is that the libguestfs git repository is too large and unwieldy. There are too many separate, unrelated projects and as a result of that the source has too many dependencies and takes too long to build and test. The project divides (sort of) naturally into layers -- the library, the bindings, the various virt tools -- and could be split along those lines into separate projects which can then be released and evolve at their own pace. My suggested split would be something like this: * libguestfs: The library, daemon and appliance. That would include the following directories in a single project: appliance bash contrib daemon docs examples gnulib lib logo test-tool tmp utils website * 1 project for each language binding: csharp erlang gobject golang haskell java lua ocaml php perl python ruby * virt-customize and related tools, we'd probably call this subproject "virt-builder". It would include virt-builder, virt-customize and virt-sysprep, since they share a lot of common code. * 1 project for each of the following items: small tools written in C (virt-cat, virt-filesystems, virt-log, virt-ls, virt-tail, virt-diff, virt-edit, virt-format, guestmount, virt-inspector, virt-make-fs, virt-rescue) guestfish virt-alignment-scan and virt-df virt-dib virt-get-kernel virt-resize virt-sparsify virt-v2v and virt-p2v virt-win-reg * I'd be inclined to drop the legacy Perl tools virt-tar, virt-list-filesystems, virt-list-partitions unless someone especially wished to step forward to maintain them. * common code and generator: Off to the side we'd somehow need to package up the common code and the generator for use by all of the above projects. It wouldn't be a separate project for downstream packagers, but instead the code would be included (ie. duplicated) in tarballs and upstream available as a side git repo that you'd need to include when building (git submodule?). This is somewhat unspecified. M4, PO, and tests would be split between the projects as appropriate. My proposal would be to do this incrementally, rather than all at once, moving the easier things out first. Thoughts? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

4 18

[nbdkit PATCH v2] plugin: add and use nbdkit_realpath
by Pino Toscano 06 Apr '18

06 Apr '18

Introduce a new helper function to resolve a path name, calling nbdkit_error on failure: other than doing what nbdkit_absolute_path does, it also checks that the file exists (and thus avoids errors later on). To help distinguish it from nbdkit_absolute_path, improve the documentation of the latter. Apply it where an existing path is required, both in nbdkit itself and in plugins. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334 --- docs/nbdkit-plugin.pod | 18 +++++++++++++++++- include/nbdkit-common.h | 1 + plugins/example2/example2.c | 2 +- plugins/file/file.c | 6 +----- plugins/gzip/gzip.c | 2 +- plugins/split/split.c | 2 +- plugins/vddk/vddk.c | 2 +- plugins/xz/xz.c | 2 +- src/plugins.c | 2 +- src/utils.c | 19 +++++++++++++++++++ 10 files changed, 44 insertions(+), 12 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 44822fc..5faba1d 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -200,13 +200,29 @@ descriptor. char *nbdkit_absolute_path (const char *filename); The utility function C<nbdkit_absolute_path> converts any path to an -absolute path. +absolute path: if it is relative, then all this function does is +prepending the current working directory to the path, with no extra +checks. If conversion was not possible, this calls C<nbdkit_error> and returns C<NULL>. Note that this function does not check that the file exists. The returned string must be freed by the caller. +=head2 C<nbdkit_realpath> + + char *nbdkit_realpath (const char *filename); + +The utility function C<nbdkit_realpath> converts any path to an +absolute path, resolving symlinks. Under the hood it uses the +C<realpath> function, and thus it fails if the path does not exist, +or it is not possible to access to any of the components of the path. + +If the path resolution was not possible, this calls C<nbdkit_error> +and returns C<NULL>. + +The returned string must be freed by the caller. + =head1 CALLBACKS =head2 C<.name> diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h index 5e69579..693213f 100644 --- a/include/nbdkit-common.h +++ b/include/nbdkit-common.h @@ -60,6 +60,7 @@ extern void nbdkit_vdebug (const char *msg, va_list args); extern char *nbdkit_absolute_path (const char *path); extern int64_t nbdkit_parse_size (const char *str); extern int nbdkit_read_password (const char *value, char **password); +extern char *nbdkit_realpath (const char *path); #ifdef __cplusplus } diff --git a/plugins/example2/example2.c b/plugins/example2/example2.c index 5bc4f94..a2d6fca 100644 --- a/plugins/example2/example2.c +++ b/plugins/example2/example2.c @@ -78,7 +78,7 @@ example2_config (const char *key, const char *value) { if (strcmp (key, "file") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3). */ - filename = nbdkit_absolute_path (value); + filename = nbdkit_realpath (value); if (!filename) return -1; } diff --git a/plugins/file/file.c b/plugins/file/file.c index f8cb3d3..b6e33de 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -65,7 +65,7 @@ file_config (const char *key, const char *value) if (strcmp (key, "file") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3). */ free (filename); - filename = nbdkit_absolute_path (value); + filename = nbdkit_realpath (value); if (!filename) return -1; } @@ -90,10 +90,6 @@ file_config_complete (void) nbdkit_error ("you must supply the file=<FILENAME> parameter after the plugin name on the command line"); return -1; } - if (access (filename, F_OK) < 0) { - nbdkit_error ("access '%s': %m", filename); - return -1; - } return 0; } diff --git a/plugins/gzip/gzip.c b/plugins/gzip/gzip.c index e9dbfdb..09dd629 100644 --- a/plugins/gzip/gzip.c +++ b/plugins/gzip/gzip.c @@ -62,7 +62,7 @@ gzip_config (const char *key, const char *value) { if (strcmp (key, "file") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3). */ - filename = nbdkit_absolute_path (value); + filename = nbdkit_realpath (value); if (!filename) return -1; } diff --git a/plugins/split/split.c b/plugins/split/split.c index 47c366d..bdcdcf7 100644 --- a/plugins/split/split.c +++ b/plugins/split/split.c @@ -76,7 +76,7 @@ split_config (const char *key, const char *value) return -1; } filenames = new_filenames; - filenames[nr_files] = nbdkit_absolute_path (value); + filenames[nr_files] = nbdkit_realpath (value); if (filenames[nr_files] == NULL) return -1; nr_files++; diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c index 1c15127..8bc1517 100644 --- a/plugins/vddk/vddk.c +++ b/plugins/vddk/vddk.c @@ -153,7 +153,7 @@ vddk_config (const char *key, const char *value) if (strcmp (key, "config") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3). */ free (config); - config = nbdkit_absolute_path (value); + config = nbdkit_realpath (value); if (!config) return -1; } diff --git a/plugins/xz/xz.c b/plugins/xz/xz.c index 437f798..f45e489 100644 --- a/plugins/xz/xz.c +++ b/plugins/xz/xz.c @@ -67,7 +67,7 @@ xz_config (const char *key, const char *value) { if (strcmp (key, "file") == 0) { /* See FILENAMES AND PATHS in nbdkit-plugin(3). */ - filename = nbdkit_absolute_path (value); + filename = nbdkit_realpath (value); if (!filename) return -1; } diff --git a/src/plugins.c b/src/plugins.c index dba3e24..595b632 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -134,7 +134,7 @@ plugin_dump_fields (struct backend *b) struct backend_plugin *p = container_of (b, struct backend_plugin, backend); char *path; - path = nbdkit_absolute_path (p->filename); + path = nbdkit_realpath (p->filename); printf ("path=%s\n", path); free (path); diff --git a/src/utils.c b/src/utils.c index 0083370..c6c8003 100644 --- a/src/utils.c +++ b/src/utils.c @@ -228,3 +228,22 @@ nbdkit_read_password (const char *value, char **password) return 0; } + +char * +nbdkit_realpath (const char *path) +{ + char *ret; + + if (path == NULL || *path == '\0') { + nbdkit_error ("cannot resolve a null or empty path"); + return NULL; + } + + ret = realpath (path, NULL); + if (ret == NULL) { + nbdkit_error ("realpath(%s): %m", path); + return NULL; + } + + return ret; +} -- 2.14.3

2 4

Re: [Libguestfs] [libvirt] [PATCH tck] Relabel SELinux when customizing virt-builder image
by Laine Stump 19 Mar '18

19 Mar '18

On 02/06/2018 10:53 AM, Pino Toscano wrote: > On Tuesday, 6 February 2018 16:40:04 CET Daniel P. Berrangé wrote: >> When you tell virt-builder to install extra RPMs, this potentially >> looses the SELinux labelling that Anaconda had originally setup. Thus we >> must tell virt-builder to enable SELinux relabelling. >> >> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> >> --- >> lib/Sys/Virt/TCK.pm | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/Sys/Virt/TCK.pm b/lib/Sys/Virt/TCK.pm >> index e9da8d2..b39f578 100644 >> --- a/lib/Sys/Virt/TCK.pm >> +++ b/lib/Sys/Virt/TCK.pm >> @@ -405,7 +405,7 @@ sub create_virt_builder_disk { >> } >> >> print "# running virt-builder $osname\n"; >> - system "virt-builder", "--install", "dsniff", "--root-password", "password:$password", "--output", $target, $osname; >> + system "virt-builder", "--install", "dsniff", "--selinux-relabel", "--root-password", "password:$password", "--output", $target, $osname; >> >> die "cannot run virt-builder: $?" if $? != 0; > > Reviewed-by: Pino Toscano <ptoscano(a)redhat.com> > This change works, but since the original image came from virt-builder, and virt-builder knows enough about the image to know that it should install packages with dnf (or yum or apt-get or whatever is appropriate for any given image), it should also have enough info available to determine on its own that the selinux labels need to be redone. Especially since the Fedora images provided by virt-builder have selinux set to enforcing, I think the default behavior in this case should be for virt-builder to relabel. This patch fixes the problem for libvirt-tck, but I can imagine that this same problem will be revisited time after time on IRC and the libguestfs mailing list (once the user takes the obligatory troubleshooting trip to discover the source of the problem). In this case the initial symptom was "a guest that was never logged into by a human was failing an automated test". There were several steps from there to "dhcpc was failing to get an IP address due to bad selinux labels", and then learning via IRC that the labels were incorrect because extra packages are installed with the image mounted on the libguestfs appliance, which runs with selinux disabled. What is preventing virt-builder from automatically making a correct determination about whether or not relabeling must be done?

2 2

guestfs_list_filesystems: skip block devices which cannot hold file system
by Mykola Ivanets 19 Mar '18

19 Mar '18

Initial discussion is here: https://www.redhat.com/archives/libguestfs/2018-January/msg00188.html. v2 was posted here: https://www.redhat.com/archives/libguestfs/2018-January/msg00246.html. v3 comparing to v2 is just a rebase with slightly changed commits comments.

3 12

[PATCH] v2v: rhv: Fix virtio-rng and memballoon OVF fragment for RHV.
by Richard W.M. Jones 28 Feb '18

28 Feb '18

Without this extra element, oVirt will crash with a Java NullPointerException (see https://bugzilla.redhat.com/1550123) Fixes commit dac5fc53acdd1e51be2957c67e1e063e2132e680. --- v2v/create_ovf.ml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml index f5e34d79f..87245fdc8 100644 --- a/v2v/create_ovf.ml +++ b/v2v/create_ovf.ml @@ -444,6 +444,9 @@ let rec create_ovf source targets guestcaps inspect e "rasd:ResourceType" [] [PCData "0"]; e "Type" [] [PCData "rng"]; e "Device" [] [PCData "virtio"]; + e "SpecParams" [] [ + e "source" [] [PCData "urandom"] + ] ] ); if guestcaps.gcaps_virtio_balloon then @@ -454,6 +457,9 @@ let rec create_ovf source targets guestcaps inspect e "rasd:ResourceType" [] [PCData "0"]; e "Type" [] [PCData "balloon"]; e "Device" [] [PCData "memballoon"]; + e "SpecParams" [] [ + e "model" [] [PCData "virtio"] + ] ] ); -- 2.13.2

2 1

[PATCH] lib: Don't abort if a signal handler calls exit(2) during a guestfs_* function.
by Richard W.M. Jones 28 Feb '18

28 Feb '18

$ virt-sparsify input output [ 0.0] Create overlay file in /tmp to protect source disk [ 0.0] Examine source disk ^C guestfs_close: g->lock: Device or resource busy Aborted (core dumped) The reason for this is because virt-sparsify calls exit(2) in the SIGINT signal handler, which causes the close_handles atexit handler to run, which calls guestfs_close. However the same handle is in the middle of a guestfs_* call, and the code in lib/handle.c catches this case and calls abort(2). (The same situation could happen from threaded code.) The solution is to ignore the case where guestfs_close is called from close_handles and the lock is held (the handle will be left open in this case, but the recovery process should reap qemu). --- lib/handle.c | 75 +++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/lib/handle.c b/lib/handle.c index 449ab42a6..19caad813 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -43,6 +43,7 @@ static int shutdown_backend (guestfs_h *g, int check_for_errors); static void close_handles (void); +static void do_close (guestfs_h *g, int ignore_recursive); gl_lock_define_initialized (static, handles_lock); static guestfs_h *handles = NULL; @@ -323,15 +324,7 @@ guestfs_impl_parse_environment_list (guestfs_h *g, char * const *strings) void guestfs_close (guestfs_h *g) { - struct hv_param *hp, *hp_next; guestfs_h **gg; - int r; - - if (g->state == NO_HANDLE) { - /* Not safe to call ANY callbacks here, so ... */ - fprintf (stderr, _("guestfs_close: called twice on the same handle\n")); - return; - } /* Remove the handle from the handles list. */ if (g->close_on_exit) { @@ -342,6 +335,45 @@ guestfs_close (guestfs_h *g) gl_lock_unlock (handles_lock); } + do_close (g, 0); +} + +static void +do_close (guestfs_h *g, int ignore_recursive) +{ + struct hv_param *hp, *hp_next; + int r; + + if (g->state == NO_HANDLE) { + /* Not safe to call ANY callbacks here, so ... */ + fprintf (stderr, _("guestfs_close: called twice on the same handle\n")); + return; + } + + /* Destroy the handle lock first so we can detect the case where + * guestfs_close is called during another guestfs_* call on the same + * handle. This can happen if exit(3) is called from a signal + * handler or another thread during a guestfs_* call, which will + * cause the close_handles atexit handler to run, calling us here + * with ignore_recursive == 1. + */ + r = glthread_recursive_lock_destroy (&g->lock); + if (r != 0) { + if (r == EBUSY && ignore_recursive) + return; + + /* If pthread_mutex_destroy returns 16 (EBUSY), this indicates + * that the lock is held somewhere. That means a programming + * error if the main program is using threads. + */ + errno = r; + perror ("guestfs_close: g->lock"); + /* While we're debugging locks in libguestfs I want this to fail + * noisily. + */ + abort (); + } + if (g->trace) { const char trace_msg[] = "close"; @@ -403,21 +435,6 @@ guestfs_close (guestfs_h *g) free (g->append); guestfs_int_free_error_data_list (g); gl_tls_key_destroy (g->error_data); - r = glthread_recursive_lock_destroy (&g->lock); - if (r != 0) { - /* If pthread_mutex_destroy returns 16 (EBUSY), this indicates - * that the lock is held somewhere. That means a programming - * error if the main program is using threads. - */ - errno = r; - perror ("guestfs_close: g->lock"); - /* While we're debugging locks in libguestfs I want this to fail - * noisily. Remove this later since there are valid times when - * this might fail such as if the program exits during a - * libguestfs operation. - */ - abort (); - } free (g); } @@ -492,7 +509,17 @@ shutdown_backend (guestfs_h *g, int check_for_errors) static void close_handles (void) { - while (handles) guestfs_close (handles); + guestfs_h *g, *g_next; + + gl_lock_lock (handles_lock); + g = handles; + handles = NULL; + while (g) { + g_next = g->next; + do_close (g, 1); + g = g_next; + } + gl_lock_unlock (handles_lock); } int -- 2.13.2

1 0

[PATCH v2 0/3] v2v: Add -o rhv-upload output mode.
by Richard W.M. Jones 28 Feb '18

28 Feb '18

This patch set is still for discussion only. See 3/3 for the current list of problems. However this will upload an image to an oVirt or RHV server, although you must have absolutely the latest snapshot version of 4.2 for it to work. Rich.

2 6

[PATCH v3 0/2] inspect: basic UTF-8 encoding for rpm
by Cédric Bosdonnat 28 Feb '18

28 Feb '18

Diff to v2: * inlined local_string_to_utf8 Cédric Bosdonnat (2): common: extract UTF-8 conversion function inspector: rpm summary and description may not be utf-8 common/utils/guestfs-utils.h | 11 +++++ common/utils/libxml2-utils.c | 69 +-------------------------- common/utils/utils.c | 64 +++++++++++++++++++++++++ inspector/expected-fedora.img.xml | 4 ++ lib/inspect-apps.c | 30 ++++++++++-- test-data/phony-guests/fedora-packages.db.txt | 4 +- 6 files changed, 108 insertions(+), 74 deletions(-) -- 2.16.1

1 2

[PATCH] v2v: remove MAC address related information
by Tomáš Golembiovský 27 Feb '18

27 Feb '18

Remove ties to MAC address because it is likely to change. The code is based on operations net-hwaddr and udev-persistent-net of virt-sysprep. Signed-off-by: Tomáš Golembiovský <tgolembi(a)redhat.com> --- v2v/convert_linux.ml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml index b273785e6..8bba74786 100644 --- a/v2v/convert_linux.ml +++ b/v2v/convert_linux.ml @@ -88,6 +88,8 @@ let convert (g : G.guestfs) inspect source output rcaps = unconfigure_kudzu (); unconfigure_prltools (); + sysprep_networking(); + let kernel = configure_kernel () in if output#keep_serial_console then ( @@ -452,6 +454,21 @@ let convert (g : G.guestfs) inspect source output rcaps = msg ) + and sysprep_networking () = + if family = `RHEL_family then + (* Remove HWADDR=... entries from ifcfg-* files. *) + let paths = g#aug_match + "/files/etc/sysconfig/network-scripts/*[label() =~ glob('ifcfg-*')]/HWADDR" in + let paths = Array.to_list paths in + if paths <> [] then ( + List.iter (fun path -> ignore (g#aug_rm path)) paths; + g#aug_save () + ); + (* Remove persistent udev rules. *) + try g#rm "/etc/udev/rules.d/70-persistent-net.rules" + with G.Error _ -> () + + and configure_kernel () = (* Previously this function would try to install kernels, but we * don't do that any longer. -- 2.16.1

5 16