Xen devel - Oct 2013 - [PATCH OSSTEST v2] 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.
---
v2: Use "local $ENV{USER}" which contrary to my previous experiments
does
actually work.
---
 Osstest/PDU/xenuse.pm |    8 ++++++++
 production-config     |    1 +
 2 files changed, 9 insertions(+)

diff --git a/Osstest/PDU/xenuse.pm b/Osstest/PDU/xenuse.pm
index 73a542d..fd3a148 100644
--- a/Osstest/PDU/xenuse.pm
+++ b/Osstest/PDU/xenuse.pm
@@ -44,7 +44,15 @@ sub pdu_power_state {
     my ($mo, $on) = @_;
     my $onoff= $on ? "on" : "off";
     my $xenuse= $c{XenUsePath} || "xenuse";
+    my $user= $c{XenUseUser} || undef;
+    $user= get_host_property($mo->{Host}, "XenUseUser", $user);
 
+    local $ENV{USER} = $ENV{USER};
+
+    if ($user) {
+	logm("XenUse overriding \$USER to $user");
+	$ENV{USER} = $user;
+    }
     system_checked($xenuse, "--$onoff",
"$mo->{Host}{Name}");
 }
 
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 v2] 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.
...
> +    my $user= $c{XenUseUser} || undef;
> +    $user= get_host_property($mo->{Host}, "XenUseUser",
$user);
I''m not sure what''s wrong with:

    my $user= get_host_property($mo->{Host}, "XenUseUser",
                                $c{XenUseUser} || undef $user);

?
> +    local $ENV{USER} = $ENV{USER};
If USER isn''t in the environment, this will set it to an empty value.
If you use the suffix if, you don''t get a { } which limits the scope
of local:

     ( logm("XenUse overriding \$USER to $user"),
       local $ENV{USER} = $user )
         if $user;

perhaps.
         
Ian.

Ian Jackson writes ("Re: [PATCH OSSTEST v2] PDU/xenuse: Support xenuse on
machine not locked by current user"):
> I''m not sure what''s wrong with:
> 
>     my $user= get_host_property($mo->{Host}, "XenUseUser",
>                                 $c{XenUseUser} || undef $user);
                                               
^^^^^^^^^^^^^^^
> 
> ?
Well, apart from that spurious c&p error...

Ian.

On Mon, 2013-10-21 at 10:44 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2] 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.
> ...
> > +    my $user= $c{XenUseUser} || undef;
> > +    $user= get_host_property($mo->{Host}, "XenUseUser",
$user);
> 
> I''m not sure what''s wrong with:
> 
>     my $user= get_host_property($mo->{Host}, "XenUseUser",
>                                 $c{XenUseUser} || undef $user);
> 
> ?
I just don''t think like a Perl programmer ;-)
> > +    local $ENV{USER} = $ENV{USER};
> 
> If USER isn''t in the environment, this will set it to an empty
value.
Ah yes. I was trying to work around the fact that "local $ENV{USER}"
by
itself would clear any existing $USER but didn''t think this through.
> If you use the suffix if, you don''t get a { } which limits the
scope
> of local:
> 
>      ( logm("XenUse overriding \$USER to $user"),
>        local $ENV{USER} = $user )
>          if $user;
> 
> perhaps.
I was trying to avoid the double "if $user", this looks like it might
work. I''ll try it.

Ian.