Libguestfs - Sep 2023 - [PATCH v2v 2/5] -o kubevirt: Add comment about future support for clock = localtime

[Alice: See patch 2]

[This patch is a bit rough, it could do with better commit messages
and some tests.  Please test it to see if it solves the Windows
conversion issue described in the thread below.]

We currently do not set any <clock/> field in guest output.  Most
Windows guests expect the BIOS to be set to localtime, whereas almost
all Linux guests would expect it to be set to UTC.  It is also
possible to configure a Windows guest to expect BIOS set to UTC.

The default is usually BIOS set to UTC, so for many Windows guests
this would be wrong.  This specifically may cause problems when
scheduling qemu-ga installation, see the thread here:

https://listman.redhat.com/archives/libguestfs/2023-September/thread.html#32556

but could cause other general issues with time in the guest.

One way to implement this would be to copy the source hypervisor
information across; however I'm not confident this information is read
correctly.  A better way is to read out what the guest is expecting
from the Windows registry.  (For Linux we just assume BIOS is always
UTC, since that's the default for almost any Linux guest which hasn't
been dual-booted with Windows, which for VMs would be incredibly
rare.)

Rich.

This change simply adds the flag to guestcaps, and sets it to always
true, so there is no change to the output.
---
 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..44b35faeb0 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_bios_utc : bool;
+  (** Is the BIOS 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..a70d43224c 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_bios_utc = true; (* almost all Linux expect BIOS to be UTC *)
     } in
 
     guestcaps
diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
index 122d95469d..f6e039be7e 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_bios_utc = true;
     } in
 
     guestcaps
diff --git a/lib/types.ml b/lib/types.ml
index 75c14fd4f6..0f659ebd9a 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_bios_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_bios_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_bios_utc
 
 type target_buses = {
   target_virtio_blk_bus : target_bus_slot array;
-- 
2.41.0

---
 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..b96c742832 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_bios_utc.  However that
+         * requires the following PR to be merged in Kubevirt:
+         * https://github.com/kubevirt/kubevirt/pull/9587
+         *)
         "utc", List []
       ]
     )
-- 
2.41.0

---
 output/create_libvirt_xml.ml | 10 ++++++++--
 tests/test-v2v-i-ova.xml     |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml
index 964acd25fd..f97272ca31 100644
--- a/output/create_libvirt_xml.ml
+++ b/output/create_libvirt_xml.ml
@@ -292,10 +292,16 @@ 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
+
+  (* The <clock> section. *)
+  let clock_section +    let offset = if guestcaps.gcaps_bios_utc then
"utc" else "localtime" in
+    e "clock" [ "offset", offset ] [] in
 
   List.push_back_list body [
-    e "os" [] os_section;
+    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.
---
 output/output_qemu.ml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/output/output_qemu.ml b/output/output_qemu.ml
index 57b1e58da2..a9d7b37071 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_bios_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 BIOS
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
---
 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 f6e039be7e..84e8f7b7d3 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 BIOS 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 bios_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 BIOS 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 BIOS 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_bios_utc = true;
+      gcaps_bios_utc = bios_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

Hi Rich,

On Mon, Sep 25, 2023 at 4:10?PM Richard W.M. Jones <rjones at redhat.com>
wrote:
> [Alice: See patch 2]
>
I'm not 100% sure about the source of this work. However, we had in
KubeVirt people interested in using localtime with Windows [1]. Yes, I see
that you pointed to that PR in patch 2. The problem with that PR is the
migration. What happens if we migrate the VM to a host that is in another
timezone? Is it going to break the application running inside the VM?

IMO, that was the major blocker of that PR. I initially suggested that we
either avoid migration as a first step or try to migrate only on nodes at
the same timezone,
Do you know if anyone has experience on this particular problem?

[1] https://github.com/kubevirt/kubevirt/pull/9587

Many thanks,
Alice
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20230925/c4fe1963/attachment.htm>

On Mon, Sep 25, 2023 at 04:26:59PM +0200, Alice Frosi
wrote:
> Hi Rich,
> 
> On Mon, Sep 25, 2023 at 4:10?PM Richard W.M. Jones <rjones at
redhat.com> wrote:
> 
>     [Alice: See patch 2]
> 
>
> I'm not 100% sure about the source of this work. However, we had in
> KubeVirt people interested in using localtime with Windows [1]. Yes,
> I see that you pointed to that PR in patch 2. The problem with that
> PR is the migration. What happens if we migrate the VM to a host
> that is in another timezone? Is it going to break the application
> running inside the VM?
Does every Windows VM running in Kubevirt have its registry adjusted
so it expects the BIOS to be set to UTC?  (ie:
https://wiki.archlinux.org/title/System_time#UTC_in_Microsoft_Windows).
This is a non-standard setting, but I can't understand how Windows VMs
would have correct clocks if this were not the case.

BTW you could find out by running:

  $ virt-win-reg window.img 'HKLM\SYSTEM'

on some Kubevirt Windows VMs, and looking for:

  [\ControlSet001\Control\TimeZoneInformation]

in the output.  See if there is a key called "RealTimeIsUniversal" and
what it is set to.

If it is the case that this is always set for existing Kubevirt
guests, then virt-v2v would have to be changed so it also sets this
registry entry when the output is Kubevirt.

I agree that migration across timezones is highly problematic for the
BIOS = localtime situation.  I don't see any solution for that except
disallowing it.
> IMO, that was the major blocker of that PR. I initially suggested that we
> either avoid migration as a first step or try to migrate only on nodes at
the
> same timezone,
> Do you know if anyone has experience on this particular problem?
I guess RHV must have encountered this before, but I don't know any
specifics.

Thanks,

Rich.
> [1]?https://github.com/kubevirt/kubevirt/pull/9587
> 
> Many thanks,
> Alice
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On 9/25/23 16:04, Richard W.M. Jones wrote:
> [Alice: See patch 2]
> 
> [This patch is a bit rough, it could do with better commit messages
> and some tests.  Please test it to see if it solves the Windows
> conversion issue described in the thread below.]
> 
> We currently do not set any <clock/> field in guest output.  Most
> Windows guests expect the BIOS to be set to localtime, whereas almost
> all Linux guests would expect it to be set to UTC.  It is also
> possible to configure a Windows guest to expect BIOS set to UTC.
> 
> The default is usually BIOS set to UTC, so for many Windows guests
> this would be wrong.  This specifically may cause problems when
> scheduling qemu-ga installation, see the thread here:
> 
>
https://listman.redhat.com/archives/libguestfs/2023-September/thread.html#32556
> 
> but could cause other general issues with time in the guest.
> 
> One way to implement this would be to copy the source hypervisor
> information across; however I'm not confident this information is read
> correctly.  A better way is to read out what the guest is expecting
> from the Windows registry.  (For Linux we just assume BIOS is always
> UTC, since that's the default for almost any Linux guest which
hasn't
> been dual-booted with Windows, which for VMs would be incredibly
> rare.)
I think the word "BIOS" is incorrectly used all over the series;
I'd
rather say "RTC" / "real time clock".

Laszlo