| Message ID | 1477451932-7756-1-git-send-email-zhou.guichun@zte.com.cn (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
On 10/25/2016 08:18 PM, zhou.guichun@zte.com.cn wrote: > From: zhouguichun <zhou.guichun@zte.com.cn> > > I/O access overrun occured in mutipath device under > the displayed capacity since that the displayed > capacity is greater than the real capacity > (Eg. a 3456M disk display as 3.4G). > > In this patch, we keep the displayed capacity three > decimals by abandoning the rest of decimals instead > of by rounding up(Eg. a 3.4567G disk display as > 3.456G), so that the displayed capacity will not be > greater than the real capacity, and the error will > also be in a permissible range. The motivation of this patch looks really weird to me. How could the rounding in snprint_size() cause an access past the end of a block device? Using the output of that function as block device size is wrong. Please use /sys/class/block/dm*/size instead. Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hello Bart, I agree with you that the size showed in /sys/class/block/dm*/size is most accurate, we should use this size. But I think once we showed the size of block device, we can't guarantee that users will not use it. So I think the size of block device showed by multipath should not greater than the real capacity. I'm looking forward to your reply. Thanks, Zhou 周桂春 Zhou GuiChun 软件开发 SoftWare Development 虚拟化长沙开发部 NIV Changsha Development Dept 湖南省长沙市岳麓区尖山路39号中电软件园八楼 T: +86 731-82057106 M: +86 13657402573 E: zhou.guichun@zte.com.cn www.zte.com.cn 发件人: Bart Van Assche <bart.vanassche@sandisk.com> 收件人: <zhou.guichun@zte.com.cn>, Christophe Varoqui <christophe.varoqui@opensvc.com>, 抄送: <tang.junhui@zte.com.cn>, <zhang.kai16@zte.com.cn>, <dm-devel@redhat.com> 日期: 2016/10/27 02:09 主题: Re: [dm-devel] [PATCH] libmultipath: I/O access overrun occured in mutipath device under the displayed capacity On 10/25/2016 08:18 PM, zhou.guichun@zte.com.cn wrote: > From: zhouguichun <zhou.guichun@zte.com.cn> > > I/O access overrun occured in mutipath device under > the displayed capacity since that the displayed > capacity is greater than the real capacity > (Eg. a 3456M disk display as 3.4G). > > In this patch, we keep the displayed capacity three > decimals by abandoning the rest of decimals instead > of by rounding up(Eg. a 3.4567G disk display as > 3.456G), so that the displayed capacity will not be > greater than the real capacity, and the error will > also be in a permissible range. The motivation of this patch looks really weird to me. How could the rounding in snprint_size() cause an access past the end of a block device? Using the output of that function as block device size is wrong. Please use /sys/class/block/dm*/size instead. Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/libmultipath/print.c b/libmultipath/print.c index 9aa41ad..4635e5a 100644 --- a/libmultipath/print.c +++ b/libmultipath/print.c @@ -11,6 +11,7 @@ #include <string.h> #include <errno.h> #include <libudev.h> +#include <math.h> #include "checkers.h" #include "vector.h" @@ -72,16 +73,19 @@ snprint_uint (char * buff, size_t len, unsigned int val) static int snprint_size (char * buff, size_t len, unsigned long long size) { - float s = (float)(size >> 1); /* start with KB */ + double s = (float)(size >> 1); /* start with KB */ char units[] = {'K','M','G','T','P'}; char *u = units; + double tmp = 0.0; while (s >= 1024 && *u != 'P') { s = s / 1024; u++; } - return snprintf(buff, len, "%.*f%c", s < 10, s, *u); + tmp = floor(s * 1000); + s = tmp / 1000; + return snprintf(buff, len, "%.3f%c", s, *u); } /* diff --git a/mpathpersist/Makefile b/mpathpersist/Makefile index 47043bb..e9691b3 100644 --- a/mpathpersist/Makefile +++ b/mpathpersist/Makefile @@ -3,7 +3,7 @@ include ../Makefile.inc CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) LIBDEPS += -lpthread -ldevmapper -L$(mpathpersistdir) -lmpathpersist \ - -L$(multipathdir) -L$(mpathcmddir) -lmpathcmd -lmultipath -ludev + -L$(multipathdir) -L$(mpathcmddir) -lmpathcmd -lmultipath -ludev -lm EXEC = mpathpersist diff --git a/multipath/Makefile b/multipath/Makefile index cad34bf..1f3f0ee 100644 --- a/multipath/Makefile +++ b/multipath/Makefile @@ -6,7 +6,7 @@ include ../Makefile.inc CFLAGS += -I$(multipathdir) -I$(mpathcmddir) LIBDEPS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath -ludev \ - -L$(mpathcmddir) -lmpathcmd + -L$(mpathcmddir) -lmpathcmd -lm EXEC = multipath diff --git a/multipathd/Makefile b/multipathd/Makefile index ae06f9e..4db6397 100644 --- a/multipathd/Makefile +++ b/multipathd/Makefile @@ -11,7 +11,7 @@ CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir) \ LIBDEPS += -ludev -ldl -L$(multipathdir) -lmultipath -L$(mpathpersistdir) \ -lmpathpersist -L$(mpathcmddir) -lmpathcmd -lurcu -lpthread \ - -ldevmapper -lreadline + -ldevmapper -lreadline -lm ifdef SYSTEMD CFLAGS += -DUSE_SYSTEMD=$(SYSTEMD)