| Message ID | 1430384845-9666-2-git-send-email-xuw2015@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 30, 2015 at 05:07:24PM +0800, xuw2015@gmail.com wrote: > From: George Wang <xuw2015@gmail.com> > > First try to find the device matches specified device path, if nothing, then > find the device by (devid, dev_uuid). This can fix the regression for > replacing an offline device which path is held in btrfs_device. I vaguely remember similar patches sent by Anand, CCed. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/05/2015 11:38 PM, David Sterba wrote: > On Thu, Apr 30, 2015 at 05:07:24PM +0800, xuw2015@gmail.com wrote: >> From: George Wang <xuw2015@gmail.com> >> >> First try to find the device matches specified device path, if nothing, then >> find the device by (devid, dev_uuid). This can fix the regression for >> replacing an offline device which path is held in btrfs_device. > > I vaguely remember similar patches sent by Anand, CCed. George, Critically we don't need this patch. right ? Anyway user of replace cli can use devid if device read fails. I think David is talking about: [PATCH] device delete by devid it was critical for device delete. since there was device delete by devid. I used device delete by devid instead of device path strcmp mainly because to maintain consistency between device replace and delete. the above patch set also provides code cleanups between device replace and delete codes. Thanks, Anand > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
fix typo. (I have to blame thunderbird's bug which vanishes some of the words as I scroll up and down in the 'write' window.). On 05/06/2015 05:15 PM, Anand Jain wrote: > > > On 05/05/2015 11:38 PM, David Sterba wrote: >> On Thu, Apr 30, 2015 at 05:07:24PM +0800, xuw2015@gmail.com wrote: >>> From: George Wang <xuw2015@gmail.com> >>> >>> First try to find the device matches specified device path, if >>> nothing, then >>> find the device by (devid, dev_uuid). This can fix the regression for >>> replacing an offline device which path is held in btrfs_device. >> >> I vaguely remember similar patches sent by Anand, CCed. > > George, > > Critically we don't need this patch. right ? > Anyway user of replace cli can use devid if device read fails. > > I think David is talking about: > [PATCH] device delete by devid > > it was critical for device delete. since there wasn't device > delete by devid. I used device delete by devid instead of > device path strcmp mainly because to maintain consistency > between device replace and delete. > the above patch set also provides code cleanups between > device replace and delete codes. > > Thanks, Anand > >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for reply. On Wed, May 6, 2015 at 5:22 PM, Anand Jain <Anand.Jain@oracle.com> wrote: > > > fix typo. (I have to blame thunderbird's bug which vanishes > some of the words as I scroll up and down in the 'write' window.). > > On 05/06/2015 05:15 PM, Anand Jain wrote: >> >> >> >> On 05/05/2015 11:38 PM, David Sterba wrote: >>> >>> On Thu, Apr 30, 2015 at 05:07:24PM +0800, xuw2015@gmail.com wrote: >>>> >>>> From: George Wang <xuw2015@gmail.com> >>>> >>>> First try to find the device matches specified device path, if >>>> nothing, then >>>> find the device by (devid, dev_uuid). This can fix the regression for >>>> replacing an offline device which path is held in btrfs_device. >>> >>> >>> I vaguely remember similar patches sent by Anand, CCed. >> >> >> George, >> >> Critically we don't need this patch. right ? Yes, I agree it. >> Anyway user of replace cli can use devid if device read fails. >> >> I think David is talking about: >> [PATCH] device delete by devid We can delete the device by devid on behalf of "btrfs_find_device". In my opinion, the dev path is easier and humanized to use. This was OK before, but now I can not replace offline device by path. So I consider it as a regression. Anyway, maybe we should support replace offline device by path in the future. Thanks, George >> >> it was critical for device delete. since there wasn't device >> delete by devid. I used device delete by devid instead of >> device path strcmp mainly because to maintain consistency >> between device replace and delete. >> the above patch set also provides code cleanups between >> device replace and delete codes. >> >> Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Critically we don't need this patch. right ? >>> Anyway user of replace cli can use devid if device read fails. > Yes, I agree it. >>> I think David is talking about: >>> [PATCH] device delete by devid >>> >>> it was critical for device delete. since there wasn't device >>> delete by devid. I used device delete by devid instead of >>> device path strcmp mainly because to maintain consistency >>> between device replace and delete. >>> the above patch set also provides code cleanups between >>> device replace and delete codes. > We can delete the device by devid on behalf of "btrfs_find_device". yes. patch-set (above) uses btrfs_find_device for device delete now. replace was already using it. > In my opinion, the dev path is easier and humanized to use. yes. good to have. in the long run. But not a regression/critical. this will conflict with my patch, can you rebase on top of above path which has some cleanups in this area as well. > This was OK before, but now I can not replace offline device > by path. > So I consider it as a regression. You mean to say you could replace the offline device using the device path before (not devid) and now you can't ? Then what patch introduced the regression ? Do you see any older version replace working with offline device using the device path ? Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 7, 2015 at 11:17 AM, Anand Jain <Anand.Jain@oracle.com> wrote: > > >>>> Critically we don't need this patch. right ? >>>> Anyway user of replace cli can use devid if device read fails. > > >> Yes, I agree it. > >>>> I think David is talking about: >>>> [PATCH] device delete by devid >>>> >>>> it was critical for device delete. since there wasn't device >>>> delete by devid. I used device delete by devid instead of >>>> device path strcmp mainly because to maintain consistency >>>> between device replace and delete. >>>> the above patch set also provides code cleanups between >>>> device replace and delete codes. > > >> We can delete the device by devid on behalf of "btrfs_find_device". > > yes. patch-set (above) uses btrfs_find_device for device delete now. > replace was already using it. > >> In my opinion, the dev path is easier and humanized to use. > > yes. good to have. in the long run. But not a regression/critical. > this will conflict with my patch, can you rebase on top of > above path which has some cleanups in this area as well. > >> This was OK before, but now I can not replace offline device >> by path. >> So I consider it as a regression. Oh, sorry, my fault. NOT regression, It should always be the srcdevid. I confused with another stuff. Thanks, George > > You mean to say you could replace the offline device using the > device path before (not devid) and now you can't ? > > Then what patch introduced the regression ? Do you see any > older version replace working with offline device using the > device path ? > > Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 8bcd2a0..c8ece13 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1888,8 +1888,37 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_fs_info *fs_info, mutex_unlock(&uuid_mutex); } -static int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path, - struct btrfs_device **device) + +/* + * Find specified device in fs_devices, caller must hold volume_mutex. + * Use @device_path as the key, do not check the devid and dev_uuid. Depends + * on usage, the caller may do more checking for ret device. + */ +static int __fast_find_device_by_path(struct btrfs_root *root, char *device_path, + struct btrfs_device **device) +{ + struct list_head *devices; + struct btrfs_device *tmp; + char *name; + + devices = &root->fs_info->fs_devices->devices; + + list_for_each_entry(tmp, devices, dev_list) { + name = rcu_str_deref(tmp->name); + if (tmp && 0 == strcmp(name, device_path)) { + *device = tmp; + return 0; + } + } + + return -ENOENT; +} + +/* + * Real read (devid, dev_uuid) from device, then find it in fs_devices + */ +static int __slow_find_device_by_path(struct btrfs_root *root, char *device_path, + struct btrfs_device **device) { int ret = 0; struct btrfs_super_block *disk_super; @@ -1915,6 +1944,19 @@ static int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path, return ret; } + +static int btrfs_find_device_by_path(struct btrfs_root *root, char *device_path, + struct btrfs_device **device) +{ + int ret; + + ret = __fast_find_device_by_path(root, device_path, device); + if (ret) + ret = __slow_find_device_by_path(root, device_path, device); + + return ret; +} + int btrfs_find_device_missing_or_by_path(struct btrfs_root *root, char *device_path, struct btrfs_device **device)