On Mon, Sep 25, 2023 at 01:25:06PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 25, 2023 at 01:14:40PM +0100, Richard W.M. Jones wrote:
> > On Mon, Sep 25, 2023 at 01:09:15PM +0200, Lee Garrett wrote:
> > > On 23.09.23 19:37, Laszlo Ersek wrote:
> > > >On 9/22/23 16:47, Lee Garrett wrote:
> > > >>On 22.09.23 14:54, Richard W.M. Jones wrote:
> > > >>>On Fri, Sep 22, 2023 at 11:40:03AM +0100, Richard W.M. Jones wrote:
> > > >>>>On Thu, Sep 21, 2023 at 07:47:52PM +0200, Lee Garrett wrote:
> > > >>>>>On 21.09.23 19:43, Richard W.M. Jones wrote:
> > > >>>>>>So this is probably another instance or variation of the timezone
> > > >>>>>>formatting problem (of schtasks).  Which version of virt-v2v is 
> > > >>>>>>this?
> > > >>>>>>I want to check that you have a version with all the latest patches 
> > > >>>>>>in
> > > >>>>>>this area.
> > > >>>>>
> > > >>>>>It's 2.2.0-1 from Debian (12) bookworm. I've verified that it
> > > >>>>>doesn't have any distro-specific patches.
> > > >>>>>
> > > >>>>>(https://salsa.debian.org/libvirt-team/virt-v2v/-/tree/debian/master/debian
> > > >>>>>would have a patches/series file in this case)
> > > >>>>
> > > >>>>The timezone fixes are:
> > > >>>>
> > > >>>>commit 597d177567234c3a539098c423649781424eeb6f
> > > >>>>Author: Laszlo Ersek <ler...@redhat.com>
> > > >>>>Date:   Tue Mar 8 15:30:51 2022 +0100
> > > >>>>
> > > >>>>      convert_windows: rewrite "configure_qemu_ga" script purely in
> > > >>>>PowerShell
> > > >>>>
> > > >>>>commit d9dc6c42ae64ba92993dbd9477f003ba73fcfa2f
> > > >>>>Author: Richard W.M. Jones <rjo...@redhat.com>
> > > >>>>Date:   Fri Nov 12 08:47:55 2021 +0000
> > > >>>>
> > > >>>>      convert/convert_windows.ml: Handle date formats with dots
> > > >>>>instead of /
> > > >>>>
> > > >>>>They are all included in >= 2.0
> > > >>>>
> > > >>>>I wonder if 597d177567 has a subtle flaw, or if we introduced a bug
> > > >>>>somewhere when refactoring this code later.
> > > >>>>
> > > >>>>Lee: Do you have a theory about exactly what is wrong with the
> > > >>>>schtasks date?  Like what was it supposed to be, assuming it was 120
> > > >>>>seconds in the future from boot time, versus what it was set to:
> > > >>>>
> > > >>>>>Firstboot-qemu-ga                        9/21/2023 4:04:00 PM   Ready
> > > >>>>
> > > >>>>Could a date or time field have not been swapped or been corrupted
> > > >>>>in some predictable way?
> > > >>>
> > > >>>Or in even simpler terms, what is the time (and timezone) that
> > > >>>this ^^^ machine was booted?
> > > >>
> > > >>I believe I have it figured out.
> > > >>The guest local time is currently 7:08 AM (a few minutes after
> > > >>firstboot/provisioning), pacific daylight time (UTC-7, though Windows
> > > >>displays it as "UTC-08:00"). This is the timezone that the guest comes
> > > >>configured with at first boot. The task is scheduled for 2:01 PM,
> > > >>meaning it's scheduled to run ~7 hours in the future.
> > > >>
> > > >>So it seems like the task was meant to be scheduled for 2:01 PM UTC (=
> > > >>7:01 AM PDT), but for some reason was scheduled for 2:01 PM *local 
> > > >>time*.
> > > >>
> > > >> From what I can see, the host machine time zone is irrelevant (UTC+2).
> > > >>
> > > >>I don't know where the timezone mixup comes from, though. Running
> > > >>`(get-date)` in the powershell at this point correctly returns the local
> > > >>time (7:08 AM). I guess during injection the time is in UTC, and
> > > >>schtasks.exe has no awareness of timezones?
> > > >
> > > >Right, I think there is a timezone disagreement between how we format 
> > > >the timestamp and how schtasks.exe takes it.
> > > >
> > > >What matters here is the /ST (start time) flag.
> > > >
> > > >Today we have (in the common submodule):
> > > >
> > > >       add "$d = (get-date).AddSeconds(120)";
> > > >       add "$dtfinfo = 
> > > > [System.Globalization.DateTimeFormatInfo]::CurrentInfo";
> > > >       add "$sdp = $dtfinfo.ShortDatePattern";
> > > >       add "$sdp = $sdp -replace 'y+', 'yyyy'";
> > > >       add "$sdp = $sdp -replace 'M+', 'MM'";
> > > >       add "$sdp = $sdp -replace 'd+', 'dd'";
> > > >       add "schtasks.exe /Create /SC ONCE `";
> > > >       add "  /ST $d.ToString('HH:mm') /SD $d.ToString($sdp) `";
> > > >       add "  /RU SYSTEM /TN Firstboot-qemu-ga `";
> > > >       add (sprintf "  /TR \"C:\\%s /forcerestart /qn /l+*vx 
> > > > C:\\%s.log\""
> > > >              msi_path msi_path);
> > > >
> > > >Note that for the /ST option's argument, we only perform the following 
> > > >steps:
> > > >
> > > >   $d = (get-date).AddSeconds(120)
> > > >
> > > >   /ST $d.ToString('HH:mm')
> > > >
> > > >This actually goes back to commit dc66e78fa37d ("windows: delay 
> > > >installation of qemu-ga MSI", 2020-03-10). The timestamp massaging we've 
> > > >since done only targeted the /SD (start date) option, not the start time 
> > > >(/ST) one!
> > > >
> > > >So the problem may be that
> > > >
> > > >   (get-date).AddSeconds(120).ToString('HH:mm')
> > > >
> > > >formats the hour:minute timestamp in UTC (why though?), but the /ST 
> > > >option takes hour:minute in local time.
> > > >
> > > >Interestingly, DateTime objects seem to have a "Kind" property, which 
> > > >may be Utc, Local, or Unspec.
> > > >
> > > >https://learn.microsoft.com/en-us/dotnet/api/system.datetime.kind
> > > >
> > > >It seems to be used when converting from UTC to local or vice versa, and 
> > > >it probably influences how ToString() behaves too. I thought "get-date" 
> > > >returned a Local one, and /ST took a local one as well, but perhaps 
> > > >"get-date" returns a UTC timestamp in this case (when run from the 
> > > >script)?
> > > 
> > > I think I have an idea what's happening. This is the part of the XML
> > > description of the guest:
> > > 
> > > <clock offset="utc"/>
> > 
> > Dan:
> > 
> > For virt-v2v of Windows guests do you think we should always force
> > <clock offset="localtime"/> (but leave it at "utc" for non-Windows)?
> 
> Generally 'localtime' is best for a default Windows install.
> 
> IIUC, there is a way to tell Windows to assume the RTC is
> always in UTC, but its default behaviour is to assume localtime.

It turns out there's a Windows registry key we could check:

  https://wiki.archlinux.org/title/System_time#UTC_in_Microsoft_Windows

According to this:

  
https://superuser.com/questions/975717/does-windows-10-support-utc-as-bios-time

... it seems like it's a QWORD (type 'b') rather than a DWORD for 64
bit Windows variants, but that doesn't matter as we would only need to
read the registry and not modify it.  The key seems to be completely
missing for normal (localtime) Windows guests that I have examined.

I checked virt-v2v and we don't do this right now.  In fact we don't
set the <clock> element at all, we just leave it up to libvirt or
other tools.

As this seems quite important, if someone wants to file a bug
(https://issues.redhat.com/ -> select "RHEL" as the project)
I can look at implementing this.

Rich.

> > I'm not clear what exactly should be the source of truth here.  I
> > don't see anything in osinfo-db, unless I'm missing something.
> 
> Yeah, we don't record anything in this area.
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

-- 
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
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to