Implement for driving libvirt via virsh
v2 cover Implement for driving libvirt via virsh
The following series switches osstest to implement the toolstack via get_host_method_object()->method rather than toolstack()->{Command}." method" etc.
This is needed because virsh differs from xm/xl in a few commands.
It also implements partial virsh support (simple lifecycle stuff, but not e.g. migration yet). Due to the ts-migration-check logic this means that the libvirt sequence works in so far as it skips/ignores the migration/save+restore related tests.
I've not kept notes on what I changes since last time (but noone has really reviewed the v1/RFC anyway ;-)). This time around is more complete in that it implements shutdown etc and the job completes and I've actually removed ->{Command} which was just an aspiration last timearound.
I've tested with:
test-amd64-amd64-libvirt on a xen-unstable flight, which failed due to
http://article.gmane.org/gmane.comp.emulators.xen.devel/224197 and that set of issues.
test-amd64-amd64-libvirt on a xen-unstable flight with the above patches added, works.
Since this touches xend code paths I've also tested using a xen-4.4-testing based flight:
test-amd64-amd64-libvirt
test-amd64-amd64-pv (uses xend)
test-amd64-amd64-xl (uses xl, duh!)
All passed.
The last patch stubs out migration a bit but doesn't actually work and obviously shouldn't be applied, it's included for reference. Note that the series still enables useful functionality even without migration (e.g. start/stop/destroy testing)
Ian.
PATCH v2 1/18 apt: lock osstest's usages of apt-get against each other
Currently we rely on all apt-get invocations being in a single ts-xen-build-prep job which can't run on a shared host.
That is a bit inflexible so instead use our own lock. We wait indefinitely and rely on osstest's existing command timeout infrastructure to catch problems.
target_install_packages*() previous estimated the time taken to install the packages based on the number of packages. This no longer applies because the install might get stuck behind some other large install. Use a 3000s (nearly an hour) timeout instead (I expect failures here to be unusual so erred on the big side)
--- Osstest/Debian.pm | 2 +- Osstest/TestSupport.pm | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)
Osstest/Debian.pm +1 -1
@@ -492,7 +492,7 @@ d-i apt-setup/another boolean false d-i apt-setup/non-free boolean false d-i apt-setup/contrib boolean false -d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, $extra_packages +d-i pkgsel/include string openssh-server, ntp, ntpdate, ethtool, chiark-utils-bin, $extra_packages $xopts{ExtraPreseed}
Osstest/TestSupport.pm +4 -3
@@ -433,17 +433,18 @@ sub target_putfile_root ($$$$;$) { sub target_run_apt { my ($ho, $timeout, @aptopts) = @_; target_cmd_root($ho, - "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y apt-get @aptopts", + "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \ + with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts", $timeout); } sub target_install_packages { my ($ho, @packages) = @_; - target_run_apt($ho, 300 + 100 * @packages, + target_run_apt($ho, 3000, qw(-y install), @packages); } sub target_install_packages_norec { my ($ho, @packages) = @_; - target_run_apt($ho, 300 + 100 * @packages, + target_run_apt($ho, 3000, qw(--no-install-recommends -y install), @packages); }
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other
Ian Campbell writes ("[PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other"):
Quoted 6 lines
Currently we rely on all apt-get invocations being in a single ts-xen-build-prep job which can't run on a shared host.
That is a bit inflexible so instead use our own lock. We wait indefinitely and rely on osstest's existing command timeout infrastructure to catch problems.
...
Osstest/TestSupport.pm sub target_putfile_root ($$$$;$) { · line 438
Quoted 12 lines
Osstest/TestSupport.pm +2
-1@@ -433,17 +433,18 @@ sub target_putfile_root ($$$$;$) { sub target_run_apt { my ($ho, $timeout, @aptopts) = @_; target_cmd_root($ho, - "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y apt-get @aptopts", + "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \ + with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts", $timeout);
I think target_run_apt ought to lose the $timeout parameter. Anyone who calls it with other than the big 3000s timeout might lose, after all.
Ian.
Re: [PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other
Quoted 9 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other"):
Quoted 6 lines
Currently we rely on all apt-get invocations being in a single ts-xen-build-prep job which can't run on a shared host.
That is a bit inflexible so instead use our own lock. We wait indefinitely and rely on osstest's existing command timeout infrastructure to catch problems.
...
Quoted 16 lines
Osstest/TestSupport.pm sub target_putfile_root ($$$$;$) { · line 438
Quoted 12 lines
Osstest/TestSupport.pm +2
-1@@ -433,17 +433,18 @@ sub target_putfile_root ($$$$;$) { sub target_run_apt { my ($ho, $timeout, @aptopts) = @_; target_cmd_root($ho, - "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y apt-get @aptopts", + "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \ + with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts", $timeout);I think
target_run_aptought to lose the $timeout parameter. Anyone who calls it with other than the big 3000s timeout might lose, after all.
Good point.
FWIW I don't think this patch is actually necessary for this series, it arose during an early version because I was installing stuff in ts-build-libvirt, but now everything is done in e.g. ts-build-prep.
I think it's still a step in the direction you wanted wrt sharing machine though, so I'll keep it in the series next time around too.
Ian.
Re: [PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other
Ian Campbell writes ("Re: [PATCH OSSTEST v2 01/18] apt: lock osstest's usages of apt-get against each other"):
Quoted 10 lines · Ian Jackson
Quoted 3 lines
I think
target_run_aptought to lose the $timeout parameter. Anyone who calls it with other than the big 3000s timeout might lose, after all.Good point.
FWIW I don't think this patch is actually necessary for this series, it arose during an early version because I was installing stuff in ts-build-libvirt, but now everything is done in e.g. ts-build-prep.
Ah.
Quoted 2 lines
I think it's still a step in the direction you wanted wrt sharing machine though, so I'll keep it in the series next time around too.
Yes, please.
Thanks,
Ian.
PATCH v2 2/18 ts-logs-capture: Collect some libvirt logs and capabilities
In reply to Ian Campbell--- ts-logs-capture | 4 ++++ 1 file changed, 4 insertions(+)
ts-logs-capture +4 -0
@@ -117,6 +117,9 @@ sub fetch_logs_host_guests () { /var/log/xen/osstest* /var/log/xen/xenstored* + /var/log/libvirt/libvirtd.log + /var/log/libvirt/libxl/* + /var/run/xenstored* /var/log/xenstored*
@@ -161,6 +164,7 @@ sub fetch_logs_host_guests () { 'lspci -vvv', 'lspci -tv', 'cat /proc/partitions', + 'virsh capabilities', ) { try_cmd_output_save($cmd); }
Signature · 1 line
2.1.1
PATCH v2 2/18 ts-logs-capture: Collect some libvirt logs and capabilities
Ian Campbell writes ("[PATCH OSSTEST v2 02/18] ts-logs-capture: Collect some libvirt logs and capabilities"):
Quoted 1 line
Signed-off-by Ian Campbell <[email protected]>
PATCH v2 3/18 Pass host to toolstack()
In reply to Ian CampbellThis will be needed in a future patch.
Everywhere already has a $ho in hand. Also cache the answer as $ho->{Toolstack}.
I scanned the source with:
find -name \*.pm -exec perl -c {} \;
for i in ts-* ; do perl -c $i; done
which reported "Not enough arguments for Osstest::TestSupport::toolstack" for each callsite which needed changing.
Also don't pass the toolstack command name directly to host_get_free_memory(). Look it up instead.
--- Osstest/TestSupport.pm | 16 ++++++++++------ ts-debian-fixup | 2 +- ts-debian-hvm-install | 4 ++-- ts-guest-localmigrate | 2 +- ts-guest-migrate | 2 +- ts-guest-saverestore | 8 ++++---- ts-guest-start | 4 ++-- ts-guest-stop | 2 +- ts-logs-capture | 2 +- ts-migrate-support-check | 4 ++-- ts-redhat-install | 2 +- ts-rumpuserxen-demo-xenstorels | 4 ++-- ts-windows-install | 2 +- ts-xen-install | 10 +++++----- 14 files changed, 34 insertions(+), 30 deletions(-)
Osstest/TestSupport.pm +10 -6
@@ -954,8 +954,9 @@ END }); } -sub host_get_free_memory($$) { - my ($ho,$toolstack) = @_; +sub host_get_free_memory($) { + my ($ho) = @_; + my $toolstack = toolstack($ho)->{Command}; # The line is as followed: # free_memory : XXXX my $info = target_cmd_output_root($ho, "$toolstack info", 10);
@@ -1318,7 +1319,7 @@ sub guest_await_shutdown ($$$) { sub guest_destroy ($$) { my ($ho,$gho) = @_; - target_cmd_root($ho, toolstack()->{Command}." destroy $gho->{Name}", 40); + target_cmd_root($ho, toolstack($ho)->{Command}." destroy $gho->{Name}", 40); } sub guest_create ($$) {
@@ -1609,7 +1610,7 @@ sub guest_check_up ($) { sub guest_get_state ($$) { my ($ho,$gho) = @_; - my $domains= target_cmd_output_root($ho, toolstack()->{Command}." list"); + my $domains= target_cmd_output_root($ho, toolstack($ho)->{Command}." list"); $domains =~ s/^Name.*\n//; foreach my $l (split /\n/, $domains) { $l =~ m/^(\S+) (?: \s+ \d+ ){3} \s+ ([-a-z]+) \s/x or die "$l ?";
@@ -1814,7 +1815,7 @@ sub guest_find_domid ($$) { my ($ho,$gho) = @_; return if defined $gho->{Domid}; my $list= target_cmd_output_root($ho, - toolstack()->{Command}." list $gho->{Name}"); + toolstack($ho)->{Command}." list $gho->{Name}"); $list =~ m/^(?!Name\s)(\S+)\s+(\d+)\s+(\d+)+(\d+)\s.*$/m or die "domain list: $list"; $1 eq $gho->{Name} or die "domain list name $1 expected $gho->{Name}";
@@ -1884,7 +1885,9 @@ our %toolstacks= }, ); -sub toolstack () { +sub toolstack ($) { + my ($ho) = @_; + return $ho->{Toolstack} if $ho->{Toolstack}; my $tsname= $r{toolstack}; $tsname= 'xend' if !defined $tsname; my $ts= $toolstacks{$tsname};
@@ -1893,6 +1896,7 @@ sub toolstack () { logm("toolstack $tsname"); $ts->{Name}= $tsname; } + $ho->{Toolstack} = $ts; return $ts; }
ts-debian-fixup +1 -1
@@ -142,7 +142,7 @@ sub otherfixupcfg () { if (@pcipt) { logm("checking passthrough device(s) are assignable: @pcipt"); my @assignables= split /\n/, - target_cmd_output_root($ho, toolstack()->{Command}. + target_cmd_output_root($ho, toolstack($ho)->{Command}. " pci-assignable-list"); foreach my $pcipt (@pcipt) { die "not assignable: $pcipt (not in: @assignables)"
ts-debian-hvm-install +2 -2
@@ -41,7 +41,7 @@ our $disk_mb= 10000; our $guesthost= "$gn.guest.osstest"; our $gho; -our $toolstack= toolstack()->{Command}; +our $toolstack= toolstack($ho)->{Command}; sub preseed () {
@@ -171,7 +171,7 @@ sub prep () { # If host has >8G free memory, create a guest with 4G memory to catch # any error that triggers cross 4G boundary -my $host_freemem_mb = host_get_free_memory($ho, $toolstack); +my $host_freemem_mb = host_get_free_memory($ho); my $ram_minslop = 100; my $ram_lots = 5000; if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
ts-guest-localmigrate +1 -1
@@ -33,7 +33,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub migrate () { guest_checkrunning($ho,$gho) or die $gho->{Name}; target_cmd_root($ho, - toolstack()->{Command} + toolstack($ho)->{Command} ." migrate $gho->{Name} localhost", $timeout{Migrate}); }
ts-guest-migrate +1 -1
@@ -32,7 +32,7 @@ sub migrate () { guest_checkrunning($sho,$gho) or die $gho->{Name}; my $err= guest_check_ip($gho); die "$err $gho->{Name}" if defined $err; target_cmd_root($sho, - toolstack()->{Command} + toolstack($sho)->{Command} ." migrate $gho->{Name} $dho->{Name}", $timeout{Migrate}); }
ts-guest-saverestore +4 -4
@@ -28,17 +28,17 @@ sub save () { guest_checkrunning($ho,$gho) or die $gho->{Name}; my $err= guest_check_ip($gho); die "$err $gho->{Name}" if defined $err; target_cmd_root($ho, - toolstack()->{Command} + toolstack($ho)->{Command} ." save $gho->{Name} image", 200); target_ping_check_down($gho); } sub restore () { target_cmd_root($ho, - toolstack()->{Command} + toolstack($ho)->{Command} ." restore " - .(toolstack()->{RestoreNeedsConfig} ? - $r{ $gho->{Guest}.'_'. toolstack()->{CfgPathVar} } : '') + .(toolstack($ho)->{RestoreNeedsConfig} ? + $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} } : '') ." image", 200); target_ping_check_up($gho); }
ts-guest-start +2 -2
@@ -26,8 +26,8 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub start () { guest_umount_lv($ho, $gho); - my $cmd= toolstack()->{Command}." create ". - $r{ $gho->{Guest}.'_'. toolstack()->{CfgPathVar} }; + my $cmd= toolstack($ho)->{Command}." create ". + $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }; target_cmd_root($ho, $cmd, 30); }
ts-guest-stop +1 -1
@@ -27,7 +27,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub stop () { guest_checkrunning($ho, $gho) or die "$gho->{Name} not running"; target_cmd_root($ho, - toolstack()->{Command} + toolstack($ho)->{Command} ." shutdown -w " .$gho->{Name}, 200); guest_checkrunning($ho, $gho) and die $gho->{Name};
ts-logs-capture +1 -1
@@ -195,7 +195,7 @@ sub fetch_logs_guest ($) { logm("cannot find domid: $@"); return; } - my $consolecmd= toolstack()->{Command}." console $gho->{Name}"; + my $consolecmd= toolstack($ho)->{Command}." console $gho->{Name}"; try_cmd_output_save("sleep 1 | $consolecmd | cat", "guest-$gho->{Name}-console");
ts-migrate-support-check +2 -2
@@ -25,9 +25,9 @@ tsreadconfig(); our $ho = selecthost($ARGV[0]); # all xend/xm platforms support migration -exit(0) if toolstack()->{Command} eq "xm"; +exit(0) if toolstack($ho)->{Command} eq "xm"; -my $help = target_cmd_output_root($ho, toolstack()->{Command}." help"); +my $help = target_cmd_output_root($ho, toolstack($ho)->{Command}." help"); my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1;
ts-redhat-install +1 -1
@@ -37,7 +37,7 @@ our $disk_mb= 50000; our $guesthost= "$gn.guest.osstest"; our $gho; -our $xl= toolstack()->{Command}; +our $xl= toolstack($ho)->{Command}; sub kickstart () {
ts-rumpuserxen-demo-xenstorels +2 -2
@@ -40,8 +40,8 @@ sub arrangepreserve () { } sub start () { - my $cmd= toolstack()->{Command}." create ". - $r{ $gho->{Guest}.'_'. toolstack()->{CfgPathVar} }; + my $cmd= toolstack($ho)->{Command}." create ". + $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }; target_cmd_root($ho, $cmd, 30); $domid = target_cmd_output_root($ho,"xl domid $gho->{Guest}");
ts-windows-install +1 -1
@@ -50,7 +50,7 @@ END } sub start () { - target_cmd_root($ho, toolstack()->{Command}. + target_cmd_root($ho, toolstack($ho)->{Command}. " create $gho->{CfgPath}", 100); }
ts-xen-install +5 -5
@@ -60,8 +60,8 @@ sub packages () { if ($r{arch} eq 'i386') { target_install_packages($ho, 'libc6-xen'); } - target_install_packages($ho, @{toolstack()->{ExtraPackages}}) - if toolstack()->{ExtraPackages}; + target_install_packages($ho, @{toolstack($ho)->{ExtraPackages}}) + if toolstack($ho)->{ExtraPackages}; } sub extract () {
@@ -101,7 +101,7 @@ sub adjustconfig () { } } print EO $extra or die $!; - }) if toolstack()->{Name} eq "xend"; + }) if toolstack($ho)->{Name} eq "xend"; my $trace_config_file; foreach my $try (qw(/etc/default/xencommons
@@ -147,7 +147,7 @@ sub setupboot () { } else { logm("No Xen console device defined for host"); } - if (toolstack()->{Dom0MemFixed}) { + if (toolstack($ho)->{Dom0MemFixed}) { $xenhopt .= " dom0_mem=512M,max:512M"; } my $append= $r{xen_boot_append};
@@ -179,7 +179,7 @@ sub setupboot () { our $initscripts_nobridge; sub setupinitd () { - my $ts= toolstack(); + my $ts= toolstack($ho); my $xencommons= '/etc/init.d/xencommons'; my $have_xencommons= !!target_cmd_output_root($ho, <<END);
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 03/18] Pass host to toolstack()
Ian Campbell writes ("[PATCH OSSTEST v2 03/18] Pass host to toolstack()"):
Quoted 1 line
This will be needed in a future patch.
PATCH v2 4/18 ts-rumpuserxen-demo-xenstorels: Use standard functions for things
In reply to Ian CampbellSpecifically guest_create and guest_find_domid.
--- ts-rumpuserxen-demo-xenstorels | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
ts-rumpuserxen-demo-xenstorels +2 -4
@@ -40,11 +40,9 @@ sub arrangepreserve () { } sub start () { - my $cmd= toolstack($ho)->{Command}." create ". - $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }; - target_cmd_root($ho, $cmd, 30); + guest_create($gho,toolstack($ho)); - $domid = target_cmd_output_root($ho,"xl domid $gho->{Guest}"); + $domid = guest_find_domid($ho, $gho); } sub await_end () {
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 04/18] ts-rumpuserxen-demo-xenstorels: Use standard functions for things
Ian Campbell writes ("[PATCH OSSTEST v2 04/18] ts-rumpuserxen-demo-xenstorels: Use standard functions for things"):
Quoted 1 line
Specifically
guest_createandguest_find_domid.
(Not sure why I did it like that.)
Ian.
PATCH v2 5/18 Toolstack: use get_host_method_object() to manage toolstack selection
In reply to Ian CampbellThis will allow us to more easily have per-toolstack methods etc.
The previous hash of toolstack parameters is now a blessed object. For now the callers don't need to change but over the following patches we will refactor things to use method calls. In particular we will be aiming to remove Command from the hash and use method calls for everything.
--- Osstest/TestSupport.pm | 37 ++++--------------------------------- Osstest/Toolstack/libvirt.pm | 34 ++++++++++++++++++++++++++++++++++ Osstest/Toolstack/xend.pm | 35 +++++++++++++++++++++++++++++++++++ Osstest/Toolstack/xl.pm | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 33 deletions(-) create mode 100644 Osstest/Toolstack/libvirt.pm create mode 100644 Osstest/Toolstack/xend.pm create mode 100644 Osstest/Toolstack/xl.pm
Osstest/TestSupport.pm +4 -33
@@ -1862,42 +1862,13 @@ sub guest_vncsnapshot_stash ($$$$) { target_getfile_root($ho,100, "$rfile", "$stash/$leaf"); } -our %toolstacks= - ('xend' => { - NewDaemons => [qw(xend)], - OldDaemonInitd => 'xend', - Command => 'xm', - CfgPathVar => 'cfgpath', - Dom0MemFixed => 1, - }, - 'xl' => { - NewDaemons => [], - Dom0MemFixed => 1, - Command => 'xl', - CfgPathVar => 'cfgpath', - RestoreNeedsConfig => 1, - }, - 'libvirt' => { - NewDaemons => [qw(libvirtd)], - Dom0MemFixed => 1, - Command => 'virsh', - ExtraPackages => [qw(libnl1 libavahi-client3)], - }, - ); - sub toolstack ($) { my ($ho) = @_; return $ho->{Toolstack} if $ho->{Toolstack}; - my $tsname= $r{toolstack}; - $tsname= 'xend' if !defined $tsname; - my $ts= $toolstacks{$tsname}; - die "$tsname ?" unless defined $ts; - if (!exists $ts->{Name}) { - logm("toolstack $tsname"); - $ts->{Name}= $tsname; - } - $ho->{Toolstack} = $ts; - return $ts; + + my $tsname= $r{toolstack} || 'xend'; + $ho->{Toolstack}= get_host_method_object($ho, 'Toolstack', $tsname); + return $ho->{Toolstack}; } sub authorized_keys () {
Osstest/Toolstack/libvirt.pm new file +34 -0
@@ -0,0 +1,34 @@ +# This is part of "osstest", an automated testing framework for Xen. +# Copyright (C) 2014 Citrix Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + + +package Osstest::Toolstack::libvirt; + +use strict; +use warnings; + +sub new { + my ($class, $ho, $methname,$asset) = @_; + return bless { Name => "libvirt", + Host => $ho, + NewDaemons => [qw(libvirtd)], + Dom0MemFixed => 1, + Command => 'virsh', + ExtraPackages => [qw(libnl1 libavahi-client3)], + }, $class; +} + +1;
Osstest/Toolstack/xend.pm new file +35 -0
@@ -0,0 +1,35 @@ +# This is part of "osstest", an automated testing framework for Xen. +# Copyright (C) 2014 Citrix Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + + +package Osstest::Toolstack::xend; + +use strict; +use warnings; + +sub new { + my ($class, $ho, $methname,$asset) = @_; + return bless { Name => "xend", + Host => $ho, + NewDaemons => [qw(xend)], + OldDaemonInitd => 'xend', + Command => 'xm', + CfgPathVar => 'cfgpath', + Dom0MemFixed => 1, + }, $class; +} + +1;
Osstest/Toolstack/xl.pm new file +35 -0
@@ -0,0 +1,35 @@ +# This is part of "osstest", an automated testing framework for Xen. +# Copyright (C) 2014 Citrix Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + + +package Osstest::Toolstack::xl; + +use strict; +use warnings; + +sub new { + my ($class, $ho, $methname,$asset) = @_; + return bless { Name => "xl", + Host => $ho, + NewDaemons => [], + Dom0MemFixed => 1, + Command => 'xl', + CfgPathVar => 'cfgpath', + RestoreNeedsConfig => 1, + }, $class; +} + +1;
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 05/18] Toolstack: use get_host_method_object() to manage toolstack selection
Ian Campbell writes ("[PATCH OSSTEST v2 05/18] Toolstack: use get_host_method_object() to manage toolstack selection"):
Quoted 7 lines
This will allow us to more easily have per-toolstack methods etc.
The previous hash of toolstack parameters is now a blessed object. For now the callers don't need to change but over the following patches we will refactor things to use method calls. In particular we will be aiming to remove Command from the hash and use method calls for everything.
PATCH v2 6/18 TestSupport: always use xl for generic operations.
In reply to Ian CampbellUnless the toolstack is xend (for compatibility with pre-xl Xen versions), when we use xm.
For several operations in TestSupport.pm the actual toolstack isn't really relevant, since we want info straight from Xen. For simplicity just use xl (or xm) in these cases, to avoid needing to implement the following specially for each toolstack:
- host_get_free_memory
- guest_get_state
- guest_find_domid
- listing assignable pci devices
--- Osstest/TestSupport.pm | 13 ++++++++++--- ts-debian-fixup | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-)
Osstest/TestSupport.pm +10 -3
@@ -936,6 +936,13 @@ sub compress_stashed($) { #---------- other stuff ---------- +sub common_toolstack () { + my $tsname= $r{toolstack} || 'xend'; + my $ts = 'xl'; + $ts = 'xm' if $tsname eq 'xend'; + return $ts; +} + sub host_reboot ($) { my ($ho) = @_; target_reboot($ho);
@@ -956,7 +963,7 @@ END sub host_get_free_memory($) { my ($ho) = @_; - my $toolstack = toolstack($ho)->{Command}; + my $toolstack = common_toolstack(); # The line is as followed: # free_memory : XXXX my $info = target_cmd_output_root($ho, "$toolstack info", 10);
@@ -1610,7 +1617,7 @@ sub guest_check_up ($) { sub guest_get_state ($$) { my ($ho,$gho) = @_; - my $domains= target_cmd_output_root($ho, toolstack($ho)->{Command}." list"); + my $domains= target_cmd_output_root($ho, common_toolstack()." list"); $domains =~ s/^Name.*\n//; foreach my $l (split /\n/, $domains) { $l =~ m/^(\S+) (?: \s+ \d+ ){3} \s+ ([-a-z]+) \s/x or die "$l ?";
@@ -1815,7 +1822,7 @@ sub guest_find_domid ($$) { my ($ho,$gho) = @_; return if defined $gho->{Domid}; my $list= target_cmd_output_root($ho, - toolstack($ho)->{Command}." list $gho->{Name}"); + common_toolstack()." list $gho->{Name}"); $list =~ m/^(?!Name\s)(\S+)\s+(\d+)\s+(\d+)+(\d+)\s.*$/m or die "domain list: $list"; $1 eq $gho->{Name} or die "domain list name $1 expected $gho->{Name}";
ts-debian-fixup +1 -1
@@ -142,7 +142,7 @@ sub otherfixupcfg () { if (@pcipt) { logm("checking passthrough device(s) are assignable: @pcipt"); my @assignables= split /\n/, - target_cmd_output_root($ho, toolstack($ho)->{Command}. + target_cmd_output_root($ho, common_toolstack(). " pci-assignable-list"); foreach my $pcipt (@pcipt) { die "not assignable: $pcipt (not in: @assignables)"
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 06/18] TestSupport: always use xl for generic operations.
Ian Campbell writes ("[PATCH OSSTEST v2 06/18] TestSupport: always use xl for generic operations."):
Quoted 4 lines
For several operations in TestSupport.pm the actual toolstack isn't really relevant, since we want info straight from Xen. For simplicity just use xl (or xm) in these cases, to avoid needing to implement the following specially for each toolstack:
...
Osstest/TestSupport.pm sub compress_stashed($) { · line 944
Quoted 6 lines
Osstest/TestSupport.pm +6
-0+sub common_toolstack () { + my $tsname= $r{toolstack} || 'xend'; + my $ts = 'xl'; + $ts = 'xm' if $tsname eq 'xend'; + return $ts; +}
Shouldn't this look this up in the host ? I can imagine scenarios where we have multiple toolstacks at different times or on different hosts (eg, to test upgrades or sidegrades).
So I think common_toolstack at the very least should take a $ho.
Ian.
PATCH v2 7/18 TestSupport: guest_create takes a $ho.
In reply to Ian CampbellAnd looks up the toolstack from it.
This is now consistent with guest_destroy.
--- Osstest/TestSupport.pm | 5 +++-- ts-debian-hvm-install | 7 ++----- ts-redhat-install | 7 ++----- ts-rumpuserxen-demo-xenstorels | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-)
Osstest/TestSupport.pm +3 -2
@@ -1330,8 +1330,9 @@ sub guest_destroy ($$) { } sub guest_create ($$) { - my ($gho,$toolstack) = @_; - target_cmd_root($gho->{Host}, "$toolstack create $gho->{CfgPath}", 100); + my ($ho,$gho) = @_; + my $ts= toolstack($ho); + target_cmd_root($ho, $ts->{Command}." create $gho->{CfgPath}", 100); }
ts-debian-hvm-install +2 -5
@@ -41,9 +41,6 @@ our $disk_mb= 10000; our $guesthost= "$gn.guest.osstest"; our $gho; -our $toolstack= toolstack($ho)->{Command}; - - sub preseed () { my $preseed_file = preseed_base('wheezy','',());
@@ -183,7 +180,7 @@ logm("Host has $host_freemem_mb MB free memory, setting guest memory size to $ra if (!$stage) { prep(); - guest_create($gho,$toolstack); + guest_create($ho,$gho); } else { $gho= selectguest($gn,$gho); }
@@ -193,6 +190,6 @@ if ($stage<2) { } guest_editconfig_nocd($gho,$emptyiso); -guest_create($gho,$toolstack); +guest_create($ho,$gho); guest_await_dhcp_tcp($gho,300); guest_check_up($gho);
ts-redhat-install +2 -5
@@ -37,9 +37,6 @@ our $disk_mb= 50000; our $guesthost= "$gn.guest.osstest"; our $gho; -our $xl= toolstack($ho)->{Command}; - - sub kickstart () { my $cryptpw= '$6$anjRJmBbJcrNJGWN$rqvGUhu8ITjvErdIA5C//w2R6b/6wAjHbaF7XF8u3lZg1XI3StthPIE6UII08scOFwASMOepCGpgtsYeCpjqb.'; my $authkeys= authorized_keys();
@@ -152,7 +149,7 @@ sub prep () { if (!$stage) { prep(); - guest_create($gho,$xl); + guest_create($ho,$gho); } else { $gho= selectguest($gn,$gho); }
@@ -162,6 +159,6 @@ if ($stage<2) { } guest_editconfig_nocd($gho,$emptyiso); -guest_create($gho,$xl); +guest_create($ho,$gho); guest_await_dhcp_tcp($gho,300); guest_check_up($gho);
ts-rumpuserxen-demo-xenstorels +1 -1
@@ -40,7 +40,7 @@ sub arrangepreserve () { } sub start () { - guest_create($gho,toolstack($ho)); + guest_create($ho, $gho); $domid = guest_find_domid($ho, $gho); }
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 07/18] TestSupport: guest_create takes a $ho.
Ian Campbell writes ("[PATCH OSSTEST v2 07/18] TestSupport: guest_create takes a $ho."):
Quoted 1 line
And looks up the toolstack from it.
...
Osstest/TestSupport.pm sub guest_destroy ($$) { · line 1333
Quoted 4 lines
sub
guest_create($$) {Osstest/TestSupport.pm +1
-2- my ($gho,$toolstack) = @_; - target_cmd_root($gho->{Host}, "$toolstack create $gho->{CfgPath}", 100); + my ($ho,$gho) = @_;
But a $gho has a $ho in $gho->{Host}. So perhaps you would rather just eliminate the $toolstack parameter ?
Ian.
PATCH v2 8/18 Toolstack: Refactor guest lifecycle.
In reply to Ian CampbellImplement destory/create as per toolstack methods, including implementing the libvirt version which previously didn't work. To do this we use the virsh capability to convert an xl/xm style config file into the correct XML.
xend basically calls into the xl helper since they are compatible.
xl/x, uses ->{Command} which will eventually become private.
--- Osstest/TestSupport.pm | 5 ++--- Osstest/Toolstack/libvirt.pm | 20 ++++++++++++++++++++ Osstest/Toolstack/xend.pm | 5 +++++ Osstest/Toolstack/xl.pm | 13 +++++++++++++ ts-guest-saverestore | 2 +- ts-guest-start | 4 +--- ts-windows-install | 7 +------ 7 files changed, 43 insertions(+), 13 deletions(-)
Osstest/TestSupport.pm +2 -3
@@ -1326,13 +1326,12 @@ sub guest_await_shutdown ($$$) { sub guest_destroy ($$) { my ($ho,$gho) = @_; - target_cmd_root($ho, toolstack($ho)->{Command}." destroy $gho->{Name}", 40); + toolstack($ho)->destroy($gho); } sub guest_create ($$) { my ($ho,$gho) = @_; - my $ts= toolstack($ho); - target_cmd_root($ho, $ts->{Command}." create $gho->{CfgPath}", 100); + toolstack($ho)->create($gho->{CfgPath}); }
Osstest/Toolstack/libvirt.pm +20 -0
@@ -20,15 +20,35 @@ package Osstest::Toolstack::libvirt; use strict; use warnings; +use Osstest::TestSupport; + sub new { my ($class, $ho, $methname,$asset) = @_; return bless { Name => "libvirt", Host => $ho, NewDaemons => [qw(libvirtd)], Dom0MemFixed => 1, + CfgPathVar => 'cfgpath', Command => 'virsh', ExtraPackages => [qw(libnl1 libavahi-client3)], }, $class; } +sub destroy ($$) { + my ($self,$gho) = @_; + my $gn = $gho->{Name}; + target_cmd_root($self->{Host}, "virsh destroy $gn", 40); +} + +sub create ($$) { + my ($self,$cfg) = @_; + my $ho = $self->{Host}; + my $lcfg = $cfg; + $lcfg =~ s,/,-,g; + $lcfg = "$ho->{Name}--$lcfg"; + target_cmd_root($ho, "virsh domxml-from-native xen-xm $cfg > $cfg.xml", 30); + target_getfile_root($ho,60,"$cfg.xml", "$stash/$lcfg"); + target_cmd_root($ho, "virsh create --file $cfg.xml", 100); +} + 1;
Osstest/Toolstack/xend.pm +5 -0
@@ -19,6 +19,7 @@ package Osstest::Toolstack::xend; use strict; use warnings; +use Osstest::Toolstack::xl; sub new { my ($class, $ho, $methname,$asset) = @_;
@@ -32,4 +33,8 @@ sub new { }, $class; } +# Defer to xl driver for most things +sub destroy { return &Osstest::Toolstack::xl::destroy; } +sub create { return &Osstest::Toolstack::xl::create; } + 1;
Osstest/Toolstack/xl.pm +13 -0
@@ -20,6 +20,8 @@ package Osstest::Toolstack::xl; use strict; use warnings; +use Osstest::TestSupport; + sub new { my ($class, $ho, $methname,$asset) = @_; return bless { Name => "xl",
@@ -32,4 +34,15 @@ sub new { }, $class; } +sub destroy ($$) { + my ($self,$gho) = @_; + my $gn = $gho->{Name}; + target_cmd_root($self->{Host}, $self->{Command}." destroy $gn", 40); +} + +sub create ($$) { + my ($self,$cfg) = @_; + target_cmd_root($self->{Host}, $self->{Command}." create $cfg", 100); +} + 1;
ts-guest-saverestore +1 -1
@@ -38,7 +38,7 @@ sub restore () { toolstack($ho)->{Command} ." restore " .(toolstack($ho)->{RestoreNeedsConfig} ? - $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} } : '') + $gho->{CfgPath} : '') ." image", 200); target_ping_check_up($gho); }
ts-guest-start +1 -3
@@ -26,9 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub start () { guest_umount_lv($ho, $gho); - my $cmd= toolstack($ho)->{Command}." create ". - $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }; - target_cmd_root($ho, $cmd, 30); + toolstack($ho)->create($r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }); } sub checkstart () {
ts-windows-install +1 -6
@@ -49,13 +49,8 @@ END store_runvar("$gho->{Guest}_pingbroken", 1); } -sub start () { - target_cmd_root($ho, toolstack($ho)->{Command}. - " create $gho->{CfgPath}", 100); -} - prep(); -start(); +guest_create($ho,$gho); guest_await_dhcp_tcp($gho,7000); guest_check_up($gho);
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
Osstest/Toolstack/libvirt.pm package Osstest::Toolstack::libvirt; · line 49
Quoted 7 lines · Ian Campbell
Osstest/Toolstack/libvirt.pm +7
-0+sub create ($$) { + my ($self,$cfg) = @_; + my $ho = $self->{Host}; + my $lcfg = $cfg; + $lcfg =~ s,/,-,g; + $lcfg = "$ho->{Name}--$lcfg"; + target_cmd_root($ho, "virsh domxml-from-native xen-xm $cfg > $cfg.xml", 30);
Given that Jim is in the process of introducing support for "xen-xl" cfg file consumption I intend to make this xen-xl unconditionally.
Testing older versions of libvirt (which don't speak xen-xl) will therefore continue to fail at the guest-start test step, as they do today, and hence it will not appears as a regression.
Testing newer versions of libvirt will pass as expected
Ian.
Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle."):
Quoted 9 lines · Ian Campbell
Osstest/Toolstack/libvirt.pm package Osstest::Toolstack::libvirt; · line 49
Quoted 7 lines
Osstest/Toolstack/libvirt.pm +7
-0+sub create ($$) { + my ($self,$cfg) = @_; + my $ho = $self->{Host}; + my $lcfg = $cfg; + $lcfg =~ s,/,-,g; + $lcfg = "$ho->{Name}--$lcfg"; + target_cmd_root($ho, "virsh domxml-from-native xen-xm $cfg > $cfg.xml", 30);
Quoted 8 lines
Given that Jim is in the process of introducing support for "xen-xl" cfg file consumption I intend to make this xen-xl unconditionally.
Testing older versions of libvirt (which don't speak xen-xl) will therefore continue to fail at the guest-start test step, as they do today, and hence it will not appears as a regression.
Testing newer versions of libvirt will pass as expected
That sounds good, thanks.
Ian.
Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
In reply to Ian CampbellIan Campbell writes ("[PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle."):
Quoted 7 lines
Implement destory/create as per toolstack methods, including implementing the libvirt version which previously didn't work. To do this we use the virsh capability to convert an xl/xm style config file into the correct XML.
xend basically calls into the xl helper since they are compatible.
xl/x, uses ->{Command} which will eventually become private.
...
ts-guest-saverestore sub restore () { · line 42
Quoted 11 lines
ts-guest-saverestore +1
-1@@ -38,7 +38,7 @@ sub restore () { toolstack($ho)->{Command} ." restore " .(toolstack($ho)->{RestoreNeedsConfig} ? - $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} } : '') + $gho->{CfgPath} : '') ." image", 200);
This hunk doesn't seem to be covered by the commit message ?
ts-guest-saverestore sub restore () { · line 43
Quoted 2 lines
ts-guest-saverestore +1
-1- target_cmd_root($ho, $cmd, 30); + toolstack($ho)->create($r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} });
Perhaps the toolstack create method should take the $gho rather than the config file pathname, like the destroy method does ?
Ian.
Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
Quoted 9 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle."):
Quoted 7 lines
Implement destory/create as per toolstack methods, including implementing the libvirt version which previously didn't work. To do this we use the virsh capability to convert an xl/xm style config file into the correct XML.
xend basically calls into the xl helper since they are compatible.
xl/x, uses ->{Command} which will eventually become private.
...
Quoted 13 lines
ts-guest-saverestore sub restore () { · line 42
Quoted 11 lines
ts-guest-saverestore +1
-1@@ -38,7 +38,7 @@ sub restore () { toolstack($ho)->{Command} ." restore " .(toolstack($ho)->{RestoreNeedsConfig} ? - $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} } : '') + $gho->{CfgPath} : '') ." image", 200);This hunk doesn't seem to be covered by the commit message ?
Indeed, not sure what was going on there. I was a bit confused about the CfgPathVar during early iterations of the series.
Quoted 5 lines
ts-guest-saverestore sub restore () { · line 43
Quoted 2 lines
ts-guest-saverestore +1
-1- target_cmd_root($ho, $cmd, 30); + toolstack($ho)->create($r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} });Perhaps the toolstack create method should take the $gho rather than the config file pathname, like the destroy method does ?
Yes. In fact towards the end of the series I end up arranging that $gho->{CfgPath} is consistently useful. Since reordering these API changes is a bit error prone I'd prefer to add a patch onto the end which reworks the create method's interface to just take $gho. I hope that's ok.
Ian.
Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 08/18] Toolstack: Refactor guest lifecycle."):
Quoted 8 lines
Quoted 2 lines
Perhaps the toolstack create method should take the $gho rather than the config file pathname, like the destroy method does ?
Yes. In fact towards the end of the series I end up arranging that $gho->{CfgPath} is consistently useful. Since reordering these API changes is a bit error prone I'd prefer to add a patch onto the end which reworks the create method's interface to just take $gho. I hope that's ok.
Sure, thanks.
Ian.
PATCH v2 9/18 Toolstack: Refactor consolecmd handling
In reply to Ian Campbell--- Osstest/Toolstack/libvirt.pm | 6 ++++++ Osstest/Toolstack/xend.pm | 1 + Osstest/Toolstack/xl.pm | 6 ++++++ ts-logs-capture | 2 +- 4 files changed, 14 insertions(+), 1 deletion(-)
Osstest/Toolstack/libvirt.pm +6 -0
@@ -51,4 +51,10 @@ sub create ($$) { target_cmd_root($ho, "virsh create --file $cfg.xml", 100); } +sub consolecmd ($$) { + my ($self,$gho) = @_; + my $gn = $gho->{Name}; + return "virsh console $gn"; +} + 1;
Osstest/Toolstack/xend.pm +1 -0
@@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } +sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } 1;
Osstest/Toolstack/xl.pm +6 -0
@@ -45,4 +45,10 @@ sub create ($$) { target_cmd_root($self->{Host}, $self->{Command}." create $cfg", 100); } +sub consolecmd ($$) { + my ($self,$gho) = @_; + my $gn = $gho->{Name}; + return $self->{Command}." console $gn"; +} + 1;
ts-logs-capture +1 -1
@@ -195,7 +195,7 @@ sub fetch_logs_guest ($) { logm("cannot find domid: $@"); return; } - my $consolecmd= toolstack($ho)->{Command}." console $gho->{Name}"; + my $consolecmd= toolstack($ho)->consolecmd($gho); try_cmd_output_save("sleep 1 | $consolecmd | cat", "guest-$gho->{Name}-console");
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Ian Campbell writes ("[PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
...
Osstest/Toolstack/xend.pm sub new { · line 39
Quoted 9 lines
Osstest/Toolstack/xend.pm +1
-0@@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } +sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; }
I hesitate to suggest this, but perhaps Exporter(3pm)'s inheritance feature (@ISA, I think) might do this more automatically ?
ts-logs-capture sub fetch_logs_guest ($) { · line 200
Quoted 12 lines
ts-logs-capture +1
-1@@ -195,7 +195,7 @@ sub fetch_logs_guest ($) { logm("cannot find domid: $@"); return; } - my $consolecmd= toolstack($ho)->{Command}." console $gho->{Name}"; + my $consolecmd= toolstack($ho)->consolecmd($gho); try_cmd_output_save("sleep 1 | $consolecmd | cat", "guest-$gho->{Name}-console");
Does `virsh console' work the same way as xl console, particularly wrt this weird thing with the sleep 1 etc. ?
Ian.
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Quoted 14 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
...Osstest/Toolstack/xend.pm sub new { · line 39
Quoted 9 lines
Osstest/Toolstack/xend.pm +1
-0@@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } +sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; }I hesitate to suggest this, but perhaps Exporter(3pm)'s inheritance feature (@ISA, I think) might do this more automatically ?
Perhaps, I can't seem to find Exporter(3pm) any where either in Debian or online. Lots of Exporter::Lite, ::Simple, ::Heavy type stuff though...
Quoted 7 lines
Quoted 7 lines
ts-logs-capture +0
-0@@ -195,7 +195,7 @@ sub fetch_logs_guest ($) { logm("cannot find domid: $@"); return;
Quoted 8 lines
ts-logs-capture sub fetch_logs_guest ($) { · line 200
Quoted 5 lines
ts-logs-capture +1
-1} - my $consolecmd= toolstack($ho)->{Command}." console $gho->{Name}"; + my $consolecmd= toolstack($ho)->consolecmd($gho); try_cmd_output_save("sleep 1 | $consolecmd | cat", "guest-$gho->{Name}-console");Does `virsh console' work the same way as xl console, particularly wrt this weird thing with the sleep 1 etc. ?
TBH I'm not sure. What is the reason for that weirdness?
Ultimately libvirt/virsh opens the pty directly as opposed to via the xenconsole binary, AFAICT.
Ian.
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Quoted 16 lines · Ian Campbell
Quoted 14 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
...Osstest/Toolstack/xend.pm sub new { · line 39
Quoted 9 lines
Osstest/Toolstack/xend.pm +1
-0@@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } +sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; }I hesitate to suggest this, but perhaps Exporter(3pm)'s inheritance feature (@ISA, I think) might do this more automatically ?
Quoted 3 lines
Perhaps, I can't seem to find Exporter(3pm) any where either in Debian or online. Lots of
Exporter::Lite, ::Simple, ::Heavy type stuff though...
Turns out that including the 3pm confuses search enginers, searching just for "perl Exporter" has found me what I need.
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Quoted 17 lines · Ian Campbell
Quoted 16 lines · Ian Campbell
Quoted 14 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
...Osstest/Toolstack/xend.pm sub new { · line 39
Quoted 9 lines
Osstest/Toolstack/xend.pm +1
-0@@ -36,5 +36,6 @@ sub new { # Defer to xl driver for most things sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } +sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; }I hesitate to suggest this, but perhaps Exporter(3pm)'s inheritance feature (@ISA, I think) might do this more automatically ?
Quoted 6 lines
Quoted 3 lines
Perhaps, I can't seem to find Exporter(3pm) any where either in Debian or online. Lots of
Exporter::Lite, ::Simple, ::Heavy type stuff though...Turns out that including the 3pm confuses search enginers, searching just for "perl Exporter" has found me what I need.
It seems that "use parent" is the right way to do this these days, in so much as the following seems to be recommended and to work. I'll fold it through the series into the appropriate places.
Ian.
Osstest/Toolstack/xend.pm +3 -10
@@ -19,7 +19,9 @@ package Osstest::Toolstack::xend; use strict; use warnings; -use Osstest::Toolstack::xl; + +# Defer to xl driver for most things +use parent qw(Osstest::Toolstack::xl); sub new { my ($class, $ho, $methname,$asset) = @_;
@@ -32,15 +34,6 @@ sub new { }, $class; } -# Defer to xl driver for most things -sub destroy { return &Osstest::Toolstack::xl::destroy; } -sub create { return &Osstest::Toolstack::xl::create; } -sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } -sub shutdown_wait { return &Osstest::Toolstack::xl::shutdown_wait; } -sub migrate { return &Osstest::Toolstack::xl::migrate; } -sub save { return &Osstest::Toolstack::xl::save; } -sub restore { return &Osstest::Toolstack::xl::restore; } - # xend always supported migration sub migrate_check ($) { return 0; }
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
Quoted 7 lines · Ian Campbell
Quoted 2 lines
Turns out that including the 3pm confuses search enginers, searching just for "perl Exporter" has found me what I need.
It seems that "use parent" is the right way to do this these days, in so much as the following seems to be recommended and to work. I'll fold it through the series into the appropriate places.
DYK, how far back is this supported ?
Ian.
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Quoted 10 lines · Ian Jackson
Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
Quoted 7 lines · Ian Campbell
Quoted 2 lines
Turns out that including the 3pm confuses search enginers, searching just for "perl Exporter" has found me what I need.
It seems that "use parent" is the right way to do this these days, in so much as the following seems to be recommended and to work. I'll fold it through the series into the appropriate places.
DYK, how far back is this supported ?
http://perldoc.perl.org/parent.html says "introduced with Perl 5.004_04.".
Ian.
Ian Jackson
Acked-byRe: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
Quoted 5 lines · Ian Jackson
Quoted 1 line
DYK, how far back is this supported ?
http://perldoc.perl.org/parent.html says "introduced with Perl 5.004_04.".
Wow. The Perl docs are really out of date - perlmod still has an example with Exporter.
(I wouldn't bother folding these changes in unless you feel like it.)
Thanks,
Ian.
Re: [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling
Quoted 11 lines · Ian Jackson
Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST v2 09/18] Toolstack: Refactor consolecmd handling"):
Quoted 5 lines · Ian Jackson
Quoted 1 line
DYK, how far back is this supported ?
http://perldoc.perl.org/parent.html says "introduced with Perl 5.004_04.".
Wow. The Perl docs are really out of date - perlmod still has an example with Exporter.
Acked-by Ian Jackson <[email protected]>
Thanks.
Quoted 1 line
(I wouldn't bother folding these changes in unless you feel like it.)
I already did it...
So I'll take the Ack as permission not to drop the Ack from patches modified in this way, hope that's ok.
Ian.
PATCH v2 10/18 Toolstack: Refactor shutdown support
In reply to Ian Campbell--- Osstest/Toolstack/libvirt.pm | 6 ++++++ Osstest/Toolstack/xend.pm | 1 + Osstest/Toolstack/xl.pm | 7 +++++++ ts-guest-stop | 5 +---- 4 files changed, 15 insertions(+), 4 deletions(-)
Osstest/Toolstack/libvirt.pm +6 -0
@@ -57,4 +57,10 @@ sub consolecmd ($$) { return "virsh console $gn"; } +sub shutdown_wait ($$) { + my ($self,$gho) = @_; + my $gn = $gho->{Name}; + die "libvirt shutdown wait not implemented yet." +} + 1;
Osstest/Toolstack/xend.pm +1 -0
@@ -37,5 +37,6 @@ sub new { sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } +sub shutdown_wait { return &Osstest::Toolstack::xl::shutdown_wait; } 1;
Osstest/Toolstack/xl.pm +7 -0
@@ -51,4 +51,11 @@ sub consolecmd ($$) { return $self->{Command}." console $gn"; } +sub shutdown_wait ($$) { + my ($self,$gho) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho,"$self->{Command} shutdown -w $gn", 200); +} + 1;
ts-guest-stop +1 -4
@@ -26,10 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub stop () { guest_checkrunning($ho, $gho) or die "$gho->{Name} not running"; - target_cmd_root($ho, - toolstack($ho)->{Command} - ." shutdown -w " - .$gho->{Name}, 200); + toolstack($ho)->shutdown_wait($gho); guest_checkrunning($ho, $gho) and die $gho->{Name}; }
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 10/18] Toolstack: Refactor shutdown support
Ian Campbell writes ("[PATCH OSSTEST v2 10/18] Toolstack: Refactor shutdown support"):
Osstest/Toolstack/xl.pm sub consolecmd ($$) { · line 58
Quoted 5 lines
Osstest/Toolstack/xl.pm +5
-0+sub shutdown_wait ($$) { + my ($self,$gho) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho,"$self->{Command} shutdown -w $gn", 200);
I think the timeout should be in the callers, not here. Different guests might take different times to shut down - but different toolstacks shouldn't.
Ian.
PATCH v2 11/18 Toolstack: Refactor migration support check.
In reply to Ian CampbellNot implemented for libvirt (the check itself that is, the hook is present).
--- Osstest/Toolstack/libvirt.pm | 5 +++++ Osstest/Toolstack/xend.pm | 3 +++ Osstest/Toolstack/xl.pm | 9 +++++++++ ts-migrate-support-check | 10 +--------- 4 files changed, 18 insertions(+), 9 deletions(-)
Osstest/Toolstack/libvirt.pm +5 -0
@@ -63,4 +63,9 @@ sub shutdown_wait ($$) { die "libvirt shutdown wait not implemented yet." } +sub migrate_check ($) { + my ($self) = @_; + die "Migration check is not yet supported on libvirt."; +} + 1;
Osstest/Toolstack/xend.pm +3 -0
@@ -39,4 +39,7 @@ sub create { return &Osstest::Toolstack::xl::create; } sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } sub shutdown_wait { return &Osstest::Toolstack::xl::shutdown_wait; } +# xend always supported migration +sub migrate_check ($) { return 0; } + 1;
Osstest/Toolstack/xl.pm +9 -0
@@ -58,4 +58,13 @@ sub shutdown_wait ($$) { target_cmd_root($ho,"$self->{Command} shutdown -w $gn", 200); } +sub migrate_check ($) { + my ($self) = @_; + my $ho = $self->{Host}; + my $help = target_cmd_output_root($ho, $self->{Command}." help"); + my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; + logm("rc=$rc"); + return $rc; +} + 1;
ts-migrate-support-check +1 -9
@@ -24,12 +24,4 @@ tsreadconfig(); our $ho = selecthost($ARGV[0]); -# all xend/xm platforms support migration -exit(0) if toolstack($ho)->{Command} eq "xm"; - -my $help = target_cmd_output_root($ho, toolstack($ho)->{Command}." help"); - -my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; - -logm("rc=$rc"); -exit($rc); +exit(toolstack($ho)->migrate_check());
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 11/18] Toolstack: Refactor migration support check.
Ian Campbell writes ("[PATCH OSSTEST v2 11/18] Toolstack: Refactor migration support check."):
Quoted 1 line
Not implemented for libvirt (the check itself that is, the hook is present).
PATCH v2 12/18 Toolstack: Refactor migration support.
In reply to Ian CampbellNote that since the previous patch arranges for ts-migration-support-check to continue to fail for libvirt the libvirt code is not actually called yet (and will die if it is). This patch is mainly included to reduce the number of users of toolstack()->{Command} closer to zero.
--- Osstest/Toolstack/libvirt.pm | 5 +++++ Osstest/Toolstack/xend.pm | 1 + Osstest/Toolstack/xl.pm | 9 +++++++++ ts-guest-localmigrate | 5 +---- ts-guest-migrate | 5 +---- 5 files changed, 17 insertions(+), 8 deletions(-)
Osstest/Toolstack/libvirt.pm +5 -0
@@ -68,4 +68,9 @@ sub migrate_check ($) { die "Migration check is not yet supported on libvirt."; } +sub migrate ($) { + my ($self,$gho,$dst,$to) = @_; + die "Migration is not yet supported on libvirt."; +} + 1;
Osstest/Toolstack/xend.pm +1 -0
@@ -38,6 +38,7 @@ sub destroy { return &Osstest::Toolstack::xl::destroy; } sub create { return &Osstest::Toolstack::xl::create; } sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } sub shutdown_wait { return &Osstest::Toolstack::xl::shutdown_wait; } +sub migrate { return &Osstest::Toolstack::xl::migrate; } # xend always supported migration sub migrate_check ($) { return 0; }
Osstest/Toolstack/xl.pm +9 -0
@@ -67,4 +67,13 @@ sub migrate_check ($) { return $rc; } +sub migrate ($$$$) { + my ($self,$gho,$dst,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + $self->{Command}." migrate $gn $dst", + $to); +} + 1;
ts-guest-localmigrate +1 -4
@@ -32,10 +32,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub migrate () { guest_checkrunning($ho,$gho) or die $gho->{Name}; - target_cmd_root($ho, - toolstack($ho)->{Command} - ." migrate $gho->{Name} localhost", - $timeout{Migrate}); + toolstack($ho)->migrate($gho, "localhost", $timeout{Migrate}); } guest_await_dhcp_tcp($gho, 5);
ts-guest-migrate +1 -4
@@ -31,10 +31,7 @@ our $gho = selectguest($ARGV[2],$sho); sub migrate () { guest_checkrunning($sho,$gho) or die $gho->{Name}; my $err= guest_check_ip($gho); die "$err $gho->{Name}" if defined $err; - target_cmd_root($sho, - toolstack($sho)->{Command} - ." migrate $gho->{Name} $dho->{Name}", - $timeout{Migrate}); + toolstack($sho)->migrate($gho, $dho->{Name}, $timeout{Migrate}); } guest_await_dhcp_tcp($gho, 5);
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Ian Campbell writes ("[PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 5 lines
Note that since the previous patch arranges for ts-migration-support-check to continue to fail for libvirt the libvirt code is not actually called yet (and will die if it is). This patch is mainly included to reduce the number of users of toolstack()->{Command} closer to zero.
...
Osstest/Toolstack/xl.pm sub migrate_check ($) { · line 77
Quoted 8 lines
Osstest/Toolstack/xl.pm +8
-0+sub migrate ($$$$) { + my ($self,$gho,$dst,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + $self->{Command}." migrate $gn $dst", + $to); +}
Again, I think you should pass the $dho rather than the name. In general it's better to pass the hash object to these kind of methods.
Ian.
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Quoted 7 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 5 lines
Note that since the previous patch arranges for ts-migration-support-check to continue to fail for libvirt the libvirt code is not actually called yet (and will die if it is). This patch is mainly included to reduce the number of users of toolstack()->{Command} closer to zero.
...
Quoted 11 lines
Osstest/Toolstack/xl.pm sub migrate_check ($) { · line 77
Quoted 8 lines
Osstest/Toolstack/xl.pm +8
-0+sub migrate ($$$$) { + my ($self,$gho,$dst,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + $self->{Command}." migrate $gn $dst", + $to); +}Again, I think you should pass the $dho rather than the name. In general it's better to pass the hash object to these kind of methods.
For ts-guest-localmigrate is there a cunning stunt you would recommend to generate a $dho with ->{Name} == "localhost"?
Or perhaps:
$dho ? $dho->{Name} : "localhost"? (i.e. $dho==undef => localhost)
Ian.
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 10 lines · Ian Jackson
Quoted 2 lines
Again, I think you should pass the $dho rather than the name. In general it's better to pass the hash object to these kind of methods.
For ts-guest-localmigrate is there a cunning stunt you would recommend to generate a $dho with ->{Name} == "localhost"?
Or perhaps:
$dho ? $dho->{Name} : "localhost"? (i.e. $dho==undef => localhost)
That would do, although it's a big magic.
Magic in a different way would be:
->migrate($ts, $gho, { %$dho, Name => 'localhost' }, $timeout)
Ian.
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Quoted 12 lines · Ian Jackson
Ian Campbell writes ("Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 10 lines · Ian Jackson
Quoted 2 lines
Again, I think you should pass the $dho rather than the name. In general it's better to pass the hash object to these kind of methods.
For ts-guest-localmigrate is there a cunning stunt you would recommend to generate a $dho with ->{Name} == "localhost"?
Or perhaps:
$dho ? $dho->{Name} : "localhost"? (i.e. $dho==undef => localhost)
Quoted 5 lines
That would do, although it's a big magic.
Magic in a different way would be:
->migrate($ts, $gho, { %$dho, Name => 'localhost' }, $timeout)
In the caller? There is no $dho in that case. But I think maybe you meant $gho? (i.e. this is supposed to produce a copy of $gho with a different name)
Ian.
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 8 lines · Ian Jackson
Quoted 3 lines
Magic in a different way would be:
->migrate($ts, $gho, { %$dho, Name => 'localhost' }, $timeout)
In the caller? There is no $dho in that case. But I think maybe you meant $gho? (i.e. this is supposed to produce a copy of $gho with a different name)
Yes. Indeed, I meant $gho.
%$gho expands to a list of (key, value, key, value, ...) which you can feed to hash constructors like { }.
So { %$gho } is a shallow copy. And later values for a particular key in the list override earlier ones.
For some other examples, git-grep '%\$'
Ian.
Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support.
In reply to Ian CampbellQuoted 15 lines · Ian Campbell
Quoted 14 lines · Ian Jackson
Ian Campbell writes ("Re: [PATCH OSSTEST v2 12/18] Toolstack: Refactor migration support."):
Quoted 10 lines · Ian Jackson
Quoted 2 lines
Again, I think you should pass the $dho rather than the name. In general it's better to pass the hash object to these kind of methods.
For ts-guest-localmigrate is there a cunning stunt you would recommend to generate a $dho with ->{Name} == "localhost"?
Or perhaps:
$dho ? $dho->{Name} : "localhost"? (i.e. $dho==undef => localhost)
That would do, although it's a big magic.
Quoted 7 lines
Quoted 3 lines
Magic in a different way would be:
->migrate($ts, $gho, { %$dho, Name => 'localhost' }, $timeout)
In the caller? There is no $dho in that case. But I think maybe you meant $gho? (i.e. this is supposed to produce a copy of $gho with a different name)
Actually in this context I think $ho is what was wanted, that's what I went with.
Ian.
PATCH v2 13/18 Toolstack: Refactor save/restore support
In reply to Ian CampbellStill stubbed out for libvirt.
--- Osstest/Toolstack/libvirt.pm | 8 ++++++++ Osstest/Toolstack/xend.pm | 2 ++ Osstest/Toolstack/xl.pm | 18 ++++++++++++++++++ ts-guest-saverestore | 12 ++---------- 4 files changed, 30 insertions(+), 10 deletions(-)
Osstest/Toolstack/libvirt.pm +8 -0
@@ -73,4 +73,12 @@ sub migrate ($) { die "Migration is not yet supported on libvirt."; } +sub save ($$$$) { + die "Save is not yet supported on libvirt."; +} + +sub restore ($$$$$) { + die "Restore is not yet supported on libvirt."; +} + 1;
Osstest/Toolstack/xend.pm +2 -0
@@ -39,6 +39,8 @@ sub create { return &Osstest::Toolstack::xl::create; } sub consolecmd { return &Osstest::Toolstack::xl::consolecmd; } sub shutdown_wait { return &Osstest::Toolstack::xl::shutdown_wait; } sub migrate { return &Osstest::Toolstack::xl::migrate; } +sub save { return &Osstest::Toolstack::xl::save; } +sub restore { return &Osstest::Toolstack::xl::restore; } # xend always supported migration sub migrate_check ($) { return 0; }
Osstest/Toolstack/xl.pm +18 -0
@@ -76,4 +76,22 @@ sub migrate ($$$$) { $to); } +sub save ($$$$) { + my ($self,$gho,$f,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho,$self->{Command}." save $gn $f", $to); +} + +sub restore ($$$$$) { + my ($self,$gho,$f,$cfg,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + $self->{Command} + ." restore " + .($self->{RestoreNeedsConfig} ? $cfg : '') + ." $f", $to); +} + 1;
ts-guest-saverestore +2 -10
@@ -27,19 +27,11 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub save () { guest_checkrunning($ho,$gho) or die $gho->{Name}; my $err= guest_check_ip($gho); die "$err $gho->{Name}" if defined $err; - target_cmd_root($ho, - toolstack($ho)->{Command} - ." save $gho->{Name} image", - 200); + toolstack($ho)->save($gho,"image",200); target_ping_check_down($gho); } sub restore () { - target_cmd_root($ho, - toolstack($ho)->{Command} - ." restore " - .(toolstack($ho)->{RestoreNeedsConfig} ? - $gho->{CfgPath} : '') - ." image", 200); + toolstack($ho)->restore($gho,"image",$gho->{CfgPath},200); target_ping_check_up($gho); }
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 13/18] Toolstack: Refactor save/restore support
Ian Campbell writes ("[PATCH OSSTEST v2 13/18] Toolstack: Refactor save/restore support"):
Quoted 1 line
Still stubbed out for libvirt.
...
Osstest/Toolstack/xl.pm sub migrate ($$$$) { · line 94
Quoted 9 lines
Osstest/Toolstack/xl.pm +9
-0+sub restore ($$$$$) { + my ($self,$gho,$f,$cfg,$to) = @_; + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + $self->{Command} + ." restore " + .($self->{RestoreNeedsConfig} ? $cfg : '') + ." $f", $to);
Why pass the $cfg explicitly to restore() when we can get it out of $gho ? I notice that the timeout here is in the argument list where it should be.
But, please call the timeout variable $timeout', not $to'. That's what it's called elsewhere.
Ian.
PATCH v2 14/18 libvirt: Implement initscript restart which has some hope of working.
In reply to Ian Campbell--- ts-libvirt-build | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ts-libvirt-build +4 -2
@@ -140,8 +140,10 @@ case "$1" in [ "$VERBOSE" != no ] && log_end_msg $? ;; restart) - stop - start + [ "$VERBOSE" != no ] && log_daemon_msg "Restarting $DESC" "$DAEMON" + start-stop-daemon --oknodo --stop --pidfile $PIDFILE --exec $DAEMON + start-stop-daemon --start --pidfile $PIDFILE --exec $DAEMON -- -d $libvirtd_opts + [ "$VERBOSE" != no ] && log_end_msg $? ;; reload|force-reload) start-stop-daemon --stop --signal 1 --quiet --pidfile \
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 14/18] libvirt: Implement initscript restart which has some hope of working.
Ian Campbell writes ("[PATCH OSSTEST v2 14/18] libvirt: Implement initscript restart which has some hope of working."):
...
ts-libvirt-build case "$1" in · line 146
Quoted 7 lines
restart)
ts-libvirt-build +4
-2- stop - start + [ "$VERBOSE" != no ] && log_daemon_msg "Restarting $DESC" "$DAEMON" + start-stop-daemon --oknodo --stop --pidfile $PIDFILE --exec $DAEMON + start-stop-daemon --start --pidfile $PIDFILE --exec $DAEMON -- -d $libvirtd_opts + [ "$VERBOSE" != no ] && log_end_msg $?
Well, the old code is pretty bad because the functions start() and stop() don't exist. But wouldn't it be better to move the code into functions, rather than duplicating it ?
(AFAICT the stop code in restart differs in that it uses --oknodo but you could pass that to your `stop' function.)
Ian.
PATCH v2 15/18 libvirt: Implement shutdown_wait
In reply to Ian Campbellvirsh does not include a --wait option to shutdown as xl and xm do, so we implement it by hand.
Needs new guest_await_destroy helper. Note the guest_await_shutdown requires on_shutdown='preserve'
--- Osstest/TestSupport.pm | 7 ++++++- Osstest/Toolstack/libvirt.pm | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-)
Osstest/TestSupport.pm +6 -1
@@ -93,7 +93,7 @@ BEGIN { guest_checkrunning guest_check_ip guest_find_ether guest_find_domid guest_check_up guest_check_up_quick guest_get_state guest_await_reboot - guest_await_shutdown guest_destroy + guest_await_shutdown guest_await_destroy guest_destroy guest_vncsnapshot_begin guest_vncsnapshot_stash guest_check_remus_ok guest_editconfig host_involves_pcipassthrough host_get_pcipassthrough_devs
@@ -1324,6 +1324,11 @@ sub guest_await_shutdown ($$$) { return guest_await_state($ho,$gho, "shutdown", "s", $timeout); } +sub guest_await_destroy ($$$) { + my ($ho,$gho, $timeout) = @_; + return guest_await_state($ho,$gho, "destroy", "", $timeout); +} + sub guest_destroy ($$) { my ($ho,$gho) = @_; toolstack($ho)->destroy($gho);
Osstest/Toolstack/libvirt.pm +3 -1
@@ -59,8 +59,10 @@ sub consolecmd ($$) { sub shutdown_wait ($$) { my ($self,$gho) = @_; + my $ho = $self->{Host}; my $gn = $gho->{Name}; - die "libvirt shutdown wait not implemented yet." + target_cmd_root($ho, "virsh shutdown $gn", 30); + guest_await_destroy($ho,$gho,200); } sub migrate_check ($) {
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 15/18] libvirt: Implement shutdown_wait
Ian Campbell writes ("[PATCH OSSTEST v2 15/18] libvirt: Implement shutdown_wait"):
Quoted 5 lines
virsh does not include a --wait option to shutdown as xl and xm do, so we implement it by hand.
Needs new
guest_await_destroyhelper. Note theguest_await_shutdownrequires on_shutdown='preserve'
I see that the existing ts-guest-stop doesn't check that the guest stopped with a correct exit code, which is rather poor.
Thanks,
Ian.
PATCH v2 16/18 Toolstack: Remove Command field for all toolstacks.
In reply to Ian CampbellNothing in generic code uses this now, so remove.
xl+xend retain as _Command for internal use only.
--- Osstest/Toolstack/libvirt.pm | 1 - Osstest/Toolstack/xend.pm | 2 +- Osstest/Toolstack/xl.pm | 18 +++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-)
Osstest/Toolstack/libvirt.pm +0 -1
@@ -29,7 +29,6 @@ sub new { NewDaemons => [qw(libvirtd)], Dom0MemFixed => 1, CfgPathVar => 'cfgpath', - Command => 'virsh', ExtraPackages => [qw(libnl1 libavahi-client3)], }, $class; }
Osstest/Toolstack/xend.pm +1 -1
@@ -27,7 +27,7 @@ sub new { Host => $ho, NewDaemons => [qw(xend)], OldDaemonInitd => 'xend', - Command => 'xm', + _Command => 'xm', CfgPathVar => 'cfgpath', Dom0MemFixed => 1, }, $class;
Osstest/Toolstack/xl.pm +9 -9
@@ -28,7 +28,7 @@ sub new { Host => $ho, NewDaemons => [], Dom0MemFixed => 1, - Command => 'xl', + _Command => 'xl', CfgPathVar => 'cfgpath', RestoreNeedsConfig => 1, }, $class;
@@ -37,31 +37,31 @@ sub new { sub destroy ($$) { my ($self,$gho) = @_; my $gn = $gho->{Name}; - target_cmd_root($self->{Host}, $self->{Command}." destroy $gn", 40); + target_cmd_root($self->{Host}, $self->{_Command}." destroy $gn", 40); } sub create ($$) { my ($self,$cfg) = @_; - target_cmd_root($self->{Host}, $self->{Command}." create $cfg", 100); + target_cmd_root($self->{Host}, $self->{_Command}." create $cfg", 100); } sub consolecmd ($$) { my ($self,$gho) = @_; my $gn = $gho->{Name}; - return $self->{Command}." console $gn"; + return $self->{_Command}." console $gn"; } sub shutdown_wait ($$) { my ($self,$gho) = @_; my $ho = $self->{Host}; my $gn = $gho->{Name}; - target_cmd_root($ho,"$self->{Command} shutdown -w $gn", 200); + target_cmd_root($ho,"$self->{_Command} shutdown -w $gn", 200); } sub migrate_check ($) { my ($self) = @_; my $ho = $self->{Host}; - my $help = target_cmd_output_root($ho, $self->{Command}." help"); + my $help = target_cmd_output_root($ho, $self->{_Command}." help"); my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1; logm("rc=$rc"); return $rc;
@@ -72,7 +72,7 @@ sub migrate ($$$$) { my $ho = $self->{Host}; my $gn = $gho->{Name}; target_cmd_root($ho, - $self->{Command}." migrate $gn $dst", + $self->{_Command}." migrate $gn $dst", $to); }
@@ -80,7 +80,7 @@ sub save ($$$$) { my ($self,$gho,$f,$to) = @_; my $ho = $self->{Host}; my $gn = $gho->{Name}; - target_cmd_root($ho,$self->{Command}." save $gn $f", $to); + target_cmd_root($ho,$self->{_Command}." save $gn $f", $to); } sub restore ($$$$$) {
@@ -88,7 +88,7 @@ sub restore ($$$$$) { my $ho = $self->{Host}; my $gn = $gho->{Name}; target_cmd_root($ho, - $self->{Command} + $self->{_Command} ." restore " .($self->{RestoreNeedsConfig} ? $cfg : '') ." $f", $to);
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks.
Ian Campbell writes ("[PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks."):
Quoted 3 lines
Nothing in generic code uses this now, so remove.
xl+xend retain as
_Commandfor internal use only.
Although personally I wouldn't bother with the rename.
Ian.
Re: [PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks.
Quoted 6 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks."):
Quoted 3 lines
Nothing in generic code uses this now, so remove.
xl+xend retain as
_Commandfor internal use only.Acked-by Ian Jackson <[email protected]>
Thanks.
Quoted 1 line
Although personally I wouldn't bother with the rename.
It was mainly to make it easier to grep for all the prexisting uses of {Command} which I was trying to eradicate. With that done perhaps it is redundant, although maybe we want to leave it as _Command so inflight patch series start using it again...
Ian.
Re: [PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 16/18] Toolstack: Remove Command field for all toolstacks."):
Quoted 7 lines · Ian Jackson
Quoted 1 line
Although personally I wouldn't bother with the rename.
It was mainly to make it easier to grep for all the prexisting uses of {Command} which I was trying to eradicate. With that done perhaps it is redundant, although maybe we want to leave it as
_Commandso inflight patch series start using it again...
Mmm, yes. Ok.
Thanks,
Ian.
PATCH v2 17/18 ts-guest-start: Use guest_create
In reply to Ian CampbellThis allows us to abolish CfgPathVar which was inconsistently used, appears redundant with $gho->{CfgPath} and in any case never set to anything other than 'cfgpath'.
I suppose it was intended to deal with toolstacks with a cfg format completely dissimilar to xm/xl's. I think if this arises in a future toolstack this functionality could be reintroduced pretty trivially via the toolstack abstraction which is added by this series.
--- Osstest/Toolstack/libvirt.pm | 1 - Osstest/Toolstack/xend.pm | 1 - Osstest/Toolstack/xl.pm | 1 - ts-guest-start | 2 +- 4 files changed, 1 insertion(+), 4 deletions(-)
Osstest/Toolstack/libvirt.pm +0 -1
@@ -28,7 +28,6 @@ sub new { Host => $ho, NewDaemons => [qw(libvirtd)], Dom0MemFixed => 1, - CfgPathVar => 'cfgpath', ExtraPackages => [qw(libnl1 libavahi-client3)], }, $class; }
Osstest/Toolstack/xend.pm +0 -1
@@ -28,7 +28,6 @@ sub new { NewDaemons => [qw(xend)], OldDaemonInitd => 'xend', _Command => 'xm', - CfgPathVar => 'cfgpath', Dom0MemFixed => 1, }, $class; }
Osstest/Toolstack/xl.pm +0 -1
@@ -29,7 +29,6 @@ sub new { NewDaemons => [], Dom0MemFixed => 1, _Command => 'xl', - CfgPathVar => 'cfgpath', RestoreNeedsConfig => 1, }, $class; }
ts-guest-start +1 -1
@@ -26,7 +26,7 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV); sub start () { guest_umount_lv($ho, $gho); - toolstack($ho)->create($r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} }); + guest_create($ho,$gho); } sub checkstart () {
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 17/18] ts-guest-start: Use guest_create
Ian Campbell writes ("[PATCH OSSTEST v2 17/18] ts-guest-start: Use guest_create"):
Quoted 8 lines
This allows us to abolish CfgPathVar which was inconsistently used, appears redundant with $gho->{CfgPath} and in any case never set to anything other than 'cfgpath'.
I suppose it was intended to deal with toolstacks with a cfg format completely dissimilar to xm/xl's. I think if this arises in a future toolstack this functionality could be reintroduced pretty trivially via the toolstack abstraction which is added by this series.
Very early in libxl's history it couldn't cope with xm-format config files and had its own format (with mandatory semicolons and various other differences). This is now irrelevant.
PATCH v2 18/18 WIP: libvirt: migration + save/restore support.
In reply to Ian CampbellNote that this remains stubbed out, since making it actually work requires more work (i.e. I need to figure out what is involved, seem to need TLS and a CA etc...)
Appears to need gnutls enabling for migration, even to localhost.
NB haven't managed to get this actually working. With GNUtls enabled
it wants a CA certificate installed:
error: Cannot read CA certificate '/etc/pki/CA/cacert.pem': No such file or directory
Other attempts give:
# virsh migrate --live debian.guest.osstest xen://
error: Requested operation is not valid: domain 'debian.guest.osstest' is already activeAlso need to try tcp:// based, by bodging libvirtd.conf to turn on tcp.
TODO: Ask Jim for advice...
--- Osstest/Toolstack/libvirt.pm | 17 +++++++++++++++++ ts-libvirt-build | 3 +++ ts-xen-build-prep | 1 + 3 files changed, 21 insertions(+)
Osstest/Toolstack/libvirt.pm +17 -0
@@ -65,12 +65,29 @@ sub shutdown_wait ($$) { sub migrate_check ($) { my ($self) = @_; + + # Localhost migration currently requires more setup on our side + # than we currently do. Once that is sorted out the code below + # will correctly determine whether the version of libvirt under + # test even supports the possibility. die "Migration check is not yet supported on libvirt."; + + my $ho = $self->{Host}; + my $caps = target_cmd_output_root($ho, "virsh capabilities"); + my $rc = ($caps =~ m/<migration_features>/) ? 0 : 1; + logm("rc=$rc"); + return $rc; } sub migrate ($) { my ($self,$gho,$dst,$to) = @_; die "Migration is not yet supported on libvirt."; + + my $ho = $self->{Host}; + my $gn = $gho->{Name}; + target_cmd_root($ho, + "virsh migrate $gn $dst", + $to); } sub save ($$$$) {
ts-libvirt-build +3 -0
@@ -55,6 +55,7 @@ sub config() { ./autogen.sh --no-git \\ --with-libxl --without-xen --without-xenapi --without-selinux \\ --without-lxc --without-vbox --without-uml \\ + --with-gnutls \\ --sysconfdir=/etc --localstatedir=/var #/ END }
@@ -80,6 +81,8 @@ END $builddir.'/dist/etc/init.d/libvirtd'); target_cmd_build($ho, 60, $builddir, <<END); chmod +x $builddir/dist/etc/init.d/libvirtd + #sed -i -e 's/\#\?listen_tls = .*/listen_tls = 0/g' $builddir/dist/etc/libvirt/libvirtd.conf + #sed -i -e 's/\#\?listen_tcp = .*/listen_tcp = 1/g' $builddir/dist/etc/libvirt/libvirtd.conf END }
ts-xen-build-prep +1 -0
@@ -174,6 +174,7 @@ sub prep () { libpci-dev libncurses5-dev libssl-dev python-dev libx11-dev git-core uuid-dev gettext gawk libsdl-dev libyajl-dev libaio-dev libpixman-1-dev + libgnutls-dev libglib2.0-dev liblzma-dev pkg-config autoconf automake libtool xsltproc libxml2-utils libxml2-dev libnl-dev
Signature · 1 line
2.1.1
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support.
Quoted 10 lines · Ian Campbell
Note that this remains stubbed out, since making it actually work requires more work (i.e. I need to figure out what is involved, seem to need TLS and a CA etc...)
Appears to need gnutls enabling for migration, even to localhost.
NB haven't managed to get this actually working. With GNUtls enabled
it wants a CA certificate installed:
error: Cannot read CA certificate '/etc/pki/CA/cacert.pem': No such file or directory
Quoted 7 lines
Other attempts give:
# virsh migrate --live debian.guest.osstest xen:// error: Requested operation is not valid: domain 'debian.guest.osstest' is already activeAlso need to try tcp:// based, by bodging libvirtd.conf to turn on tcp.
Tried this. It didn't make any difference with regard to the error message you saw, i.e. it's the same as above.
I've also played with changing domain name and UUID (since they are what libvirt is checking), but it didn't work either. Error messages were different but libvirt still refuses to migrate to localhost.
On the other hand, save and restore worked just fine. I have a patch to implement save and restore, which is literally calling "virsh save" and "virsh restore".
One way to move forward is to separate the concept of migration support and save / restore support, so that we can test save / restore for libvirt, even if we can test local migration at the moment.
Of course if we can get local migration to work that would be best.
Quoted 2 lines
TODO: Ask Jim for advice...
Jim, any suggestion?
Wei.
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support.
In reply to Ian CampbellIan Campbell writes ("[PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support."):
Quoted 3 lines
Note that this remains stubbed out, since making it actually work requires more work (i.e. I need to figure out what is involved, seem to need TLS and a CA etc...)
How annoying.
Quoted 3 lines
NB haven't managed to get this actually working. With GNUtls enabled
it wants a CA certificate installed:
error: Cannot read CA certificate '/etc/pki/CA/cacert.pem': No such file or directory
Let me pick a nit:
Osstest/Toolstack/libvirt.pm sub shutdown_wait ($$) { · line 77
Quoted 3 lines
Osstest/Toolstack/libvirt.pm +3
-0+ my $ho = $self->{Host}; + my $caps = target_cmd_output_root($ho, "virsh capabilities"); + my $rc = ($caps =~ m/<migration_features>/) ? 0 : 1;
You should probaby escape the < > with \ in case later Perls make them metacharacters.
Thanks,
Ian.
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support.
Quoted 9 lines · Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support."):
Quoted 4 lines
Note that this remains stubbed out, since making it actually work requires more work (i.e. I need to figure out what is involved, seem to need TLS and a CA etc...)
How annoying.
You can get around this by disabling TLS in /etc/libvirt/libvirtd.conf. Then simply use ssh to connect to the dst host, e.g. 'virsh migrate --live dom xen+ssh://dst-host'.
Alternatively, enable listen_tcp then migrate with 'virsh migrate --live xen+tcp://dst-host'
Quoted 11 lines
ts-libvirt-build +1
-0@@ -80,6 +81,8 @@ END $builddir.'/dist/etc/init.d/libvirtd'); target_cmd_build($ho, 60, $builddir, <<END); chmod +x $builddir/dist/etc/init.d/libvirtd + #sed -i -e 's/\#\?listen_tls = .*/listen_tls = 0/g'
$builddir/dist/etc/libvirt/libvirtd.conf
+ #sed -i -e 's/\#\?listen_tcp = .*/listen_tcp = 1/g'$builddir/dist/etc/libvirt/libvirtd.conf
END
}
Yes, something like that :-). I'd disable TLS and use the xen+ssh connection URI.
Regards,
Jim
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support.
Quoted 1 line · Jim Fehlig
Ian Jackson wrote:
Thanks both. I've CCd Wei who has picked up the libvirt migration stuff based on this series.
Quoted 35 lines
Quoted 9 lines
Ian Campbell writes ("[PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support."):
Quoted 4 lines
Note that this remains stubbed out, since making it actually work requires more work (i.e. I need to figure out what is involved, seem to need TLS and a CA etc...)
How annoying.
You can get around this by disabling TLS in
/etc/libvirt/libvirtd.conf. Then simply use ssh to connect to the dst host, e.g. 'virsh migrate --live dom xen+ssh://dst-host'.Alternatively, enable
listen_tcpthen migrate with 'virsh migrate --live xen+tcp://dst-host'Quoted 11 lines
ts-libvirt-build +1
-0@@ -80,6 +81,8 @@ END $builddir.'/dist/etc/init.d/libvirtd'); target_cmd_build($ho, 60, $builddir, <<END); chmod +x $builddir/dist/etc/init.d/libvirtd + #sed -i -e 's/\#\?listen_tls = .*/listen_tls = 0/g'
$builddir/dist/etc/libvirt/libvirtd.conf
+ #sed -i -e 's/\#\?listen_tcp = .*/listen_tcp = 1/g'$builddir/dist/etc/libvirt/libvirtd.conf
END
}Yes, something like that :-). I'd disable TLS and use the xen+ssh connection URI.
Regards,
Jim
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support. [and 1 more messages]
In reply to Jim FehligJim Fehlig writes ("Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support."):
Quoted 3 lines
You can get around this by disabling TLS in
/etc/libvirt/libvirtd.conf. Then simply use ssh to connect to the dst host, e.g. 'virsh migrate --live dom xen+ssh://dst-host'.
Is this the way people would normally use libvirt to do migration ?
(osstest's xl migration test already uses ssh for migration, but that's because that's the most convenient and usual way to use `xl migrate'.)
Ian.
Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support. [and 1 more messages]
Quoted 9 lines · Ian Jackson
Jim Fehlig writes ("Re: [PATCH OSSTEST v2 18/18] WIP: libvirt: migration + save/restore support."):
Quoted 4 lines
You can get around this by disabling TLS in
/etc/libvirt/libvirtd.conf. Then simply use ssh to connect to the dst host, e.g. 'virsh migrate --live dom xen+ssh://dst-host'.Is this the way people would normally use libvirt to do migration ?
Yes, it is a common approach for the same reasons. It is convenient and configured "out of the box".
Regards,
Jim
Quoted 7 lines
(osstest's xl migration test already uses ssh for migration, but that's because that's the most convenient and usual way to use `xl migrate'.)
Ian.