[pve-devel] [PATCH pve-container] Add new pct fsck command to check the mountpoints of a container

[Wolfgang Bumiller]

some minor things to finalize this:

On Wed, Oct 14, 2015 at 10:09:17AM +0200, Emmanuel Kasper wrote:
> * the filesystem specific command will be called automatically by fsck
> * the -a flag ensures that the filesystem can be fixed without any questions
> * the -f flag forces a filesystem check even if the fs seems clean
> (flags similar to what the fsck systemd unit uses)
> ---
>  src/PVE/CLI/pct.pm | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
> index 8c56f89..f98406b 100755
> --- a/src/PVE/CLI/pct.pm
> +++ b/src/PVE/CLI/pct.pm
> @@ -124,6 +124,90 @@ __PACKAGE__->register_method ({
>  	exec('lxc-attach', '-n', $param->{vmid}, '--', @{$param->{'extra-args'}});
>      }});
>  
> +    __PACKAGE__->register_method ({
> +    name => 'fsck',
> +    path => 'fsck',
> +    method => 'PUT',
> +    description => "Run a filesystem check on a container volume",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::LXC::complete_ctid_stopped }),
> +		force => {

1 level of indentation missing in this block

> +		optional => 1,
> +		type => 'boolean',
> +		description => "Force checking, even if the filesystem seems clean",
> +		default => 0,
> +		},
> +                device => {

the above line is space-indented and needs s/ {8}/\t/g
the block inside needs 1 more level of indentation

> +		optional => 1,
> +		type => 'string',
> +		description => "A volume on which to run the filesystem check",
> +		enum => [PVE::LXC::mountpoint_names()],
> +		},
> +    },
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +	my ($param) = @_;
> +	my $vmid = $param->{'vmid'};
> +	my $device = defined($param->{'device'}) ? $param->{'device'} : 'rootfs';
> +
> +	my $command = ['fsck', '-a', '-l'];
> +	push(@$command, '-f') if $param->{force};
> +
> +	# critical path: all of this will be done while the container is locked
> +	my $do_fsck = sub {

1 level of indentation missing for this sub (it's hard to find the end)
And the if clauses should be indented by 4, but currently use a whole
tab (8).

> +	my $conf = PVE::LXC::load_config($vmid);
> +	my $storage_cfg = PVE::Cluster::cfs_read_file("storage.cfg");

I missed this in the first patch. The cfs_read_file is used in the code,
too, but we have a PVE::Storage::config() which is a better fit.

> +	defined($conf->{$device}) || die "cannot run command on unexisting mountpoint $device\n";
> +
> +	my $mount_point = PVE::LXC::parse_ct_mountpoint($conf->{$device});
> +	my $volid = $mount_point->{volume};
> +
> +	my $path;
> +	my $storage = PVE::Storage::parse_volume_id($volid, 1);
> +
> +	if ($storage) {
> +		# skip zfs datasets
> +		my (undef, undef, undef, undef, undef, undef, $format) =
> +			PVE::Storage::parse_volname($storage_cfg, $volid);
> +		my $storage_id = PVE::Storage::parse_volume_id($volid);

There's already $storage, no need to parse it twice.

> +		my $storage_id_cfg = PVE::Storage::storage_config($storage_cfg, $storage_id);
> +
> +		if (($storage_id_cfg->{'type'} eq 'zfspool') && ($format eq 'subvol')) {
> +			die "cannot run command, $device does not point to a block device or volume\n";
> +		}
> +
> +		$path = PVE::Storage::path($storage_cfg, $volid);
> +
> +	} else {
> +		# todo: move both checks to utility function
> +		# skip bind mounts
> +		if ($volid =~ m|^/.+| && $volid !~ m|^/dev/.+|) {
> +			die "cannot run command, $device does not point to a block device or volume\n";
> +		}
> +		# pass block devices directly
> +		elsif ($volid =~ m|^/dev/.+|) {
> +			$path = $volid;
> +		}
> +		else {
> +			die "unknown storage configuration";
> +		}
> +	}
> +
> +	push(@$command, $path);
> +
> +	PVE::LXC::check_running($vmid) && die "cannot run command on active container\n";
> +	PVE::Tools::run_command($command);
> +	};
> +
> +	PVE::LXC::lock_container($vmid, undef, $do_fsck);
> +	return undef;
> +    }});
> +
>  our $cmddef = {
>      list=> [ 'PVE::API2::LXC', 'vmlist', [], { node => $nodename }, sub {
>  	my $res = shift;
> @@ -165,6 +249,7 @@ our $cmddef = {
>      enter => [ __PACKAGE__, 'enter', ['vmid']],
>      unlock => [ __PACKAGE__, 'unlock', ['vmid']],
>      exec => [ __PACKAGE__, 'exec', ['vmid', 'extra-args']],
> +    fsck => [ __PACKAGE__, 'fsck', ['vmid']],
>      
>      destroy => [ 'PVE::API2::LXC', 'destroy_vm', ['vmid'], 
>  		 { node => $nodename }, $upid_exit ],
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>