On 07/21/2017 08:15 AM, Mariusz Tkaczyk wrote:
GET_MISMATCH option doesn't exist for containers so sysfs_read fails if
this information is requested. Set options according to the device using
information from /proc/mdstat.
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxx>
Reviewed-by: Tomasz Majchrzak <tomasz.majchrzak@xxxxxxxxx>
---
Monitor.c | 50 ++++++++++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 22 deletions(-)
Hi Mariusz,
Sorry for the late response, I am currently traveling so there have been
some delays.
I am fine with the principle of this patch, but I have a few nits:
diff --git a/Monitor.c b/Monitor.c
index 48c451c..8d753ca 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -459,22 +459,41 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
mdu_array_info_t array;
struct mdstat_ent *mse = NULL, *mse2;
char *dev = st->devname;
- int fd;
+ int fd = -1;
int i;
int remaining_disks;
int last_disk;
int new_array = 0;
- int retval;
+ int retval = 0;
+ int is_container;
+ unsigned long array_only_flags = 0;
You initialize array_only_flags here.
[snip]
@@ -482,11 +501,11 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
if (md_get_array_info(fd, &array) < 0)
goto disappeared;
- if (st->devnm[0] == 0)
- strcpy(st->devnm, fd2devnm(fd));
+ array_only_flags |= !is_container ? GET_MISMATCH : 0;
+
... and again here.
Could we try and avoid that obfuscated approach and simply do
if (!is_container)
array_only_flags |= GET_MISMATCH
That would avoid the double initialization and is a lot more readable.
Second issue, you are moving to global initialization for a number of
variables. That is pretty much always bad as it hides things - please
avoid that unless absolutely necessary.
Thanks,
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
