* nfs-utils: blkmapd fixex @ 2014-08-07 7:26 Christoph Hellwig 2014-08-07 7:26 ` [PATCH 1/2] blkmapd: fix broken multipath handling Christoph Hellwig 2014-08-07 7:26 ` [PATCH 2/2] blkmapd: dump useful device information to syslog Christoph Hellwig 0 siblings, 2 replies; 5+ messages in thread From: Christoph Hellwig @ 2014-08-07 7:26 UTC (permalink / raw) To: linux-nfs This series fixed the broken ѕupport for dm-multipath devices in blkmapd, and adds some useful logging to syslog that allows triaging pnfs block layout setup issues. This work was sponsored by NetApp, Inc. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] blkmapd: fix broken multipath handling 2014-08-07 7:26 nfs-utils: blkmapd fixex Christoph Hellwig @ 2014-08-07 7:26 ` Christoph Hellwig 2014-08-13 14:53 ` Steve Dickson 2014-08-07 7:26 ` [PATCH 2/2] blkmapd: dump useful device information to syslog Christoph Hellwig 1 sibling, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2014-08-07 7:26 UTC (permalink / raw) To: linux-nfs We do want to use the dm-multipath device if it exists, which the code is generally prepared for, except that this check excludes them early. In addition this will also add the passive path to the device list, which is harmless if an active one exists as that or the multipath device will be preferred, and at least allows us to work if it doesn't. Also fix up the check if an path needs to be updated to remove the silly partition check - pNFS block offset are relative to the device so partion should never match it instead of the full device. On the other hand the simplistic check easily creates false positives, e.g. dm-10 is considered a partition of dm-1. Signed-off-by: Christoph Hellwig <hch@lst.de> --- utils/blkmapd/device-discovery.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c index df4627e..bcfb060 100644 --- a/utils/blkmapd/device-discovery.c +++ b/utils/blkmapd/device-discovery.c @@ -77,16 +77,6 @@ struct bl_disk_path *bl_get_path(const char *filepath, return tmp; } -/* Check whether valid_path is a substring(partition) of path */ -int bl_is_partition(struct bl_disk_path *valid_path, struct bl_disk_path *path) -{ - if (!strncmp(valid_path->full_path, path->full_path, - strlen(valid_path->full_path))) - return 1; - - return 0; -} - /* * For multipath devices, devices state could be PASSIVE/ACTIVE/PSEUDO, * where PSEUDO > ACTIVE > PASSIVE. Device with highest state is used to @@ -95,19 +85,13 @@ int bl_is_partition(struct bl_disk_path *valid_path, struct bl_disk_path *path) * If device-mapper multipath support is a must, pseudo devices should * exist for each multipath device. If not, active device path will be * chosen for device creation. - * Treat partition as invalid path. */ -int bl_update_path(struct bl_disk_path *path, enum bl_path_state_e state, - struct bl_disk *disk) +int bl_update_path(enum bl_path_state_e state, struct bl_disk *disk) { struct bl_disk_path *valid_path = disk->valid_path; - if (valid_path) { - if (valid_path->state >= state) { - if (bl_is_partition(valid_path, path)) - return 0; - } - } + if (valid_path && valid_path->state >= state) + return 0; return 1; } @@ -170,9 +154,6 @@ void bl_add_disk(char *filepath) ap_state = bldev_read_ap_state(fd); close(fd); - if (ap_state != BL_PATH_STATE_ACTIVE) - return; - for (disk = visible_disk_list; disk != NULL; disk = disk->next) { /* Already scanned or a partition? * XXX: if released each time, maybe not need to compare @@ -216,7 +197,7 @@ void bl_add_disk(char *filepath) path->next = disk->paths; disk->paths = path; /* check whether we need to update disk info */ - if (bl_update_path(path, path->state, disk)) { + if (bl_update_path(path->state, disk)) { disk->dev = dev; disk->size = size; disk->valid_path = path; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] blkmapd: fix broken multipath handling 2014-08-07 7:26 ` [PATCH 1/2] blkmapd: fix broken multipath handling Christoph Hellwig @ 2014-08-13 14:53 ` Steve Dickson 0 siblings, 0 replies; 5+ messages in thread From: Steve Dickson @ 2014-08-13 14:53 UTC (permalink / raw) To: Christoph Hellwig, linux-nfs On 08/07/2014 03:26 AM, Christoph Hellwig wrote: > We do want to use the dm-multipath device if it exists, which the > code is generally prepared for, except that this check excludes them > early. In addition this will also add the passive path to the device > list, which is harmless if an active one exists as that or the multipath > device will be preferred, and at least allows us to work if it doesn't. > > Also fix up the check if an path needs to be updated to remove the silly > partition check - pNFS block offset are relative to the device so partion > should never match it instead of the full device. On the other hand the > simplistic check easily creates false positives, e.g. dm-10 is considered > a partition of dm-1. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Committed... steved. > --- > utils/blkmapd/device-discovery.c | 27 ++++----------------------- > 1 file changed, 4 insertions(+), 23 deletions(-) > > diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c > index df4627e..bcfb060 100644 > --- a/utils/blkmapd/device-discovery.c > +++ b/utils/blkmapd/device-discovery.c > @@ -77,16 +77,6 @@ struct bl_disk_path *bl_get_path(const char *filepath, > return tmp; > } > > -/* Check whether valid_path is a substring(partition) of path */ > -int bl_is_partition(struct bl_disk_path *valid_path, struct bl_disk_path *path) > -{ > - if (!strncmp(valid_path->full_path, path->full_path, > - strlen(valid_path->full_path))) > - return 1; > - > - return 0; > -} > - > /* > * For multipath devices, devices state could be PASSIVE/ACTIVE/PSEUDO, > * where PSEUDO > ACTIVE > PASSIVE. Device with highest state is used to > @@ -95,19 +85,13 @@ int bl_is_partition(struct bl_disk_path *valid_path, struct bl_disk_path *path) > * If device-mapper multipath support is a must, pseudo devices should > * exist for each multipath device. If not, active device path will be > * chosen for device creation. > - * Treat partition as invalid path. > */ > -int bl_update_path(struct bl_disk_path *path, enum bl_path_state_e state, > - struct bl_disk *disk) > +int bl_update_path(enum bl_path_state_e state, struct bl_disk *disk) > { > struct bl_disk_path *valid_path = disk->valid_path; > > - if (valid_path) { > - if (valid_path->state >= state) { > - if (bl_is_partition(valid_path, path)) > - return 0; > - } > - } > + if (valid_path && valid_path->state >= state) > + return 0; > return 1; > } > > @@ -170,9 +154,6 @@ void bl_add_disk(char *filepath) > ap_state = bldev_read_ap_state(fd); > close(fd); > > - if (ap_state != BL_PATH_STATE_ACTIVE) > - return; > - > for (disk = visible_disk_list; disk != NULL; disk = disk->next) { > /* Already scanned or a partition? > * XXX: if released each time, maybe not need to compare > @@ -216,7 +197,7 @@ void bl_add_disk(char *filepath) > path->next = disk->paths; > disk->paths = path; > /* check whether we need to update disk info */ > - if (bl_update_path(path, path->state, disk)) { > + if (bl_update_path(path->state, disk)) { > disk->dev = dev; > disk->size = size; > disk->valid_path = path; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] blkmapd: dump useful device information to syslog 2014-08-07 7:26 nfs-utils: blkmapd fixex Christoph Hellwig 2014-08-07 7:26 ` [PATCH 1/2] blkmapd: fix broken multipath handling Christoph Hellwig @ 2014-08-07 7:26 ` Christoph Hellwig 2014-08-13 14:54 ` Steve Dickson 1 sibling, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2014-08-07 7:26 UTC (permalink / raw) To: linux-nfs Dump some information about used devices to syslog so that an admin can troubleshoot failing blocklayout mounts. Signed-off-by: Christoph Hellwig <hch@lst.de> --- utils/blkmapd/device-discovery.c | 6 +++++- utils/blkmapd/device-process.c | 2 ++ utils/blkmapd/dm-device.c | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c index bcfb060..b52afe2 100644 --- a/utils/blkmapd/device-discovery.c +++ b/utils/blkmapd/device-discovery.c @@ -148,7 +148,11 @@ void bl_add_disk(char *filepath) dev = sb.st_rdev; serial = bldev_read_serial(fd, filepath); - if (dm_is_dm_major(major(dev))) + if (!serial) { + BL_LOG_ERR("%s: no serial found for %s\n", + __func__, filepath); + ap_state = BL_PATH_STATE_PASSIVE; + } else if (dm_is_dm_major(major(dev))) ap_state = BL_PATH_STATE_PSEUDO; else ap_state = bldev_read_ap_state(fd); diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c index 5fe3dff..f53a616 100644 --- a/utils/blkmapd/device-process.c +++ b/utils/blkmapd/device-process.c @@ -181,6 +181,8 @@ static int map_sig_to_device(struct bl_sig *sig, struct bl_volume *vol) /* FIXME: should we use better algorithm for disk scan? */ mapped = verify_sig(disk, sig); if (mapped) { + BL_LOG_INFO("%s: using device %s\n", + __func__, disk->valid_path->full_path); vol->param.bv_dev = disk->dev; vol->bv_size = disk->size; break; diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c index 0f4f148..8ffb19e 100644 --- a/utils/blkmapd/dm-device.c +++ b/utils/blkmapd/dm-device.c @@ -400,6 +400,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols) } dev = node->bv_vols[0]->param.bv_dev; tmp = table->params; + BL_LOG_INFO("%s: major %u minor %u", __func__, + MAJOR(dev), MINOR(dev)); if (!dm_format_dev(tmp, DM_PARAMS_LEN, MAJOR(dev), MINOR(dev))) { free(table); @@ -459,6 +461,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols) strcpy(table->target_type, "linear"); tmp = table->params; dev = node->bv_vols[i]->param.bv_dev; + BL_LOG_INFO("%s: major %u minor %u", __func__, + MAJOR(dev), MINOR(dev)); if (!dm_format_dev(tmp, DM_PARAMS_LEN, MAJOR(dev), MINOR(dev))) { free(table); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] blkmapd: dump useful device information to syslog 2014-08-07 7:26 ` [PATCH 2/2] blkmapd: dump useful device information to syslog Christoph Hellwig @ 2014-08-13 14:54 ` Steve Dickson 0 siblings, 0 replies; 5+ messages in thread From: Steve Dickson @ 2014-08-13 14:54 UTC (permalink / raw) To: Christoph Hellwig, linux-nfs On 08/07/2014 03:26 AM, Christoph Hellwig wrote: > Dump some information about used devices to syslog so that an admin > can troubleshoot failing blocklayout mounts. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Committed... after fixing a couple format warnings... steved. > --- > utils/blkmapd/device-discovery.c | 6 +++++- > utils/blkmapd/device-process.c | 2 ++ > utils/blkmapd/dm-device.c | 4 ++++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c > index bcfb060..b52afe2 100644 > --- a/utils/blkmapd/device-discovery.c > +++ b/utils/blkmapd/device-discovery.c > @@ -148,7 +148,11 @@ void bl_add_disk(char *filepath) > > dev = sb.st_rdev; > serial = bldev_read_serial(fd, filepath); > - if (dm_is_dm_major(major(dev))) > + if (!serial) { > + BL_LOG_ERR("%s: no serial found for %s\n", > + __func__, filepath); > + ap_state = BL_PATH_STATE_PASSIVE; > + } else if (dm_is_dm_major(major(dev))) > ap_state = BL_PATH_STATE_PSEUDO; > else > ap_state = bldev_read_ap_state(fd); > diff --git a/utils/blkmapd/device-process.c b/utils/blkmapd/device-process.c > index 5fe3dff..f53a616 100644 > --- a/utils/blkmapd/device-process.c > +++ b/utils/blkmapd/device-process.c > @@ -181,6 +181,8 @@ static int map_sig_to_device(struct bl_sig *sig, struct bl_volume *vol) > /* FIXME: should we use better algorithm for disk scan? */ > mapped = verify_sig(disk, sig); > if (mapped) { > + BL_LOG_INFO("%s: using device %s\n", > + __func__, disk->valid_path->full_path); > vol->param.bv_dev = disk->dev; > vol->bv_size = disk->size; > break; > diff --git a/utils/blkmapd/dm-device.c b/utils/blkmapd/dm-device.c > index 0f4f148..8ffb19e 100644 > --- a/utils/blkmapd/dm-device.c > +++ b/utils/blkmapd/dm-device.c > @@ -400,6 +400,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols) > } > dev = node->bv_vols[0]->param.bv_dev; > tmp = table->params; > + BL_LOG_INFO("%s: major %u minor %u", __func__, > + MAJOR(dev), MINOR(dev)); > if (!dm_format_dev(tmp, DM_PARAMS_LEN, > MAJOR(dev), MINOR(dev))) { > free(table); > @@ -459,6 +461,8 @@ uint64_t dm_device_create(struct bl_volume *vols, int num_vols) > strcpy(table->target_type, "linear"); > tmp = table->params; > dev = node->bv_vols[i]->param.bv_dev; > + BL_LOG_INFO("%s: major %u minor %u", __func__, > + MAJOR(dev), MINOR(dev)); > if (!dm_format_dev(tmp, DM_PARAMS_LEN, > MAJOR(dev), MINOR(dev))) { > free(table); > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-13 14:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-07 7:26 nfs-utils: blkmapd fixex Christoph Hellwig 2014-08-07 7:26 ` [PATCH 1/2] blkmapd: fix broken multipath handling Christoph Hellwig 2014-08-13 14:53 ` Steve Dickson 2014-08-07 7:26 ` [PATCH 2/2] blkmapd: dump useful device information to syslog Christoph Hellwig 2014-08-13 14:54 ` Steve Dickson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).