[dm-devel] [PATCH 04/13] libmultipath: return error numbers from sysfs_get_XXX
Christophe Varoqui
christophe.varoqui at opensvc.com
Sun Nov 17 17:34:53 UTC 2013
declare_sysfs_get_str(devtype);
-declare_sysfs_get_str(cutype);
declare_sysfs_get_str(vendor);
declare_sysfs_get_str(model);
declare_sysfs_get_str(rev);
This part seems correct, as this function is not used anywhere, but
orthogonal to the patch.
Can you confirm this slip is safe ?
Best regards,
Christophe Varoqui
www.opensvc.com
On Fri, Nov 15, 2013 at 11:29 AM, Hannes Reinecke <hare at suse.de> wrote:
> Instead of returning hand-crafted error values from sysfs_get_XXX
> functions we should be using the standard error numbers.
>
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> ---
> libmultipath/discovery.c | 40 +++++++++++++++++++++-------------------
> libmultipath/discovery.h | 2 +-
> libmultipath/propsel.c | 2 +-
> libmultipath/structs_vec.c | 3 +--
> 4 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 3292358..d5557d9 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -140,31 +140,34 @@ path_discovery (vector pathvec, struct config *
> conf, int flag)
> }
>
> #define declare_sysfs_get_str(fname) \
> -extern int \
> +extern ssize_t \
> sysfs_get_##fname (struct udev_device * udev, char * buff, size_t len) \
> { \
> + ssize_t ret; \
> const char * attr; \
> const char * devname; \
> \
> + if (!udev) \
> + return -ENOSYS; \
> + \
> devname = udev_device_get_sysname(udev); \
> \
> attr = udev_device_get_sysattr_value(udev, #fname); \
> if (!attr) { \
> condlog(3, "%s: attribute %s not found in sysfs", \
> devname, #fname); \
> - return 1; \
> + return -ENXIO; \
> } \
> if (strlen(attr) > len) { \
> condlog(3, "%s: overflow in attribute %s", \
> devname, #fname); \
> - return 2; \
> + return -EINVAL; \
> } \
> - strlcpy(buff, attr, len); \
> - return 0; \
> + ret = strlcpy(buff, attr, len); \
> + return ret; \
> }
>
> declare_sysfs_get_str(devtype);
> -declare_sysfs_get_str(cutype);
> declare_sysfs_get_str(vendor);
> declare_sysfs_get_str(model);
> declare_sysfs_get_str(rev);
> @@ -180,7 +183,7 @@ sysfs_get_timeout(struct path *pp, unsigned int
> *timeout)
> unsigned int t;
>
> if (!pp->udev || pp->bus != SYSFS_BUS_SCSI)
> - return 1;
> + return -ENOSYS;
>
> parent = pp->udev;
> while (parent) {
> @@ -192,7 +195,7 @@ sysfs_get_timeout(struct path *pp, unsigned int
> *timeout)
> }
> if (!attr) {
> condlog(3, "%s: No timeout value in sysfs", pp->dev);
> - return 1;
> + return -ENXIO;
> }
>
> r = sscanf(attr, "%u\n", &t);
> @@ -200,7 +203,7 @@ sysfs_get_timeout(struct path *pp, unsigned int
> *timeout)
> if (r != 1) {
> condlog(3, "%s: Cannot parse timeout attribute '%s'",
> pp->dev, attr);
> - return 1;
> + return -EINVAL;
> }
>
> *timeout = t;
> @@ -650,17 +653,17 @@ scsi_sysfs_pathinfo (struct path * pp)
> if (!attr_path || pp->sg_id.host_no == -1)
> return 1;
>
> - if (sysfs_get_vendor(parent, pp->vendor_id, SCSI_VENDOR_SIZE))
> + if (sysfs_get_vendor(parent, pp->vendor_id, SCSI_VENDOR_SIZE) <= 0)
> return 1;
>
> condlog(3, "%s: vendor = %s", pp->dev, pp->vendor_id);
>
> - if (sysfs_get_model(parent, pp->product_id, SCSI_PRODUCT_SIZE))
> + if (sysfs_get_model(parent, pp->product_id, SCSI_PRODUCT_SIZE) <=
> 0)
> return 1;
>
> condlog(3, "%s: product = %s", pp->dev, pp->product_id);
>
> - if (sysfs_get_rev(parent, pp->rev, SCSI_REV_SIZE))
> + if (sysfs_get_rev(parent, pp->rev, SCSI_REV_SIZE) <= 0)
> return 1;
>
> condlog(3, "%s: rev = %s", pp->dev, pp->rev);
> @@ -712,7 +715,7 @@ ccw_sysfs_pathinfo (struct path * pp)
>
> condlog(3, "%s: vendor = %s", pp->dev, pp->vendor_id);
>
> - if (sysfs_get_devtype(parent, attr_buff, FILE_NAME_SIZE))
> + if (sysfs_get_devtype(parent, attr_buff, FILE_NAME_SIZE) <= 0)
> return 1;
>
> if (!strncmp(attr_buff, "3370", 4)) {
> @@ -772,17 +775,17 @@ cciss_sysfs_pathinfo (struct path * pp)
> if (!attr_path || pp->sg_id.host_no == -1)
> return 1;
>
> - if (sysfs_get_vendor(parent, pp->vendor_id, SCSI_VENDOR_SIZE))
> + if (sysfs_get_vendor(parent, pp->vendor_id, SCSI_VENDOR_SIZE) <= 0)
> return 1;
>
> condlog(3, "%s: vendor = %s", pp->dev, pp->vendor_id);
>
> - if (sysfs_get_model(parent, pp->product_id, SCSI_PRODUCT_SIZE))
> + if (sysfs_get_model(parent, pp->product_id, SCSI_PRODUCT_SIZE) <=
> 0)
> return 1;
>
> condlog(3, "%s: product = %s", pp->dev, pp->product_id);
>
> - if (sysfs_get_rev(parent, pp->rev, SCSI_REV_SIZE))
> + if (sysfs_get_rev(parent, pp->rev, SCSI_REV_SIZE) <= 0)
> return 1;
>
> condlog(3, "%s: rev = %s", pp->dev, pp->rev);
> @@ -816,7 +819,7 @@ common_sysfs_pathinfo (struct path * pp)
> condlog(4, "%s: udev not initialised", pp->dev);
> return 1;
> }
> - if (sysfs_get_dev(pp->udev, pp->dev_t, BLK_DEV_SIZE)) {
> + if (sysfs_get_dev(pp->udev, pp->dev_t, BLK_DEV_SIZE) <= 0) {
> condlog(3, "%s: no 'dev' attribute in sysfs", pp->dev);
> return 1;
> }
> @@ -956,8 +959,7 @@ get_state (struct path * pp, int daemon)
> if (daemon)
> checker_set_async(c);
> if (!conf->checker_timeout &&
> - (pp->bus != SYSFS_BUS_SCSI ||
> - sysfs_get_timeout(pp, &(c->timeout))))
> + sysfs_get_timeout(pp, &(c->timeout)) <= 0)
> c->timeout = DEF_TIMEOUT;
> state = checker_check(c);
> condlog(3, "%s: state = %s", pp->dev, checker_state_name(state));
> diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
> index d049ead..3d2d0ce 100644
> --- a/libmultipath/discovery.h
> +++ b/libmultipath/discovery.h
> @@ -26,7 +26,7 @@
>
> struct config;
>
> -int sysfs_get_dev (struct udev_device *udev, char * buff, size_t len);
> +ssize_t sysfs_get_dev (struct udev_device *udev, char * buff, size_t len);
> int path_discovery (vector pathvec, struct config * conf, int flag);
>
> int do_tur (char *);
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index e47d0ca..f092227 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -351,7 +351,7 @@ out:
> condlog(3, "%s: checker timeout = %u s (config file
> default)",
> pp->dev, c->timeout);
> }
> - else if (pp->udev && sysfs_get_timeout(pp, &c->timeout) == 0)
> + else if (sysfs_get_timeout(pp, &c->timeout) > 0)
> condlog(3, "%s: checker timeout = %u ms (sysfs setting)",
> pp->dev, c->timeout);
> else {
> diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
> index 14ea1c7..abb2c56 100644
> --- a/libmultipath/structs_vec.c
> +++ b/libmultipath/structs_vec.c
> @@ -451,8 +451,7 @@ verify_paths(struct multipath * mpp, struct vectors *
> vecs, vector rpvec)
> /*
> * see if path is in sysfs
> */
> - if (!pp->udev || sysfs_get_dev(pp->udev, pp->dev_t,
> - BLK_DEV_SIZE)) {
> + if (sysfs_get_dev(pp->udev, pp->dev_t, BLK_DEV_SIZE) <= 0)
> {
> if (pp->state != PATH_DOWN) {
> condlog(1, "%s: removing valid path %s in
> state %d",
> mpp->alias, pp->dev, pp->state);
> --
> 1.8.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20131117/737b7d4e/attachment.htm>
More information about the dm-devel
mailing list