[tools 1/3] testbot/LibvirtTool: Wait a bit between shutting down and reverting a VM.

[Francois Gouget]

Also rename ShutDown() to the more descriptive ShutDownIfOffSnapshot()
and document it.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
Without this delay reverting from a PCI-passthrough configuration to a
regular VGA live snapshot sometimes results in a black VM screen.
---
 testbot/bin/LibvirtTool.pl | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl
index ef94a2be09..1fc7dc1d4c 100755
--- a/testbot/bin/LibvirtTool.pl
+++ b/testbot/bin/LibvirtTool.pl
@@ -280,7 +280,20 @@ sub GetPrivilegedTA($)
   return $TA->SetTime() ? $TA : $VM->GetAgent(1);
 }
 
-sub ShutDown()
+=pod
+=over 12
+
+=item C<ShutDownIfOffSnapshot()>
+
+Attempt a clean shutdown of the VM if it is running a snapshot that requests
+them (snapshot name containing '-off').
+
+Returns 0 if the shutdown failed, 2 if it was successful and 1 if the VM was
+already powered off or a shutdown was not needed.
+=back
+=cut
+
+sub ShutDownIfOffSnapshot()
 {
   my $Domain = $VM->GetDomain();
   my $CurrentSnapshot = $Domain->GetSnapshotName();
@@ -311,7 +324,7 @@ sub ShutDown()
     {
       Debug(Elapsed($Start), " Successfully shut down $VMKey\n");
       LogMsg "Successfully shut down $VMKey\n";
-      return 1;
+      return 2;
     }
     sleep(1);
   }
@@ -365,7 +378,7 @@ sub Monitor()
     if ($IsReady and $VM->GetDomain()->IsPoweredOn())
     {
       # Try to perform a clean shutdown if requested
-      ShutDown();
+      ShutDownIfOffSnapshot();
 
       my $ErrMessage = $VM->GetDomain()->PowerOff();
       if (defined $ErrMessage)
@@ -391,7 +404,7 @@ sub Monitor()
 sub PowerOff()
 {
   # Try to perform a clean shutdown if requested
-  ShutDown();
+  ShutDownIfOffSnapshot();
 
   # Power off the VM no matter what its initial status is
   $CurrentStatus = $VM->Status;
@@ -431,7 +444,7 @@ sub CheckOff()
     if ($SnapshotName eq $VM->IdleSnapshot)
     {
       # Try to perform a clean shutdown if requested
-      ShutDown();
+      ShutDownIfOffSnapshot();
 
       my $ErrMessage = $VM->GetDomain()->PowerOff();
       FatalError("$ErrMessage\n") if (defined $ErrMessage);
@@ -612,8 +625,10 @@ sub Revert()
     $VM->Save();
   }
 
-  # Before reverting, try to perform a clean shutdown if requested
-  ShutDown();
+  # Before reverting, try to perform a clean shutdown if requested.
+  # After a shutdown a small wait is also sometimes required, at least when
+  # switching from a PCI-passthrough configuration to a regular one.
+  sleep(1) if (ShutDownIfOffSnapshot() == 2);
 
   # Revert the VM (and power it on if necessary)
   Debug(Elapsed($Start), " Reverting $VMKey to $DomainSnapshot\n");
-- 
2.30.2

[tools 2/3] testbot/LibvirtTool: Skip creating the intermediate -live snapshot.

[Francois Gouget]

Basing the localized snapshots on the plain -live snapshot is not very
useful and skipping this intermediate step makes the code simpler.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/bin/LibvirtTool.pl | 43 +++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl
index 1fc7dc1d4c..90df848df1 100755
--- a/testbot/bin/LibvirtTool.pl
+++ b/testbot/bin/LibvirtTool.pl
@@ -604,6 +604,7 @@ sub Revert()
     # Add some extra time to set up the VM locale and reboot it
     $ExtraTimeout += $VMToolTimeout;
     $SetLocale = "$1-$2";
+    $CreateSnapshot = 1;
     Debug(Elapsed($Start), " $VMKey does not yet have a $DomainSnapshot-$SetLocale snapshot\n");
   }
   if (!$Domain->HasSnapshot($DomainSnapshot) and $DomainSnapshot =~ s/-live$//)
@@ -656,7 +657,7 @@ sub Revert()
   # Mark the VM as sleeping which allows the scheduler to abort the revert in
   # favor of higher priority tasks. But don't allow interruptions in the
   # middle of snapshot creation!
-  if (!$CreateSnapshot and !$SetLocale)
+  if (!$CreateSnapshot)
   {
     return 1 if (ChangeStatus("reverting", "sleeping"));
   }
@@ -665,10 +666,22 @@ sub Revert()
   # reboot so reset start.count in that case.
   SetupTestAgentd($Booting, ($CreateSnapshot or $SetLocale), $SetLocale);
 
+  # Set up the VM locale
+  if ($SetLocale)
+  {
+    Debug(Elapsed($Start), " Setting up the $SetLocale locale on $VMKey\n");
+    my @Cmd = ("$BinDir/SetWinLocale", $VM->Hostname, "--default", $SetLocale);
+    push @Cmd, "--debug" if ($Debug);
+    if (system(@Cmd))
+    {
+      FatalError("Could not set the $VMKey locale to $SetLocale\n");
+    }
+    $Booting = 1;
+  }
+
   if ($CreateSnapshot)
   {
-    $DomainSnapshot .= "-live";
-    CreateSnapshot($Domain, $DomainSnapshot);
+    CreateSnapshot($Domain, $VM->IdleSnapshot);
 
     if ($VM->Type eq "build" or $VM->Type eq "wine")
     {
@@ -687,23 +700,11 @@ sub Revert()
         Debug(Elapsed($Start), " Added a job to update and rebuild Wine on $VMKey\n");
       }
     }
-  }
 
-  # Set up the VM locale
-  if ($SetLocale)
-  {
-    Debug(Elapsed($Start), " Setting up the $SetLocale locale on $VMKey\n");
-    my @Cmd = ("$BinDir/SetWinLocale", $VM->Hostname, "--default", $SetLocale);
-    push @Cmd, "--debug" if ($Debug);
-    if (system(@Cmd))
-    {
-      FatalError("Could not set the $VMKey locale to $SetLocale\n");
-    }
-
-    $DomainSnapshot .= "-$SetLocale";
-    CreateSnapshot($Domain, $DomainSnapshot);
+    # The activity monitor does not like it when VMs skip the sleeping step
+    return 1 if (ChangeStatus("reverting", "sleeping"));
   }
-  elsif (!$CreateSnapshot)
+  else
   {
     my $Sleep = ($Booting and $SleepAfterBoot > $SleepAfterRevert) ?
                 $SleepAfterBoot : $SleepAfterRevert;
@@ -712,12 +713,6 @@ sub Revert()
     sleep($Sleep);
   }
 
-  if ($CreateSnapshot or $SetLocale)
-  {
-    # The activity monitor does not like it when VMs skip the sleeping step
-    return 1 if (ChangeStatus("reverting", "sleeping"));
-  }
-
   return ChangeStatus($CurrentStatus, "idle", "done");
 }
 
-- 
2.30.2

[tools 3/3] testbot/LibvirtTool: Clean up the snapshot name parsing.

[Francois Gouget]

This also makes it easier to extend it and pass all the configuration
around.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/bin/LibvirtTool.pl | 95 ++++++++++++++++++++++----------------
 1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl
index 90df848df1..cfdb251694 100755
--- a/testbot/bin/LibvirtTool.pl
+++ b/testbot/bin/LibvirtTool.pl
@@ -6,7 +6,7 @@
 # network trouble, and thus are best performed in a separate process.
 #
 # Copyright 2009 Ge van Geldorp
-# Copyright 2012-2019 Francois Gouget
+# Copyright 2012-2021 Francois Gouget
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -565,6 +565,27 @@ sub SetupTestAgentd($$$)
   $TA->Disconnect();
 }
 
+sub GetSnapshotConfig($)
+{
+  my ($Config) = @_;
+
+  while (1)
+  {
+    if ($Config->{base} =~ s/-([a-z]{2}-[A-Z]{2})$//)
+    {
+      $Config->{locale} = $1;
+    }
+    elsif ($Config->{base} =~ s/-(live)$//)
+    {
+      $Config->{$1} = 1;
+    }
+    else
+    {
+      last;
+    }
+  }
+}
+
 sub CreateSnapshot($$)
 {
   my ($Domain, $SnapshotName) = @_;
@@ -592,35 +613,32 @@ sub Revert()
     return 1;
   }
   $CurrentStatus = "reverting";
-  my $DomainSnapshot = $VM->IdleSnapshot;
-  my $ExtraTimeout = 0;
-  my ($SetLocale, $CreateSnapshot);
+  my $Config = { base => $VM->IdleSnapshot };
 
   my $Domain = $VM->GetDomain();
-  if (($VM->Type eq "win32" or $VM->Type eq "win64") and
-      !$Domain->HasSnapshot($DomainSnapshot) and
-      $DomainSnapshot =~ s/-([a-z]{2})-([A-Z]{2})$//)
-  {
-    # Add some extra time to set up the VM locale and reboot it
-    $ExtraTimeout += $VMToolTimeout;
-    $SetLocale = "$1-$2";
-    $CreateSnapshot = 1;
-    Debug(Elapsed($Start), " $VMKey does not yet have a $DomainSnapshot-$SetLocale snapshot\n");
-  }
-  if (!$Domain->HasSnapshot($DomainSnapshot) and $DomainSnapshot =~ s/-live$//)
-  {
-    # Add some extra time to boot the VM and create the live snapshot
-    $ExtraTimeout += $WaitForBoot + $VMToolTimeout / 2;
-    $CreateSnapshot = 1;
-    Debug(Elapsed($Start), " $VMKey does not yet have a $DomainSnapshot-live snapshot\n");
-  }
-  if (!$Domain->HasSnapshot($DomainSnapshot))
-  {
-    FatalError("Could not find $VMKey\'s $DomainSnapshot snapshot\n");
-  }
-  if ($ExtraTimeout)
+  if (!$Domain->HasSnapshot($Config->{base}))
   {
+    $Config->{create} = 1;
+    GetSnapshotConfig($Config);
+    if ($Config->{locale} and $VM->Type !~ /^win(?:32|64)$/)
+    {
+      FatalError("Creating locale snapshots ($Config->{locale}) is not supported for ". $VM->Type ." VMs ($VMKey)\n");
+    }
+    if (!$Config->{live})
+    {
+      FatalError("Only live snapshots can be created ($VMKey)\n");
+    }
+    if (!$Domain->HasSnapshot($Config->{base}))
+    {
+      FatalError("Could not find $VMKey\'s $Config->{base} snapshot\n");
+    }
+
+    # Extend the deadline to have time to set up the VM and (re)boot it.
+    # Add $WaitForBoot even if no reboot is needed in the end.
+    my $ExtraTimeout = $WaitForBoot + $VMToolTimeout;
     Debug(Elapsed($Start), " Extending the $VMKey revert deadline by ${ExtraTimeout}s\n");
+    # ChildDeadline is undefined if the VM (or another sharing the same
+    # hypervisor domain) is in maintenance mode.
     my $Deadline = $VM->ChildDeadline || (time() + $VMToolTimeout);
     $VM->ChildDeadline($Deadline + $ExtraTimeout);
     $VM->Save();
@@ -632,8 +650,8 @@ sub Revert()
   sleep(1) if (ShutDownIfOffSnapshot() == 2);
 
   # Revert the VM (and power it on if necessary)
-  Debug(Elapsed($Start), " Reverting $VMKey to $DomainSnapshot\n");
-  my ($ErrMessage, $Booting) = $Domain->RevertToSnapshot($DomainSnapshot);
+  Debug(Elapsed($Start), " Reverting $VMKey to $Config->{base}\n");
+  my ($ErrMessage, $Booting) = $Domain->RevertToSnapshot($Config->{base});
   if (defined $ErrMessage)
   {
     # Libvirt/QEmu is buggy and cannot revert a running VM from one hardware
@@ -646,40 +664,39 @@ sub Revert()
       FatalError("Could not power off $VMKey: $ErrMessage\n");
     }
 
-    Debug(Elapsed($Start), " Reverting $VMKey to $DomainSnapshot... again\n");
-    ($ErrMessage, $Booting) = $Domain->RevertToSnapshot($DomainSnapshot);
+    Debug(Elapsed($Start), " Reverting $VMKey to $Config->{base}... again\n");
+    ($ErrMessage, $Booting) = $Domain->RevertToSnapshot($Config->{base});
   }
   if (defined $ErrMessage)
   {
-    FatalError("Could not revert $VMKey to $DomainSnapshot: $ErrMessage\n");
+    FatalError("Could not revert $VMKey to $Config->{base}: $ErrMessage\n");
   }
 
   # Mark the VM as sleeping which allows the scheduler to abort the revert in
   # favor of higher priority tasks. But don't allow interruptions in the
   # middle of snapshot creation!
-  if (!$CreateSnapshot)
+  if (!$Config->{create})
   {
     return 1 if (ChangeStatus("reverting", "sleeping"));
   }
 
   # Set up the TestAgent server. Note that setting the locale will require a
   # reboot so reset start.count in that case.
-  SetupTestAgentd($Booting, ($CreateSnapshot or $SetLocale), $SetLocale);
+  SetupTestAgentd($Booting, $Config->{create}, $Config->{locale});
 
   # Set up the VM locale
-  if ($SetLocale)
+  if ($Config->{locale})
   {
-    Debug(Elapsed($Start), " Setting up the $SetLocale locale on $VMKey\n");
-    my @Cmd = ("$BinDir/SetWinLocale", $VM->Hostname, "--default", $SetLocale);
+    Debug(Elapsed($Start), " Setting up the $Config->{locale} locale on $VMKey\n");
+    my @Cmd = ("$BinDir/SetWinLocale", $VM->Hostname, "--default", $Config->{locale});
     push @Cmd, "--debug" if ($Debug);
     if (system(@Cmd))
     {
-      FatalError("Could not set the $VMKey locale to $SetLocale\n");
+      FatalError("Could not set the $VMKey locale to $Config->{locale}\n");
     }
-    $Booting = 1;
   }
 
-  if ($CreateSnapshot)
+  if ($Config->{create})
   {
     CreateSnapshot($Domain, $VM->IdleSnapshot);
 
-- 
2.30.2