[Libguestfs] [PATCH v4] windows: delay installation of qemu-ga MSI
Instead of running firstboot script during early boot schedule a task delayed for 2 minutes. During the first boot, after virt-v2v conversion, Windows installs the drivers injected by virt-v2v. When this installation is finished Windows enforces some kind of internal reboot. This unfortunately terminates any running firstboot scripts thus killing the installation of qemu-ga MSI. This is just a best-effort mitigation. It can still happen (e.g. with slow disk drives) that the drivers are not yet installed when the delayed installation starts. On the other hand we cannot delay it too much otherwise we risk that the users logs in and will be doing some work when the MSI installation starts. After MSI installation finishes the VM needs to be rebooted which would be annoying if that would happen under users hands. Although this is not a best fix (that may come later as it is more complex, e.g. introducing waiting mechanism), the delay as it is defined works in most cases. And it dramaticaly improves the situations -- originaly I experienced more than 90% failure rate. Signed-off-by: Tomáš Golembiovský --- common | 2 +- v2v/convert_windows.ml | 17 +++-- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/common b/common index ea10827b..5371257c 16 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit ea10827b4cfb3cfe5f782421c01d2902e5f73f90 +Subproject commit 5371257c3cf27fb09d5f2e31ba378b0e6ccf5df6 diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml index 0fda1d4e..9b90f611 100644 --- a/v2v/convert_windows.ml +++ b/v2v/convert_windows.ml @@ -428,16 +428,13 @@ popd and configure_qemu_ga files = List.iter ( fun msi_path -> - let fb_script = "\ -echo Installing qemu-ga from " ^ msi_path ^ " -\"\\" ^ msi_path ^ "\" /norestart /qn /l+*vx \"%~dpn0.log\" -set elvl=!errorlevel! -echo Done installing qemu-ga error_level=!elvl! -if !elvl! == 0 ( - echo Restarting Windows... - shutdown /r /f /c \"rebooted by firstboot script\" -) -" in + let fb_script = sprintf "\ +echo Removing any previously scheduled qemu-ga installation +schtasks.exe /Delete /TN Firstboot-qemu-ga /F +echo Scheduling delayed installation of qemu-ga from %s +powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\%s /forcerestart /qn /l+*vx C:\\%s.log\\\"\" + " + msi_path msi_path msi_path in Firstboot.add_firstboot_script g inspect.i_root ("install " ^ msi_path) fb_script; ) files -- 2.25.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v2 3/4] appliance: Pass root=UUID= instead of appliance device name (RHBZ#1804207).
Appliance device names are not reliable since the kernel no longer enumerates virtio-scsi devices serially. Instead get the UUID of the appliance and pass this as the parameter. Note this requires supermin >= 5.1.18 (from around July 2017). --- docs/guestfs-building.pod | 2 +- lib/appliance-kcmdline.c | 57 ++- lib/guestfs-internal.h| 2 +- lib/launch-direct.c | 34 +-- lib/launch-libvirt.c | 19 +++-- m4/guestfs-appliance.m4 | 3 ++- 6 files changed, 67 insertions(+), 50 deletions(-) diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod index 005626515..8e3da1d32 100644 --- a/docs/guestfs-building.pod +++ b/docs/guestfs-building.pod @@ -83,7 +83,7 @@ I. I. The following features must be enabled: C, C, C, C. -=item supermin E 5.1.0 +=item supermin E 5.1.18 I. For alternatives, see L below. diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c index 8eb4999af..211cc4687 100644 --- a/lib/appliance-kcmdline.c +++ b/lib/appliance-kcmdline.c @@ -26,8 +26,10 @@ #include #include #include +#include #include "c-ctype.h" +#include "ignore-value.h" #include "guestfs.h" #include "guestfs-internal.h" @@ -54,15 +56,53 @@ #define EARLYPRINTK "earlyprintk=pl011,0x900" #endif +COMPILE_REGEXP (re_uuid, "UUID=([-0-9a-f]+)", 0) + +static void +read_uuid (guestfs_h *g, void *retv, const char *line, size_t len) +{ + char **ret = retv; + + *ret = match1 (g, line, re_uuid); +} + +/** + * Given a disk image containing an extX filesystem, return the UUID. + * The L command does the hard work. + */ +static char * +get_root_uuid (guestfs_h *g, const char *appliance) +{ + CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g); + char *ret = NULL; + int r; + + guestfs_int_cmd_add_arg (cmd, "file"); + guestfs_int_cmd_add_arg (cmd, "--"); + guestfs_int_cmd_add_arg (cmd, appliance); + guestfs_int_cmd_set_stdout_callback (cmd, read_uuid, &ret, 0); + r = guestfs_int_cmd_run (cmd); + if (r == -1) { +if (ret) free (ret); +return NULL; + } + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { +guestfs_int_external_command_failed (g, r, "file", NULL); +if (ret) free (ret); +return NULL; + } + + return ret; +} + /** * Construct the Linux command line passed to the appliance. This is * used by the C and C backends, and is simply * located in this file because it's a convenient place for this * common code. * - * The C parameter must be the full device name of the - * appliance disk and must have already been adjusted to take into - * account virtio-blk or virtio-scsi; eg C. + * The C parameter is the filename of the appliance + * (could be NULL) from which we obtain the root UUID. * * The C parameter can contain the following flags logically * or'd together (or 0): @@ -80,7 +120,8 @@ * be freed by the caller. */ char * -guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, +guestfs_int_appliance_command_line (guestfs_h *g, +const char *appliance, int flags) { CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (argv); @@ -164,8 +205,12 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, guestfs_int_add_string (g, &argv, "8250.nr_uarts=1"); /* Tell supermin about the appliance device. */ - if (appliance_dev) -guestfs_int_add_sprintf (g, &argv, "root=%s", appliance_dev); + if (appliance) { +CLEANUP_FREE char *uuid = get_root_uuid (g, appliance); +if (!uuid) + return NULL; +guestfs_int_add_sprintf (g, &argv, "root=UUID=%s", uuid); + } /* SELinux - deprecated setting, never worked and should not be enabled. */ if (g->selinux) diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 8c6affa54..fe761a784 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -688,7 +688,7 @@ extern int guestfs_int_build_appliance (guestfs_h *g, char **kernel, char **init const char *guestfs_int_get_cpu_model (int kvm); /* appliance-kcmdline.c */ -extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, int flags); +extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance, int flags); #define APPLIANCE_COMMAND_LINE_IS_TCG 1 /* appliance-uefi.c */ diff --git a/lib/launch-direct.c b/lib/launch-direct.c index 1718041a5..2de57ea32 100644 --- a/lib/launch-direct.c +++ b/lib/launch-direct.c @@ -61,8 +61,6 @@ struct backend_direct_data { char guestfsd_sock[UNIX_PATH_MAX]; /* Path to daemon socket. */ }; -static char *make_appliance_dev (guestfs_h *g); - static char * create_cow_overlay_direct (guestfs_h *g, void *datav, struct drive *drv) { @@ -382,7 +380,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) int uefi_flags; CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL;
[Libguestfs] [PATCH v2 2/4] daemon: Print device names when they are translated.
This helps to debug problems with the new device name translation code. We can think about removing this later when the code is known to work well. --- daemon/device-name-translation.c | 8 1 file changed, 8 insertions(+) diff --git a/daemon/device-name-translation.c b/daemon/device-name-translation.c index 30173f5c2..b17f3c683 100644 --- a/daemon/device-name-translation.c +++ b/daemon/device-name-translation.c @@ -195,6 +195,10 @@ device_name_translation (const char *device) return NULL; } + /* If the device name is different, print the translation. */ + if (STRNEQ (device, ret)) +fprintf (stderr, "device name translated: %s -> %s\n", device, ret); + /* Now check the device is openable. */ fd = open (ret, O_RDONLY|O_CLOEXEC); if (fd >= 0) { @@ -281,5 +285,9 @@ reverse_device_name_translation (const char *device) } } + /* If the device name is different, print the translation. */ + if (STRNEQ (device, ret)) +fprintf (stderr, "reverse device name translated: %s -> %s\n", device, ret); + return ret; } -- 2.25.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v2 0/4] daemon: Translate device names if Linux device is unstable (RHBZ#1804207).
v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00220.html This patch series is a little bit better. It's still a bit of a hack. The _real_ fix for this is outlined in the TODO file (see patch 1) but that requires a lot more work than we could do before 1.42 is released, unless we delay 1.42 for a lot longer. I'm hoping with this to have something which works for 1.42, and then fix it properly in 1.43. Patch 2 is a debugging patch. It's separated out because we might consider reverting it in future once we are confident that the approach is working. Patch 3 is the second change necessary to how we launch the supermin appliance. This actually stands alone so could be reviewed separately. It requires a modest update to our baseline supermin version. Patch 4 should be ignored, it's just for my (additional) debugging. Rich. ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH v2 1/4] daemon: Translate device names if Linux device ordering is unstable (RHBZ#1804207).
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é. --- TODO | 13 +++ daemon/daemon.h | 2 + daemon/device-name-translation.c | 195 +-- daemon/devsparts.ml | 23 +++- daemon/guestfsd.c| 5 + daemon/stubs-macros.h| 10 +- lib/canonical-name.c | 5 + 7 files changed, 235 insertions(+), 18 deletions(-) diff --git a/TODO b/TODO index 066f633d4..064386ac2 100644 --- a/TODO +++ b/TODO @@ -189,6 +189,19 @@ might be replaced by direct use of the library (if this is easier). But it is very hard to be compatible between RHEL6 and RHEL5 when using the library directly. +Properly fix device name translation + + +Currently for Device parameters we pass a string name like "/dev/sda" +or "/dev/sdb2" or "/dev/VG/LV" to the daemon. Since Linux broke +device enumeration (RHBZ#1804207) this works less well, requiring +non-trivial translation within the daemon. + +It would be far better if in the generator we mapped "/dev/XXX" to a +proper structure. Device index + partition | LV | MD | ... +This would be then used everywhere inside the daemon and mean we would +no longer need device name translation at all. + Visualization - diff --git a/daemon/daemon.h b/daemon/daemon.h index 170fb2537..24cf8585d 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -215,6 +215,8 @@ extern void notify_progress_no_ratelimit (uint64_t position, uint64_t total, con /* device-name-translation.c */ extern char *device_name_translation (const char *device); +extern void device_name_translation_init (void); +extern void device_name_translation_free (void); extern char *reverse_device_name_translation (const char *device); /* stubs.c (auto-generated) */ diff --git a/daemon/device-name-translation.c b/daemon/device-name-translation.c index ed826bbae..30173f5c2 100644 --- a/daemon/device-name-translation.c +++ b/daemon/device-name-translation.c @@ -27,12 +27,125 @@ #include #include #include +#include +#include + +#include "c-ctype.h" #include "daemon.h" +static char **cache; +static size_t cache_size; + +/** + * Cache daemon disk mapping. + * + * When the daemon starts up, populate a cache with the contents + * of /dev/disk/by-path. It's easiest to use C here + * since the names are sorted awkwardly. + */ +void +device_name_translation_init (void) +{ + const char *by_path = "/dev/disk/by-path"; + CLEANUP_FREE char *out = NULL, *err = NULL; + CLEANUP_FREE_STRING_LIST char **lines = NULL; + size_t i, j; + int r; + + device_name_translation_free (); + + r = command (&out, &err, "ls", "-1v", by_path, NULL); + if (r == -1) +error (EXIT_FAILURE, 0, + "failed to initialize device name translation cache: %s", err); + + lines = split_lines (out); + if (lines == NULL) +error (EXIT_FAILURE, errno, "split_lines"); + + cache_size = guestfs_int_count_strings (lines); + cache = calloc (cache_size, sizeof (char *)); + if (cache == NULL) +error (EXIT_FAILURE, errno, "calloc"); + + /* Delete entries for partitions. (Mark them as NULL in the array + * and skip them below.) + */ + for (i = 0; i < cache_size; ++i) { +if (strstr (lines[i], "-part")) + lines[i] = NULL; + } + + /* Look up each device name. It should be a symlink to /dev/sdX. */ + for (i = j = 0; i < cache_size; ++i) { +if (lines[i] != NULL) { + CLEANUP_FREE char *full; + char *device; + + if (asprintf (&full, "%s/%s", by_path, lines[i]) == -1) +error (EXIT_FAILURE, errno, "asprintf"); + + device = realpath (full, NULL); + if (device == NULL) +error (EXIT_FAILURE, errno, "realpath: %s", full); + + /* Don't add the root device to the cache. */ + if (is_root_device (device)) +continue; + + cache[j++] = device; +} + } + + /* This is the final cache size because we deleted entries above. */ + cache_size = j; +} + +void +device_name_translation_free (void) +{ + size_t i; + + for (i = 0; i < cache_size; ++i) +free (cache[i]); + free (cache); + cache = NULL; + cache_size = 0; +} + /** * Perform device name translation. * + * Libguestfs defines a few standard formats for device names. + * (see also L and + * L). They are: + * + * =over 4 + * + * =item F + * + * =item F + * + * =item F + * + * These mean the Nth partition on the Xth device. Because + * Linux no longer enumera
[Libguestfs] [PATCH v2 4/4] DEBUG - DROP THIS PATCH LATER
--- daemon/device-name-translation.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/daemon/device-name-translation.c b/daemon/device-name-translation.c index b17f3c683..39266fdd7 100644 --- a/daemon/device-name-translation.c +++ b/daemon/device-name-translation.c @@ -100,6 +100,12 @@ device_name_translation_init (void) /* This is the final cache size because we deleted entries above. */ cache_size = j; + + /* XXX */ + for (i = 0; i < cache_size; ++i) { +fprintf (stderr, "device name translation cache: %zu -> %s\n", + i, cache[i]); + } } void @@ -180,6 +186,8 @@ device_name_translation (const char *device) strcpy (dev, start); dev[len] = '\0'; +fprintf (stderr, "DEVICE=%s START=%s LEN=%zu\n", device, start, len); + i = guestfs_int_drive_index (dev); if (i >= 0 && i < (ssize_t) cache_size) { /* Append the partition name if present. */ @@ -273,6 +281,8 @@ reverse_device_name_translation (const char *device) reply_with_perror ("asprintf"); return NULL; } + fprintf (stderr, "REVERSE %s -> %s, index %zu, cache[%zu]=%s, len %zu\n", + device, ret, i, i, cache[i], len); break; } } -- 2.25.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v3] windows: delay installation of qemu-ga MSI
On Thu, Mar 05, 2020 at 12:47:58PM +0100, Tomáš Golembiovský wrote: > On Thu, Mar 05, 2020 at 08:49:04AM +, Richard W.M. Jones wrote: > > On Tue, Mar 03, 2020 at 03:45:49PM +0100, Tomáš Golembiovský wrote: > > > Instead of running firstboot script during early boot schedule a task > > > delayed for 2 minutes. > > > > > > During the first boot, after virt-v2v conversion, Windows installs the > > > drivers injected by virt-v2v. When this installation is finished > > > Windows enforces some kind of internal reboot. This unfortunately > > > terminates any running firstboot scripts thus killing the installation > > > of qemu-ga MSI. > > > > > > This is just a best-effort mitigation. It can still happen (e.g. with > > > slow disk drives) that the drivers are not yet installed when the > > > delayed installation starts. On the other hand we cannot delay it too > > > much otherwise we risk that the users logs in and will be doing some > > > work when the MSI installation starts. After MSI installation finishes > > > the VM needs to be rebooted which would be annoying if that would happen > > > under users hands. Although this is not a best fix (that may come later > > > as it is more complex, e.g. introducing waiting mechanism), the delay as > > > it is defined works in most cases. And it dramaticaly improves the > > > situations -- originaly I experienced more than 90% failure rate. > > > > > > Signed-off-by: Tomáš Golembiovský > > > --- > > > common | 2 +- > > > v2v/convert_windows.ml | 12 > > > 2 files changed, 5 insertions(+), 9 deletions(-) > > > > > > diff --git a/common b/common > > > index ea10827b..5371257c 16 > > > --- a/common > > > +++ b/common > > > @@ -1 +1 @@ > > > -Subproject commit ea10827b4cfb3cfe5f782421c01d2902e5f73f90 > > > +Subproject commit 5371257c3cf27fb09d5f2e31ba378b0e6ccf5df6 > > > diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml > > > index 0fda1d4e..bed5989a 100644 > > > --- a/v2v/convert_windows.ml > > > +++ b/v2v/convert_windows.ml > > > @@ -429,14 +429,10 @@ popd > > > List.iter ( > > > fun msi_path -> > > > let fb_script = "\ > > > -echo Installing qemu-ga from " ^ msi_path ^ " > > > -\"\\" ^ msi_path ^ "\" /norestart /qn /l+*vx \"%~dpn0.log\" > > > -set elvl=!errorlevel! > > > -echo Done installing qemu-ga error_level=!elvl! > > > -if !elvl! == 0 ( > > > - echo Restarting Windows... > > > - shutdown /r /f /c \"rebooted by firstboot script\" > > > -) > > > +echo Removing any previously scheduled qemu-ga installation > > > +schtasks.exe /Delete /TN Firstboot-qemu-ga /F > > > +echo Scheduling delayed installation of qemu-ga from " ^ msi_path ^ " > > > +powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe > > > /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') > > > /RU SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\" ^ msi_path ^ " > > > /forcerestart /qn /l+*vx C:\\" ^ msi_path ^ ".log\\\"\" > > > " in > > >Firstboot.add_firstboot_script g inspect.i_root > > > ("install " ^ msi_path) fb_script; > > > > It could be easier to follow if you use sprintf here, something like: > > > > let fb_script = sprintf "\ > > echo Removing any previously scheduled qemu-ga installation > > schtasks.exe /Delete /TN Firstboot-qemu-ga /F > > echo Scheduling delayed installation of qemu-ga from %s > > powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe > > /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU > > SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\%s /forcerestart /qn /l+*vx > > C:\\%s.log\\\"\" > > msi_path msi_path msi_path in > > Ok, I can change that. > > > > > The other possible problem is this seems to install a firstboot script > > for every element of the list qemu_ga_files. How long is this list? > > At the moment it is just one. Until something changes in virtio-win ISO > or in our internal logic (like in virtio_iso_path_matches_qemu_ga()) it > will always be one. > > > > > Do filenames on this list need some kind of quoting? The filenames > > don't, but they seem to contain a path that comes from the virtio-win > > ISO. > > Those are really just file names without path. See > https://github.com/libguestfs/virt-v2v/blob/master/v2v/windows_virtio.ml#L339 I'll ACK this, but I think using sprintf is better. 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 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v3] windows: delay installation of qemu-ga MSI
On Thu, Mar 05, 2020 at 08:49:04AM +, Richard W.M. Jones wrote: > On Tue, Mar 03, 2020 at 03:45:49PM +0100, Tomáš Golembiovský wrote: > > Instead of running firstboot script during early boot schedule a task > > delayed for 2 minutes. > > > > During the first boot, after virt-v2v conversion, Windows installs the > > drivers injected by virt-v2v. When this installation is finished > > Windows enforces some kind of internal reboot. This unfortunately > > terminates any running firstboot scripts thus killing the installation > > of qemu-ga MSI. > > > > This is just a best-effort mitigation. It can still happen (e.g. with > > slow disk drives) that the drivers are not yet installed when the > > delayed installation starts. On the other hand we cannot delay it too > > much otherwise we risk that the users logs in and will be doing some > > work when the MSI installation starts. After MSI installation finishes > > the VM needs to be rebooted which would be annoying if that would happen > > under users hands. Although this is not a best fix (that may come later > > as it is more complex, e.g. introducing waiting mechanism), the delay as > > it is defined works in most cases. And it dramaticaly improves the > > situations -- originaly I experienced more than 90% failure rate. > > > > Signed-off-by: Tomáš Golembiovský > > --- > > common | 2 +- > > v2v/convert_windows.ml | 12 > > 2 files changed, 5 insertions(+), 9 deletions(-) > > > > diff --git a/common b/common > > index ea10827b..5371257c 16 > > --- a/common > > +++ b/common > > @@ -1 +1 @@ > > -Subproject commit ea10827b4cfb3cfe5f782421c01d2902e5f73f90 > > +Subproject commit 5371257c3cf27fb09d5f2e31ba378b0e6ccf5df6 > > diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml > > index 0fda1d4e..bed5989a 100644 > > --- a/v2v/convert_windows.ml > > +++ b/v2v/convert_windows.ml > > @@ -429,14 +429,10 @@ popd > > List.iter ( > > fun msi_path -> > > let fb_script = "\ > > -echo Installing qemu-ga from " ^ msi_path ^ " > > -\"\\" ^ msi_path ^ "\" /norestart /qn /l+*vx \"%~dpn0.log\" > > -set elvl=!errorlevel! > > -echo Done installing qemu-ga error_level=!elvl! > > -if !elvl! == 0 ( > > - echo Restarting Windows... > > - shutdown /r /f /c \"rebooted by firstboot script\" > > -) > > +echo Removing any previously scheduled qemu-ga installation > > +schtasks.exe /Delete /TN Firstboot-qemu-ga /F > > +echo Scheduling delayed installation of qemu-ga from " ^ msi_path ^ " > > +powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe > > /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU > > SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\" ^ msi_path ^ " /forcerestart /qn > > /l+*vx C:\\" ^ msi_path ^ ".log\\\"\" > > " in > >Firstboot.add_firstboot_script g inspect.i_root > > ("install " ^ msi_path) fb_script; > > It could be easier to follow if you use sprintf here, something like: > > let fb_script = sprintf "\ > echo Removing any previously scheduled qemu-ga installation > schtasks.exe /Delete /TN Firstboot-qemu-ga /F > echo Scheduling delayed installation of qemu-ga from %s > powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe > /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU > SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\%s /forcerestart /qn /l+*vx > C:\\%s.log\\\"\" > msi_path msi_path msi_path in Ok, I can change that. > > The other possible problem is this seems to install a firstboot script > for every element of the list qemu_ga_files. How long is this list? At the moment it is just one. Until something changes in virtio-win ISO or in our internal logic (like in virtio_iso_path_matches_qemu_ga()) it will always be one. > > Do filenames on this list need some kind of quoting? The filenames > don't, but they seem to contain a path that comes from the virtio-win > ISO. Those are really just file names without path. See https://github.com/libguestfs/virt-v2v/blob/master/v2v/windows_virtio.ml#L339 Tomas > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-builder quickly builds VMs from scratch > http://libguestfs.org/virt-builder.1.html > -- Tomáš Golembiovský ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] Having integration tests "test the future" with QEMU
On Wed, Mar 04, 2020 at 03:48:34PM +0100, Markus Armbruster wrote: > I understand libguestfs comes with integration tests. You might be > interested in something I'm developing for QEMU 5.0 to permit "testing > the future". I'd like to ensure it is actually useful before I > continue. Let me know what you think. > > = Motivation = > > When layers above QEMU somehow miss feature deprecation, things continue > to work until the deprecated interface goes away, at which point we have > an entirely avoidable regression. > > We've been trying to get better at communicating feature deprecation to > the layers above QEMU. An obvious first step was systematic > documentation. > > However, documentation is all too easy to miss. Even if you don't miss > it, you may need to continue using the deprecated feature with old > versions of QEMU, and that logic needs to be tested. > > I'm working on a way to run QEMU with the deprecated features disabled. > This permits "testing the future". > > >From my cover letter: > > This series extends QMP introspection to cover deprecation. > Additionally, new option -compat lets you configure what to do when > deprecated interfaces get used. This is intended for testing users of > the management interfaces. It is experimental. > > -compat deprecated-input= configures what to do when > deprecated input is received. Available policies: > > * accept: Accept deprecated commands and arguments (default) > * reject: Reject them > * crash: Crash > > -compat deprecated-output= configures what to do when > deprecated output is sent. Available output policies: > > * accept: Emit deprecated command results and events (default) > * hide: Suppress them > > For now, -compat covers only deprecated syntactic aspects of QMP. We > may want to extend it to cover semantic aspects, CLI, and experimental > features. Yes we would add a -compat option when running qemu (only when running non-production builds of libguestfs) so that it would fail loudly when we use a deprecated feature. This is the best way I've heard of so far to get advanced warning of deprecated features. Rich. > If you want to learn more, check out the last three commit messages in > the series. > > = Patches = > > [PATCH v2 00/30] Configurable policy for handling deprecated interfaces > Message-Id: <20200303163505.32041-1-arm...@redhat.com> > https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00645.html > > = Git = > > Tag patchew/20200303163505.32041-1-arm...@redhat.com in repository > https://github.com/patchew-project/qemu.git > > ___ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] Bug: readdir() returning null on large number of files
On Wed, Mar 04, 2020 at 03:28:33PM +0300, Sergey Ivanov wrote: > There's a bug (https://bugzilla.redhat.com/show_bug.cgi?id=1674392) and > it's almost a year since it has been reported. Is there any > plans/suggestions on its fixing? Didn't find any commits regarding this > issue but maybe I'm just blind. It's open source, we don't have a guarantee of fixing any particular bug. I suggest if it's important for you that you take a look at fixing it yourself. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v3] windows: delay installation of qemu-ga MSI
On Tue, Mar 03, 2020 at 03:45:49PM +0100, Tomáš Golembiovský wrote: > Instead of running firstboot script during early boot schedule a task > delayed for 2 minutes. > > During the first boot, after virt-v2v conversion, Windows installs the > drivers injected by virt-v2v. When this installation is finished > Windows enforces some kind of internal reboot. This unfortunately > terminates any running firstboot scripts thus killing the installation > of qemu-ga MSI. > > This is just a best-effort mitigation. It can still happen (e.g. with > slow disk drives) that the drivers are not yet installed when the > delayed installation starts. On the other hand we cannot delay it too > much otherwise we risk that the users logs in and will be doing some > work when the MSI installation starts. After MSI installation finishes > the VM needs to be rebooted which would be annoying if that would happen > under users hands. Although this is not a best fix (that may come later > as it is more complex, e.g. introducing waiting mechanism), the delay as > it is defined works in most cases. And it dramaticaly improves the > situations -- originaly I experienced more than 90% failure rate. > > Signed-off-by: Tomáš Golembiovský > --- > common | 2 +- > v2v/convert_windows.ml | 12 > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/common b/common > index ea10827b..5371257c 16 > --- a/common > +++ b/common > @@ -1 +1 @@ > -Subproject commit ea10827b4cfb3cfe5f782421c01d2902e5f73f90 > +Subproject commit 5371257c3cf27fb09d5f2e31ba378b0e6ccf5df6 > diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml > index 0fda1d4e..bed5989a 100644 > --- a/v2v/convert_windows.ml > +++ b/v2v/convert_windows.ml > @@ -429,14 +429,10 @@ popd > List.iter ( > fun msi_path -> > let fb_script = "\ > -echo Installing qemu-ga from " ^ msi_path ^ " > -\"\\" ^ msi_path ^ "\" /norestart /qn /l+*vx \"%~dpn0.log\" > -set elvl=!errorlevel! > -echo Done installing qemu-ga error_level=!elvl! > -if !elvl! == 0 ( > - echo Restarting Windows... > - shutdown /r /f /c \"rebooted by firstboot script\" > -) > +echo Removing any previously scheduled qemu-ga installation > +schtasks.exe /Delete /TN Firstboot-qemu-ga /F > +echo Scheduling delayed installation of qemu-ga from " ^ msi_path ^ " > +powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe > /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU > SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\" ^ msi_path ^ " /forcerestart /qn > /l+*vx C:\\" ^ msi_path ^ ".log\\\"\" > " in >Firstboot.add_firstboot_script g inspect.i_root > ("install " ^ msi_path) fb_script; It could be easier to follow if you use sprintf here, something like: let fb_script = sprintf "\ echo Removing any previously scheduled qemu-ga installation schtasks.exe /Delete /TN Firstboot-qemu-ga /F echo Scheduling delayed installation of qemu-ga from %s powershell.exe -command \"$d = (get-date).AddSeconds(120); schtasks.exe /Create /SC ONCE /ST $d.ToString('HH:mm') /SD $d.ToString('MM/dd/') /RU SYSTEM /TN Firstboot-qemu-ga /TR \\\"C:\\%s /forcerestart /qn /l+*vx C:\\%s.log\\\"\" msi_path msi_path msi_path in The other possible problem is this seems to install a firstboot script for every element of the list qemu_ga_files. How long is this list? Do filenames on this list need some kind of quoting? The filenames don't, but they seem to contain a path that comes from the virtio-win ISO. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs