From db34e93936779c7b1d607a11bdea50995addc5b1 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Mon, 17 Aug 2026 11:10:49 +0200 Subject: [PATCH] Check and report shared physical devices across array disks --- cmdline/app.h | 19 ++++++++----- cmdline/device.c | 67 +++++++++++++++++++++++++++++++++++++++++----- cmdline/mingwapp.c | 19 +++++++++---- cmdline/thermal.c | 6 ++++- cmdline/unixapp.c | 41 +++++++++++++++++++--------- 5 files changed, 120 insertions(+), 32 deletions(-) diff --git a/cmdline/app.h b/cmdline/app.h index 28584a06..fbe8c897 100644 --- a/cmdline/app.h +++ b/cmdline/app.h @@ -315,9 +315,6 @@ struct smart_attr { #define POWER_ACTIVE 1 #define POWER_UNKNOWN -1 -/** - * Device info entry. - */ struct devinfo_struct { uint64_t device; /**< Device ID. */ char name[PATH_MAX]; /**< Name of the disk combined with the split index if any */ @@ -330,6 +327,7 @@ struct devinfo_struct { char wfile[PATH_MAX]; /**< File device in Windows format. Like \\.\PhysicalDriveX, or \\?\Volume{X}. */ #endif struct devinfo_struct* parent; /**< Pointer at the parent if any. */ + int is_array; /**< 1 if this is an array disk (data or parity), 0 if extra disk. */ struct devinfo_struct* split; /**< Pointer at first split if this one is not the first. */ struct smart_attr smart[SMART_COUNT]; /**< All smart values. */ uint64_t info[INFO_COUNT]; /**< Informational attributes not related to SMART telemetry. */ @@ -356,12 +354,21 @@ void device_name_set(devinfo_t* dev, const char* name, int index); #define DEVICE_DOWNIFUP 5 /** - * Query all the "high" level devices with the specified operation, - * and produces a list of "low" level devices to operate on. + * Sync filesystems associated with the "high" level devices. + */ +void devsync(tommy_list* high); + +/** + * Query all the "high" level devices and produce a list of "low" level devices. * * The passed "low" device list must be already initialized. */ -int devquery(tommy_list* high, tommy_list* low, int operation); +int devquery(tommy_list* high, tommy_list* low); + +/** + * Execute an operation on the "low" level devices. + */ +int devrun(tommy_list* low, int operation); /** * Fill with fake data the device list. diff --git a/cmdline/device.c b/cmdline/device.c index 24cc22c4..5088c1a0 100644 --- a/cmdline/device.c +++ b/cmdline/device.c @@ -640,7 +640,7 @@ static void state_smart(struct snapraid_state* state, size_t n, tommy_list* low) unsigned j; size_t device_pad; size_t serial_pad; - int have_parent; + int have_array; double array_failure_rate; double p_at_least_one_failure; int make_it_fail = 0; @@ -649,7 +649,7 @@ static void state_smart(struct snapraid_state* state, size_t n, tommy_list* low) /* compute lengths for padding */ device_pad = 0; serial_pad = 0; - have_parent = 0; + have_array = 0; for (i = tommy_list_head(low); i != 0; i = i->next) { size_t len; devinfo_t* devinfo = i->data; @@ -662,8 +662,8 @@ static void state_smart(struct snapraid_state* state, size_t n, tommy_list* low) if (len > serial_pad) serial_pad = len; - if (devinfo->parent != 0) - have_parent = 1; + if (devinfo->parent && devinfo->parent->is_array) + have_array = 1; } printf("SnapRAID SMART report:\n"); @@ -756,8 +756,10 @@ static void state_smart(struct snapraid_state* state, size_t n, tommy_list* low) /* this happens only if no data */ printf(" -"); } else { - /* use only the disks in the array */ - if (devinfo->parent != 0 || !have_parent) + int is_array = devinfo->parent && devinfo->parent->is_array; + + /* compute failure rate only for array disks (or fallback to all disks if none are array members) */ + if (is_array || !have_array) array_failure_rate += afr; printf("%4.0f%%", poisson_prob_at_least_one_failure(afr) * 100); @@ -976,6 +978,41 @@ static void state_devices(struct snapraid_state* state, tommy_list* low) } } +static void state_devices_check(struct snapraid_state* state, tommy_list* low) +{ + tommy_node* i; + + if (state->opt.no_warnings || state->opt.skip_device) + return; + + for (i = tommy_list_head(low); i != 0; i = i->next) { + devinfo_t* d1 = i->data; + tommy_node* j; + + for (j = i->next; j != 0; j = j->next) { + devinfo_t* d2 = j->data; + + if (d1->device == d2->device && d1->parent && d2->parent) { + devinfo_t* p1 = d1->parent; + devinfo_t* p2 = d2->parent; + + /* only check conflict between array members (data/parity) */ + if (!p1->is_array || !p2->is_array) + continue; + + /* ignore extents of the same member or splits of the same parity level */ + devinfo_t* a1 = p1->split ? p1->split : p1; + devinfo_t* a2 = p2->split ? p2->split : p2; + if (a1 == a2) + continue; + + log_error(EUSER, "WARNING! Disks '%s' and '%s' share physical device '%s'.\n", p1->name, p2->name, d1->file); + log_error(EUSER, "This configuration does not provide independent physical fault domains.\n"); + } + } + } +} + int devtest(tommy_list* high, tommy_list* low, int operation) { tommy_node* i; @@ -1080,6 +1117,7 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter entry->device = disk->mount_device; device_name_set(entry, disk->name, 0); + entry->is_array = 1; pathcpy(entry->mount, sizeof(entry->mount), disk->dir); pathcpy(entry->smartctl, sizeof(entry->smartctl), disk->smartctl); pathcpy(entry->smartctl_info, sizeof(entry->smartctl_info), disk->smartctl_info); @@ -1108,6 +1146,7 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter entry->device = state->parity[j].split_map[s].device; device_name_set(entry, lev_config_name(j), s); + entry->is_array = 1; pathcpy(entry->mount, sizeof(entry->mount), state->parity[j].split_map[s].path); pathcpy(entry->smartctl, sizeof(entry->smartctl), state->parity[j].smartctl); pathcpy(entry->smartctl_info, sizeof(entry->smartctl_info), state->parity[j].smartctl_info); @@ -1140,6 +1179,7 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter entry->device = extra->device; device_name_set(entry, extra->name, 0); + entry->is_array = 0; pathcpy(entry->mount, sizeof(entry->mount), extra->dir); pathcpy(entry->smartctl, sizeof(entry->smartctl), extra->smartctl); pathcpy(entry->smartctl_info, sizeof(entry->smartctl_info), extra->smartctl_info); @@ -1156,7 +1196,11 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter if (state->opt.fake_device) { ret = devtest(&high, &low, operation); } else { - ret = devquery(&high, &low, operation); + /* issue pending filesystem writes before the underlying devices enter standby */ + if (operation == DEVICE_DOWN) + devsync(&high); + + ret = devquery(&high, &low); } /* if the list is empty, it's not supported in this platform */ @@ -1177,6 +1221,14 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter } else { state_stat(state, &high); + state_devices_check(state, &low); + + if (!state->opt.fake_device) { + ret = devrun(&low, operation); + if (ret != 0) + goto bail; + } + if (operation == DEVICE_SMART) state_smart(state, state->level + tommy_list_count(&state->disklist), &low); @@ -1190,6 +1242,7 @@ int state_device(struct snapraid_state* state, int operation, tommy_list* filter state_devices(state, &low); } +bail: tommy_list_foreach(&high, free); tommy_list_foreach(&low, free); diff --git a/cmdline/mingwapp.c b/cmdline/mingwapp.c index c5dc7be1..d4345f22 100644 --- a/cmdline/mingwapp.c +++ b/cmdline/mingwapp.c @@ -851,9 +851,7 @@ static int devtree(devinfo_t* parent, tommy_list* list) } for (i = 0; i < vde->NumberOfDiskExtents; ++i) { - devinfo_t* devinfo; - - devinfo = calloc_nofail(1, sizeof(devinfo_t)); + devinfo_t* devinfo = calloc_nofail(1, sizeof(devinfo_t)); pathcpy(devinfo->name, sizeof(devinfo->name), parent->name); pathcpy(devinfo->smartctl, sizeof(devinfo->smartctl), parent->smartctl); @@ -1747,10 +1745,14 @@ static int device_thread(tommy_list* list, void* (*func)(void* arg)) return 0; } -int devquery(tommy_list* high, tommy_list* low, int operation) +void devsync(tommy_list* high) +{ + (void)high; +} + +int devquery(tommy_list* high, tommy_list* low) { tommy_node* i; - void* (*func)(void* arg) = 0; /* for each device */ for (i = tommy_list_head(high); i != 0; i = i->next) { @@ -1782,6 +1784,13 @@ int devquery(tommy_list* high, tommy_list* low, int operation) } } + return 0; +} + +int devrun(tommy_list* low, int operation) +{ + void* (*func)(void* arg) = 0; + switch (operation) { case DEVICE_UP : func = thread_spinup; break; case DEVICE_DOWN : func = thread_spindown; break; diff --git a/cmdline/thermal.c b/cmdline/thermal.c index 854191e9..53199f01 100644 --- a/cmdline/thermal.c +++ b/cmdline/thermal.c @@ -138,6 +138,7 @@ void state_thermal(struct snapraid_state* state, time_t now) entry->device = disk->mount_device; device_name_set(entry, disk->name, 0); + entry->is_array = 1; pathcpy(entry->mount, sizeof(entry->mount), disk->mount_point); pathcpy(entry->smartctl, sizeof(entry->smartctl), disk->smartctl); pathcpy(entry->smartctl_info, sizeof(entry->smartctl_info), disk->smartctl_info); @@ -156,6 +157,7 @@ void state_thermal(struct snapraid_state* state, time_t now) entry->device = state->parity[j].split_map[s].device; device_name_set(entry, lev_config_name(j), s); + entry->is_array = 1; pathcpy(entry->mount, sizeof(entry->mount), state->parity[j].split_map[s].path); pathcpy(entry->smartctl, sizeof(entry->smartctl), state->parity[j].smartctl); pathcpy(entry->smartctl_info, sizeof(entry->smartctl_info), state->parity[j].smartctl_info); @@ -173,7 +175,9 @@ void state_thermal(struct snapraid_state* state, time_t now) if (state->opt.fake_device) { ret = devtest(&high, &low, DEVICE_SMART); } else { - ret = devquery(&high, &low, DEVICE_SMART); + ret = devquery(&high, &low); + if (ret == 0 && !tommy_list_empty(&low)) + ret = devrun(&low, DEVICE_SMART); } /* on error, just disable thermal gathering */ diff --git a/cmdline/unixapp.c b/cmdline/unixapp.c index 86277183..a2c7fbad 100644 --- a/cmdline/unixapp.c +++ b/cmdline/unixapp.c @@ -3669,10 +3669,28 @@ static int device_thread(tommy_list* list, void* (*func)(void* arg)) return 0; } -int devquery(tommy_list* high, tommy_list* low, int operation) +void devsync(tommy_list* high) { - void* (*func)(void* arg) = 0; +#if HAVE_SYNCFS + tommy_node* i; + for (i = tommy_list_head(high); i != 0; i = i->next) { + devinfo_t* devinfo = i->data; + int f; + + f = open(devinfo->mount, O_RDONLY); + if (f >= 0) { + syncfs(f); + close(f); + } + } +#else + (void)high; +#endif +} + +int devquery(tommy_list* high, tommy_list* low) +{ #if HAVE_LINUX_DEVICE tommy_node* i; struct stat st; @@ -3690,17 +3708,6 @@ int devquery(tommy_list* high, tommy_list* low, int operation) devinfo_t* devinfo = i->data; uint64_t device = devinfo->device; -#if HAVE_SYNCFS - if (operation == DEVICE_DOWN) { - /* flush the high level filesystem before spinning down */ - int f = open(devinfo->mount, O_RDONLY); - if (f >= 0) { - syncfs(f); - close(f); - } - } -#endif - tommy_list devlist; tommy_list_init(&devlist); @@ -3753,8 +3760,16 @@ int devquery(tommy_list* high, tommy_list* low, int operation) } #else (void)high; + (void)low; #endif + return 0; +} + +int devrun(tommy_list* low, int operation) +{ + void* (*func)(void* arg) = 0; + switch (operation) { case DEVICE_UP : func = thread_spinup; break; case DEVICE_DOWN : func = thread_spindown; break;