Francois Gouget : testbot: Allow multiple VMs to share access to a hypervisor domain.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 035ad3d5ab6ec66d91d10961bf834db5f0c6d7eb
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=035ad3d5ab6ec66d91d10961...

Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   Thu Mar  1 11:02:25 2018 +0100

testbot: Allow multiple VMs to share access to a hypervisor domain.

This makes it possible to define two or more VM instances that have the
same VirtURI and VirtDomain values and differ only in their IdleSnapshot
field. This can be used to perform tests on multiple configurations of a
given Windows version, such as testing different locales, etc.

However a hypervisor domain only has one current snapshot. So the
TestBot can only use one of the VM instances that depend on this
hypervisor domain at any time.

An important consequence is that $VM->Status does not necessarily
matches the state of the hypervisor domain. So $VM->Status == off does
not mean that the TestBot is free to revert the domain to that VM
snapshot. The status of the other VMs using the same hypervisor domain
much be checked first. This is handled by some extra _CanScheduleOnVM()
calls.

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

---

 testbot/lib/WineTestBot/Jobs.pm | 91 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 77 insertions(+), 14 deletions(-)

diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm
index eea1686..b870d7b 100644
--- a/testbot/lib/WineTestBot/Jobs.pm
+++ b/testbot/lib/WineTestBot/Jobs.pm
@@ -519,22 +519,67 @@ sub _GetMaxReverts($)
 
 Checks if a task or VM operation can be performed on the specified VM.
 
+We allow multiple VM instances to refer to different snapshots of the same
+hypervisor domain (that is VM objects that have identical VirtURI and
+VirtDomain fields but different values for IdleSnapshot). This is typically
+used to test different configurations of the same base virtual machine.
+
+However a hypervisor domain cannot run two snapshots simultaneously so this
+function is used to ensure the scheduler does not simultaneously assign the
+same hypervisor domain to two VM instances.
+
 =back
 =cut
 
-sub _CanScheduleOnVM($$)
+sub _CanScheduleOnVM($$;$)
 {
-  my ($Sched, $VM) = @_;
+  my ($Sched, $VM, $Steal) = @_;
+
+  my $DomainKey = $VM->VirtURI ." ". $VM->VirtDomain;
+  my $DomainVM = $Sched->{domains}->{$DomainKey};
 
-  return 1 if ($VM->Status eq "off");
+  if (!$DomainVM or $DomainVM->Status eq "off")
+  {
+    $Sched->{domains}->{$DomainKey} = $VM;
+    return 1;
+  }
 
-  # If the VM is busy it cannot be taken over for a new task
   my $VMKey = $VM->GetKey();
-  return 0 if ($Sched->{busyvms}->{$VMKey});
+  if ($Sched->{busyvms}->{$VMKey})
+  {
+    # If the VM is busy it cannot be taken over for a new task
+    return 0;
+  }
+
+  my $DomainVMKey = $DomainVM->GetKey();
+  if ($VMKey eq $DomainVMKey)
+  {
+    # Already ours. Use it if it is not busy
+    return !$VM->ChildPid;
+  }
 
-  # A process may be working on the VM even though it is not busy (e.g. if it
-  # is sleeping). In that case just wait.
-  return !$VM->ChildPid;
+  # We cannot schedule anything on this VM if we cannot take the hypervisor
+  # domain from its current owner. Note that we can always take over dirty VMs
+  # if we did not start an operation on them yet (i.e. if they are in lambvms).
+  if (!$Sched->{lambvms}->{$DomainVMKey} or
+      (!$Steal and ($VM->Status eq "off" or $DomainVM->Status ne "dirty")))
+  {
+    return 0;
+  }
+
+  # $DomainVM is either dirty (with no child process), idle or sleeping.
+  # Just mark it off and let the caller poweroff or revert the
+  # hypervisor domain as needed for the new VM.
+  $DomainVM->KillChild(); # For the sleeping case
+  my $Host = _GetSchedHost($Sched, $DomainVM);
+  $Host->{$DomainVM->Status}--;
+  $Host->{active}--;
+  $DomainVM->Status("off");
+  $DomainVM->Save();
+  # off VMs are neither in busyvms nor lambvms
+  delete $Sched->{lambvms}->{$DomainVMKey};
+  $Sched->{domains}->{$DomainKey} = $VM;
+  return 1;
 }
 
 =pod
@@ -542,8 +587,8 @@ sub _CanScheduleOnVM($$)
 
 =item C<_CheckAndClassifyVMs()>
 
-Checks the VMs state consistency, counts the VMs in each state and classifies
-them.
+Checks the VMs state consistency, counts the VMs in each state, classifies
+them, and determines which VM owns each hypervisor domain.
 
 =over
 
@@ -577,6 +622,14 @@ Puts the VMs in one of three sets:
 
 =item *
 
+Determines which VM should have exclusive access to each hypervisor domain.
+This is normally the VM that is currently using it, but if all a given
+hypervisor domain's VMs are off, one of them is picked at random. In any case
+if a VM is not in the busyvms set, the hypervisor domain can be taken away from
+it if necessary.
+
+=item *
+
 Each VM is given a priority describing the likelihood that it will be needed
 by a future job. When no other VM is running this can be used to decide which
 VMs to start in advance.
@@ -699,8 +752,11 @@ sub _CheckAndClassifyVMs()
       }
       elsif ($VM->Status eq "offline")
       {
-        my $ErrMessage = $VM->RunMonitor();
-        return ($ErrMessage, undef) if (defined $ErrMessage);
+        if (_CanScheduleOnVM($Sched, $VM))
+        {
+          my $ErrMessage = $VM->RunMonitor();
+          return ($ErrMessage, undef) if (defined $ErrMessage);
+        }
         # Ignore the VM for this round since we cannot use it
         $Sched->{busyvms}->{$VMKey} = 1;
       }
@@ -720,6 +776,8 @@ sub _CheckAndClassifyVMs()
       # Note that off VMs are neither in busyvms nor lambvms
     }
 
+    _CanScheduleOnVM($Sched, $VM);
+
     $Sched->{nicefuture}->{$VMKey} =
         ($VM->Role eq "base" ? 0 :
          $VM->Role eq "winetest" ? 10 :
@@ -965,7 +1023,8 @@ sub _ScheduleTasks($)
           # count them against the running VM limit.
           if ($Host->{sleeping} + $Host->{running} + $Host->{dirty} < $Host->{MaxRunningVMs} and
               ($Host->{reverting} == 0 or
-               $Host->{reverting} <= $Host->{MaxRevertsWhileRunningVMs}))
+               $Host->{reverting} <= $Host->{MaxRevertsWhileRunningVMs}) and
+              _CanScheduleOnVM($Sched, $VM))
           {
             $Sched->{busyvms}->{$VMKey} = 1;
             $VM->RecordStatus($Sched->{records}, join(" ", "running", $Job->Id, $Step->No, $Task->No));
@@ -1159,7 +1218,10 @@ sub _RevertVMs($$)
 
     # Skip this VM if the previous step's tasks are not about to run yet
     next if (_HasMissingDependencies($Sched, $NeededVMs, $VMKey));
-    next if (!_CanScheduleOnVM($Sched, $VM));
+
+    # Don't steal the hypervisor domain for a VM we will only need later
+    my $Steal = (_GetNiceness($NeededVMs, $VMKey) < $NEXT_BASE);
+    next if (!_CanScheduleOnVM($Sched, $VM, $Steal));
 
     my $NeedsSacrifice;
     if (_GetNiceness($NeededVMs, $VMKey) >= $FUTURE_BASE)
@@ -1224,6 +1286,7 @@ sub _PowerOffDirtyVMs($)
   {
     my $VM = $Sched->{VMs}->GetItem($VMKey);
     next if ($VM->Status ne "dirty");
+    next if (!_CanScheduleOnVM($Sched, $VM));
 
     $VM->RecordStatus($Sched->{records}, "dirty poweroff");
     my $ErrMessage = $VM->RunPowerOff();