[PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[Ian Campbell]

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. Wei has a followup series (or at
least a WIP?) which adds migration support. I've dropped my WIP patch
from last time in favour of that.

Since last time I've addressed all of Ian's review on v2. Lots of
passing $gho to things instead of individual properties, switch to a
better inheritance scheme for Osstest::Toolstack::xend, 

*	 TestSupport: Add helper to wait for a guest to shutdown
	 apt: lock osstest's usages of apt-get against each other
A	 ts-logs-capture: Collect some libvirt logs and capabilities
A	 Pass host to toolstack()
A	 ts-rumpuserxen-demo-xenstorels: Use standard functions for things
	 Toolstack: use get_host_method_object() to manage toolstack selection
	 TestSupport: always use xl for generic operations.
	 TestSupport: guest_create and guest_destroy take only a $gho.
	 Toolstack: Refactor guest lifecycle.
	 Toolstack: Refactor consolecmd handling
	 Toolstack: Refactor shutdown support
A	 Toolstack: Refactor migration support check.
	 Toolstack: Refactor migration support.
	 Toolstack: Refactor save/restore support
	 libvirt: Implement initscript restart which has some hope of working.
A	 libvirt: Implement shutdown_wait
A	 Toolstack: Remove Command field for all toolstacks.
A	 ts-guest-start: Use guest_create
N	 Toolstack: Pass $gho to create method

* == "TestSupport: Add helper to wait for a guest to shutdown") wasn't
posted last time, it was actually part of the "add distro domU testing
flight" series, but it is needed here too and I expect this one to land
first.

N == New
A == Acked

Ian.

[PATCH v3 01/19] TestSupport: Add helper to wait for a guest to shutdown

[Ian Campbell]

Refactor the guts of guest_await_reboot into a helper and use for both
shutdown and reboot handling.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Refactor common code with guest_await_reboot.
---
 Osstest/TestSupport.pm | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 5ac66e5..b983bd4 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -92,7 +92,8 @@ BEGIN {
                       guest_umount_lv guest_await guest_await_dhcp_tcp
                       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_destroy
+                      guest_get_state guest_await_reboot
+                      guest_await_shutdown guest_destroy
                       guest_vncsnapshot_begin guest_vncsnapshot_stash
 		      guest_check_remus_ok guest_editconfig
                       host_involves_pcipassthrough host_get_pcipassthrough_devs
@@ -1295,17 +1296,28 @@ sub report_once ($$$) {
     $ho->{$k}= $msg;
 }
 
-sub guest_await_reboot ($$$) {
-    my ($ho,$gho, $timeout) = @_;
-    poll_loop($timeout, 30, "await reboot request from $gho->{Guest}", sub {
+sub guest_await_state ($$$$$) {
+    my ($ho,$gho, $what,$wait_st,$timeout) = @_;
+
+    poll_loop($timeout, 30, "await $what request from $gho->{Guest}", sub {
         my $st= guest_get_state($ho,$gho);
-        return undef if $st eq 'sr';
+        return undef if $st eq $wait_st;
         fail("guest unexpectedly shutdown; state is '$st'")
             if $st =~ m/^s/ || $st eq '';
-        return "guest state is $st";
+        return "guest state is \"$st\"";
     });
 }
 
+sub guest_await_reboot ($$$) {
+    my ($ho,$gho, $timeout) = @_;
+    return guest_await_state($ho,$gho, "reboot", "sr", $timeout);
+}
+
+sub guest_await_shutdown ($$$) {
+    my ($ho,$gho, $timeout) = @_;
+    return guest_await_state($ho,$gho, "shutdown", "s", $timeout);
+}
+
 sub guest_destroy ($$) {
     my ($ho,$gho) = @_;
     target_cmd_root($ho, toolstack()->{Command}." destroy $gho->{Name}", 40);
-- 
2.1.4

Re: [PATCH v3 01/19] TestSupport: Add helper to wait for a guest to shutdown

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 01/19] TestSupport: Add helper to wait for a guest to shutdown"):
> Refactor the guts of guest_await_reboot into a helper and use for both
> shutdown and reboot handling.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 02/19] apt: lock osstest's usages of apt-get against each other

[Ian Campbell]

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)

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Drop timeout parameter to target_run_apt
    Reindent target_run_apt more sensibly
---
 Osstest/Debian.pm      |  2 +-
 Osstest/TestSupport.pm | 12 +++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 7081a29..d4bf62d 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -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}
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index b983bd4..17fb4ce 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -434,20 +434,18 @@ sub target_putfile_root ($$$$;$) {
     tputfileex('root', @_);
 }
 sub target_run_apt {
-    my ($ho, $timeout, @aptopts) = @_;
+    my ($ho, @aptopts) = @_;
     target_cmd_root($ho,
-   "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y apt-get @aptopts",
-                    $timeout);
+        "DEBIAN_PRIORITY=critical UCF_FORCE_CONFFOLD=y \
+            with-lock-ex -w /var/lock/osstest-apt apt-get @aptopts", 3000);
 }
 sub target_install_packages {
     my ($ho, @packages) = @_;
-    target_run_apt($ho, 300 + 100 * @packages,
-		   qw(-y install), @packages);
+    target_run_apt($ho, qw(-y install), @packages);
 }
 sub target_install_packages_norec {
     my ($ho, @packages) = @_;
-    target_run_apt($ho, 300 + 100 * @packages,
-		   qw(--no-install-recommends -y install), @packages);
+    target_run_apt($ho, qw(--no-install-recommends -y install), @packages);
 }
 
 sub target_somefile_getleaf ($$$) {
-- 
2.1.4

Re: [PATCH v3 02/19] apt: lock osstest's usages of apt-get against each other

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 02/19] 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.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 03/19] ts-logs-capture: Collect some libvirt logs and capabilities

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-logs-capture | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ts-logs-capture b/ts-logs-capture
index 21974a9..6cf51c1 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -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);
         }
-- 
2.1.4

[PATCH v3 04/19] Pass host to toolstack()

[Ian Campbell]

This 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.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 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(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 17fb4ce..28d8767 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -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 ($$) {
@@ -1611,7 +1612,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 ?";
@@ -1816,7 +1817,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}";
@@ -1886,7 +1887,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};
@@ -1895,6 +1898,7 @@ sub toolstack () {
         logm("toolstack $tsname");
         $ts->{Name}= $tsname;
     }
+    $ho->{Toolstack} = $ts;
     return $ts;
 }
 
diff --git a/ts-debian-fixup b/ts-debian-fixup
index b560501..ee1ec98 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -148,7 +148,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)"
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 37eade2..0148eef 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -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) {
diff --git a/ts-guest-localmigrate b/ts-guest-localmigrate
index e50e93a..f3381da 100755
--- a/ts-guest-localmigrate
+++ b/ts-guest-localmigrate
@@ -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});
 }
diff --git a/ts-guest-migrate b/ts-guest-migrate
index 17ac8a0..65e7b42 100755
--- a/ts-guest-migrate
+++ b/ts-guest-migrate
@@ -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});
 }
diff --git a/ts-guest-saverestore b/ts-guest-saverestore
index 81671c8..9e04ae9 100755
--- a/ts-guest-saverestore
+++ b/ts-guest-saverestore
@@ -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);
 }
diff --git a/ts-guest-start b/ts-guest-start
index 057afe6..bfbb734 100755
--- a/ts-guest-start
+++ b/ts-guest-start
@@ -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);
 }
 
diff --git a/ts-guest-stop b/ts-guest-stop
index cc7db4c..0e3a863 100755
--- a/ts-guest-stop
+++ b/ts-guest-stop
@@ -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};
diff --git a/ts-logs-capture b/ts-logs-capture
index 6cf51c1..841ad5a 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -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");
 
diff --git a/ts-migrate-support-check b/ts-migrate-support-check
index ffae1b3..c70b77a 100755
--- a/ts-migrate-support-check
+++ b/ts-migrate-support-check
@@ -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;
 
diff --git a/ts-redhat-install b/ts-redhat-install
index 56d4129..a0b1fab 100755
--- a/ts-redhat-install
+++ b/ts-redhat-install
@@ -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 () {
diff --git a/ts-rumpuserxen-demo-xenstorels b/ts-rumpuserxen-demo-xenstorels
index 6db7024..a2a6a77 100755
--- a/ts-rumpuserxen-demo-xenstorels
+++ b/ts-rumpuserxen-demo-xenstorels
@@ -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}");
diff --git a/ts-windows-install b/ts-windows-install
index 0a69f8e..4b06310 100755
--- a/ts-windows-install
+++ b/ts-windows-install
@@ -50,7 +50,7 @@ END
 }
 
 sub start () {
-    target_cmd_root($ho, toolstack()->{Command}.
+    target_cmd_root($ho, toolstack($ho)->{Command}.
                     " create $gho->{CfgPath}", 100);
 }
 
diff --git a/ts-xen-install b/ts-xen-install
index 4d34d1f..7cfe344 100755
--- a/ts-xen-install
+++ b/ts-xen-install
@@ -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);
-- 
2.1.4

[PATCH v3 05/19] ts-rumpuserxen-demo-xenstorels: Use standard functions for things

[Ian Campbell]

Specifically guest_create and guest_find_domid.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 ts-rumpuserxen-demo-xenstorels | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ts-rumpuserxen-demo-xenstorels b/ts-rumpuserxen-demo-xenstorels
index a2a6a77..6698848 100755
--- a/ts-rumpuserxen-demo-xenstorels
+++ b/ts-rumpuserxen-demo-xenstorels
@@ -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 () {
-- 
2.1.4

[PATCH v3 06/19] Toolstack: use get_host_method_object() to manage toolstack selection

[Ian Campbell]

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.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: Use "use parent" to pull in xl baseline for xend.
---
 Osstest/TestSupport.pm       | 37 ++++---------------------------------
 Osstest/Toolstack/libvirt.pm | 34 ++++++++++++++++++++++++++++++++++
 Osstest/Toolstack/xend.pm    | 38 ++++++++++++++++++++++++++++++++++++++
 Osstest/Toolstack/xl.pm      | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 111 insertions(+), 33 deletions(-)
 create mode 100644 Osstest/Toolstack/libvirt.pm
 create mode 100644 Osstest/Toolstack/xend.pm
 create mode 100644 Osstest/Toolstack/xl.pm

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 28d8767..14d6b47 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1864,42 +1864,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 () {
diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
new file mode 100644
index 0000000..90fe434
--- /dev/null
+++ b/Osstest/Toolstack/libvirt.pm
@@ -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;
diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm
new file mode 100644
index 0000000..720bcc8
--- /dev/null
+++ b/Osstest/Toolstack/xend.pm
@@ -0,0 +1,38 @@
+# 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;
+
+# Defer to xl driver for most things
+use parent qw(Osstest::Toolstack::xl);
+
+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;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
new file mode 100644
index 0000000..0b66201
--- /dev/null
+++ b/Osstest/Toolstack/xl.pm
@@ -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;
-- 
2.1.4

[PATCH v3 07/19] TestSupport: always use xl for generic operations.

[Ian Campbell]

Unless 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

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: common_toolstack() takes a $ho and checks if toolstack($ho) is
    xend.
---
 Osstest/TestSupport.pm | 14 +++++++++++---
 ts-debian-fixup        |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 14d6b47..37df50e 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -936,6 +936,14 @@ sub compress_stashed($) {
 
 #---------- other stuff ----------
 
+sub common_toolstack ($) {
+    my ($ho) =  @_;
+    my $tsname = toolstack($ho);
+    my $ts = 'xl';
+    $ts = 'xm' if $tsname eq 'xend';
+    return $ts;
+}
+
 sub host_reboot ($) {
     my ($ho) = @_;
     target_reboot($ho);
@@ -956,7 +964,7 @@ END
 
 sub host_get_free_memory($) {
     my ($ho) = @_;
-    my $toolstack = toolstack($ho)->{Command};
+    my $toolstack = common_toolstack($ho);
     # The line is as followed:
     # free_memory       :   XXXX
     my $info = target_cmd_output_root($ho, "$toolstack info", 10);
@@ -1612,7 +1620,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($ho)." 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 ?";
@@ -1817,7 +1825,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($ho)." 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}";
diff --git a/ts-debian-fixup b/ts-debian-fixup
index ee1ec98..beae049 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -148,7 +148,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($ho).
                                    " pci-assignable-list");
         foreach my $pcipt (@pcipt) {
             die "not assignable: $pcipt (not in: @assignables)"
-- 
2.1.4

Re: [PATCH v3 07/19] TestSupport: always use xl for generic operations.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 07/19] TestSupport: always use xl for generic operations."):
> Unless the toolstack is xend (for compatibility with pre-xl Xen
> versions), when we use xm.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

> 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

I think at the very least "guest_get_state" should be implemented
per-toolstack to exercise the toolstack's query functionality.  But
that's not a blocker for this series.

Ian.

Re: [PATCH v3 07/19] TestSupport: always use xl for generic operations.

[Ian Campbell]

On Tue, 2015-01-27 at 16:40 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH v3 07/19] TestSupport: always use xl for generic operations."):
> > Unless the toolstack is xend (for compatibility with pre-xl Xen
> > versions), when we use xm.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks.

> > 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
> 
> I think at the very least "guest_get_state" should be implemented
> per-toolstack to exercise the toolstack's query functionality.  But
> that's not a blocker for this series.

True. I'll see if I can make that happen for v4.

Ian.

Re: [PATCH v3 07/19] TestSupport: always use xl for generic operations.

[Ian Campbell]

On Tue, 2015-01-27 at 17:09 +0000, Ian Campbell wrote:
> On Tue, 2015-01-27 at 16:40 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("[PATCH v3 07/19] TestSupport: always use xl for generic operations."):
> > > Unless the toolstack is xend (for compatibility with pre-xl Xen
> > > versions), when we use xm.
> > 
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> Thanks.
> 
> > > 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
> > 
> > I think at the very least "guest_get_state" should be implemented
> > per-toolstack to exercise the toolstack's query functionality.  But
> > that's not a blocker for this series.
> 
> True. I'll see if I can make that happen for v4.

... I didn't.

I've just looked now and I don't see a way to extract the necessary
granularity info via virsh needed to replicate the interface which the
callers expect. I can just get running or not.

Possibly with some refactoring the callers could be made to ask
questions which virsh can answer, but I don't propose to work on that
right now.

Ian.

[PATCH v3 08/19] TestSupport: guest_create and guest_destroy take only a $gho.

[Ian Campbell]

The host can be looked up from $gho->{Host} and the toolstack can be
looked up from the host.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: This replaces "TestSupport: guest_create takes a $ho"
---
 Osstest/TestSupport.pm         | 12 +++++++-----
 ts-debian-hvm-install          |  9 +++------
 ts-guest-destroy               |  2 +-
 ts-guest-destroy-hard          |  2 +-
 ts-redhat-install              |  9 +++------
 ts-rumpuserxen-demo-xenstorels |  2 +-
 6 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 37df50e..6124d19 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1325,14 +1325,16 @@ sub guest_await_shutdown ($$$) {
     return guest_await_state($ho,$gho, "shutdown", "s", $timeout);
 }
 
-sub guest_destroy ($$) {
-    my ($ho,$gho) = @_;
+sub guest_destroy ($) {
+    my ($gho) = @_;
+    my $ho = $gho->{Host};
     target_cmd_root($ho, toolstack($ho)->{Command}." destroy $gho->{Name}", 40);
 }
 
-sub guest_create ($$) {
-    my ($gho,$toolstack) = @_;
-    target_cmd_root($gho->{Host}, "$toolstack create $gho->{CfgPath}", 100);
+sub guest_create ($) {
+    my ($gho) = @_;
+    my $ho = $gho->{Host};
+    target_cmd_root($ho, toolstack($ho)->{Command}." create $gho->{CfgPath}", 100);
 }
 
 
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 0148eef..449b96c 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -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,16 +180,16 @@ logm("Host has $host_freemem_mb MB free memory, setting guest memory size to $ra
 
 if (!$stage) {
     prep();
-    guest_create($gho,$toolstack);
+    guest_create($gho);
 } else {
     $gho= selectguest($gn,$gho);
 }
 if ($stage<2) {
     guest_await_reboot($ho,$gho,2000);
-    guest_destroy($ho,$gho);
+    guest_destroy($gho);
 }
 
 guest_editconfig_nocd($gho,$emptyiso);
-guest_create($gho,$toolstack);
+guest_create($gho);
 guest_await_dhcp_tcp($gho,300);
 guest_check_up($gho);
diff --git a/ts-guest-destroy b/ts-guest-destroy
index 738650a..b4340f6 100755
--- a/ts-guest-destroy
+++ b/ts-guest-destroy
@@ -25,7 +25,7 @@ tsreadconfig();
 our ($ho,$gho) = ts_get_host_guest(@ARGV);
 
 sub destroy () {
-    guest_destroy($ho, $gho);
+    guest_destroy($gho);
     guest_checkrunning($ho, $gho) and die $gho->{Name};
 }
 
diff --git a/ts-guest-destroy-hard b/ts-guest-destroy-hard
index 457ac72..57d7100 100755
--- a/ts-guest-destroy-hard
+++ b/ts-guest-destroy-hard
@@ -24,4 +24,4 @@ tsreadconfig();
 
 our ($ho,$gho) = ts_get_host_guest(@ARGV);
 
-guest_destroy($ho, $gho);
+guest_destroy($gho);
diff --git a/ts-redhat-install b/ts-redhat-install
index a0b1fab..22c3061 100755
--- a/ts-redhat-install
+++ b/ts-redhat-install
@@ -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,16 +149,16 @@ sub prep () {
 
 if (!$stage) {
     prep();
-    guest_create($gho,$xl);
+    guest_create($gho);
 } else {
     $gho= selectguest($gn,$gho);
 }
 if ($stage<2) {
     guest_await_reboot($ho,$gho,2000);
-    guest_destroy($ho,$gho);
+    guest_destroy($gho);
 }
 
 guest_editconfig_nocd($gho,$emptyiso);
-guest_create($gho,$xl);
+guest_create($gho);
 guest_await_dhcp_tcp($gho,300);
 guest_check_up($gho);
diff --git a/ts-rumpuserxen-demo-xenstorels b/ts-rumpuserxen-demo-xenstorels
index 6698848..ed46843 100755
--- a/ts-rumpuserxen-demo-xenstorels
+++ b/ts-rumpuserxen-demo-xenstorels
@@ -40,7 +40,7 @@ sub arrangepreserve () {
 }
 
 sub start () {
-    guest_create($gho,toolstack($ho));
+    guest_create($gho);
 
     $domid = guest_find_domid($ho, $gho);
 }
-- 
2.1.4

Re: [PATCH v3 08/19] TestSupport: guest_create and guest_destroy take only a $gho.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 08/19] TestSupport: guest_create and guest_destroy take only a $gho."):
> The host can be looked up from $gho->{Host} and the toolstack can be
> looked up from the host.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 09/19] Toolstack: Refactor guest lifecycle.

[Ian Campbell]

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/xm, uses ->{Command} which will eventually become private.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Rebased of prototype changes to guest_create/destroy (only $gho)
    Use xen-xl format to domxml-from-native rather than xen-xm
    Moved spurious change in ts-save-restore to correct home later in
    the series.
    Suggested switch to passing $gho to ->start has been deferred to a
    new patch towards the end of the series
---
 Osstest/TestSupport.pm       |  4 ++--
 Osstest/Toolstack/libvirt.pm | 20 ++++++++++++++++++++
 Osstest/Toolstack/xl.pm      | 13 +++++++++++++
 ts-guest-start               |  4 +---
 ts-windows-install           |  7 +------
 5 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 6124d19..5c753b4 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1328,13 +1328,13 @@ sub guest_await_shutdown ($$$) {
 sub guest_destroy ($) {
     my ($gho) = @_;
     my $ho = $gho->{Host};
-    target_cmd_root($ho, toolstack($ho)->{Command}." destroy $gho->{Name}", 40);
+    toolstack($ho)->destroy($gho);
 }
 
 sub guest_create ($) {
     my ($gho) = @_;
     my $ho = $gho->{Host};
-    target_cmd_root($ho, toolstack($ho)->{Command}." create $gho->{CfgPath}", 100);
+    toolstack($ho)->create($gho->{CfgPath});
 }
 
 
diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 90fe434..0d09ffc 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -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-xl $cfg > $cfg.xml", 30);
+    target_getfile_root($ho,60,"$cfg.xml", "$stash/$lcfg");
+    target_cmd_root($ho, "virsh create --file $cfg.xml", 100);
+}
+
 1;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 0b66201..12417ca 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -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;
diff --git a/ts-guest-start b/ts-guest-start
index bfbb734..fb6a174 100755
--- a/ts-guest-start
+++ b/ts-guest-start
@@ -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 () {
diff --git a/ts-windows-install b/ts-windows-install
index 4b06310..7740ee6 100755
--- a/ts-windows-install
+++ b/ts-windows-install
@@ -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($gho);
 
 guest_await_dhcp_tcp($gho,7000);
 guest_check_up($gho);
-- 
2.1.4

Re: [PATCH v3 09/19] Toolstack: Refactor guest lifecycle.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 09/19] Toolstack: Refactor guest lifecycle."):
> 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.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 10/19] Toolstack: Refactor consolecmd handling

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Osstest/Toolstack/libvirt.pm | 6 ++++++
 Osstest/Toolstack/xl.pm      | 6 ++++++
 ts-logs-capture              | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 0d09ffc..fb9b9a9 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -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;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 12417ca..4997775 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -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;
diff --git a/ts-logs-capture b/ts-logs-capture
index 841ad5a..dbca13a 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -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");
 
-- 
2.1.4

Re: [PATCH v3 10/19] Toolstack: Refactor consolecmd handling

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 10/19] Toolstack: Refactor consolecmd handling"):
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 11/19] Toolstack: Refactor shutdown support

[Ian Campbell]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Timeout is set in the caller, not the method handler. "Different
    guests might take different times to shut down - but different
    toolstacks shouldn't."
---
 Osstest/Toolstack/libvirt.pm | 6 ++++++
 Osstest/Toolstack/xl.pm      | 7 +++++++
 ts-guest-stop                | 5 +----
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index fb9b9a9..7f4180e 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -57,4 +57,10 @@ sub consolecmd ($$) {
     return "virsh console $gn";
 }
 
+sub shutdown_wait ($$$) {
+    my ($self,$gho,$timeout) = @_;
+    my $gn = $gho->{Name};
+    die "libvirt shutdown wait not implemented yet."
+}
+
 1;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 4997775..f0c15fb 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -51,4 +51,11 @@ sub consolecmd ($$) {
     return $self->{Command}." console $gn";
 }
 
+sub shutdown_wait ($$$) {
+    my ($self,$gho,$timeout) = @_;
+    my $ho = $self->{Host};
+    my $gn = $gho->{Name};
+    target_cmd_root($ho,"$self->{Command} shutdown -w $gn", $timeout);
+}
+
 1;
diff --git a/ts-guest-stop b/ts-guest-stop
index 0e3a863..378f334 100755
--- a/ts-guest-stop
+++ b/ts-guest-stop
@@ -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, 200);
     guest_checkrunning($ho, $gho) and die $gho->{Name};
 }
 
-- 
2.1.4

Re: [PATCH v3 11/19] Toolstack: Refactor shutdown support

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 11/19] Toolstack: Refactor shutdown support"):
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v3: Timeout is set in the caller, not the method handler. "Different
>     guests might take different times to shut down - but different
>     toolstacks shouldn't."

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 12/19] Toolstack: Refactor migration support check.

[Ian Campbell]

Not implemented for libvirt (the check itself that is, the hook is
present).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 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(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 7f4180e..b74c8cb 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -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;
diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm
index 720bcc8..8e77a05 100644
--- a/Osstest/Toolstack/xend.pm
+++ b/Osstest/Toolstack/xend.pm
@@ -35,4 +35,7 @@ sub new {
     }, $class;
 }
 
+# xend always supported migration
+sub migrate_check ($) { return 0; }
+
 1;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index f0c15fb..dcf3b08 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -58,4 +58,13 @@ sub shutdown_wait ($$$) {
     target_cmd_root($ho,"$self->{Command} shutdown -w $gn", $timeout);
 }
 
+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;
diff --git a/ts-migrate-support-check b/ts-migrate-support-check
index c70b77a..cd41f68 100755
--- a/ts-migrate-support-check
+++ b/ts-migrate-support-check
@@ -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());
-- 
2.1.4

[PATCH v3 13/19] Toolstack: Refactor migration support.

[Ian Campbell]

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.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Pass a $ho object as the destination, rather than it's name.
---
 Osstest/Toolstack/libvirt.pm |  5 +++++
 Osstest/Toolstack/xl.pm      | 10 ++++++++++
 ts-guest-localmigrate        |  6 ++----
 ts-guest-migrate             |  5 +----
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index b74c8cb..3f84be8 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -68,4 +68,9 @@ sub migrate_check ($) {
     die "Migration check is not yet supported on libvirt.";
 }
 
+sub migrate ($) {
+    my ($self,$gho,$dst,$timeout) = @_;
+    die "Migration is not yet supported on libvirt.";
+}
+
 1;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index dcf3b08..4f710dd 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -67,4 +67,14 @@ sub migrate_check ($) {
     return $rc;
 }
 
+sub migrate ($$$$) {
+    my ($self,$gho,$dho,$timeout) = @_;
+    my $sho = $self->{Host};
+    my $dst = $dho->{Name};
+    my $gn = $gho->{Name};
+    target_cmd_root($sho,
+		    $self->{Command}." migrate $gn $dst",
+		    $timeout);
+}
+
 1;
diff --git a/ts-guest-localmigrate b/ts-guest-localmigrate
index f3381da..a831521 100755
--- a/ts-guest-localmigrate
+++ b/ts-guest-localmigrate
@@ -32,10 +32,8 @@ 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});
+    # Migrate to same host, via localhost interface.
+    toolstack($ho)->migrate($gho, { %$ho, Name => "localhost" }, $timeout{Migrate});
 }
 
 guest_await_dhcp_tcp($gho, 5);
diff --git a/ts-guest-migrate b/ts-guest-migrate
index 65e7b42..b77d0de 100755
--- a/ts-guest-migrate
+++ b/ts-guest-migrate
@@ -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, $timeout{Migrate});
 }
 
 guest_await_dhcp_tcp($gho, 5);
-- 
2.1.4

Re: [PATCH v3 13/19] Toolstack: Refactor migration support.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 13/19] Toolstack: Refactor migration support."):
> 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.
...
> +++ b/ts-guest-localmigrate
> @@ -32,10 +32,8 @@ our ($ho,$gho) = ts_get_host_guest(@ARGV);
...
> +    # Migrate to same host, via localhost interface.
> +    toolstack($ho)->migrate($gho, { %$ho, Name => "localhost" }, $timeout{Migrate});

Can you wrap this line please ?  With that done,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks,
Ian.

[PATCH v3 14/19] Toolstack: Refactor save/restore support

[Ian Campbell]

Use $gho->{CfgPath} rather than constructing it again.

Still stubbed out for libvirt.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Switch to CfgPath defered from incorrect home in earlier patch to
    here.
    RestoreNeedsConfig is an xl/xm ism, not relevant to libvirt, so
    remove $cfg parameter from restore method.
---
 Osstest/Toolstack/libvirt.pm | 10 ++++++++++
 Osstest/Toolstack/xl.pm      | 19 +++++++++++++++++++
 ts-guest-saverestore         | 12 ++----------
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 3f84be8..471b42e 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -73,4 +73,14 @@ sub migrate ($) {
     die "Migration is not yet supported on libvirt.";
 }
 
+sub save ($$$$) {
+    my ($self,$gho,$f,$timeout) = @_;
+    die "Save is not yet supported on libvirt.";
+}
+
+sub restore ($$$$) {
+    my ($self,$gho,$f,$timeout) = @_;
+    die "Restore is not yet supported on libvirt.";
+}
+
 1;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 4f710dd..b730783 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -77,4 +77,23 @@ sub migrate ($$$$) {
 		    $timeout);
 }
 
+sub save ($$$$) {
+    my ($self,$gho,$f,$timeout) = @_;
+    my $ho = $self->{Host};
+    my $gn = $gho->{Name};
+    target_cmd_root($ho,$self->{Command}." save $gn $f", $timeout);
+}
+
+sub restore ($$$$) {
+    my ($self,$gho,$f,$timeout) = @_;
+    my $ho = $self->{Host};
+    my $gn = $gho->{Name};
+    my $cfg = $self->{RestoreNeedsConfig} ? $gho->{CfgPath} : '';
+    target_cmd_root($ho,
+		    $self->{Command}
+		    ." restore "
+		    .$cfg
+		    ." $f", $timeout);
+}
+
 1;
diff --git a/ts-guest-saverestore b/ts-guest-saverestore
index 9e04ae9..7e13d90 100755
--- a/ts-guest-saverestore
+++ b/ts-guest-saverestore
@@ -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} ?
-		      $r{ $gho->{Guest}.'_'. toolstack($ho)->{CfgPathVar} } : '')
-		    ." image", 200);
+    toolstack($ho)->restore($gho,"image", 200);
     target_ping_check_up($gho);
 }
 
-- 
2.1.4

Re: [PATCH v3 14/19] Toolstack: Refactor save/restore support

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 14/19] Toolstack: Refactor save/restore support"):
> Use $gho->{CfgPath} rather than constructing it again.
> 
> Still stubbed out for libvirt.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 15/19] libvirt: Implement initscript restart which has some hope of working.

[Ian Campbell]

... by refactoring start/stop actions into the functions which are
expected by restart.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Refactor into start/stop functions
---
 ts-libvirt-build | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/ts-libvirt-build b/ts-libvirt-build
index 8c5d1fc..7322d73 100755
--- a/ts-libvirt-build
+++ b/ts-libvirt-build
@@ -128,22 +128,32 @@ test -x $DAEMON || exit 0
 
 PIDFILE=/var/run/$NAME.pid
 
+start() {
+	start-stop-daemon --start --pidfile $PIDFILE \
+	    --exec $DAEMON $@ -- -d $libvirtd_opts
+}
+
+stop() {
+	start-stop-daemon --stop --pidfile $PIDFILE \
+	    --exec $DAEMON $@
+}
+
 case "$1" in
   start)
 	[ "$VERBOSE" != no ] && log_daemon_msg "Starting $DESC" "$NAME"
-	start-stop-daemon --start --pidfile $PIDFILE \
-	    --exec $DAEMON -- -d $libvirtd_opts
+	start
 	[ "$VERBOSE" != no ] && log_end_msg $?
 	;;
   stop)
 	[ "$VERBOSE" != no ] && log_daemon_msg "Stopping $DESC" "$NAME"
-	start-stop-daemon --stop --pidfile $PIDFILE \
-	    --exec $DAEMON
+	stop
 	[ "$VERBOSE" != no ] && log_end_msg $?
 	;;
   restart)
-	stop
+	[ "$VERBOSE" != no ] && log_daemon_msg "Restarting $DESC" "$DAEMON"
+	stop --oknodo
 	start
+	[ "$VERBOSE" != no ] && log_end_msg $?
 	;;
   reload|force-reload)
 	start-stop-daemon --stop --signal 1 --quiet --pidfile \
-- 
2.1.4

Re: [PATCH v3 15/19] libvirt: Implement initscript restart which has some hope of working.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 15/19] libvirt: Implement initscript restart which has some hope of working."):
> ... by refactoring start/stop actions into the functions which are
> expected by restart.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH v3 16/19] libvirt: Implement shutdown_wait

[Ian Campbell]

virsh 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'

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: Drop $ho parameter frm guest_await_destroy, for consistency with
    changes to guest_destroy.
---
 Osstest/TestSupport.pm       | 8 +++++++-
 Osstest/Toolstack/libvirt.pm | 4 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 5c753b4..427ead4 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -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
@@ -1331,6 +1331,12 @@ sub guest_destroy ($) {
     toolstack($ho)->destroy($gho);
 }
 
+sub guest_await_destroy ($$) {
+    my ($gho, $timeout) = @_;
+    my $ho = $gho->{Host};
+    return guest_await_state($ho,$gho, "destroy", "", $timeout);
+}
+
 sub guest_create ($) {
     my ($gho) = @_;
     my $ho = $gho->{Host};
diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 471b42e..28c5cb1 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -59,8 +59,10 @@ sub consolecmd ($$) {
 
 sub shutdown_wait ($$$) {
     my ($self,$gho,$timeout) = @_;
+    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($gho,$timeout);
 }
 
 sub migrate_check ($) {
-- 
2.1.4

[PATCH v3 17/19] Toolstack: Remove Command field for all toolstacks.

[Ian Campbell]

Nothing in generic code uses this now, so remove.

xl+xend retain as _Command for internal use only.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 Osstest/Toolstack/libvirt.pm |  1 -
 Osstest/Toolstack/xend.pm    |  2 +-
 Osstest/Toolstack/xl.pm      | 18 +++++++++---------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 28c5cb1..f5f099a 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -29,7 +29,6 @@ sub new {
 		   NewDaemons => [qw(libvirtd)],
 		   Dom0MemFixed => 1,
 		   CfgPathVar => 'cfgpath',
-		   Command => 'virsh',
 		   ExtraPackages => [qw(libnl1 libavahi-client3)],
     }, $class;
 }
diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm
index 8e77a05..ed9071b 100644
--- a/Osstest/Toolstack/xend.pm
+++ b/Osstest/Toolstack/xend.pm
@@ -29,7 +29,7 @@ sub new {
 		   Host => $ho,
 		   NewDaemons => [qw(xend)],
 		   OldDaemonInitd => 'xend',
-		   Command => 'xm',
+		   _Command => 'xm',
 		   CfgPathVar => 'cfgpath',
 		   Dom0MemFixed => 1,
     }, $class;
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index b730783..7938649 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -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,$timeout) = @_;
     my $ho = $self->{Host};
     my $gn = $gho->{Name};
-    target_cmd_root($ho,"$self->{Command} shutdown -w $gn", $timeout);
+    target_cmd_root($ho,"$self->{_Command} shutdown -w $gn", $timeout);
 }
 
 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;
@@ -73,7 +73,7 @@ sub migrate ($$$$) {
     my $dst = $dho->{Name};
     my $gn = $gho->{Name};
     target_cmd_root($sho,
-		    $self->{Command}." migrate $gn $dst",
+		    $self->{_Command}." migrate $gn $dst",
 		    $timeout);
 }
 
@@ -81,7 +81,7 @@ sub save ($$$$) {
     my ($self,$gho,$f,$timeout) = @_;
     my $ho = $self->{Host};
     my $gn = $gho->{Name};
-    target_cmd_root($ho,$self->{Command}." save $gn $f", $timeout);
+    target_cmd_root($ho,$self->{_Command}." save $gn $f", $timeout);
 }
 
 sub restore ($$$$) {
@@ -90,7 +90,7 @@ sub restore ($$$$) {
     my $gn = $gho->{Name};
     my $cfg = $self->{RestoreNeedsConfig} ? $gho->{CfgPath} : '';
     target_cmd_root($ho,
-		    $self->{Command}
+		    $self->{_Command}
 		    ." restore "
 		    .$cfg
 		    ." $f", $timeout);
-- 
2.1.4

[PATCH v3 18/19] ts-guest-start: Use guest_create

[Ian Campbell]

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.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: Adjust for dropping $ho param to guest_create
---
 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(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index f5f099a..e9d3952 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -28,7 +28,6 @@ sub new {
 		   Host => $ho,
 		   NewDaemons => [qw(libvirtd)],
 		   Dom0MemFixed => 1,
-		   CfgPathVar => 'cfgpath',
 		   ExtraPackages => [qw(libnl1 libavahi-client3)],
     }, $class;
 }
diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm
index ed9071b..1d5d059 100644
--- a/Osstest/Toolstack/xend.pm
+++ b/Osstest/Toolstack/xend.pm
@@ -30,7 +30,6 @@ sub new {
 		   NewDaemons => [qw(xend)],
 		   OldDaemonInitd => 'xend',
 		   _Command => 'xm',
-		   CfgPathVar => 'cfgpath',
 		   Dom0MemFixed => 1,
     }, $class;
 }
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 7938649..5498c0a 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -29,7 +29,6 @@ sub new {
 		   NewDaemons => [],
 		   Dom0MemFixed => 1,
 		   _Command => 'xl',
-		   CfgPathVar => 'cfgpath',
 		   RestoreNeedsConfig => 1,
     }, $class;
 }
diff --git a/ts-guest-start b/ts-guest-start
index fb6a174..1aa9e69 100755
--- a/ts-guest-start
+++ b/ts-guest-start
@@ -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($gho);
 }
 
 sub checkstart () {
-- 
2.1.4

Re: [PATCH v3 18/19] ts-guest-start: Use guest_create

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 18/19] ts-guest-start: Use guest_create"):
> 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.

It was intended to deal with precisely that.  Early versions of xl did
not understand xm config files as generated by Debian's
xen-create-image, so they had to be converted.

Ian.

Re: [PATCH v3 18/19] ts-guest-start: Use guest_create

[Ian Campbell]

On Tue, 2015-01-27 at 17:21 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH v3 18/19] ts-guest-start: Use guest_create"):
> > 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.
> 
> It was intended to deal with precisely that.  Early versions of xl did
> not understand xm config files as generated by Debian's
> xen-create-image, so they had to be converted.

I think you told me that last time too and I just forgot to update the
changelog, sorry.

I shall replace the first sentence with "This was intended to deal with
toolstacks with a different cfg format (such as very early versions of
xl) but is no longer relevant".

Ian.

[PATCH v3 19/19] Toolstack: Pass $gho to create method

[Ian Campbell]

The cfg file can be obtained from $gho->{CfgPath}. This is more
consistent with other methods.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: New patch, implementing suggesting from review of "Toolstack:
    Refactor guest lifecycle."
---
 Osstest/TestSupport.pm       | 2 +-
 Osstest/Toolstack/libvirt.pm | 3 ++-
 Osstest/Toolstack/xl.pm      | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 427ead4..086ac10 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1340,7 +1340,7 @@ sub guest_await_destroy ($$) {
 sub guest_create ($) {
     my ($gho) = @_;
     my $ho = $gho->{Host};
-    toolstack($ho)->create($gho->{CfgPath});
+    toolstack($ho)->create($gho);
 }
 
 
diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index e9d3952..8bd7f4f 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -39,8 +39,9 @@ sub destroy ($$) {
 }
 
 sub create ($$) {
-    my ($self,$cfg) = @_;
+    my ($self,$gho) = @_;
     my $ho = $self->{Host};
+    my $cfg = $gho->{CfgPath};
     my $lcfg = $cfg;
     $lcfg =~ s,/,-,g;
     $lcfg = "$ho->{Name}--$lcfg";
diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
index 5498c0a..71aefd5 100644
--- a/Osstest/Toolstack/xl.pm
+++ b/Osstest/Toolstack/xl.pm
@@ -40,7 +40,8 @@ sub destroy ($$) {
 }
 
 sub create ($$) {
-    my ($self,$cfg) = @_;
+    my ($self,$gho) = @_;
+    my $cfg = $gho->{CfgPath};
     target_cmd_root($self->{Host}, $self->{_Command}." create $cfg", 100);
 }
 
-- 
2.1.4

Re: [PATCH v3 19/19] Toolstack: Pass $gho to create method

[Ian Jackson]

Ian Campbell writes ("[PATCH v3 19/19] Toolstack: Pass $gho to create method"):
> The cfg file can be obtained from $gho->{CfgPath}. This is more
> consistent with other methods.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[Ian Campbell]

On Mon, 2015-01-26 at 14:34 +0000, Ian Campbell wrote:
> Since last time I've addressed all of Ian's review on v2.

You have now acked all of this series. I don't think it needs a resend
for the minor changes you asked for.

Once the series which supports the new ARM hardware passes the push gate
are you OK with this going in the next batch?

Ian.

Re: [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh"):
> On Mon, 2015-01-26 at 14:34 +0000, Ian Campbell wrote:
> > Since last time I've addressed all of Ian's review on v2.
> 
> You have now acked all of this series. I don't think it needs a resend
> for the minor changes you asked for.
> 
> Once the series which supports the new ARM hardware passes the push gate
> are you OK with this going in the next batch?

Yes.  Would you care to pick up my short linux-3.16 series too ?

I've pushed it to
  xenbits.xen.org:/home/iwj/ext/osstest.git#wip.branch-setup
and you can rebase it as appropriate.

Thanks,
Ian.

Re: [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[Ian Campbell]

On Wed, 2015-01-28 at 13:14 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh"):
> > On Mon, 2015-01-26 at 14:34 +0000, Ian Campbell wrote:
> > > Since last time I've addressed all of Ian's review on v2.
> > 
> > You have now acked all of this series. I don't think it needs a resend
> > for the minor changes you asked for.
> > 
> > Once the series which supports the new ARM hardware passes the push gate
> > are you OK with this going in the next batch?
> 
> Yes.  Would you care to pick up my short linux-3.16 series too ?

Sure.

Is there anything else lurking around? What about Wei's XSM stuff?

> 
> I've pushed it to
>   xenbits.xen.org:/home/iwj/ext/osstest.git#wip.branch-setup
> and you can rebase it as appropriate.
> 
> Thanks,
> Ian.

Re: [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH v3 OSSTEST 0/19] Implement for driving libvirt via virsh"):
> On Wed, 2015-01-28 at 13:14 +0000, Ian Jackson wrote:
> > Yes.  Would you care to pick up my short linux-3.16 series too ?
> 
> Sure.
> 
> Is there anything else lurking around? What about Wei's XSM stuff?

That's a whole series and is awaiting a review from me.

Ian.