Also rename ShutDown() to the more descriptive ShutDownIfOffSnapshot() and document it.
Signed-off-by: Francois Gouget fgouget@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");
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@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"); }
This also makes it easier to extend it and pass all the configuration around.
Signed-off-by: Francois Gouget fgouget@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);
-
Francois Gouget