Bug#335230: Random Code so far to be cleaned

2005-10-24 Thread Erik van Konijnenburg

Thanks; that was fast.

In the following comment, there are occasional notes "so far we did
it differently".  That does not necessarily mean your approach is wrong,
however I would prefer not to introduce variations in style if we don't
have to.  The unity in style should make it easier for future generations
to familiarize themselves with the code ...

In other situations, I found it effective to first build analysis code
that is invoked with the --test option, that just shows which evms devices
are available and what are their underlying devices.

Note that I haven't played with EVMS yet; these are mostly issues of fitting
in with the existing code, not test results.

On Mon, Oct 24, 2005 at 06:33:42PM +0200, Marco Amadori wrote:
> diff -urN yaird-0.0.11/perl/Evms.pm yaird-0.0.11-evms/perl/Evms.pm
> --- yaird-0.0.11/perl/Evms.pm 1970-01-01 01:00:00.0 +0100
> +++ yaird-0.0.11-evms/perl/Evms.pm2005-10-24 18:31:24.0 +0200
> @@ -0,0 +1,70 @@
> +#!perl -w
> +#
> +# Evms -- encapsulate evms_gather output
> +#   Copyright (C) 2005  Erik van Konijnenburg, Marco Amadori
> +#
> +#   This program is free software; you can redistribute it and/or modify
> +#   it under the terms of the GNU General Public License as published by
> +#   the Free Software Foundation; either version 2 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 General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program; if not, write to the Free Software
> +#   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> +#
> +
> +use strict;
> +use warnings;
> +use Base;
> +use Conf;
Conf is probably redundant

> +use base 'Obj';
> +package Evms;
These two lines may need to be swapped.

> +
> +sub fill {
> + my $self = shift;
> + $self->SUPER::fill();
> + $self->takeArgs ('path', 'devno', 'disks', 'plugins', 'version');
> +}
> +
> +sub path { return $_[0]->{path}; }
> +sub devno{ return $_[0]->{devno}; } # 4 what?
> +sub disks{ return $_[0]->{disks}; }

ok ...

> +sub plugins  { return $_[0]->{plugins}; } # not implemented
> +sub version  { return $_[0]->{version}; }

can these differ between devices in a single system?
if not, not much sense in making them attributes of individual disks..

> +
> +sub string {
> + my $self = shift;
> + my $path = $self->path;
> + my $devno = $self->devno;
> + my $disks = join (',', @{$self->disks});
> + my $plugins = join (',', @{$self->plugins});
> + return "$path($devno) = $plugins on $disks";
> +}
> +
> +
> +sub findDisksByName ($) {

So far, the find functions have been kept in a module
named like YyyTab.pm, separate from the YyyDev.pm, the YyyDev
then contains just access functions like string() and
friends.

perhaps we need an EvmsTab.pm?

> +# should provide a list from evms_query disks 
> +# like the output of the shell command
> +# evms_query disks /dev/evms/lvm2/safe/usr64 | xargs -i echo /dev/\{\}
> +# /dev/sda
> +# /dev/sdb
> +# /dev/sdc
> + my ($name) = @_;
> + 
> + return $evms->disks;

$evms?  from where?  So far, a module EvmsTab contained a variable
$evmsTab.  A find() function then does init(), and init() will initialise
$evmsTab if it is not yet defined; doing so by parsing output of evms_query.
The find() function can walk $evmsTab afterwards.

Note that this is easier to integrate if you have a findByDevno()
that returns an Evms object given a major:minor for a device, or undef.

> + 
> + 
> +sub findVersion () {
> +# equivalent of evms_query info | head -1 | cut -d ' ' -f 3
> +# e.g. from the output "EVMS Version: 2.5.3 \
> +#  EVMS Engine API Version: 10.1.0" gives "2.5.3"
> +
> + return $evms->version;
> +}
> +
> +1;
> diff -urN yaird-0.0.11/perl/Plan.pm yaird-0.0.11-evms/perl/Plan.pm
> --- yaird-0.0.11/perl/Plan.pm 2005-08-07 22:57:40.0 +0200
> +++ yaird-0.0.11-evms/perl/Plan.pm2005-10-24 18:28:12.375146768 +0200
> @@ -1,7 +1,7 @@
>  #!perl -w
>  #
>  # Plan -- high-level stuff
> -#   Copyright (C) 2005  Erik van Konijnenburg
> +#   Copyright (C) 2005  Erik van Konijnenburg, Marco Amadori
>  #
>  #   This program is free software; you can redistribute it and/or modify
>  #   it under the terms of the GNU General Public License as published by
> @@ -25,6 +25,7 @@
>  use FsTab;
>  use ActiveBlockDevTab;
>  use LvmTab;
> +use Evms;
>  use Hardware;
>  use ModProbe;
>  use RaidTab;
> @@ -83,6 +84,7 @@
>   }
>  
>   my $ok = 0;
> + $ok || ($ok = tryEvms ($actions,$device,[$device,@{$working}]));

Tricky.  Trying Evms even before verifying a device may be a partition
of other device?  The partition check is simple, cheap and foolproof;
a good 

Bug#335230: Random Code so far to be cleaned

2005-10-24 Thread Marco Amadori
diff -urN yaird-0.0.11/perl/Evms.pm yaird-0.0.11-evms/perl/Evms.pm
--- yaird-0.0.11/perl/Evms.pm   1970-01-01 01:00:00.0 +0100
+++ yaird-0.0.11-evms/perl/Evms.pm  2005-10-24 18:31:24.0 +0200
@@ -0,0 +1,70 @@
+#!perl -w
+#
+# Evms -- encapsulate evms_gather output
+#   Copyright (C) 2005  Erik van Konijnenburg, Marco Amadori
+#
+#   This program is free software; you can redistribute it and/or modify
+#   it under the terms of the GNU General Public License as published by
+#   the Free Software Foundation; either version 2 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 General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program; if not, write to the Free Software
+#   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+#
+
+use strict;
+use warnings;
+use Base;
+use Conf;
+use base 'Obj';
+package Evms;
+
+sub fill {
+   my $self = shift;
+   $self->SUPER::fill();
+   $self->takeArgs ('path', 'devno', 'disks', 'plugins', 'version');
+}
+
+sub path   { return $_[0]->{path}; }
+sub devno  { return $_[0]->{devno}; } # 4 what?
+sub disks  { return $_[0]->{disks}; }
+sub plugins{ return $_[0]->{plugins}; } # not implemented
+sub version{ return $_[0]->{version}; }
+
+sub string {
+   my $self = shift;
+   my $path = $self->path;
+   my $devno = $self->devno;
+   my $disks = join (',', @{$self->disks});
+   my $plugins = join (',', @{$self->plugins});
+   return "$path($devno) = $plugins on $disks";
+}
+
+
+sub findDisksByName ($) {
+# should provide a list from evms_query disks 
+# like the output of the shell command
+# evms_query disks /dev/evms/lvm2/safe/usr64 | xargs -i echo /dev/\{\}
+# /dev/sda
+# /dev/sdb
+# /dev/sdc
+   my ($name) = @_;
+   
+   return $evms->disks;
+   
+   
+sub findVersion () {
+# equivalent of evms_query info | head -1 | cut -d ' ' -f 3
+# e.g. from the output "EVMS Version: 2.5.3 \
+#  EVMS Engine API Version: 10.1.0" gives "2.5.3"
+
+   return $evms->version;
+}
+
+1;
diff -urN yaird-0.0.11/perl/Plan.pm yaird-0.0.11-evms/perl/Plan.pm
--- yaird-0.0.11/perl/Plan.pm   2005-08-07 22:57:40.0 +0200
+++ yaird-0.0.11-evms/perl/Plan.pm  2005-10-24 18:28:12.375146768 +0200
@@ -1,7 +1,7 @@
 #!perl -w
 #
 # Plan -- high-level stuff
-#   Copyright (C) 2005  Erik van Konijnenburg
+#   Copyright (C) 2005  Erik van Konijnenburg, Marco Amadori
 #
 #   This program is free software; you can redistribute it and/or modify
 #   it under the terms of the GNU General Public License as published by
@@ -25,6 +25,7 @@
 use FsTab;
 use ActiveBlockDevTab;
 use LvmTab;
+use Evms;
 use Hardware;
 use ModProbe;
 use RaidTab;
@@ -83,6 +84,7 @@
}
 
my $ok = 0;
+   $ok || ($ok = tryEvms ($actions,$device,[$device,@{$working}]));
$ok || ($ok = tryParent ($actions,$device,[$device,@{$working}]));
$ok || ($ok = tryDmCrypt ($actions,$device,[$device,@{$working}]));
$ok || ($ok = tryLvm ($actions,$device,[$device,@{$working}]));
@@ -93,6 +95,35 @@
}
Base::debug ("device: completed $name");
 }
+#
+# tryEvms -- 
+#
+
+sub tryEvms ($$$) {
+   my ($actions, $device, $working) = @_;
+
+   my $name = $device->name; # If name is like /dev/evms/root, if is like 
dm-3 
I'm lost
+   if ($name !~ /^dm-\d+$/) {
+   return 0;
+   }
+
+   for my $evmsDisk (@{Evms::findDisksByName($name)}) 
+   {
+   my $pdev = ActiveBlockDevTab::findByPath ($evmsDisk);
+   my $rc = tryHardware ($actions, $pdev, $working);
+   if ($rc !~ 1) {
+   # Base::fatal ("Can't find EVMS info for $name");
+   return 0;
+   }
+   
+   }
+   my $version = Evms::findVersion ()
+   ModProbe::addModules ($actions, [ "dm-mod" ]);
+   $actions->add ("evms_activate", $version);
+   
+   return 1;
+}
+
 
 #
 # tryParent -- If it's a partition, do the whole
diff -urN yaird-0.0.11/templates/Debian.cfg 
yaird-0.0.11-evms/templates/Debian.cfg
--- yaird-0.0.11/templates/Debian.cfg   2005-08-07 22:57:40.0 +0200
+++ yaird-0.0.11-evms/templates/Debian.cfg  2005-10-24 17:57:31.434012360 
+0200
@@ -250,6 +250,23 @@
!/sbin/vgchange -a y ''
END SCRIPT
END TEMPLATE
+   
+   TEMPLATE evms_activate
+   BEGIN
+   DIRECTORY "/lib/evms/" # Instead of this 
two lines
+   FILE "/lib/evms//*.so" # should be used 
addTreeLibrary or similar
+
+   FILE "/sbin/evms_activate"
+   FILE "/etc/evms.conf"
+   SCRI