| Message ID | 1457583778-4042-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 10, 2016 at 12:22:58PM +0800, Anand Jain wrote: > Optional Label may or may not be set, or it might be set at some time > later. However while debugging to search through the kernel logs the > scripts would need the logs to be consistent, so logs search key words > shouldn't depend on the optional variables, instead fsid is better. I think the label is a useful information, as it's set by the user. So if I'm looking to the log, I'll recognize the labels, not the device or fsid. It would be better to show all of them, ie. label, fsid, device and transid. The line will get longer, but I hope it's ok. Proposed order of the fields: - device PATH - devid ID - fsid UUID - transid TID > - if (disk_super->label[0]) { > - printk(KERN_INFO "BTRFS: device label %s ", disk_super->label); > - } else { > - printk(KERN_INFO "BTRFS: device fsid %pU ", disk_super->fsid); > - } > - > - printk(KERN_CONT "devid %llu transid %llu %s\n", devid, transid, path); > + printk(KERN_INFO "BTRFS: device fsid %pU devid %llu transid %llu %s\n", > + disk_super->fsid, devid, transid, path); -- 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 03/17/2016 12:18 AM, David Sterba wrote: > On Thu, Mar 10, 2016 at 12:22:58PM +0800, Anand Jain wrote: >> Optional Label may or may not be set, or it might be set at some time >> later. However while debugging to search through the kernel logs the >> scripts would need the logs to be consistent, so logs search key words >> shouldn't depend on the optional variables, instead fsid is better. > > I think the label is a useful information, as it's set by the user. So > if I'm looking to the log, I'll recognize the labels, not the device or > fsid. > > It would be better to show all of them, ie. label, fsid, device and > transid. The line will get longer, but I hope it's ok. > > Proposed order of the fields: > - device PATH > - devid ID > - fsid UUID > - transid TID (I am not too particular about the below but just my opinion.) The patch titled in the ML: Btrfs: fix fs logging for multi device Would prefix BTRFS: <fsid>: to most of the logs in dmesg. So I guess if we have following then BTRFS: <fsid>: <devid ID> (<device UUID>*) <transid> <device PATH> is better. For end users, I hope we provide all those requisites through btrfs-progs cli, and they wouldn't have to review dmesg. Further, 'btrfs fi show' provides the FSID to label mapping. So I hope the next set of targeted community, the troubleshooters will be familiar with the FSID, and they could do dmesg | grep "BTRFS: <fsid>:" To filter to get logs of one btrfs which they want to troubleshoot. (as there may be more than one btrfs in the system). [*] May be in future. (There is a bug that we might fails to know / assemble right set devices as per last assembled-volume and to fix this, its better create a new device UUID for the replace target device instead of copying the device UUID of source device (bit vague of now). If this is successful, then device UUID will be useful to printk here). Thanks, Anand >> - if (disk_super->label[0]) { >> - printk(KERN_INFO "BTRFS: device label %s ", disk_super->label); >> - } else { >> - printk(KERN_INFO "BTRFS: device fsid %pU ", disk_super->fsid); >> - } >> - >> - printk(KERN_CONT "devid %llu transid %llu %s\n", devid, transid, path); >> + printk(KERN_INFO "BTRFS: device fsid %pU devid %llu transid %llu %s\n", >> + disk_super->fsid, devid, transid, path); > -- > 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
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index dc2db98..af176d6 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -998,13 +998,8 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder, ret = device_list_add(path, disk_super, devid, fs_devices_ret); if (ret > 0) { - if (disk_super->label[0]) { - printk(KERN_INFO "BTRFS: device label %s ", disk_super->label); - } else { - printk(KERN_INFO "BTRFS: device fsid %pU ", disk_super->fsid); - } - - printk(KERN_CONT "devid %llu transid %llu %s\n", devid, transid, path); + printk(KERN_INFO "BTRFS: device fsid %pU devid %llu transid %llu %s\n", + disk_super->fsid, devid, transid, path); ret = 0; } if (!ret && fs_devices_ret)
Optional Label may or may not be set, or it might be set at some time later. However while debugging to search through the kernel logs the scripts would need the logs to be consistent, so logs search key words shouldn't depend on the optional variables, instead fsid is better. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v2: fix commit log fs/btrfs/volumes.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)