| Message ID | 75f38ba99fde2f94066fb4578914241c0e2a8f9d.1636408300.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,RESEND] btrfs: allow single disk devices to mount with older generations | expand |
Ping? I find test case btrfs/219 needs this patch. Thanks, Anand On 09/11/2021 05:52, Josef Bacik wrote: > We have this check to make sure we don't accidentally add older devices > that may have disappeared and re-appeared with an older generation from > being added to an fs_devices. This makes sense, we don't want stale > disks in our file system. However for single disks this doesn't really > make sense. I've seen this in testing, but I was provided a reproducer > from a project that builds btrfs images on loopback devices. The > loopback device gets cached with the new generation, and then if it is > re-used to generate a new file system we'll fail to mount it because the > new fs is "older" than what we have in cache. > > Fix this by simply ignoring this check if we're a single disk file > system, as we're not going to cause problems for the fs by allowing the > disk to be mounted with an older generation than what is in our cache. > > I've also added a error message for this case, as it was kind of > annoying to find originally. > > Reviewed-by: Anand Jain <anand.jain@oracle.com> > Reported-by: Daan De Meyer <daandemeyer@fb.com> > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/volumes.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 68bb3709834a..9dfdc7680c41 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -777,6 +777,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, > struct rcu_string *name; > u64 found_transid = btrfs_super_generation(disk_super); > u64 devid = btrfs_stack_device_id(&disk_super->dev_item); > + bool multi_disk = btrfs_super_num_devices(disk_super) > 1; > bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) & > BTRFS_FEATURE_INCOMPAT_METADATA_UUID); > bool fsid_change_in_progress = (btrfs_super_flags(disk_super) & > @@ -909,7 +910,8 @@ static noinline struct btrfs_device *device_list_add(const char *path, > * tracking a problem where systems fail mount by subvolume id > * when we reject replacement on a mounted FS. > */ > - if (!fs_devices->opened && found_transid < device->generation) { > + if (multi_disk && !fs_devices->opened && > + found_transid < device->generation) { > /* > * That is if the FS is _not_ mounted and if you > * are here, that means there is more than one > @@ -917,6 +919,10 @@ static noinline struct btrfs_device *device_list_add(const char *path, > * with larger generation number or the last-in if > * generation are equal. > */ > + btrfs_warn_in_rcu(device->fs_info, > + "old device %s not being added for fsid:devid for %pU:%llu", > + rcu_str_deref(device->name), > + disk_super->fsid, devid); > mutex_unlock(&fs_devices->device_list_mutex); > return ERR_PTR(-EEXIST); > }
Josef, Maybe this patch got lost. Could you please resend this patch again? - Anand On 09/11/2021 05:52, Josef Bacik wrote: > We have this check to make sure we don't accidentally add older devices > that may have disappeared and re-appeared with an older generation from > being added to an fs_devices. This makes sense, we don't want stale > disks in our file system. However for single disks this doesn't really > make sense. I've seen this in testing, but I was provided a reproducer > from a project that builds btrfs images on loopback devices. The > loopback device gets cached with the new generation, and then if it is > re-used to generate a new file system we'll fail to mount it because the > new fs is "older" than what we have in cache. > > Fix this by simply ignoring this check if we're a single disk file > system, as we're not going to cause problems for the fs by allowing the > disk to be mounted with an older generation than what is in our cache. > > I've also added a error message for this case, as it was kind of > annoying to find originally. > > Reviewed-by: Anand Jain <anand.jain@oracle.com> > Reported-by: Daan De Meyer <daandemeyer@fb.com> > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/volumes.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 68bb3709834a..9dfdc7680c41 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -777,6 +777,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, > struct rcu_string *name; > u64 found_transid = btrfs_super_generation(disk_super); > u64 devid = btrfs_stack_device_id(&disk_super->dev_item); > + bool multi_disk = btrfs_super_num_devices(disk_super) > 1; > bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) & > BTRFS_FEATURE_INCOMPAT_METADATA_UUID); > bool fsid_change_in_progress = (btrfs_super_flags(disk_super) & > @@ -909,7 +910,8 @@ static noinline struct btrfs_device *device_list_add(const char *path, > * tracking a problem where systems fail mount by subvolume id > * when we reject replacement on a mounted FS. > */ > - if (!fs_devices->opened && found_transid < device->generation) { > + if (multi_disk && !fs_devices->opened && > + found_transid < device->generation) { > /* > * That is if the FS is _not_ mounted and if you > * are here, that means there is more than one > @@ -917,6 +919,10 @@ static noinline struct btrfs_device *device_list_add(const char *path, > * with larger generation number or the last-in if > * generation are equal. > */ > + btrfs_warn_in_rcu(device->fs_info, > + "old device %s not being added for fsid:devid for %pU:%llu", > + rcu_str_deref(device->name), > + disk_super->fsid, devid); > mutex_unlock(&fs_devices->device_list_mutex); > return ERR_PTR(-EEXIST); > }
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 68bb3709834a..9dfdc7680c41 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -777,6 +777,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, struct rcu_string *name; u64 found_transid = btrfs_super_generation(disk_super); u64 devid = btrfs_stack_device_id(&disk_super->dev_item); + bool multi_disk = btrfs_super_num_devices(disk_super) > 1; bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) & BTRFS_FEATURE_INCOMPAT_METADATA_UUID); bool fsid_change_in_progress = (btrfs_super_flags(disk_super) & @@ -909,7 +910,8 @@ static noinline struct btrfs_device *device_list_add(const char *path, * tracking a problem where systems fail mount by subvolume id * when we reject replacement on a mounted FS. */ - if (!fs_devices->opened && found_transid < device->generation) { + if (multi_disk && !fs_devices->opened && + found_transid < device->generation) { /* * That is if the FS is _not_ mounted and if you * are here, that means there is more than one @@ -917,6 +919,10 @@ static noinline struct btrfs_device *device_list_add(const char *path, * with larger generation number or the last-in if * generation are equal. */ + btrfs_warn_in_rcu(device->fs_info, + "old device %s not being added for fsid:devid for %pU:%llu", + rcu_str_deref(device->name), + disk_super->fsid, devid); mutex_unlock(&fs_devices->device_list_mutex); return ERR_PTR(-EEXIST); }