[Tools 4/4] testbot: Avoid blocking virtualization functions in the Engine initialization.

Francois Gouget fgouget at codeweavers.com
Wed Oct 18 13:16:44 CDT 2017


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 at codeweavers.com>
---

Note that this patch depends on the previous one to leave the dirty VMs 
alone.

 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 9c461df0..02c361c4 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 64f0575b..fcc0e814 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 5fbff186..20d619ac 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 a6b2a8b3..377eb0ad 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.
-- 
2.14.2



More information about the wine-patches mailing list