[lvm-devel] [RFC][PATCH] lvm2: limit accesses to broken devices (v2)

[Petr Rockai]

Hello Taka,

Takahiro Yasui <takahiro.yasui at hds.com> writes:
>  int dev_read(struct device *dev, uint64_t offset, size_t len, void *buffer)
>  {
>  	struct device_area where;
> +	int ret;
>  
>  	if (!dev->open_count)
>  		return_0;
>  
> +	if (!_dev_is_valid(dev))
> +		return 0;
Maybe return_0 here?

> +
>  	where.dev = dev;
>  	where.start = offset;
>  	where.size = len;
>  
> -	return _aligned_io(&where, buffer, 0);
> +	ret = _aligned_io(&where, buffer, 0);
> +	if (!ret)
> +		_dev_inc_error_count(dev);
> +
> +	return ret;
>  }
>  
>  /*
> @@ -662,17 +684,25 @@ int dev_append(struct device *dev, size_
>  int dev_write(struct device *dev, uint64_t offset, size_t len, void *buffer)
>  {
>  	struct device_area where;
> +	int ret;
>  
>  	if (!dev->open_count)
>  		return_0;
>  
> +	if (!_dev_is_valid(dev))
> +		return 0;
(and here)

> +
>  	where.dev = dev;
>  	where.start = offset;
>  	where.size = len;
>  
>  	dev->flags |= DEV_ACCESSED_W;
>  
> -	return _aligned_io(&where, buffer, 1);
> +	ret = _aligned_io(&where, buffer, 1);
> +	if (!ret)
> +		_dev_inc_error_count(dev);
> +
> +	return ret;
Hmm. Should write errors count towards this? What happens if the device
just rejects writes -- could this break code by yanking the device
completely? This means, IIUIC, that any subsequent rescan that may
happen won't see the device even if the failure was due to failing
writes and the device reads just fine.

I guess this is a corner case, but I would be surprised if there were
any significant use-cases for tracking write errors for this purpose:
the primary reason for the code is repeated read errors, I believe?

Of course, not writing to the device that cannot be read is certainly
prudent -- do not remove the check near the top of dev_write if you
decide to not track write errors as per above.

Other than this, I think the patch looks OK.

Yours,
   Petr.