From 3e813577dd07ec3a65ca386feaf9d893024cd1da Mon Sep 17 00:00:00 2001 From: Akhil Mohan Date: Tue, 6 Oct 2020 13:45:41 +0530 Subject: [PATCH] fix(sysfs,udev): add additional verbose logs in udev probe - add more verbose logs in udev probe - log errors from sysfs package - add log from smart probe Signed-off-by: Akhil Mohan --- api-service/node/services/listBlockDevice.go | 2 +- cmd/ndm_daemonset/probe/smartprobe.go | 6 +++ cmd/ndm_daemonset/probe/udevprobe.go | 50 +++++++++++++++++++- pkg/sysfs/syspath.go | 10 ++-- pkg/udevevent/event.go | 2 +- 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/api-service/node/services/listBlockDevice.go b/api-service/node/services/listBlockDevice.go index 8e0abcd5f..76db7ff68 100644 --- a/api-service/node/services/listBlockDevice.go +++ b/api-service/node/services/listBlockDevice.go @@ -160,7 +160,7 @@ func GetAllTypes(BL *v1alpha1.BlockDeviceList) error { // GetDependents should not be called on sparse devices, hence this block comes later. sysfsDevice, err := sysfs.NewSysFsDeviceFromDevPath(bd.Spec.Path) if err != nil { - klog.Errorf("could not get sysfs device for %s", bd.Spec.Path) + klog.Errorf("could not get sysfs device for %s, err: %v", bd.Spec.Path, err) continue } depDevices, err := sysfsDevice.GetDependents() diff --git a/cmd/ndm_daemonset/probe/smartprobe.go b/cmd/ndm_daemonset/probe/smartprobe.go index dd97aa7af..be86ce888 100644 --- a/cmd/ndm_daemonset/probe/smartprobe.go +++ b/cmd/ndm_daemonset/probe/smartprobe.go @@ -106,13 +106,19 @@ func (sp *smartProbe) FillBlockDeviceDetails(blockDevice *blockdevice.BlockDevic if blockDevice.Capacity.Storage == 0 && deviceBasicSCSIInfo.Capacity != 0 { blockDevice.Capacity.Storage = deviceBasicSCSIInfo.Capacity + klog.V(4).Infof("device: %s, Capacity: %d filled by smart-probe", + blockDevice.DevPath, blockDevice.Capacity.Storage) } if blockDevice.DeviceAttributes.LogicalBlockSize == 0 && deviceBasicSCSIInfo.LBSize != 0 { blockDevice.DeviceAttributes.LogicalBlockSize = deviceBasicSCSIInfo.LBSize + klog.V(4).Infof("device: %s, LogicalBlockSize: %d filled by smart-probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.LogicalBlockSize) } if blockDevice.DeviceAttributes.PhysicalBlockSize == 0 && deviceBasicSCSIInfo.PBSize != 0 { blockDevice.DeviceAttributes.PhysicalBlockSize = deviceBasicSCSIInfo.PBSize + klog.V(4).Infof("device: %s, PhysicalBlockSize: %d filled by smart-probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.PhysicalBlockSize) } } diff --git a/cmd/ndm_daemonset/probe/udevprobe.go b/cmd/ndm_daemonset/probe/udevprobe.go index 48355dcb2..15cf97779 100644 --- a/cmd/ndm_daemonset/probe/udevprobe.go +++ b/cmd/ndm_daemonset/probe/udevprobe.go @@ -188,6 +188,29 @@ func (up *udevProbe) scan() error { deviceDetails.DeviceAttributes.DeviceType = newUdevice.GetPropertyValue(libudevwrapper.UDEV_DEVTYPE) deviceDetails.SysPath = newUdevice.GetSyspath() deviceDetails.DevPath = newUdevice.GetPath() + + // log the details only if present, to avoid log flooding + if deviceDetails.DeviceAttributes.WWN != "" { + klog.V(4).Infof("device: %s, WWN: %s filled during udev scan", + deviceDetails.DevPath, deviceDetails.DeviceAttributes.WWN) + } + if deviceDetails.DeviceAttributes.Serial != "" { + klog.V(4).Infof("device: %s, Serial: %s filled during udev scan", + deviceDetails.DevPath, deviceDetails.DeviceAttributes.Serial) + } + if deviceDetails.PartitionInfo.PartitionTableUUID != "" { + klog.V(4).Infof("device: %s, PartitionTableUUID: %s filled during udev scan", + deviceDetails.DevPath, deviceDetails.PartitionInfo.PartitionTableUUID) + } + if deviceDetails.PartitionInfo.PartitionEntryUUID != "" { + klog.V(4).Infof("device: %s, PartitionEntryUUID: %s filled during udev scan", + deviceDetails.DevPath, deviceDetails.PartitionInfo.PartitionEntryUUID) + } + if deviceDetails.FSInfo.FileSystemUUID != "" { + klog.V(4).Infof("device: %s, FileSystemUUID: %s filled during udev scan", + deviceDetails.DevPath, deviceDetails.FSInfo.FileSystemUUID) + } + diskInfo = append(diskInfo, deviceDetails) // get the dependents of the block device @@ -195,12 +218,12 @@ func (up *udevProbe) scan() error { sysfsDevice, err := sysfs.NewSysFsDeviceFromDevPath(deviceDetails.DevPath) // TODO if error occurs a rescan may be required if err != nil { - klog.Errorf("could not get sysfs device for %s", deviceDetails.DevPath) + klog.Errorf("could not get sysfs device for %s, err: %v", deviceDetails.DevPath, err) } else { dependents, err := sysfsDevice.GetDependents() // TODO if error occurs need to do a scan from the beginning if err != nil { - klog.Error("error getting dependent devices for ", deviceDetails.DevPath) + klog.Errorf("error getting dependent devices for %s, err: %v", deviceDetails.DevPath, err) } else { deviceDetails.DependentDevices = dependents klog.Infof("Dependents of %s : %+v", deviceDetails.DevPath, dependents) @@ -236,6 +259,29 @@ func (up *udevProbe) FillBlockDeviceDetails(blockDevice *blockdevice.BlockDevice blockDevice.DeviceAttributes.Serial = udevDiskDetails.Serial blockDevice.DeviceAttributes.Vendor = udevDiskDetails.Vendor blockDevice.DeviceAttributes.IDType = udevDiskDetails.IDType + + // log only if details are present to prevent log flooding + if blockDevice.DeviceAttributes.Model != "" { + klog.V(4).Infof("device: %s, Model: %s filled by udev probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.Model) + } + if blockDevice.DeviceAttributes.WWN != "" { + klog.V(4).Infof("device: %s, WWN: %s filled by udev probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.WWN) + } + if blockDevice.DeviceAttributes.Serial != "" { + klog.V(4).Infof("device: %s, Serial: %s filled by udev probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.Serial) + } + if blockDevice.DeviceAttributes.Vendor != "" { + klog.V(4).Infof("device: %s, Vendor: %s filled by udev probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.Vendor) + } + if blockDevice.DeviceAttributes.IDType != "" { + klog.V(4).Infof("device: %s, IDType: %s filled by udev probe", + blockDevice.DevPath, blockDevice.DeviceAttributes.IDType) + } + if len(udevDiskDetails.ByIdDevLinks) != 0 { blockDevice.DevLinks = append(blockDevice.DevLinks, blockdevice.DevLink{ Kind: libudevwrapper.BY_ID_LINK, diff --git a/pkg/sysfs/syspath.go b/pkg/sysfs/syspath.go index 7220a8332..5f7e122f1 100644 --- a/pkg/sysfs/syspath.go +++ b/pkg/sysfs/syspath.go @@ -205,7 +205,7 @@ func (s Device) GetDependents() (blockdevice.DependentBlockDevices, error) { // GetLogicalBlockSize gets the logical block size, the caller should handle if 0 LB size is returned func (s Device) GetLogicalBlockSize() (int64, error) { - logicalBlockSize, err := readSysFSFileAsInt64(s.sysPath + "/queue/logical_block_size") + logicalBlockSize, err := readSysFSFileAsInt64(s.sysPath + "queue/logical_block_size") if err != nil { return 0, err } @@ -214,7 +214,7 @@ func (s Device) GetLogicalBlockSize() (int64, error) { // GetPhysicalBlockSize gets the physical block size of the device func (s Device) GetPhysicalBlockSize() (int64, error) { - physicalBlockSize, err := readSysFSFileAsInt64(s.sysPath + "/queue/physical_block_size") + physicalBlockSize, err := readSysFSFileAsInt64(s.sysPath + "queue/physical_block_size") if err != nil { return 0, err } @@ -223,7 +223,7 @@ func (s Device) GetPhysicalBlockSize() (int64, error) { // GetHardwareSectorSize gets the hardware sector size of the device func (s Device) GetHardwareSectorSize() (int64, error) { - hardwareSectorSize, err := readSysFSFileAsInt64(s.sysPath + "/queue/hw_sector_size") + hardwareSectorSize, err := readSysFSFileAsInt64(s.sysPath + "queue/hw_sector_size") if err != nil { return 0, err } @@ -232,7 +232,7 @@ func (s Device) GetHardwareSectorSize() (int64, error) { // GetDriveType gets the drive type of the device based on the rotational value. Can be HDD or SSD func (s Device) GetDriveType() (string, error) { - rotational, err := readSysFSFileAsInt64(s.sysPath + "/queue/rotational") + rotational, err := readSysFSFileAsInt64(s.sysPath + "queue/rotational") if err != nil { return "", err } @@ -255,7 +255,7 @@ func (s Device) GetCapacityInBytes() (int64, error) { // Ref: https://elixir.bootlin.com/linux/v4.4/source/fs/block_dev.c#L487 // // Therefore, to get the capacity of the device it needs to always multiplied with 512 - numberOfBlocks, err := readSysFSFileAsInt64(s.sysPath + "/size") + numberOfBlocks, err := readSysFSFileAsInt64(s.sysPath + "size") if err != nil { return 0, err } else if numberOfBlocks == 0 { diff --git a/pkg/udevevent/event.go b/pkg/udevevent/event.go index 0c6baf2ea..90623b449 100644 --- a/pkg/udevevent/event.go +++ b/pkg/udevevent/event.go @@ -71,7 +71,7 @@ func (e *event) process(device *libudevwrapper.UdevDevice) { if action != libudevwrapper.UDEV_ACTION_REMOVE { sysfsDevice, err := sysfs.NewSysFsDeviceFromDevPath(deviceDetails.DevPath) if err != nil { - klog.Errorf("could not get sysfs device for %s", deviceDetails.DevPath) + klog.Errorf("could not get sysfs device for %s, err: %v", deviceDetails.DevPath, err) } else { dependents, err := sysfsDevice.GetDependents() // TODO if error occurs need to do a scan from the beginning