Re: [PATCH v2 1/2] Btrfs: add more valid checks for superblock

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

 



On Fri, Jun 03, 2016 at 12:05:14PM -0700, Liu Bo wrote:
> @@ -6648,6 +6648,7 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>  	struct btrfs_key found_key;
>  	int ret;
>  	int slot;
> +	u64 total_dev = 0;
>  
>  	root = root->fs_info->chunk_root;
>  
> @@ -6689,6 +6690,7 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>  			ret = read_one_dev(root, leaf, dev_item);
>  			if (ret)
>  				goto error;
> +			total_dev++;
>  		} else if (found_key.type == BTRFS_CHUNK_ITEM_KEY) {
>  			struct btrfs_chunk *chunk;
>  			chunk = btrfs_item_ptr(leaf, slot, struct btrfs_chunk);
> @@ -6698,6 +6700,28 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
>  		}
>  		path->slots[0]++;
>  	}
> +
> +	/*
> +	 * After loading chunk tree, we've got all device information,
> +	 * do another round of validation check.
> +	 */
> +	if (total_dev != root->fs_info->fs_devices->total_devices) {
> +		btrfs_err(root->fs_info,
> +	   "super_num_devices(%llu) mismatch with num_devices(%llu) found here",
> +			  btrfs_super_num_devices(root->fs_info->super_copy),
> +			  total_dev);
> +		ret = -EINVAL;

Turns out this check is too strict in combination with an intermediate
state and a bug in device deletion.

We've had several reports where a filesystem becomes unmountable due to
the above check, where the wrong value is just stored in the superblock
and is fixable by simply setting it to the right value. The right value
is number of dev items found in the dev tree, which is exactly the
total_dev that we read here.

The intermediate state can be caused by removing the device item in
btrfs_rm_device (see comment before btrfs_rm_dev_item call). This
apparently happens, when the device deletion is not finished, ie. the
superblock is not overwritten with a new copy.

So: do you agree to make it just a warning, and fixup the superblock
num_devices here? A userspace fix is also possible, but when this
happens and the filesystem is not mountable, a fix from outside of the
filesystem might be hard.

> +		goto error;
> +	}
> +	if (btrfs_super_total_bytes(root->fs_info->super_copy) <
> +	    root->fs_info->fs_devices->total_rw_bytes) {
> +		btrfs_err(root->fs_info,
> +	"super_total_bytes(%llu) mismatch with fs_devices total_rw_bytes(%llu)",
> +			  btrfs_super_total_bytes(root->fs_info->super_copy),
> +			  root->fs_info->fs_devices->total_rw_bytes);
> +		ret = -EINVAL;
> +		goto error;
> +	}
>  	ret = 0;
>  error:
>  	unlock_chunks(root);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux