[pve-devel] [PATCH v4 pve-container] Add new pct fsck command, to check the rootfs of a container for consistency

[Wolfgang Bumiller]

On Mon, Oct 12, 2015 at 10:51:29AM +0200, Emmanuel Kasper wrote:
> * the filesystem specific command will be called automatically by fsck (at the
> moment ext4)
> * the -y flag ensures that the filesystem can be fixed automcatically in
> a non-interactive session
> * the -f flag forces a filesystem check even if the fs seems clean
> ---
>  src/PVE/CLI/pct.pm | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
> index 8c56f89..0acfdae 100755
> --- a/src/PVE/CLI/pct.pm
> +++ b/src/PVE/CLI/pct.pm
> @@ -124,6 +124,70 @@ __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 => {
> +		optional => 1,
> +		type => 'boolean',
> +		description => "Force checking, even if the filesystem seems clean",
> +		default => 0,
> +		},
> +                device => {
> +		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 = $param->{'device'};
> +
> +	my $conf = PVE::LXC::load_config($vmid);
> +	my $storage_cfg = PVE::Cluster::cfs_read_file("storage.cfg");
> +
> +	my $volume_entry;
> +
> +	if (defined($device)) {
> +		!defined($conf->{$device}) &&
> +			die "cannot run command on unexisting device $device\n";
> +
> +		$volume_entry = PVE::LXC::parse_ct_mountpoint($conf->{$device});
> +
> +		#skip bind mounts
> +		$volume_entry->{volume} =~ m|^/| &&
> +			die "cannot run command, $device does not look like a block device\n";
> +
> +	} else {
> +		$volume_entry = PVE::LXC::parse_ct_mountpoint($conf->{rootfs});
> +	}

I don't think parsing and bind-mount check need to be part of the 'if'
clause. Checking in case of the rootfs, too, doesn't harm, and you might
want to be able to put a rootfs on a block device, too. (Though this
currently doesn't seem to be possible).

Also, while you do want to skip bind-mounts, you don't want to skip
block devices, so this should be m|^/(?!dev/)| (or add
&& ... !~ m|^/dev/|) ;-)

> +
> +	my $path = PVE::Storage::path($storage_cfg, $volume_entry->{volume}, undef);
> +
> +	my $command = ['fsck', '-a', '-T', '-l'];
> +	push(@$command, '-f') if $param->{force};
> +	push(@$command, $path);
> +
> +	# critical path: all of this will be done while the container is locked
> +	my $do_fsck = sub {
> +		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);

Shouldn't the lock be held from load_config on?
If between load_config() and lock_container() someone replaces that
particular disk the fsck command would error, no?

> +	return undef;
> +    }});
> +
>  our $cmddef = {
>      list=> [ 'PVE::API2::LXC', 'vmlist', [], { node => $nodename }, sub {
>  	my $res = shift;
> @@ -165,6 +229,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
>