Libguestfs - Sep 2023 - [PATCH v2v v2 3/6] output/create_libvirt_xml.ml: Refactor os_section

The main changes are:

 - BIOS -> RTC passim

 - Split out the minor refactoring patch

 - Enhanced commit messages

 - Add Laszlo's R-b tag

Rich.

Almost every Linux guest expects the motherboard Real Time Clock (RTC)
to be set to UTC and they adjust the time displayed based on their
timezone (which may be different for each user).

Most Windows guests expect the RTC to be set to the local time.
Windows can be configured to use a UTC clock.  We can detect this by
looking at the Windows registry.

To cope with this difference we need to add a guestcaps flag based on
what we think the guest is expecting.  (We might also use the source
hypervisor RTC setting, but it is not thought to be as reliable as
inspecting the guest.)

This change simply adds the flag to guestcaps, and sets it to always
true, so there is no change to the output.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/types.mli              | 5 +++++
 convert/convert_linux.ml   | 1 +
 convert/convert_windows.ml | 1 +
 lib/types.ml               | 3 +++
 4 files changed, 10 insertions(+)

diff --git a/lib/types.mli b/lib/types.mli
index 65ef2e35cf..3446bb64bd 100644
--- a/lib/types.mli
+++ b/lib/types.mli
@@ -277,6 +277,11 @@ type guestcaps = {
   gcaps_virtio_1_0 : bool;
   (** The guest supports the virtio devices that it does at the virtio-1.0
       protocol level. *)
+
+  gcaps_rtc_utc : bool;
+  (** Is the RTC set to UTC ([true]) or localtime ([false])?  For
+      Linux guests this is always true.  For Windows we find out
+      what the guest is expecting by looking at the registry. *)
 }
 (** Guest capabilities after conversion.  eg. Was virtio found or installed? *)
 
diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
index 8d7020848f..bd54568b10 100644
--- a/convert/convert_linux.ml
+++ b/convert/convert_linux.ml
@@ -220,6 +220,7 @@ let convert (g : G.guestfs) source inspect i_firmware _
keep_serial_console _        gcaps_arch = Utils.kvm_arch inspect.i_arch;
       gcaps_arch_min_version = arch_min_version;
       gcaps_virtio_1_0 = virtio_1_0;
+      gcaps_rtc_utc = true; (* almost all Linux expect RTC to be UTC *)
     } in
 
     guestcaps
diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
index 122d95469d..7e3aa8d789 100644
--- a/convert/convert_windows.ml
+++ b/convert/convert_windows.ml
@@ -275,6 +275,7 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips        gcaps_arch = Utils.kvm_arch inspect.i_arch;
       gcaps_arch_min_version = 0;
       gcaps_virtio_1_0 = virtio_win_installed.Inject_virtio_win.virtio_1_0;
+      gcaps_rtc_utc = true;
     } in
 
     guestcaps
diff --git a/lib/types.ml b/lib/types.ml
index 75c14fd4f6..d6f9a266c5 100644
--- a/lib/types.ml
+++ b/lib/types.ml
@@ -399,6 +399,7 @@ type guestcaps = {
   gcaps_arch : string;
   gcaps_arch_min_version : int;
   gcaps_virtio_1_0 : bool;
+  gcaps_rtc_utc : bool;
 }
 and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
 and guestcaps_net_type = Virtio_net | E1000 | RTL8139
@@ -429,6 +430,7 @@ let string_of_guestcaps gcaps             gcaps_arch = %s\n\
            gcaps_arch_min_version = %d\n\
            gcaps_virtio_1_0 = %b\n\
+           gcaps_rtc_utc = %b\n\
           "
   (string_of_block_type gcaps.gcaps_block_bus)
   (string_of_net_type gcaps.gcaps_net_bus)
@@ -440,6 +442,7 @@ let string_of_guestcaps gcaps    gcaps.gcaps_arch
   gcaps.gcaps_arch_min_version
   gcaps.gcaps_virtio_1_0
+  gcaps.gcaps_rtc_utc
 
 type target_buses = {
   target_virtio_blk_bus : target_bus_slot array;
-- 
2.41.0

Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 output/create_kubevirt_yaml.ml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/output/create_kubevirt_yaml.ml b/output/create_kubevirt_yaml.ml
index 689555e4dc..e3a3f422df 100644
--- a/output/create_kubevirt_yaml.ml
+++ b/output/create_kubevirt_yaml.ml
@@ -54,6 +54,11 @@ let create_kubevirt_yaml source inspect
           "pit", Assoc [ "tickPolicy", String
"delay" ];
           "rtc", Assoc [ "tickPolicy", String
"catchup" ];
         ];
+        (* XXX Note that we may need to set "localtime" here
+         * depending on guestcaps.gcaps_rtc_utc.  However that
+         * requires the following PR to be merged in Kubevirt:
+         * https://github.com/kubevirt/kubevirt/pull/9587
+         *)
         "utc", List []
       ]
     )
-- 
2.41.0

Minor refactoring of how <os> section is generated in XML output.
There is no change in the output.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 output/create_libvirt_xml.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml
index 964acd25fd..12586fde17 100644
--- a/output/create_libvirt_xml.ml
+++ b/output/create_libvirt_xml.ml
@@ -292,10 +292,10 @@ let create_libvirt_xml ?pool source inspect
            e "nvram" ["template", vars_template] [] ] in
 
     List.push_back_list os loader;
-    !os in
+    e "os" [] !os in
 
   List.push_back_list body [
-    e "os" [] os_section;
+    os_section;
 
     e "on_poweroff" [] [PCData "destroy"];
     e "on_reboot" [] [PCData "restart"];
-- 
2.41.0

Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 output/create_libvirt_xml.ml | 6 ++++++
 tests/test-v2v-i-ova.xml     | 1 +
 2 files changed, 7 insertions(+)

diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml
index 12586fde17..30119d1357 100644
--- a/output/create_libvirt_xml.ml
+++ b/output/create_libvirt_xml.ml
@@ -294,8 +294,14 @@ let create_libvirt_xml ?pool source inspect
     List.push_back_list os loader;
     e "os" [] !os in
 
+  (* The <clock> section. *)
+  let clock_section +    let offset = if guestcaps.gcaps_rtc_utc then
"utc" else "localtime" in
+    e "clock" [ "offset", offset ] [] in
+
   List.push_back_list body [
     os_section;
+    clock_section;
 
     e "on_poweroff" [] [PCData "destroy"];
     e "on_reboot" [] [PCData "restart"];
diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml
index e5907ea1cc..a41827bfd5 100644
--- a/tests/test-v2v-i-ova.xml
+++ b/tests/test-v2v-i-ova.xml
@@ -18,6 +18,7 @@
   <os>
     <type arch='x86_64' machine='q35'>hvm</type>
   </os>
+  <clock offset='utc'/>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
-- 
2.41.0

I didn't set the -rtc flag in the normal (UTC) case as that is the
default.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 output/output_qemu.ml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/output/output_qemu.ml b/output/output_qemu.ml
index 57b1e58da2..1a5f7f7147 100644
--- a/output/output_qemu.ml
+++ b/output/output_qemu.ml
@@ -158,6 +158,8 @@ module QEMU = struct
         arg_list "-device" ["vmgenid"; sprintf
"guid=%s" genid; "id=vmgenid0"]
     );
 
+    if not guestcaps.gcaps_rtc_utc then arg "-rtc"
"base=localtime";
+
     arg_list "-machine" (machine_str ::
                            (if smm then ["smm=on"] else []) @
                            ["accel=kvm:tcg"]);
-- 
2.41.0

Read HKLM\SYSTEM\CurrentControlSet\Control\TimeZoneInformation key
"RealTimeIsUniversal" to see if the Windows guest is expecting RTC
set to localtime (not present) or UTC (present and set to 1).

See: https://wiki.archlinux.org/title/System_time#UTC_in_Microsoft_Windows
See:
https://listman.redhat.com/archives/libguestfs/2023-September/thread.html#32556
Reported-by: Lee Garrett
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/convert_windows.ml | 38 +++++++++++++++++++++++++++++++++++++-
 tests/test-v2v-i-ova.xml   |  2 +-
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
index 7e3aa8d789..34cf341b8e 100644
--- a/convert/convert_windows.ml
+++ b/convert/convert_windows.ml
@@ -103,6 +103,42 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips    (* If the Windows guest has AV installed. *)
   let has_antivirus = Windows.detect_antivirus inspect in
 
+  (* Does the guest expect the RTC to be set to UTC or localtime?
+   * See https://wiki.archlinux.org/title/System_time#UTC_in_Microsoft_Windows
+   * Note this might be a QWORD on 64 bit Windows instances.
+   *)
+  let rtc_utc +    Registry.with_hive_readonly g inspect.i_windows_system_hive
+      (fun reg ->
+       try
+         let key_path = [ "Control"; "TimeZoneInformation"
] in
+         let path = inspect.i_windows_current_control_set :: key_path in
+         let node +           match Registry.get_node reg path with
+           | None -> raise Not_found
+           | Some node -> node in
+         let valueh = g#hivex_node_get_value node
"RealTimeIsUniversal" in
+         if valueh = 0L then raise Not_found;
+         let t = g#hivex_value_type valueh in
+         let data = g#hivex_value_value valueh in
+         let is_utc +           match t with
+           | 0_L (* REG_NONE *) -> false (* localtime *)
+           | 4_L (* REG_DWORD *) -> data = "\001\000\000\000"
+           | 11_L (* REG_QWORD *) -> data =
"\001\000\000\000\000\000\000\000"
+           | _ (* who knows ... *) ->
+             warning (f_"unknown CurrentControlSet\\Control\\\
+                         TimeZoneInformation key RealTimeIsUniversal \
+                         type 0x%Lx, assuming RTC set to UTC") t;
+             true in
+         is_utc
+       with Not_found ->
+         (* If the key is not found then by default we assume
+          * that Windows is expecting the RTC to be set to localtime.
+          *)
+         false
+      ) in
+
   (* Open the software hive (readonly) and find the Xen PV uninstaller,
    * if it exists.
    *)
@@ -275,7 +311,7 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips        gcaps_arch = Utils.kvm_arch inspect.i_arch;
       gcaps_arch_min_version = 0;
       gcaps_virtio_1_0 = virtio_win_installed.Inject_virtio_win.virtio_1_0;
-      gcaps_rtc_utc = true;
+      gcaps_rtc_utc = rtc_utc;
     } in
 
     guestcaps
diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml
index a41827bfd5..fa7b4dbfc5 100644
--- a/tests/test-v2v-i-ova.xml
+++ b/tests/test-v2v-i-ova.xml
@@ -18,7 +18,7 @@
   <os>
     <type arch='x86_64' machine='q35'>hvm</type>
   </os>
-  <clock offset='utc'/>
+  <clock offset='localtime'/>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
-- 
2.41.0