Francois Gouget : testbot: Avoid blocking virtualization functions in the Engine initialization.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 548f7e305da1257b7b5e9a61d597c3d3be89f442
URL:    http://source.winehq.org/git/tools.git/?a=commit;h=548f7e305da1257b7b5e9a61d...

Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   Wed Oct 18 20:16:44 2017 +0200

testbot: Avoid blocking virtualization functions in the Engine initialization.

The Cleanup() method now uses LibvirtTool to verify that idle VMs are
still good and to power off VMs.
Note that the VMs are marked dirty while LibvirtTool performs the idle
check to prevent the scheduler from using them.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 testbot/bin/Engine.pl                    | 56 ++++++++++++++------------------
 testbot/bin/LibvirtTool.pl               | 24 ++++++++++++--
 testbot/lib/WineTestBot/LibvirtDomain.pm | 21 ++++++++++++
 testbot/lib/WineTestBot/VMs.pm           | 17 ++++++++++
 4 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index 9c461df..02c361c 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -112,32 +112,27 @@ sub Cleanup(;$$)
       foreach my $Task (@{$Tasks->GetItems()})
       {
         my $TaskKey = join("/", $Job->Id, $Step->No, $Task->No);
+        my $ChildPid = $Task->ChildPid;
+        $ChildPid = undef if (defined $ChildPid and !kill(0, $ChildPid));
 
         my $Requeue;
-        if (!defined $Task->ChildPid)
+        if (!defined $ChildPid)
         {
           # That task's process died somehow.
           $Requeue = 1;
         }
-        elsif (!$Task->VM->GetDomain()->IsPoweredOn())
+        elsif ($Task->VM->Status ne "running")
         {
-          # A running task should have a powered on VM.
+          # The Task and VM status should match.
           $Requeue = 1;
-          # Kill the task process if it's still there.
-          kill("TERM", $Task->ChildPid);
+          kill("TERM", $ChildPid);
         }
         elsif ($KillTasks)
         {
           # Kill the task and requeue. Note that since the VM is still running
           # we're not in the computer reboot case so ChildPid is probably
           # still valid.
-          kill("TERM", $Task->ChildPid);
-          $Requeue = 1;
-        }
-        elsif (!kill(0, $Task->ChildPid))
-        {
-          # The event that caused the WineTestBot server to restart probably
-          # also killed the task's child process.
+          kill("TERM", $ChildPid);
           $Requeue = 1;
         }
         else
@@ -181,36 +176,35 @@ sub Cleanup(;$$)
       next;
     }
 
-    my $Domain = $VM->GetDomain();
-    if ($Domain->IsPoweredOn())
+    if ($VM->HasRunningChild())
     {
       if ($KillVMs)
       {
         $VM->KillChild();
+        $VM->RunPowerOff();
       }
-      elsif ($VM->Status eq "idle")
+      elsif (!$VM->CanHaveChild())
       {
-        # Assume these are still ready for use
-        next;
-      }
-      elsif ($VM->CanHaveChild() and $VM->HasRunningChild())
-      {
-        # This VM is still being reverted. Let that process run its course.
-        LogMsg "$VMKey is being reverted\n";
-        next;
+        # The VM should not have a process.
+        $VM->KillChild();
+        $VM->RunPowerOff();
       }
-      LogMsg "Powering off $VMKey\n";
-      $Domain->PowerOff();
+      # else let the process finish its work
     }
     else
     {
-      next if ($VM->Status eq "off");
-      LogMsg "Marking $VMKey as off\n";
+      if ($VM->Status eq "idle")
+      {
+        $VM->RunCheckIdle();
+      }
+      else
+      {
+        # Power off the VM, even if its status is already off.
+        # This is the simplest way to resync the VM status field.
+        # Also powering off a powered off VM will detect offline VMs.
+        $VM->RunPowerOff();
+      }
     }
-
-    $VM->Status('off');
-    $VM->ChildPid(undef);
-    $VM->Save();
   }
 }
 
diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl
index 64f0575..fcc0e81 100755
--- a/testbot/bin/LibvirtTool.pl
+++ b/testbot/bin/LibvirtTool.pl
@@ -104,7 +104,7 @@ while (@ARGV)
   {
     $LogOnly = 1;
   }
-  elsif ($Arg =~ /^(?:poweroff|revert)$/)
+  elsif ($Arg =~ /^(?:checkidle|poweroff|revert)$/)
   {
     $Action = $Arg;
   }
@@ -163,7 +163,7 @@ if (!defined $Usage)
 }
 if (defined $Usage)
 {
-  print "Usage: $Name0 [--debug] [--log-only] [--help] (poweroff|revert) VMName\n";
+  print "Usage: $Name0 [--debug] [--log-only] [--help] (checkidle|poweroff|revert) VMName\n";
   exit $Usage;
 }
 
@@ -261,6 +261,20 @@ sub PowerOff()
   return ChangeStatus(undef, "off", "done");
 }
 
+sub CheckIdle()
+{
+  $CurrentStatus = "dirty";
+  my $IsPoweredOn = $VM->GetDomain()->IsPoweredOn();
+  return ChangeStatus("dirty", "offline", "done") if (!defined $IsPoweredOn);
+  return ChangeStatus("dirty", "off", "done") if (!$IsPoweredOn);
+
+  my ($ErrMessage, $SnapshotName) = $VM->GetDomain()->GetSnapshotName();
+  FatalError("$ErrMessage\n") if (defined $ErrMessage);
+  return PowerOff() if ($SnapshotName ne $VM->IdleSnapshot);
+
+  return ChangeStatus("dirty", "idle", "done");
+}
+
 sub Revert()
 {
   my $VM = CreateVMs()->GetItem($VMKey);
@@ -318,7 +332,11 @@ sub Revert()
 
 
 my $Rc;
-if ($Action eq "poweroff")
+if ($Action eq "checkidle")
+{
+  $Rc = CheckIdle();
+}
+elsif ($Action eq "poweroff")
 {
   $Rc = PowerOff();
 }
diff --git a/testbot/lib/WineTestBot/LibvirtDomain.pm b/testbot/lib/WineTestBot/LibvirtDomain.pm
index 5fbff18..20d619a 100644
--- a/testbot/lib/WineTestBot/LibvirtDomain.pm
+++ b/testbot/lib/WineTestBot/LibvirtDomain.pm
@@ -202,6 +202,27 @@ sub _GetSnapshot($$)
   return (_eval_err() || "Snapshot '$SnapshotName' not found", undef, undef);
 }
 
+sub GetSnapshotName($)
+{
+  my ($self) = @_;
+
+  my ($ErrMessage, $Domain) = $self->_GetDomain();
+  return ($ErrMessage, undef) if (defined $ErrMessage);
+
+  my $SnapshotName;
+  eval
+  {
+    if ($Domain->has_current_snapshot())
+    {
+      my $Snapshot = $Domain->current_snapshot();
+      $SnapshotName = $Snapshot->get_name() if ($Snapshot);
+    }
+  };
+  return $self->_Reset(_eval_err(), undef) if ($@);
+
+  return (undef, $SnapshotName);
+}
+
 sub RevertToSnapshot($)
 {
   my ($self) = @_;
diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm
index a6b2a8b..377eb0a 100644
--- a/testbot/lib/WineTestBot/VMs.pm
+++ b/testbot/lib/WineTestBot/VMs.pm
@@ -399,6 +399,23 @@ sub _RunVMTool($$$)
 =pod
 =over 12
 
+=item C<RunCheckIdle()>
+
+If the virtual machine state matches that of this VM instance, sets its status
+to idle. If not the VM is simply marked as off. While this is happening the
+VM status is set to dirty so the job scheduler does not try to use it.
+
+This operation can take a long time so it is performed in a separate process.
+
+=back
+=cut
+
+sub RunCheckIdle($)
+{
+  my ($self) = @_;
+  return $self->_RunVMTool("dirty", ["--log-only", "checkidle", $self->GetKey()]);
+}
+
 =item C<RunPowerOff()>
 
 Powers off the VM so that it stops using resources.