Xen devel - Oct 2013 - [PATCH OSSTEST] PDU/xenuse: Support xenuse on machine not locked by current user

xenuse checks that whoever is trying to reboot a machine "owns" that
machine
via a locking mechanism. This is usually fine doesn''t work well when
one wants
to book a machine out of the osstest pool for adhoc testing. In this case
machine ownership is maintained by osstest ahd the machine remains locked to
osstest as far as xenuse is concerned.

Therefore add a global configuration option and perhost override to override
$USER (which xenuse obeys) to osstest.

I used eval + save/restore the original user so as not to pollute the whole
process, it might have been better to actually fork?
---
 Osstest/PDU/xenuse.pm |   16 ++++++++++++++--
 production-config     |    1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Osstest/PDU/xenuse.pm b/Osstest/PDU/xenuse.pm
index 73a542d..da966e3 100644
--- a/Osstest/PDU/xenuse.pm
+++ b/Osstest/PDU/xenuse.pm
@@ -44,8 +44,20 @@ sub pdu_power_state {
     my ($mo, $on) = @_;
     my $onoff= $on ? "on" : "off";
     my $xenuse= $c{XenUsePath} || "xenuse";
-
-    system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
+    my $user= $c{XenUseUser} || undef;
+    $user= get_host_property($mo->{Host}, "XenUseUser", $user);
+
+    my $saved_user = $ENV{USER} if $user;
+    eval {
+	if ($user) {
+	    logm("XenUse overriding \$USER to $user");
+	    $ENV{USER} = $user;
+	}
+
+	system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
+    };
+    $ENV{USER} = $saved_user if $user;
+    die $@ if $@;
 }
 
 1;
diff --git a/production-config b/production-config
index de9e55c..fb9d750 100644
--- a/production-config
+++ b/production-config
@@ -75,6 +75,7 @@ TftpPxeGroup osstest
 TftpDiVersion 2013-09-23
 
 XenUsePath /usr/groups/xencore/systems/bin/xenuse
+XenUseUser osstest
 
 # We use the IP address because Citrix can''t manage reliable
nameservice
 #DebianMirrorHost debian.uk.xensource.com
-- 
1.7.10.4

Ian Campbell writes ("[PATCH OSSTEST] PDU/xenuse: Support xenuse on machine
not locked by current user"):
> xenuse checks that whoever is trying to reboot a machine "owns"
that machine
> via a locking mechanism. This is usually fine doesn''t work well
when one want...
> I used eval + save/restore the original user so as not to pollute the whole
> process, it might have been better to actually fork?
Well, I can think of two better ways:

1. Perl dynamic scoping ("local"):

   {
       local $ENV{USER} = $user;
       system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
   }

IMO this is idiomatic Perl.

2. env(1) from coreutils:

   my @cmd = ($xenuse, "--$onoff", "$mo->{Host}{Name}");
   if ($user) {
       unshift @cmd, ''env'', "''USER=$user";
   }
   system_checked(@cmd);

Ian.

On Thu, 2013-10-17 at 11:25 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] PDU/xenuse: Support xenuse on
machine not locked by current user"):
> > xenuse checks that whoever is trying to reboot a machine
"owns" that machine
> > via a locking mechanism. This is usually fine doesn''t work
well when one want...
> > I used eval + save/restore the original user so as not to pollute the
whole
> > process, it might have been better to actually fork?
> 
> Well, I can think of two better ways:
> 
> 1. Perl dynamic scoping ("local"):
> 
>    {
>        local $ENV{USER} = $user;
>        system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
>    }
> 
> IMO this is idiomatic Perl.
I tried this and it didn''t work. I think local causes $ENV to loose its
magic properties. That kind of makes sense I think since you can''t
actually create a local copy of your actual environment so it would have
to jump through what I imagine would be some clever hoops to unwind when
the local $ENV went out of scope, I''ve no idea if that''s even
possible
in Perl.
> 2. env(1) from coreutils:
> 
>    my @cmd = ($xenuse, "--$onoff",
"$mo->{Host}{Name}");
>    if ($user) {
>        unshift @cmd, ''env'',
"''USER=$user";
>    }
>    system_checked(@cmd);
I can do this one assuming I''m right about method #1.
> 
> Ian.

Ian Campbell writes ("Re: [PATCH OSSTEST] PDU/xenuse: Support xenuse on
machine not locked by current user"):
> On Thu, 2013-10-17 at 11:25 +0100, Ian Jackson wrote:
> > 1. Perl dynamic scoping ("local"):
> > 
> >    {
> >        local $ENV{USER} = $user;
> >        system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
> >    }
> > 
> > IMO this is idiomatic Perl.
> 
> I tried this and it didn''t work. I think local causes $ENV to
loose its
> magic properties.
mariner:~> perl -e ''{ local $ENV{USER}="foo"; system
"printenv"; }; system "printenv"'' | grep
USERUSER=foo
USER=iwj
mariner:~>

I''m not sure what you tried...

Ian.

On Thu, 2013-10-17 at 11:48 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST] PDU/xenuse: Support xenuse
on machine not locked by current user"):
> > On Thu, 2013-10-17 at 11:25 +0100, Ian Jackson wrote:
> > > 1. Perl dynamic scoping ("local"):
> > > 
> > >    {
> > >        local $ENV{USER} = $user;
> > >        system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
> > >    }
> > > 
> > > IMO this is idiomatic Perl.
> > 
> > I tried this and it didn''t work. I think local causes $ENV to
loose its
> > magic properties.
> 
> mariner:~> perl -e ''{ local $ENV{USER}="foo"; system
"printenv"; }; system "printenv"'' | grep USER>
USER=foo
> USER=iwj
> mariner:~>
> 
> I''m not sure what you tried...
Essentially the same thing, or so I thought.

Looks like I either buggered it up or misinterpreted the results!

Ian.