[Libguestfs] [PATCH v2v 3/5] -o libvirt: Add <clock offset="utc|localtime"> to libvirt XML

[Laszlo Ersek]

On 9/25/23 16:04, Richard W.M. Jones wrote:
> ---
>  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"];

I don't think we need to change what "os_section" is bound to, here; I
understand the idea is to increase consistency, but for me it makes the
patch harder to read -- ultimately it is a refactoring, and then adding
in clock_section is the new thing.

If you can split these apart, that's optimal; if not, this is viable too
IMO.

Thanks
Laszlo

> 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>