Thank you for your contribution! Sashiko AI review found 3 potential issues to consider: - [Low] scsi: CONFIG_SCSI_MULTIPATH depends on SCSI_MOD instead of SCSI - [Low] scsi: duplicate MODULE_DESCRIPTION and MODULE_LICENSE in scsi_multipath.c - [Low] scsi-multipath: off-by-one truncation check in scsi_mpath_unique_lun_id() --
commit 006413118882829d81f9e30417cb6f7346390426 Author: John Garry <[email protected]> scsi-multipath: introduce basic SCSI device support This commit introduces struct scsi_mpath_device, the scsi_mod.multipath module parameter, and per-device multipath allocation and release hooks during SCSI LUN scanning and device teardown. > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 90b65bebec703..2491fdf6f563e 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -76,6 +76,16 @@ config SCSI_LIB_KUNIT_TEST > > If unsure say N. > > +config SCSI_MULTIPATH > + bool "SCSI multipath support (EXPERIMENTAL)" > + depends on SCSI_MOD > + select LIBMULTIPATH > + help > + This option enables support for native SCSI multipath support for > + SCSI host. > + > + If unsure say N. > + [Severity: Low] Should SCSI_MULTIPATH depend on SCSI instead of SCSI_MOD? SCSI_MOD defaults to y when SCSI=n and BLOCK=y: drivers/scsi/Kconfig: config SCSI_MOD tristate default y if SCSI=n || SCSI=y default m if SCSI=m depends on BLOCK When CONFIG_SCSI=n and CONFIG_BLOCK=y, CONFIG_SCSI_MULTIPATH=y can be enabled, which selects CONFIG_LIBMULTIPATH=y while skipping scsi_mod.o and drivers/scsi/scsi_multipath.o. [ ... ] > diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c > new file mode 100644 > index 0000000000000..97f7513b47122 > --- /dev/null > +++ b/drivers/scsi/scsi_multipath.c > @@ -0,0 +1,150 @@ [ ... ] > +static int scsi_mpath_unique_lun_id(struct scsi_device *sdev) > +{ > + struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev; > + int ret; > + > + ret = scsi_vpd_lun_id(sdev, scsi_mpath_dev->device_id_str, > + SCSI_MPATH_DEVICE_ID_LEN); > + if (ret < 0) > + return ret; > + else if (ret == 0) > + return -EINVAL; > + else if (ret > SCSI_MPATH_DEVICE_ID_LEN) > + return -EINVAL; > + > + return 0; > +} [Severity: Low] Should the truncation check test ret >= SCSI_MPATH_DEVICE_ID_LEN instead of ret > SCSI_MPATH_DEVICE_ID_LEN? In scsi_vpd_lun_id(), a T10 Vendor ID designator with 252 or more bytes is clamped to id_len - 4 (252) and formatted with snprintf(id, id_len, "t10.%*pE", ...), which truncates device_id_str at 255 characters plus the NUL terminator and returns 256 (SCSI_MPATH_DEVICE_ID_LEN). Testing ret > SCSI_MPATH_DEVICE_ID_LEN accepts that truncated identifier. [ ... ] > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("scsi_multipath"); [Severity: Low] Does linking scsi_multipath.o into scsi_mod.o emit duplicate MODULE_LICENSE and conflicting MODULE_DESCRIPTION entries in scsi_mod's .modinfo section? drivers/scsi/scsi.c already defines MODULE_DESCRIPTION("SCSI core") and MODULE_LICENSE("GPL") for scsi_mod. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
