January 2023 - Libguestfs - libguestfs List Archives

[cross-project PATCH v2] NBD 64-bit extensions
by Eric Blake 06 Mar '23

06 Mar '23

This is a cover letter for a set of multi-project patch series all designed to implement 64-bit operations in NBD, and demonstrate interoperability of the new extension between projects. v1 of the project was attempted nearly a year ago: https://lists.nongnu.org/archive/html/qemu-devel/2021-12/msg00453.html Since then, I've addressed a lot of preliminary cleanups in libnbd to make it easier to test things, and incorporated review comments issued on v1, including: - no orthogonality between simple/structured replies and 64-bit mode: once extended headers are negotiated, all transmission traffic (both from client to server and server to client) uses just one header size - add support for the client to pass a payload on commands to the server, and demonstrate it by further implementing a way to pass a flag with NBD_CMD_BLOCK_STATUS that says the client is passing a payload to request status of just a subset of the negotiated contexts, rather than all possible contexts that were earlier negotiated during NBD_OPT_SET_META_CONTEXT - tweaks to the header layouts: tweak block status to provide 64-bit flags values (although "base:allocation" will remain usable with just 32-bit flags); reply with offset rather than padding and with fields rearranged for maximal sharing between client and server layouts - word-smithing of the NBD proposed protocol; split things into several smaller patches, where we can choose how much to take - more unit tests added in qemu and libnbd - the series end with RFC patches on whether to support 64-bit hole responses to NBD_CMD_READ, even though our current limitations say servers don't have to accept more than a 32M read request and therefore will never have that big of a hole to read) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org

5 64

[PATCH libnbd] generator: Pass LISTEN_FDNAMES=nbd with systemd socket activation
by Richard W.M. Jones 07 Feb '23

07 Feb '23

systemd allows sockets passed through socket activation to be named with the protocol they require. We only ever pass one socket, name it. This environment variable is currently ignored by qemu-nbd and nbdkit, but might be used by qemu-storage-daemon: https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg06114.html --- generator/states-connect-socket-activation.c | 41 +++++++++++--------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c index 9a83834915..22f06d4fd3 100644 --- a/generator/states-connect-socket-activation.c +++ b/generator/states-connect-socket-activation.c @@ -34,16 +34,18 @@ /* This is baked into the systemd socket activation API. */ #define FIRST_SOCKET_ACTIVATION_FD 3 -/* == strlen ("LISTEN_PID=") | strlen ("LISTEN_FDS=") */ -#define PREFIX_LENGTH 11 - extern char **environ; /* Prepare environment for calling execvp when doing systemd socket * activation. Takes the current environment and copies it. Removes - * any existing LISTEN_PID or LISTEN_FDS and replaces them with new - * variables. env[0] is "LISTEN_PID=..." which is filled in by - * CONNECT_SA.START, and env[1] is "LISTEN_FDS=1". + * any existing LISTEN_PID, LISTEN_FDS or LISTEN_FDNAMES, and replaces + * them with new variables. + * + * env[0] is "LISTEN_PID=..." which is filled in by CONNECT_SA.START + * + * env[1] is "LISTEN_FDS=1" + * + * env[2] is "LISTEN_FDNAMES=nbd" */ static int prepare_socket_activation_environment (string_vector *env) @@ -53,26 +55,29 @@ prepare_socket_activation_environment (string_vector *env) assert (env->len == 0); - /* Reserve slots env[0] and env[1]. */ + /* Reserve slots env[0]..env[2] */ + if (string_vector_reserve (env, 3) == -1) + goto err; p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); if (p == NULL) goto err; - if (string_vector_append (env, p) == -1) { - free (p); - goto err; - } + string_vector_append (env, p); p = strdup ("LISTEN_FDS=1"); if (p == NULL) goto err; - if (string_vector_append (env, p) == -1) { - free (p); + string_vector_append (env, p); + p = strdup ("LISTEN_FDNAMES=nbd"); + if (p == NULL) goto err; - } + string_vector_append (env, p); - /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */ + /* Append the current environment, but remove the special + * environment variables. + */ for (i = 0; environ[i] != NULL; ++i) { - if (strncmp (environ[i], "LISTEN_PID=", PREFIX_LENGTH) != 0 && - strncmp (environ[i], "LISTEN_FDS=", PREFIX_LENGTH) != 0) { + if (strncmp (environ[i], "LISTEN_PID=", 11) != 0 && + strncmp (environ[i], "LISTEN_FDS=", 11) != 0 && + strncmp (environ[i], "LISTEN_FDNAMES=", 15) != 0) { char *copy = strdup (environ[i]); if (copy == NULL) goto err; @@ -194,7 +199,7 @@ CONNECT_SA.START: char buf[32]; const char *v = nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf); - strcpy (&env.ptr[0][PREFIX_LENGTH], v); + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v); /* Restore SIGPIPE back to SIG_DFL. */ signal (SIGPIPE, SIG_DFL); -- 2.39.0

4 11

[p2v PATCH 00/11] Expose virt-v2v's "-oo"; re-enable openstack
by Laszlo Ersek 02 Feb '23

02 Feb '23

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1792141 Let the user pass "-oo" options from the kernel cmdline and from the GUI to virt-v2v. This is primarily useful with the OpenStack output mode, so reenable that mode. Cc: Alban Lecorps <alban.lecorps(a)ubisoft.com> Laszlo Alban Lecorps (1): Introduce "p2v.output.misc" for passing "-oo" options to virt-v2v Laszlo Ersek (10): test-virt-p2v-cmdline: turn option list into a shell array guestfs-utils: import guestfs_int_join_strings() gui: factor out entry_text_dup() gui: factor out tgl_btn_is_act() gui: flatten get_phys_topo_from_conv_dlg() gui: wrap overlong function declaration lines gui: wrap string literals gui: expose "p2v.output.misc" (-oo) Reenable the OpenStack output mode make-kickstart: add URLs for RHEL-9 conversion.c | 9 ++ generate-p2v-config.pl | 10 ++ gui.c | 168 ++++++++++++++++---- libguestfs/guestfs-utils.c | 35 ++++ libguestfs/guestfs-utils.h | 1 + ssh.c | 5 +- test-virt-p2v-cmdline.sh | 24 ++- virt-p2v-make-kickstart.in | 13 ++ virt-p2v.pod | 2 + 9 files changed, 230 insertions(+), 37 deletions(-)

2 17

[PATCH libnbd v2 0/4] Pass LISTEN_FDNAMES with systemd socket activation
by Richard W.M. Jones 01 Feb '23

01 Feb '23

This is an alternative approach to https://listman.redhat.com/archives/libguestfs/2023-January/030535.html After discussing this with Dan Berrange we came to the conclusion that you really might want to set LISTEN_FDNAMES to arbitrary short strings (or not set it). Especially when talking to qemu-storage-daemon which would allow you to use these names on the command line. Rich.

3 15

Preliminary release notes for libguestfs 1.50, guestfs-tools 1.50
by Richard W.M. Jones 31 Jan '23

31 Jan '23

$SUBJECT here ... https://github.com/libguestfs/libguestfs/blob/master/docs/guestfs-release-n… https://github.com/rwmjones/guestfs-tools/blob/master/docs/guestfs-tools-re… Any extra stuff that's pending for either project that isn't too invasive for 1.50, then let me know. By the way this will be the 28th major release of libguestfs. Depending on how you count -- we weren't very good about having stable releases in the early days. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit

2 2

[libnbd PATCH v2 0/3] Improve nbdsh -u handling
by Eric Blake 31 Jan '23

31 Jan '23

v1 was here: https://listman.redhat.com/archives/libguestfs/2022-October/030216.html Since then, I've incorporated changes based on Rich's feedback: swap order of patches 2 and 3 less change in patch 1 (including no unsafe eval(%s) for --uri) in patch 2, include -c in list of snippets to store, and use dict of lambdas to map back to the desired action Eric Blake (3): nbdsh: Refactor handling of -c nbdsh: Allow -u interleaved with -c nbdsh: Improve --help and initial banner contents. python/nbdsh.py | 142 +++++++++++++++++++++++++++------------------ sh/test-context.sh | 26 ++++----- sh/test-error.sh | 37 +++++++----- 3 files changed, 119 insertions(+), 86 deletions(-) -- 2.38.1

3 15

[nbdkit PATCH 0/2] retry: add support for retrying .open
by Eric Blake 31 Jan '23

31 Jan '23

In https://bugzilla.redhat.com/show_bug.cgi?id=1841820, it was pointed out that the retry filter not retrying .open means that an ssh connection (such as in a vmx+ssh v2v conversion) fails when the ssh connection itself cannot be retried. A year ago, this was an inherent limitation of our retry implementation; but in the meantime, my work to allow filters to open independent backends has made it feasible to implement. Eric Blake (2): retry: Add in retry support during .open retry: Test previous patch tests/Makefile.am | 2 + server/backend.c | 7 ++- filters/retry/retry.c | 98 ++++++++++++++++++++++++---------------- tests/test-retry-open.sh | 85 ++++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 42 deletions(-) create mode 100755 tests/test-retry-open.sh -- 2.39.1

2 5

libnbd | Failed pipeline for master | 5a02c7d2
by GitLab 30 Jan '23

30 Jan '23

Pipeline #762187433 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: 5a02c7d2 ( https://gitlab.com/nbdkit/libnbd/-/commit/5a02c7d2cc6a201f9e5531c0c20c2f3c2… ) Commit Message: Revert "nbdsh: Improve --help and initial banne... Commit Author: Richard W_M_ Jones ( https://gitlab.com/rwmjones ) Pipeline #762187433 ( https://gitlab.com/nbdkit/libnbd/-/pipelines/762187433 ) triggered by Richard W_M_ Jones ( https://gitlab.com/rwmjones ) had 1 failed job. Job #3685688517 ( https://gitlab.com/nbdkit/libnbd/-/jobs/3685688517/raw ) Stage: builds Name: x86_64-macos-11 -- You're receiving this email because of your account on gitlab.com.

1 0

[PATCH v2v v2] -o rhv-upload: Improve error message for invalid or missing -os parameter
by Richard W.M. Jones 28 Jan '23

28 Jan '23

For -o rhv-upload, the -os parameter specifies the storage domain. Because the RHV API allows globs when searching for a domain, if you used a parameter like -os 'data*' then this would confuse the Python code, since it can glob to the name of a storage domain, but then later fail when we try to exact match the storage domain we found. The result of this was a confusing error in the precheck script: IndexError: list index out of range This fix validates the output storage parameter before trying to use it. Since valid storage domain names cannot contain glob characters or spaces, it avoids the problems above and improves the error message that the user sees: $ virt-v2v [...] -o rhv-upload -os '' ... RuntimeError: The storage domain (-os) parameter ‘’ is not valid virt-v2v: error: failed server prechecks, see earlier errors $ virt-v2v [...] -o rhv-upload -os 'data*' ... RuntimeError: The storage domain (-os) parameter ‘data*’ is not valid virt-v2v: error: failed server prechecks, see earlier errors Although the IndexError should no longer happen, I also added a try...catch around that code to improve the error in case it still happens. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1986386 Reported-by: Junqin Zhou Thanks: Nir Soffer --- output/rhv-upload-precheck.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/output/rhv-upload-precheck.py b/output/rhv-upload-precheck.py index 1dc1b8498a..ba125611ba 100644 --- a/output/rhv-upload-precheck.py +++ b/output/rhv-upload-precheck.py @@ -18,6 +18,7 @@ import json import logging +import re import sys from urllib.parse import urlparse @@ -46,6 +47,15 @@ output_password = output_password.rstrip() parsed = urlparse(params['output_conn']) username = parsed.username or "admin@internal" +# Check the storage domain name is valid +# (https://bugzilla.redhat.com/show_bug.cgi?id=1986386#c1) +# Also this means it cannot contain spaces or glob symbols, so +# the search below is valid. +output_storage = params['output_storage'] +if not re.match('^[-a-zA-Z0-9_]+$', output_storage): + raise RuntimeError("The storage domain (-os) parameter ‘%s’ is not valid" % + output_storage) + # Connect to the server. connection = sdk.Connection( url=params['output_conn'], @@ -60,28 +70,33 @@ system_service = connection.system_service() # Check whether there is a datacenter for the specified storage. data_centers = system_service.data_centers_service().list( - search='storage.name=%s' % params['output_storage'], + search='storage.name=%s' % output_storage, case_sensitive=True, ) if len(data_centers) == 0: storage_domains = system_service.storage_domains_service().list( - search='name=%s' % params['output_storage'], + search='name=%s' % output_storage, case_sensitive=True, ) if len(storage_domains) == 0: # The storage domain does not even exist. raise RuntimeError("The storage domain ‘%s’ does not exist" % - (params['output_storage'])) + output_storage) # The storage domain is not attached to a datacenter # (shouldn't happen, would fail on disk creation). raise RuntimeError("The storage domain ‘%s’ is not attached to a DC" % - (params['output_storage'])) + output_storage) datacenter = data_centers[0] # Get the storage domain. storage_domains = connection.follow_link(datacenter.storage_domains) -storage_domain = [sd for sd in storage_domains if sd.name == params['output_storage']][0] +try: + storage_domain = [sd for sd in storage_domains + if sd.name == output_storage][0] +except IndexError: + raise RuntimeError("The storage domain ‘%s’ does not exist" % + output_storage) # Get the cluster. clusters = connection.follow_link(datacenter.clusters) @@ -90,7 +105,7 @@ if len(clusters) == 0: raise RuntimeError("The cluster ‘%s’ is not part of the DC ‘%s’, " "where the storage domain ‘%s’ is" % (params['rhv_cluster'], datacenter.name, - params['output_storage'])) + output_storage)) cluster = clusters[0] cpu = cluster.cpu if cpu.architecture == types.Architecture.UNDEFINED: -- 2.39.0

2 1

Proposal for tools to inspect drivers and inject Windows virtio drivers
by Richard W.M. Jones 27 Jan '23

27 Jan '23

We've had requests from products that use libguestfs & virt-v2v to provide additional tooling for: (a) Inspecting a virtual machine disk image to find out what virtual devices it needs (ie. what drivers are installed in the guest). (b) Taking a Windows disk image and injecting virtio drivers from the virtio-win suite. Virt-v2v does both operations as a part of importing guests from VMware to KVM, but it doesn't expose these as separate operations. - - - For (a), you might run the tool against a disk image and it would display various facts (similar to virt-inspector https://libguestfs.org/virt-inspector.1.html) $ virt-drivers -a linux.img <operatingsystems> <operatingsystem> <root>/dev/sda2</root> <boot_drivers> [list of drivers from the initramfs in a format TBD] </boot_drivers> <drivers> [list of kernel modules] </drivers> <boot_loader> [extra stuff about the bootloader configuration, list of kernels, default kernel, grub1 or grub2, config file, ...] </boot_loader> I propose a completely new tool added to guestfs-tools to do this, which will basically pull the current kernel module and grub analysis code from virt-v2v into a new library in libguestfs-common. For Windows we actually don't do this in virt-v2v at the moment, so that would need to be completely new code, likely parsing the DriverDatabase from the Windows registry. - - - For (b), you could specify the location of the Window disk image and the virtio-win path/ISO to have it attempt to install the drivers in the disk image: $ virt-customize -a windows.img \ --inject-virtio-win /usr/share/virtio-win/virtio-win-1.2.3.iso This would largely involve taking the current virtio-win code from virt-v2v, turning it into a library, and then adding a new module into libguestfs-common/mlcustomize. (It would be a good time to refactor this code.) - - - Good? Bad? Let me know what you think ... I think one large danger is that injecting virtio-win drivers into existing Windows images is a very invasive operation with a large potential to go wrong. It would be better to work with the tools that create these images so that they're able to inject virtio-win drivers at initial creation. (Or "Inbox" the drivers with Microsoft, but there may be issues there). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit

2 6