[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Hello Taka,

Takahiro Yasui <takahiro yasui 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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]