Francois Gouget : testbot: Improve LibvirtTool.pl and make it more easily extensible.

[Alexandre Julliard]

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

Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   Mon Oct  9 11:34:26 2017 +0200

testbot: Improve LibvirtTool.pl and make it more easily extensible.

Always check the status field before changing it, and always report
status change errors.
Only notify the administrator that the VM is offline if changing the
status to offline succeeded.

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

---

 testbot/bin/LibvirtTool.pl     | 238 +++++++++++++++++++++++++----------------
 testbot/lib/WineTestBot/VMs.pm |   2 +-
 2 files changed, 149 insertions(+), 91 deletions(-)

diff --git a/testbot/bin/LibvirtTool.pl b/testbot/bin/LibvirtTool.pl
index 5cbb0bf..aa256f3 100755
--- a/testbot/bin/LibvirtTool.pl
+++ b/testbot/bin/LibvirtTool.pl
@@ -1,12 +1,12 @@
 #!/usr/bin/perl -Tw
 # -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*-
 #
-# Reverts a VM so that it is ready to run jobs. Note that in addition to the
-# hypervisor revert operation this implies letting the VM settle down and
-# checking that it responds to our commands. If this fails the administrator
-# is notified and the VM is marked as offline.
+# Reverts a VM so that it is ready to run jobs.
+# This operation can take quite a bit of time, particularly in case of
+# network trouble, and thus is best performed in a separate process.
 #
 # Copyright 2009 Ge van Geldorp
+# Copyright 2012-2017 Francois Gouget
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -57,45 +57,41 @@ sub Error(@)
   LogMsg @_;
 }
 
-sub FatalError($$)
+sub NotifyAdministrator($$)
 {
-  my ($ErrMessage, $VM) = @_;
-  Error $ErrMessage;
+  my ($Subject, $Body) = @_;
 
-  # Get the up-to-date VM status and update it if nobody else changed it
-  my $VMKey = $VM->GetKey();
-  $VM = CreateVMs()->GetItem($VMKey);
-  if ($VM->Status eq "reverting" or $VM->Status eq "sleeping")
+  if (open(my $fh, "|/usr/sbin/sendmail -oi -t -odq"))
   {
-    $VM->Status("offline");
-    $VM->ChildPid(undef);
-    $VM->Save();
-  }
-
-  my $VMSnapshot = $VM->IdleSnapshot;
-  open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq");
-  print SENDMAIL <<"EOF";
+    LogMsg "Notifying administrator: $Subject\n";
+    print $fh <<"EOF";
 From: $RobotEMail
 To: $AdminEMail
-Subject: VM $VMKey offline
-
-Reverting $VMKey to $VMSnapshot failed:
+Subject: $Subject
 
-$ErrMessage
-
-The VM has been put offline.
+$Body
 EOF
-  close(SENDMAIL);
-
-  exit 1;
+    close($fh);
+  }
+  else
+  {
+    LogMsg "Could not send administrator notification: $!\n";
+    LogMsg "  Subject: $Subject\n";
+    LogMsg "  Body: $Body\n";
+  }
 }
 
+
+#
+# Setup and command line processing
+#
+
 $ENV{PATH} = "/usr/bin:/bin";
 delete $ENV{ENV};
 
 
 # Grab the command line options
-my ($Usage, $VMKey);
+my ($Usage, $Action, $VMKey);
 while (@ARGV)
 {
   my $Arg = shift @ARGV;
@@ -103,6 +99,10 @@ while (@ARGV)
   {
     $Debug = 1;
   }
+  elsif ($Arg eq "revert")
+  {
+    $Action = $Arg;
+  }
   elsif ($Arg =~ /^(?:-\?|-h|--help)$/)
   {
     $Usage = 0;
@@ -127,8 +127,14 @@ while (@ARGV)
 }
 
 # Check parameters
+my $VM;
 if (!defined $Usage)
 {
+  if (!defined $Action)
+  {
+    Error "you must specify the action to perform\n";
+    $Usage = 2;
+  }
   if (!defined $VMKey)
   {
     Error "you must specify the VM name\n";
@@ -137,92 +143,144 @@ if (!defined $Usage)
   elsif ($VMKey =~ /^([a-zA-Z0-9_]+)$/)
   {
     $VMKey = $1;
+    $VM = CreateVMs()->GetItem($VMKey);
+    if (!defined $VM)
+    {
+      Error "VM $VMKey does not exist\n";
+      $Usage = 2;
+    }
   }
   else
   {
     Error "'$VMKey' is not a valid VM name\n";
     $Usage = 2;
   }
-
 }
 if (defined $Usage)
 {
-    print "Usage: $Name0 [--debug] [--help] VMName\n";
-    exit $Usage;
+  print "Usage: $Name0 [--debug] [--help] revert VMName\n";
+  exit $Usage;
 }
 
-my $VM = CreateVMs()->GetItem($VMKey);
-if (!defined $VM)
-{
-  Error "VM $VMKey does not exist\n";
-  exit 1;
-}
-if (!$Debug and $VM->Status ne "reverting")
-{
-  FatalError "The VM is not ready to be reverted (" . $VM->Status . ")\n", $VM;
-}
+
+#
+# Main
+#
+
 my $Start = Time();
-LogMsg "Reverting $VMKey to ", $VM->IdleSnapshot, "\n";
 
-# Some QEmu/KVM versions are buggy and cannot revert a running VM
-Debug(Elapsed($Start), " Powering off the VM\n");
-my $Domain = $VM->GetDomain();
-my $ErrMessage = $Domain->PowerOff(1);
-if (defined $ErrMessage)
-{
-  Error "$ErrMessage\n";
-  LogMsg "Trying the revert anyway\n";
-}
+my $CurrentStatus;
 
-Debug(Elapsed($Start), " Reverting $VMKey to ", $VM->IdleSnapshot, "\n");
-$ErrMessage = $Domain->RevertToSnapshot();
-if (defined($ErrMessage))
+sub FatalError($)
 {
-  FatalError "Could not revert $VMKey to " . $VM->IdleSnapshot . ": $ErrMessage\n",
-             $VM;
+  my ($ErrMessage) = @_;
+  Error $ErrMessage;
+
+  # Put the VM offline if nobody else modified its status before us
+  $VM = CreateVMs()->GetItem($VMKey);
+  $VM->Status("offline") if ($VM->Status eq $CurrentStatus);
+  $VM->ChildPid(undef);
+  my ($ErrProperty, $SaveErrMessage) = $VM->Save();
+  if (defined $SaveErrMessage)
+  {
+    LogMsg "Could not put the $VMKey VM offline: $SaveErrMessage ($ErrProperty)\n";
+  }
+  elsif ($VM->Status eq "offline")
+  {
+    NotifyAdministrator("Putting the $VMKey VM offline",
+                        "Could not perform the $Action operation on the $VMKey VM:\n".
+                        "\n$ErrMessage\n".
+                        "The VM has been put offline.");
+  }
+  exit 1;
 }
 
-# Get the up-to-date VM status and exit if someone else changed it
-$VM = CreateVMs()->GetItem($VMKey);
-exit 1 if ($VM->Status ne "reverting");
-$VM->Status("sleeping");
-(my $ErrProperty, $ErrMessage) = $VM->Save();
-if (defined($ErrMessage))
+sub ChangeStatus($$;$)
 {
-  FatalError "Could not change status for VM $VMKey: $ErrMessage\n", $VM;
+  my ($From, $To, $Done) = @_;
+
+  # Get the up-to-date VM status
+  $VM = CreateVMs()->GetItem($VMKey);
+  if (!$VM or (defined $From and $VM->Status ne $From))
+  {
+    LogMsg "Not changing status\n";
+    return undef;
+  }
+
+  $VM->Status($To);
+  $VM->ChildPid(undef) if ($Done);
+  my ($ErrProperty, $ErrMessage) = $VM->Save();
+  if (defined $ErrMessage)
+  {
+    FatalError("Could not change the $VMKey VM status: $ErrMessage\n");
+  }
+  $CurrentStatus = $To;
+  return 1;
 }
 
-Debug(Elapsed($Start), " Trying the TestAgent connection\n");
-LogMsg "Waiting for ", $VM->Name, " (up to ${WaitForToolsInVM}s per attempt)\n";
-my $TA = $VM->GetAgent();
-$TA->SetConnectTimeout($WaitForToolsInVM);
-my $Success = $TA->Ping();
-$TA->Disconnect();
-if (!$Success)
+sub Revert()
 {
-  $ErrMessage = $TA->GetLastError();
-  FatalError "Tools in $VMKey not responding: $ErrMessage\n", $VM;
+  my $VM = CreateVMs()->GetItem($VMKey);
+  if (!$Debug and $VM->Status ne "reverting")
+  {
+    Error("The VM is not ready to be reverted (". $VM->Status .")\n");
+    return 1;
+  }
+  $CurrentStatus = "reverting";
+
+  # Some QEmu/KVM versions are buggy and cannot revert a running VM
+  Debug(Elapsed($Start), " Powering off the VM\n");
+  my $Domain = $VM->GetDomain();
+  my $ErrMessage = $Domain->PowerOff(1);
+  if (defined $ErrMessage)
+  {
+    LogMsg "Could not power off $VMKey: $ErrMessage\n";
+    LogMsg "Trying the revert anyway...\n";
+  }
+
+  Debug(Elapsed($Start), " Reverting $VMKey to ", $VM->IdleSnapshot, "\n");
+  $ErrMessage = $Domain->RevertToSnapshot();
+  if (defined $ErrMessage)
+  {
+    FatalError("Could not revert $VMKey to ". $VM->IdleSnapshot .": $ErrMessage\n");
+  }
+
+  # The VM is now sleeping which may allow some tasks to run
+  return 1 if (!ChangeStatus("reverting", "sleeping"));
+
+  Debug(Elapsed($Start), " Trying the TestAgent connection\n");
+  LogMsg "Waiting for ". $VM->Name ." (up to ${WaitForToolsInVM}s per attempt)\n";
+  my $TA = $VM->GetAgent();
+  $TA->SetConnectTimeout($WaitForToolsInVM);
+  my $Success = $TA->Ping();
+  $TA->Disconnect();
+  if (!$Success)
+  {
+    $ErrMessage = $TA->GetLastError();
+    FatalError("Cannot connect to the $VMKey TestAgent: $ErrMessage\n");
+  }
+
+  if ($SleepAfterRevert != 0)
+  {
+    Debug(Elapsed($Start), " Sleeping\n");
+    LogMsg "Letting ". $VM->Name  ." settle down for ${SleepAfterRevert}s\n";
+    sleep($SleepAfterRevert);
+  }
+
+  return ChangeStatus("sleeping", "idle", "done") ? 0 : 1;
 }
 
-if ($SleepAfterRevert != 0)
+
+my $Rc;
+if ($Action eq "revert")
 {
-  Debug(Elapsed($Start), " Sleeping\n");
-  LogMsg "Letting ", $VM->Name, " settle for ${SleepAfterRevert}s\n";
-  sleep($SleepAfterRevert);
+  $Rc = Revert();
 }
-Debug(Elapsed($Start), " Done\n");
-
-# Get the up-to-date VM status and exit if someone else changed it
-$VM = CreateVMs()->GetItem($VMKey);
-exit 1 if ($VM->Status ne "sleeping");
-$VM->Status("idle");
-$VM->ChildPid(undef);
-($ErrProperty, $ErrMessage) = $VM->Save();
-if (defined($ErrMessage))
+else
 {
-  FatalError "Could not change status for VM $VMKey: $ErrMessage\n", $VM;
+  Error("Unsupported action $Action!\n");
+  $Rc = 1;
 }
+LogMsg "$Action on $VMKey completed in ", Elapsed($Start), " s\n";
 
-LogMsg "Revert of $VMKey completed in ", Elapsed($Start), " s\n";
-
-exit 0;
+exit $Rc;
diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm
index 4dd7cc9..1c6cdd7 100644
--- a/testbot/lib/WineTestBot/VMs.pm
+++ b/testbot/lib/WineTestBot/VMs.pm
@@ -276,7 +276,7 @@ sub RunRevert($)
     delete $ENV{ENV};
     require WineTestBot::Log;
     WineTestBot::Log::SetupRedirects();
-    exec("$BinDir/LibvirtTool.pl", $self->GetKey()) or
+    exec("$BinDir/LibvirtTool.pl", "revert", $self->GetKey()) or
     WineTestBot::Log::LogMsg("Unable to exec LibvirtTool.pl: $!\n");
     exit(1);
   }