* [PATCH] mdadm/util:integrate fstat and stat into a utility function
@ 2017-03-30 4:46 Zhilong Liu
2017-03-30 18:46 ` Jes Sorensen
0 siblings, 1 reply; 2+ messages in thread
From: Zhilong Liu @ 2017-03-30 4:46 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
mdadm/util: lots of dupilicate codes about stat and fstat
check the block device, move them into a utility function
to make it concise.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Assemble.c | 9 +--------
Build.c | 10 +---------
Create.c | 5 +----
Dump.c | 3 +--
Grow.c | 4 +---
Incremental.c | 27 +++------------------------
Manage.c | 9 +--------
mdadm.h | 1 +
super-intel.c | 9 +--------
util.c | 15 +++++++++++++++
10 files changed, 26 insertions(+), 66 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 6a6a56b..d3bfaa2 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -204,14 +204,7 @@ static int select_devices(struct mddev_dev *devlist,
pr_err("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- pr_err("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- devname);
+ } else if (check_block_dev(devname)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
if (st) {
diff --git a/Build.c b/Build.c
index a5fcc06..4fa606c 100644
--- a/Build.c
+++ b/Build.c
@@ -67,16 +67,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
missing_disks++;
continue;
}
- if (stat(dv->devname, &stb)) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- return 1;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- dv->devname);
+ if (check_block_dev(dv->devname))
return 1;
- }
}
if (s->raiddisks != subdevs) {
diff --git a/Create.c b/Create.c
index 10e7d10..dbd9ce3 100644
--- a/Create.c
+++ b/Create.c
@@ -327,11 +327,8 @@ int Create(struct supertype *st, char *mddev,
dname, strerror(errno));
exit(2);
}
- if (fstat(dfd, &stb) != 0 ||
- (stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (check_block_dev(dname)) {
close(dfd);
- pr_err("%s is not a block device\n",
- dname);
exit(2);
}
close(dfd);
diff --git a/Dump.c b/Dump.c
index 7bdbf6f..29fc02a 100644
--- a/Dump.c
+++ b/Dump.c
@@ -131,8 +131,7 @@ int Dump_metadata(char *dev, char *dir, struct context *c,
if (de->d_name[0] == '.')
continue;
xasprintf(&p, "/dev/disk/by-id/%s", de->d_name);
- if (stat(p, &stb) != 0 ||
- (stb.st_mode & S_IFMT) != S_IFBLK ||
+ if (check_block_dev(p) != 0 ||
stb.st_rdev != dstb.st_rdev) {
/* Not this one */
free(p);
diff --git a/Grow.c b/Grow.c
index b86b53e..063996d 100755
--- a/Grow.c
+++ b/Grow.c
@@ -145,9 +145,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
free(st);
return 1;
}
- fstat(nfd, &stb);
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device!\n", newdev);
+ if (check_block_dev(newdev)) {
close(nfd);
free(st);
return 1;
diff --git a/Incremental.c b/Incremental.c
index 81afc7e..b2fad39 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -108,18 +108,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
struct createinfo *ci = conf_get_create_info();
- if (stat(devname, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("stat failed for %s: %s.\n",
- devname, strerror(errno));
+ if (check_block_dev(devname))
return rv;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
- return rv;
- }
dfd = dev_open(devname, O_RDONLY);
if (dfd < 0) {
if (c->verbose >= 0)
@@ -159,8 +149,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
devlist = conf_get_devs();
for (;devlist; devlist = devlist->next) {
struct stat st2;
- if (stat(devlist->devname, &st2) == 0 &&
- (st2.st_mode & S_IFMT) == S_IFBLK &&
+ if (check_block_dev(devlist->devname) == 0 &&
st2.st_rdev == stb.st_rdev)
break;
}
@@ -175,18 +164,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
/* 2/ Find metadata, reject if none appropriate (check
* version/name from args) */
- if (fstat(dfd, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("fstat failed for %s: %s.\n",
- devname, strerror(errno));
+ if (check_block_dev(devname))
goto out;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
- goto out;
- }
dinfo.disk.major = major(stb.st_rdev);
dinfo.disk.minor = minor(stb.st_rdev);
diff --git a/Manage.c b/Manage.c
index 55218d9..8687084 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1543,17 +1543,10 @@ int Manage_subdevs(char *devname, int fd,
close(tfd);
} else {
int open_err = errno;
- if (stat(dv->devname, &stb) != 0) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- goto abort;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (check_block_dev(dv->devname) != 0) {
if (dv->disposition == 'M')
/* non-fatal. Also improbable */
continue;
- pr_err("%s is not a block device.\n",
- dv->devname);
goto abort;
}
if (dv->disposition == 'r')
diff --git a/mdadm.h b/mdadm.h
index dbf1f92..ad43a35 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1416,6 +1416,7 @@ extern int parse_cluster_confirm_arg(char *inp, char **devname, int *slot);
extern int check_ext2(int fd, char *name);
extern int check_reiser(int fd, char *name);
extern int check_raid(int fd, char *name);
+extern int check_block_dev(char *dev);
extern int check_partitions(int fd, char *dname,
unsigned long long freesize,
unsigned long long size);
diff --git a/super-intel.c b/super-intel.c
index 785488a..21b5887 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6584,14 +6584,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
dprintf("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- dprintf("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- dprintf("%s is not a block device.\n",
- devname);
+ } else if (check_block_dev(devname)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
struct supertype *cst;
diff --git a/util.c b/util.c
index 683c869..e7aeb54 100644
--- a/util.c
+++ b/util.c
@@ -750,6 +750,21 @@ int ask(char *mesg)
}
#endif /* MDASSEMBLE */
+int check_block_dev(char *dev)
+{
+ struct stat stb;
+
+ if (stat(dev, &stb) != 0) {
+ pr_err("stat failed for %s: %s\n", dev, strerror(errno));
+ return -1;
+ }
+ if ((S_IFMT & stb.st_mode) != S_IFBLK) {
+ pr_err("%s is not a block device.\n", dev);
+ return -1;
+ }
+ return 0;
+}
+
int is_standard(char *dev, int *nump)
{
/* tests if dev is a "standard" md dev name.
--
2.10.2
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] mdadm/util:integrate fstat and stat into a utility function
2017-03-30 4:46 [PATCH] mdadm/util:integrate fstat and stat into a utility function Zhilong Liu
@ 2017-03-30 18:46 ` Jes Sorensen
0 siblings, 0 replies; 2+ messages in thread
From: Jes Sorensen @ 2017-03-30 18:46 UTC (permalink / raw)
To: Zhilong Liu; +Cc: linux-raid
On 03/30/2017 12:46 AM, Zhilong Liu wrote:
> mdadm/util: lots of dupilicate codes about stat and fstat
> check the block device, move them into a utility function
> to make it concise.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> Assemble.c | 9 +--------
> Build.c | 10 +---------
> Create.c | 5 +----
> Dump.c | 3 +--
> Grow.c | 4 +---
> Incremental.c | 27 +++------------------------
> Manage.c | 9 +--------
> mdadm.h | 1 +
> super-intel.c | 9 +--------
> util.c | 15 +++++++++++++++
> 10 files changed, 26 insertions(+), 66 deletions(-)
I'm a little mixed on this one. Defaulting to stat() rather than fstat()
seems like a sledgehammer approach for the cases where we already have
the fd open. I would prefer to default to fstat or have two versions I'd
say.
Cheers,
Jes
> diff --git a/Assemble.c b/Assemble.c
> index 6a6a56b..d3bfaa2 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -204,14 +204,7 @@ static int select_devices(struct mddev_dev *devlist,
> pr_err("cannot open device %s: %s\n",
> devname, strerror(errno));
> tmpdev->used = 2;
> - } else if (fstat(dfd, &stb)< 0) {
> - /* Impossible! */
> - pr_err("fstat failed for %s: %s\n",
> - devname, strerror(errno));
> - tmpdev->used = 2;
> - } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - pr_err("%s is not a block device.\n",
> - devname);
> + } else if (check_block_dev(devname)) {
> tmpdev->used = 2;
> } else if (must_be_container(dfd)) {
> if (st) {
> diff --git a/Build.c b/Build.c
> index a5fcc06..4fa606c 100644
> --- a/Build.c
> +++ b/Build.c
> @@ -67,16 +67,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
> missing_disks++;
> continue;
> }
> - if (stat(dv->devname, &stb)) {
> - pr_err("Cannot find %s: %s\n",
> - dv->devname, strerror(errno));
> - return 1;
> - }
> - if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - pr_err("%s is not a block device.\n",
> - dv->devname);
> + if (check_block_dev(dv->devname))
> return 1;
> - }
> }
>
> if (s->raiddisks != subdevs) {
> diff --git a/Create.c b/Create.c
> index 10e7d10..dbd9ce3 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -327,11 +327,8 @@ int Create(struct supertype *st, char *mddev,
> dname, strerror(errno));
> exit(2);
> }
> - if (fstat(dfd, &stb) != 0 ||
> - (stb.st_mode & S_IFMT) != S_IFBLK) {
> + if (check_block_dev(dname)) {
> close(dfd);
> - pr_err("%s is not a block device\n",
> - dname);
> exit(2);
> }
> close(dfd);
> diff --git a/Dump.c b/Dump.c
> index 7bdbf6f..29fc02a 100644
> --- a/Dump.c
> +++ b/Dump.c
> @@ -131,8 +131,7 @@ int Dump_metadata(char *dev, char *dir, struct context *c,
> if (de->d_name[0] == '.')
> continue;
> xasprintf(&p, "/dev/disk/by-id/%s", de->d_name);
> - if (stat(p, &stb) != 0 ||
> - (stb.st_mode & S_IFMT) != S_IFBLK ||
> + if (check_block_dev(p) != 0 ||
> stb.st_rdev != dstb.st_rdev) {
> /* Not this one */
> free(p);
> diff --git a/Grow.c b/Grow.c
> index b86b53e..063996d 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -145,9 +145,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
> free(st);
> return 1;
> }
> - fstat(nfd, &stb);
> - if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - pr_err("%s is not a block device!\n", newdev);
> + if (check_block_dev(newdev)) {
> close(nfd);
> free(st);
> return 1;
> diff --git a/Incremental.c b/Incremental.c
> index 81afc7e..b2fad39 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -108,18 +108,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
>
> struct createinfo *ci = conf_get_create_info();
>
> - if (stat(devname, &stb) < 0) {
> - if (c->verbose >= 0)
> - pr_err("stat failed for %s: %s.\n",
> - devname, strerror(errno));
> + if (check_block_dev(devname))
> return rv;
> - }
> - if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - if (c->verbose >= 0)
> - pr_err("%s is not a block device.\n",
> - devname);
> - return rv;
> - }
> dfd = dev_open(devname, O_RDONLY);
> if (dfd < 0) {
> if (c->verbose >= 0)
> @@ -159,8 +149,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
> devlist = conf_get_devs();
> for (;devlist; devlist = devlist->next) {
> struct stat st2;
> - if (stat(devlist->devname, &st2) == 0 &&
> - (st2.st_mode & S_IFMT) == S_IFBLK &&
> + if (check_block_dev(devlist->devname) == 0 &&
> st2.st_rdev == stb.st_rdev)
> break;
> }
> @@ -175,18 +164,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
> /* 2/ Find metadata, reject if none appropriate (check
> * version/name from args) */
>
> - if (fstat(dfd, &stb) < 0) {
> - if (c->verbose >= 0)
> - pr_err("fstat failed for %s: %s.\n",
> - devname, strerror(errno));
> + if (check_block_dev(devname))
> goto out;
> - }
> - if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - if (c->verbose >= 0)
> - pr_err("%s is not a block device.\n",
> - devname);
> - goto out;
> - }
>
> dinfo.disk.major = major(stb.st_rdev);
> dinfo.disk.minor = minor(stb.st_rdev);
> diff --git a/Manage.c b/Manage.c
> index 55218d9..8687084 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -1543,17 +1543,10 @@ int Manage_subdevs(char *devname, int fd,
> close(tfd);
> } else {
> int open_err = errno;
> - if (stat(dv->devname, &stb) != 0) {
> - pr_err("Cannot find %s: %s\n",
> - dv->devname, strerror(errno));
> - goto abort;
> - }
> - if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> + if (check_block_dev(dv->devname) != 0) {
> if (dv->disposition == 'M')
> /* non-fatal. Also improbable */
> continue;
> - pr_err("%s is not a block device.\n",
> - dv->devname);
> goto abort;
> }
> if (dv->disposition == 'r')
> diff --git a/mdadm.h b/mdadm.h
> index dbf1f92..ad43a35 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1416,6 +1416,7 @@ extern int parse_cluster_confirm_arg(char *inp, char **devname, int *slot);
> extern int check_ext2(int fd, char *name);
> extern int check_reiser(int fd, char *name);
> extern int check_raid(int fd, char *name);
> +extern int check_block_dev(char *dev);
> extern int check_partitions(int fd, char *dname,
> unsigned long long freesize,
> unsigned long long size);
> diff --git a/super-intel.c b/super-intel.c
> index 785488a..21b5887 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6584,14 +6584,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
> dprintf("cannot open device %s: %s\n",
> devname, strerror(errno));
> tmpdev->used = 2;
> - } else if (fstat(dfd, &stb)< 0) {
> - /* Impossible! */
> - dprintf("fstat failed for %s: %s\n",
> - devname, strerror(errno));
> - tmpdev->used = 2;
> - } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
> - dprintf("%s is not a block device.\n",
> - devname);
> + } else if (check_block_dev(devname)) {
> tmpdev->used = 2;
> } else if (must_be_container(dfd)) {
> struct supertype *cst;
> diff --git a/util.c b/util.c
> index 683c869..e7aeb54 100644
> --- a/util.c
> +++ b/util.c
> @@ -750,6 +750,21 @@ int ask(char *mesg)
> }
> #endif /* MDASSEMBLE */
>
> +int check_block_dev(char *dev)
> +{
> + struct stat stb;
> +
> + if (stat(dev, &stb) != 0) {
> + pr_err("stat failed for %s: %s\n", dev, strerror(errno));
> + return -1;
> + }
> + if ((S_IFMT & stb.st_mode) != S_IFBLK) {
> + pr_err("%s is not a block device.\n", dev);
> + return -1;
> + }
> + return 0;
> +}
> +
> int is_standard(char *dev, int *nump)
> {
> /* tests if dev is a "standard" md dev name.
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-30 18:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-30 4:46 [PATCH] mdadm/util:integrate fstat and stat into a utility function Zhilong Liu
2017-03-30 18:46 ` Jes Sorensen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox