From patchwork Tue Jul 16 20:53:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734914 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7EFC355887 for ; Tue, 16 Jul 2024 20:54:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.50 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163246; cv=none; b=QeOOQ84EcNwfQrXDtfkztKAZqZmQyslFC/03P2DwLzzxXymwmeQTWxaNOaye+3IRuyWsVNGeU6krUzujpNG0IE9NzyOXvnb3xvZ7EG7eihWTsNpex7VZeDN3l8d2CKAqeZotCQ1mxQqSmojfHoOnzNRwj0ezWozaDmvzgbg2to4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163246; c=relaxed/simple; bh=5RO51V7xJzwU0XhIudh0K4jZQ6UWQ7JJb+c7bU761to=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=IIzl/Z51wwCNCWJIFvme1toeKhmcWTvN6pVMj3e7pveJeHZZxU38eHXvTbkTUfi9NqxcAOptZlN+L8c1TcEXWLbyy21vIKK32DxP/Dox1rQN2IBenYjnrCWAhxm55Vm415iN5Y6ngku10/2afgXJw2cjbe72e3pH+pza56pr+1I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=L0Sd9Ooq; arc=none smtp.client-ip=209.85.218.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="L0Sd9Ooq" Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a6fd513f18bso634408666b.3 for ; Tue, 16 Jul 2024 13:54:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163242; x=1721768042; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=IlnZZgouYKqeRlDFe32qKe2eoGq+s4uUcCTh60/bbVA=; b=L0Sd9OoqIjyFgF8BNV8Rop7QHhsTh8mdj8wdiSn/cOSaV9/a+kCfsbHJsUHv/yiiRJ 5C1uMS8FogxgfXthKF7/gRRTFQANuVaTmmpTGLFIxVyqIZ+NdxK/pbUrQ/A11B6f4zEk 85hJvQBBCeTN5Nn6JGyypuriteAp3vhjopUNGfAAmOVIoarErtqaN1zP4E4FNq9e5lwu wNPfXjU6+Qzs1PnQRxyd5eWR83VEmHRQdpbLVuL1zBNf9P6Bi9Aql791o1JkFUTE+1Qu 1PKhcwtijbTaAExHfuzNed6aEX+tSV2azvaSpO+GCMQGaQWtO/LdvA9ie0/gYRpQSB0D JiaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163242; x=1721768042; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=IlnZZgouYKqeRlDFe32qKe2eoGq+s4uUcCTh60/bbVA=; b=K1N/D7BOtUETKnlhmKjp5r9uuCnIo+uUuoEloAlvzee0lgpa6pRvQWxXSrHbaPsi7C Rfn3A7f6M6X9mIDyWUJ1yMaWBaujlVSxj+kr4FFZU8R+jYUfZzO8HxlHLBONGIcdz5Nh Rg+/Eohwh0pe8Qa+/B9UV4rPaFlkfGaOyUj9Z6MLWzLG9z9jSREfahziDV43A77py2d/ p1n5N4CrFVUEJ33O9lLK9EqeQ+mvNgY1xAMt3xWDcUpmjZgRRXgId1W2Nod4HMvr627i upyKk7yJL4meQ9z7aYhcxsXxzRXVVCHDIU8VTps1EMMoLZDwCGGt3Jw9rYwLH+BPfWwa Ca8g== X-Gm-Message-State: AOJu0YzHSsIVwR9EGBGi4Fw5qCkoYnR9xYh29aXWURfaw0mp/wIkAz3e xSRL0RV9MRnFmUYpx78hYwtGnGphhZ7l0pxr8sXZ3gk7VUo9UAAf0DLXdbuS1Es= X-Google-Smtp-Source: AGHT+IF4WxmWTk5nOH1khOrICIu7Sg/reionf2BB949BsXheHAv8jmetSY1d0vVHWpeRB4U8JPnuGA== X-Received: by 2002:a17:906:e087:b0:a77:e0ed:8bc with SMTP id a640c23a62f3a-a79ea43a6f6mr224343966b.4.1721163241526; Tue, 16 Jul 2024 13:54:01 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id a640c23a62f3a-a79bc821650sm358454466b.195.2024.07.16.13.54.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:01 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 16/49] libmultipath: add cleanup_dm_task(), and use it in devmapper.c Date: Tue, 16 Jul 2024 22:53:39 +0200 Message-ID: <20240716205344.146310-2-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 This allows us to get rid of a lot of goto statements, and generally obtain cleaner code. Signed-off-by: Martin Wilck --- libmultipath/devmapper.c | 261 ++++++++++++++++----------------------- 1 file changed, 108 insertions(+), 153 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 8996c1d..7dac9fa 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -91,6 +91,12 @@ int libmp_dm_task_run(struct dm_task *dmt) return r; } +static void cleanup_dm_task(struct dm_task **pdmt) +{ + if (*pdmt) + dm_task_destroy(*pdmt); +} + __attribute__((format(printf, 4, 5))) static void dm_write_log (int level, const char *file, int line, const char *f, ...) { @@ -203,8 +209,8 @@ static void init_dm_drv_version(void) static int dm_tgt_version (unsigned int *version, char *str) { - int r = 2; - struct dm_task *dmt; + bool found = false; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct dm_versions *target; struct dm_versions *last_target; unsigned int *v; @@ -220,31 +226,28 @@ static int dm_tgt_version (unsigned int *version, char *str) if (!libmp_dm_task_run(dmt)) { dm_log_error(2, DM_DEVICE_LIST_VERSIONS, dmt); condlog(0, "Cannot communicate with kernel DM"); - goto out; + return 1; } target = dm_task_get_versions(dmt); do { last_target = target; if (!strncmp(str, target->name, strlen(str))) { - r = 1; + found = true; break; } target = (void *) target + target->next; } while (last_target != target); - if (r == 2) { + if (!found) { condlog(0, "DM %s kernel driver not loaded", str); - goto out; + return 1; } v = target->version; version[0] = v[0]; version[1] = v[1]; version[2] = v[2]; - r = 0; -out: - dm_task_destroy(dmt); - return r; + return 0; } static void init_dm_mpath_version(void) @@ -383,18 +386,18 @@ libmp_dm_task_create(int task) static int dm_simplecmd (int task, const char *name, int flags, uint16_t udev_flags) { - int r = 0; + int r; int udev_wait_flag = (((flags & DMFL_NEED_SYNC) || udev_flags) && (task == DM_DEVICE_RESUME || task == DM_DEVICE_REMOVE)); uint32_t cookie = 0; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; if (!(dmt = libmp_dm_task_create (task))) return 0; if (!dm_task_set_name (dmt, name)) - goto out; + return 0; dm_task_skip_lockfs(dmt); /* for DM_DEVICE_RESUME */ #ifdef LIBDM_API_FLUSH @@ -408,7 +411,7 @@ dm_simplecmd (int task, const char *name, int flags, uint16_t udev_flags) { if (udev_wait_flag && !dm_task_set_cookie(dmt, &cookie, DM_UDEV_DISABLE_LIBRARY_FALLBACK | udev_flags)) - goto out; + return 0; r = libmp_dm_task_run (dmt); if (!r) @@ -416,8 +419,6 @@ dm_simplecmd (int task, const char *name, int flags, uint16_t udev_flags) { if (udev_wait_flag) libmp_udev_wait(cookie); -out: - dm_task_destroy (dmt); return r; } @@ -440,8 +441,9 @@ static int dm_addmap (int task, const char *target, struct multipath *mpp, char * params, int ro, uint16_t udev_flags) { int r = 0; - struct dm_task *dmt; - char *prefixed_uuid = NULL; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; + char __attribute__((cleanup(cleanup_charp))) *prefixed_uuid = NULL; + uint32_t cookie = 0; if (task == DM_DEVICE_CREATE && strlen(mpp->wwid) == 0) { @@ -457,10 +459,10 @@ dm_addmap (int task, const char *target, struct multipath *mpp, return 0; if (!dm_task_set_name (dmt, mpp->alias)) - goto addout; + return 0; if (!dm_task_add_target (dmt, 0, mpp->size, target, params)) - goto addout; + return 0; if (ro) dm_task_set_ro(dmt); @@ -469,10 +471,10 @@ dm_addmap (int task, const char *target, struct multipath *mpp, if (asprintf(&prefixed_uuid, UUID_PREFIX "%s", mpp->wwid) < 0) { condlog(0, "cannot create prefixed uuid : %s", strerror(errno)); - goto addout; + return 0; } if (!dm_task_set_uuid(dmt, prefixed_uuid)) - goto freeout; + return 0; dm_task_skip_lockfs(dmt); #ifdef LIBDM_API_FLUSH dm_task_no_flush(dmt); @@ -481,33 +483,28 @@ dm_addmap (int task, const char *target, struct multipath *mpp, if (mpp->attribute_flags & (1 << ATTR_MODE) && !dm_task_set_mode(dmt, mpp->mode)) - goto freeout; + return 0; if (mpp->attribute_flags & (1 << ATTR_UID) && !dm_task_set_uid(dmt, mpp->uid)) - goto freeout; + return 0; if (mpp->attribute_flags & (1 << ATTR_GID) && !dm_task_set_gid(dmt, mpp->gid)) - goto freeout; + return 0; + condlog(2, "%s: %s [0 %llu %s %s]", mpp->alias, task == DM_DEVICE_RELOAD ? "reload" : "addmap", mpp->size, target, params); if (task == DM_DEVICE_CREATE && !dm_task_set_cookie(dmt, &cookie, udev_flags)) - goto freeout; + return 0; r = libmp_dm_task_run (dmt); if (!r) dm_log_error(2, task, dmt); if (task == DM_DEVICE_CREATE) - libmp_udev_wait(cookie); -freeout: - if (prefixed_uuid) - free(prefixed_uuid); - -addout: - dm_task_destroy (dmt); + libmp_udev_wait(cookie); if (r) mpp->need_reload = false; @@ -648,46 +645,41 @@ int dm_map_present(const char * str) int dm_get_map(const char *name, unsigned long long *size, char **outparams) { - int r = DMP_ERR; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; uint64_t start, length; char *target_type = NULL; char *params = NULL; if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE))) - return r; + return DMP_ERR; if (!dm_task_set_name(dmt, name)) - goto out; + return DMP_ERR; errno = 0; if (!libmp_dm_task_run(dmt)) { dm_log_error(3, DM_DEVICE_TABLE, dmt); if (dm_task_get_errno(dmt) == ENXIO) - r = DMP_NOT_FOUND; - goto out; + return DMP_NOT_FOUND; + else + return DMP_ERR; } - r = DMP_NOT_FOUND; /* Fetch 1st target */ if (dm_get_next_target(dmt, NULL, &start, &length, &target_type, ¶ms) != NULL || !params) /* more than one target or not found target */ - goto out; + return DMP_NOT_FOUND; if (size) *size = length; if (!outparams) - r = DMP_OK; + return DMP_OK; else { *outparams = strdup(params); - r = *outparams ? DMP_OK : DMP_ERR; + return *outparams ? DMP_OK : DMP_ERR; } - -out: - dm_task_destroy(dmt); - return r; } static int @@ -767,7 +759,7 @@ is_mpath_part(const char *part_name, const char *map_name) int dm_get_status(const char *name, char **outstatus) { int r = DMP_ERR; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; uint64_t start, length; char *target_type = NULL; char *status = NULL; @@ -796,7 +788,7 @@ int dm_get_status(const char *name, char **outstatus) goto out; if (!status) { - condlog(2, "get null status."); + condlog(2, "got null status."); goto out; } @@ -808,9 +800,9 @@ int dm_get_status(const char *name, char **outstatus) } out: if (r != DMP_OK) - condlog(0, "%s: error getting map status string", name); + condlog(0, "%s: %s: error getting map status string: %d", + __func__, name, r); - dm_task_destroy(dmt); return r; } @@ -823,7 +815,7 @@ out: int dm_type(const char *name, char *type) { int r = 0; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; uint64_t start, length; char *target_type = NULL; char *params; @@ -850,7 +842,6 @@ int dm_type(const char *name, char *type) r = 1; out: - dm_task_destroy(dmt); return r; } @@ -863,7 +854,7 @@ out: int dm_is_mpath(const char *name) { int r = -1; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct dm_info info; uint64_t start, length; char *target_type = NULL; @@ -874,38 +865,36 @@ int dm_is_mpath(const char *name) goto out; if (!dm_task_set_name(dmt, name)) - goto out_task; + goto out; if (!libmp_dm_task_run(dmt)) { dm_log_error(3, DM_DEVICE_TABLE, dmt); - goto out_task; + goto out; } if (!dm_task_get_info(dmt, &info)) - goto out_task; + goto out; r = 0; if (!info.exists) - goto out_task; + goto out; uuid = dm_task_get_uuid(dmt); if (!uuid || strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN) != 0) - goto out_task; + goto out; /* Fetch 1st target */ if (dm_get_next_target(dmt, NULL, &start, &length, &target_type, ¶ms) != NULL) /* multiple targets */ - goto out_task; + goto out; if (!target_type || strcmp(target_type, TGT_MPATH) != 0) - goto out_task; + goto out; r = 1; -out_task: - dm_task_destroy(dmt); out: if (r < 0) condlog(3, "%s: dm command failed in %s: %s", name, __FUNCTION__, strerror(errno)); @@ -1131,10 +1120,10 @@ dm_flush_map_nopaths(const char *mapname, int deferred_remove __DR_UNUSED__) return _dm_flush_map(mapname, flags, 0); } -int dm_flush_maps (int retries) +int dm_flush_maps(int retries) { int r = DM_FLUSH_FAIL; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct dm_names *names; unsigned next = 0; @@ -1143,15 +1132,15 @@ int dm_flush_maps (int retries) if (!libmp_dm_task_run (dmt)) { dm_log_error(3, DM_DEVICE_LIST, dmt); - goto out; + return r; } if (!(names = dm_task_get_names (dmt))) - goto out; + return r; r = DM_FLUSH_OK; if (!names->dev) - goto out; + return r; do { int ret; @@ -1163,16 +1152,13 @@ int dm_flush_maps (int retries) names = (void *) names + next; } while (next); -out: - dm_task_destroy (dmt); return r; } int dm_message(const char * mapname, char * message) { - int r = 1; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; if (!(dmt = libmp_dm_task_create(DM_DEVICE_TARGET_MSG))) return 1; @@ -1191,13 +1177,10 @@ dm_message(const char * mapname, char * message) goto out; } - r = 0; + return 0; out: - if (r) - condlog(0, "DM message failed [%s]", message); - - dm_task_destroy(dmt); - return r; + condlog(0, "DM message failed [%s]", message); + return 1; } int @@ -1305,12 +1288,10 @@ out: return NULL; } -int -dm_get_maps (vector mp) +int dm_get_maps(vector mp) { struct multipath * mpp; - int r = 1; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct dm_names *names; unsigned next = 0; @@ -1322,15 +1303,15 @@ dm_get_maps (vector mp) if (!libmp_dm_task_run(dmt)) { dm_log_error(3, DM_DEVICE_LIST, dmt); - goto out; + return 1; } if (!(names = dm_task_get_names(dmt))) - goto out; + return 1; if (!names->dev) { - r = 0; /* this is perfectly valid */ - goto out; + /* this is perfectly valid */ + return 0; } do { @@ -1339,11 +1320,11 @@ dm_get_maps (vector mp) mpp = dm_get_multipath(names->name); if (!mpp) - goto out; + return 1; if (!vector_alloc_slot(mp)) { free_multipath(mpp, KEEP_PATHS); - goto out; + return 1; } vector_set_slot(mp, mpp); @@ -1352,11 +1333,7 @@ next: names = (void *) names + next; } while (next); - r = 0; - goto out; -out: - dm_task_destroy (dmt); - return r; + return 0; } int @@ -1419,33 +1396,29 @@ do_foreach_partmaps (const char * mapname, int (*partmap_func)(const char *, void *), void *data) { - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; + char __attribute__((cleanup(cleanup_charp))) *params = NULL; struct dm_names *names; unsigned next = 0; - char *params = NULL; unsigned long long size; char dev_t[32]; - int r = 1; char *p; if (!(dmt = libmp_dm_task_create(DM_DEVICE_LIST))) return 1; - if (!libmp_dm_task_run(dmt)) { - dm_log_error(3, DM_DEVICE_LIST, dmt); - goto out; - } + if (!libmp_dm_task_run(dmt)) + return 1; if (!(names = dm_task_get_names(dmt))) - goto out; + return 1; - if (!names->dev) { - r = 0; /* this is perfectly valid */ - goto out; - } + if (!names->dev) + /* this is perfectly valid */ + return 0; if (dm_dev_t(mapname, &dev_t[0], 32)) - goto out; + return 1; do { if ( @@ -1471,8 +1444,8 @@ do_foreach_partmaps (const char * mapname, (p = strstr(params, dev_t)) && !isdigit(*(p + strlen(dev_t))) ) { - if ((r = partmap_func(names->name, data)) != 0) - goto out; + if (partmap_func(names->name, data) != 0) + return 1; } free(params); @@ -1481,11 +1454,7 @@ do_foreach_partmaps (const char * mapname, names = (void *) names + next; } while (next); - r = 0; -out: - free(params); - dm_task_destroy (dmt); - return r; + return 0; } struct remove_data { @@ -1623,7 +1592,7 @@ int dm_rename (const char * old, char * new, char *delim, int skip_kpartx) { int r = 0; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; uint32_t cookie = 0; uint16_t udev_flags = DM_UDEV_DISABLE_LIBRARY_FALLBACK | ((skip_kpartx == SKIP_KPARTX_ON)? MPATH_UDEV_NO_KPARTX_FLAG : 0); @@ -1634,22 +1603,19 @@ dm_rename (const char * old, char * new, char *delim, int skip_kpartx) return r; if (!dm_task_set_name(dmt, old)) - goto out; + return r; if (!dm_task_set_newname(dmt, new)) - goto out; + return r; if (!dm_task_set_cookie(dmt, &cookie, udev_flags)) - goto out; + return r; + r = libmp_dm_task_run(dmt); if (!r) dm_log_error(2, DM_DEVICE_RENAME, dmt); libmp_udev_wait(cookie); - -out: - dm_task_destroy(dmt); - return r; } @@ -1672,9 +1638,10 @@ void dm_reassign_deps(char *table, const char *dep, const char *newdep) int dm_reassign_table(const char *name, char *old, char *new) { - int r = 0, modified = 0; + int modified = 0; uint64_t start, length; - struct dm_task *dmt, *reload_dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *reload_dmt = NULL; char *target, *params = NULL; char *buff; void *next = NULL; @@ -1683,16 +1650,16 @@ int dm_reassign_table(const char *name, char *old, char *new) return 0; if (!dm_task_set_name(dmt, name)) - goto out; + return 0; if (!libmp_dm_task_run(dmt)) { dm_log_error(3, DM_DEVICE_TABLE, dmt); - goto out; + return 0; } if (!(reload_dmt = libmp_dm_task_create(DM_DEVICE_RELOAD))) - goto out; + return 0; if (!dm_task_set_name(reload_dmt, name)) - goto out_reload; + return 0; do { next = dm_get_next_target(dmt, next, &start, &length, @@ -1705,13 +1672,13 @@ int dm_reassign_table(const char *name, char *old, char *new) */ condlog(1, "%s: invalid target found in map %s", __func__, name); - goto out_reload; + return 0; } buff = strdup(params); if (!buff) { condlog(3, "%s: failed to replace target %s, " "out of memory", name, target); - goto out_reload; + return 0; } if (strcmp(target, TGT_MPATH) && strstr(params, old)) { condlog(3, "%s: replace target %s %s", @@ -1729,18 +1696,12 @@ int dm_reassign_table(const char *name, char *old, char *new) if (!libmp_dm_task_run(reload_dmt)) { dm_log_error(3, DM_DEVICE_RELOAD, reload_dmt); condlog(3, "%s: failed to reassign targets", name); - goto out_reload; + return 0; } dm_simplecmd_noflush(DM_DEVICE_RESUME, name, MPATH_UDEV_RELOAD_FLAG); } - r = 1; - -out_reload: - dm_task_destroy(reload_dmt); -out: - dm_task_destroy(dmt); - return r; + return 1; } @@ -1752,10 +1713,9 @@ out: int dm_reassign(const char *mapname) { struct dm_deps *deps; - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct dm_info info; char dev_t[32], dm_dep[32]; - int r = 0; unsigned int i; if (dm_dev_t(mapname, &dev_t[0], 32)) { @@ -1769,21 +1729,21 @@ int dm_reassign(const char *mapname) } if (!dm_task_set_name(dmt, mapname)) - goto out; + return 0; if (!libmp_dm_task_run(dmt)) { dm_log_error(3, DM_DEVICE_DEPS, dmt); - goto out; + return 0; } if (!dm_task_get_info(dmt, &info)) - goto out; + return 0; if (!(deps = dm_task_get_deps(dmt))) - goto out; + return 0; if (!info.exists) - goto out; + return 0; for (i = 0; i < deps->count; i++) { sprintf(dm_dep, "%d:%d", @@ -1792,15 +1752,12 @@ int dm_reassign(const char *mapname) sysfs_check_holders(dm_dep, dev_t); } - r = 1; -out: - dm_task_destroy (dmt); - return r; + return 1; } int dm_setgeometry(struct multipath *mpp) { - struct dm_task *dmt; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; struct path *pp; char heads[4], sectors[4]; char cylinders[10], start[32]; @@ -1825,7 +1782,7 @@ int dm_setgeometry(struct multipath *mpp) return 0; if (!dm_task_set_name(dmt, mpp->alias)) - goto out; + return 0; /* What a sick interface ... */ snprintf(heads, 4, "%u", pp->geom.heads); @@ -1834,14 +1791,12 @@ int dm_setgeometry(struct multipath *mpp) snprintf(start, 32, "%lu", pp->geom.start); if (!dm_task_set_geometry(dmt, cylinders, heads, sectors, start)) { condlog(3, "%s: Failed to set geometry", mpp->alias); - goto out; + return 0; } r = libmp_dm_task_run(dmt); if (!r) dm_log_error(3, DM_DEVICE_SET_GEOMETRY, dmt); -out: - dm_task_destroy(dmt); return r; } From patchwork Tue Jul 16 20:53:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734915 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7C29F54F95 for ; Tue, 16 Jul 2024 20:54:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163247; cv=none; b=VXLyfnYfSS7u4GF7WHQoA0r846zzTNo2u6s8vl3pfeKeMxjVC1yK7QhVbiIVKWNhlYW2DJ8e5IcLL5TsGZg6Fbo94a0IBo/GZrvuX3uhsja4loquJR+KOED6Y/JoxzNDYiy8mX76frttpd/yNxaVKVuI3Q+sgRjMg+sTUJKcOSM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163247; c=relaxed/simple; bh=25aTLBSwx45Y2cW6TD/Eca/s3A1eufYBGFvwt3cI56M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=FDlys19j+JCSTtUrXpPBqKBzFKwrvTGlc7aHAN736e6QRFZgcQdf6S9AK5m1IDORtju9lmO+WEKUcOxLxXqkKLx1IspXgxcPuMKyaeyfWmyAph/aUUdRJOA66VyUFKfbiLOyW4WtwLlV2LERT++zeVMG4YuWb7T8OPB+8ECZ3Uc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=SxatAP2L; arc=none smtp.client-ip=209.85.218.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="SxatAP2L" Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-a797c62565aso632191666b.2 for ; Tue, 16 Jul 2024 13:54:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163243; x=1721768043; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=iHfDgT4SGfrlVm8O8aF0oJ2gK3X/pDDi8cs2q4aF/Ew=; b=SxatAP2LYKCRH6FQV+MGr35Gm6PABpnJY244RFFlKdio4s7moCbbqGuDYv3hXTNoGO p9TVy8TqncBo9dXk0HpXbSdnaLY+EgXY+nIdB4YulQavSpuxTN78BkwTgWCCalWUXl4U WrZATQSdS9EWtEvRlNWQjl9cUthfGNDFifSsfA+hVPEtfVagvnc9U6Am8CdiH59FkPzI 0nOlPY5XSRliZ59JuB6xhxUihADo03d2vSzYJBwFbUuzQA1gNo9SDihe3urqHpOcLLtD XvuwBAsHBfV8jqs9N+qTTI5ML+zjDu1iojUIGumPYIAI7TkJPXbVC00RQwdLeepbRRtW K3Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163243; x=1721768043; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=iHfDgT4SGfrlVm8O8aF0oJ2gK3X/pDDi8cs2q4aF/Ew=; b=eeE7wI2oEmBsmYoQcTVyJ8yPO7x4oLYiEcUsiQt0n8fBuPj6IHkb55UIJ4v7HXCj72 39wVgDKdigBTq7Ig7c3MM9OW1EUTLI5p9QhKKmHwhbz4ocdMAygvg3gowYFJlvbAObdC uTQG2qVK4Ee+Ehtq4VvlRaEIYE00FWPLKkcUetpeBpJTACFqLI+XuVZTlpBXDioCpY9e RDlsAMfv6cpLlaZkhbnDIh5GaAWcb0nzrAMA5KqJx/z9aJY44H7sScyuoO0sT8K21vvP 73FLBp3qqOJ4FCXdJ4nG6YtZhrK0Vu8LOssJ7bD3HqMaCdttdW6lDC/aauVNB4ch4wRQ erJQ== X-Gm-Message-State: AOJu0Yx/j9JO2GDED1u+ZTVm6qXuRD/EPpOfViyKq4nzewJ5TQqsfaTZ vcreH5gVSd+sCyHpbc6SOVD3wWjRhni1XrIO4NNJkOTGpoVe3xfkZFvmL8B0/oA= X-Google-Smtp-Source: AGHT+IEgsGAvQCVnUXsZxr0HARjwdXisHHjlQ+vcK+CPcbAmqFHnUVJFQ+k6TjATmlYBrqdzZhT3gQ== X-Received: by 2002:a17:906:6819:b0:a6e:d339:c095 with SMTP id a640c23a62f3a-a79eaa71843mr219668866b.47.1721163242593; Tue, 16 Jul 2024 13:54:02 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id a640c23a62f3a-a79bc80101fsm367345066b.179.2024.07.16.13.54.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:02 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 19/49] libmultipath: add libmp_mapinfo() Date: Tue, 16 Jul 2024 22:53:40 +0200 Message-ID: <20240716205344.146310-3-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 libmp_mapinfo() is intended as a generic abstraction for retrieving information from the kernel device-mapper driver. It retrieves the information that the caller needs, with a minimal set of DM ioctls, and never more then 2 ioctl calls. libdm's DM_DEVICE_TABLE and DM_DEVICE_STATUS calls map to the kernel's DM_TABLE_STATUS ioctl, with or without the DM_STATUS_TABLE_FLAG set, respectively. DM_TABLE_STATUS always retrieves the basic map status (struct dm_info) and the map UUID and name, too. Note: I'd prefer to use an unnamed struct instead of _u in union libmp_map_identifer. But doing using an unnamed struct and and initializing the union like this in a function argument: func((mapid_t) { .major = major, .minor = minor }) is not part of C99, and not supported in gcc 4.8, which we still support. Likewise, the following syntax for initializing an empty struct: (mapinfo_t) { 0 } is not supported on all architectures we support (notably clang 3.5 under Debian Jessie). Signed-off-by: Martin Wilck --- libmultipath/devmapper.c | 194 +++++++++++++++++++++++++++++- libmultipath/devmapper.h | 70 +++++++++++ libmultipath/libmultipath.version | 3 +- 3 files changed, 265 insertions(+), 2 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 5492a86..f90a8df 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -14,7 +14,6 @@ #include #include #include -#include #include "util.h" #include "vector.h" @@ -604,6 +603,199 @@ has_dm_info(const struct multipath *mpp) return (mpp && mpp->dmi.exists != 0); } +static int libmp_set_map_identifier(int flags, mapid_t id, struct dm_task *dmt) +{ + switch (flags & __DM_MAP_BY_MASK) { + case DM_MAP_BY_UUID: + if (!id.str || !(*id.str)) + return 0; + return dm_task_set_uuid(dmt, id.str); + case DM_MAP_BY_NAME: + if (!id.str || !(*id.str)) + return 0; + return dm_task_set_name(dmt, id.str); + case DM_MAP_BY_DEV: + if (!dm_task_set_major(dmt, id._u.major)) + return 0; + return dm_task_set_minor(dmt, id._u.minor); + case DM_MAP_BY_DEVT: + if (!dm_task_set_major(dmt, major(id.devt))) + return 0; + return dm_task_set_minor(dmt, minor(id.devt)); + default: + condlog(0, "%s: invalid by_id", __func__); + return 0; + } +} + +static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *map_id) +{ + /* avoid libmp_mapinfo__ in log messages */ + static const char fname__[] = "libmp_mapinfo"; + struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; + struct dm_info dmi; + int rc, ioctl_nr; + uint64_t start, length = 0; + char *target_type = NULL, *params = NULL; + const char *name = NULL, *uuid = NULL; + char __attribute__((cleanup(cleanup_charp))) *tmp_target = NULL; + char __attribute__((cleanup(cleanup_charp))) *tmp_status = NULL; + bool tgt_set = false; + + /* + * If both info.target and info.status are set, we need two + * ioctls. Call this function recursively. + * If successful, tmp_target will be non-NULL. + */ + if (info.target && info.status) { + rc = libmp_mapinfo__(flags, id, + (mapinfo_t) { .target = &tmp_target }, + map_id); + if (rc != DMP_OK) + return rc; + tgt_set = true; + } + + /* + * The DM_DEVICE_TABLE and DM_DEVICE_STATUS ioctls both fetch the basic + * information from DM_DEVICE_INFO, too. + * Choose the most lightweight ioctl to fetch all requested info. + */ + if (info.target && !info.status) + ioctl_nr = DM_DEVICE_TABLE; + else if (info.status || info.size || flags & __MAPINFO_TGT_TYPE) + ioctl_nr = DM_DEVICE_STATUS; + else + ioctl_nr = DM_DEVICE_INFO; + + if (!(dmt = libmp_dm_task_create(ioctl_nr))) + return DMP_ERR; + + if (!libmp_set_map_identifier(flags, id, dmt)) { + condlog(2, "%s: failed to set map identifier to %s", fname__, map_id); + return DMP_ERR; + } + + if (!libmp_dm_task_run(dmt)) { + dm_log_error(3, ioctl_nr, dmt); + if (dm_task_get_errno(dmt) == ENXIO) { + condlog(2, "%s: map %s not found", fname__, map_id); + return DMP_NOT_FOUND; + } else + return DMP_ERR; + } + + condlog(4, "%s: DM ioctl %d succeeded for %s", + fname__, ioctl_nr, map_id); + + if (!dm_task_get_info(dmt, &dmi)) { + condlog(2, "%s: dm_task_get_info() failed for %s ", fname__, map_id); + return DMP_ERR; + } else if(!dmi.exists) { + condlog(2, "%s: map %s doesn't exist", fname__, map_id); + return DMP_NOT_FOUND; + } + + if (info.target || info.status || info.size || flags & __MAPINFO_TGT_TYPE) { + if (dm_get_next_target(dmt, NULL, &start, &length, + &target_type, ¶ms) != NULL) { + condlog(2, "%s: map %s has multiple targets", fname__, map_id); + return DMP_NOT_FOUND; + } + if (!params) { + condlog(2, "%s: map %s has no targets", fname__, map_id); + return DMP_NOT_FOUND; + } + if (flags & __MAPINFO_TGT_TYPE) { + const char *tgt_type = flags & MAPINFO_MPATH_ONLY ? TGT_MPATH : TGT_PART; + + if (strcmp(target_type, tgt_type)) { + condlog(3, "%s: target type mismatch: \"%s\" != \"%s\"", + fname__, tgt_type, target_type); + return DMP_NO_MATCH; + } + } + } + + /* + * Check possible error conditions. + * If error is returned, don't touch any output parameters. + */ + if ((info.name && !(name = dm_task_get_name(dmt))) + || (info.uuid && !(uuid = dm_task_get_uuid(dmt))) + || (info.status && !(tmp_status = strdup(params))) + || (info.target && !tmp_target && !(tmp_target = strdup(params)))) + return DMP_ERR; + + if (info.name) { + strlcpy(info.name, name, WWID_SIZE); + condlog(4, "%s: %s: name: \"%s\"", fname__, map_id, info.name); + } + if (info.uuid) { + strlcpy(info.uuid, uuid, DM_UUID_LEN); + condlog(4, "%s: %s: uuid: \"%s\"", fname__, map_id, info.uuid); + } + + if (info.size) { + *info.size = length; + condlog(4, "%s: %s: size: %lld", fname__, map_id, *info.size); + } + + if (info.dmi) { + memcpy(info.dmi, &dmi, sizeof(*info.dmi)); + condlog(4, "%s: %s %d:%d, %d targets, %s table, %s, %s, %d opened, %u events", + fname__, map_id, + info.dmi->major, info.dmi->minor, + info.dmi->target_count, + info.dmi->live_table ? "live" : + info.dmi->inactive_table ? "inactive" : "no", + info.dmi->suspended ? "suspended" : "active", + info.dmi->read_only ? "ro" : "rw", + info.dmi->open_count, + info.dmi->event_nr); + } + + if (info.target) { + *info.target = steal_ptr(tmp_target); + if (!tgt_set) + condlog(4, "%s: %s: target: \"%s\"", fname__, map_id, *info.target); + } + + if (info.status) { + *info.status = steal_ptr(tmp_status); + condlog(4, "%s: %s: status: \"%s\"", fname__, map_id, *info.status); + } + + return DMP_OK; +} + +/* Helper: format a string describing the map for log messages */ +static const char* libmp_map_identifier(int flags, mapid_t id, char buf[BLK_DEV_SIZE]) +{ + switch (flags & __DM_MAP_BY_MASK) { + case DM_MAP_BY_NAME: + case DM_MAP_BY_UUID: + return id.str; + case DM_MAP_BY_DEV: + safe_snprintf(buf, BLK_DEV_SIZE, "%d:%d", id._u.major, id._u.minor); + return buf; + case DM_MAP_BY_DEVT: + safe_snprintf(buf, BLK_DEV_SIZE, "%d:%d", major(id.devt), minor(id.devt)); + return buf; + default: + safe_snprintf(buf, BLK_DEV_SIZE, "*invalid*"); + return buf; + } +} + +int libmp_mapinfo(int flags, mapid_t id, mapinfo_t info) +{ + char idbuf[BLK_DEV_SIZE]; + + return libmp_mapinfo__(flags, id, info, + libmp_map_identifier(flags, id, idbuf)); +} + int dm_get_info(const char *name, struct dm_info *info) { diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 9438c2d..4ccaaa7 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -1,5 +1,6 @@ #ifndef _DEVMAPPER_H #define _DEVMAPPER_H +#include #include "autoconfig.h" #include "structs.h" @@ -31,8 +32,77 @@ enum { DMP_ERR, DMP_OK, DMP_NOT_FOUND, + DMP_NO_MATCH, }; +/** + * enum mapinfo_flags: input flags for libmp_mapinfo() + */ +enum __mapinfo_flags { + /** DM_MAP_BY_NAME: identify map by device-mapper name from @name */ + DM_MAP_BY_NAME = 0, + /** DM_MAP_BY_UUID: identify map by device-mapper UUID from @uuid */ + DM_MAP_BY_UUID, + /** DM_MAP_BY_DEV: identify map by major/minor number from @dmi */ + DM_MAP_BY_DEV, + /** DM_MAP_BY_DEVT: identify map by a dev_t */ + DM_MAP_BY_DEVT, + __DM_MAP_BY_MASK = (1 << 8) - 1, + /* Fail if target type is not multipath */ + MAPINFO_MPATH_ONLY = (1 << 8), + /* Fail if target type is not "partition" (linear) */ + MAPINFO_PART_ONLY = (1 << 9), + __MAPINFO_TGT_TYPE = (MAPINFO_MPATH_ONLY | MAPINFO_PART_ONLY), +}; + +typedef union libmp_map_identifier { + const char *str; + struct { + int major; + int minor; + } _u; + dev_t devt; +} mapid_t; + +typedef struct libmp_map_info { + /** @name: name of the map. + * If non-NULL, it must point to an array of WWID_SIZE bytes + */ + char *name; + /** @uuid: UUID of the map. + * If non-NULL it must point to an array of DM_UUID_LEN bytes + */ + char *uuid; + /** @dmi: Basic info, must point to a valid dm_info buffer if non-NULL */ + struct dm_info *dmi; + /** @target: target params, *@target will be allocated if @target is non-NULL*/ + char **target; + /** @size: target size. */ + unsigned long long *size; + /** @status: target status, *@status will be allocated if @status is non-NULL */ + char **status; +} mapinfo_t; + +/** + * libmp_mapinfo(): obtain information about a map from the kernel + * @param flags: see __mapinfo_flags above. + * Exactly one of DM_MAP_BY_NAME, DM_MAP_BY_UUID, and DM_MAP_BY_DEV must be set. + * @param id: string or major/minor to identify the map to query + * @param info: output parameters, see above. Non-NULL elements will be filled in. + * @returns: + * DMP_OK if successful. + * DMP_NOT_FOUND if the map wasn't found, or has no or multiple targets. + * DMP_NO_MATCH if the map didn't match @tgt_type (see above). + * DMP_ERR if some other error occurred. + * + * This function obtains the requested information for the device-mapper map + * identified by the input parameters. + * Output parameters are only filled in if the return value is DMP_OK. + * For target / status / size information, the map's table should contain + * only one target (usually multipath or linear). + */ +int libmp_mapinfo(int flags, mapid_t id, mapinfo_t info); + int dm_prereq(unsigned int *v); void skip_libmp_dm_init(void); void libmp_dm_exit(void); diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index f58cb1d..48c2b67 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 { put_multipath_config; }; -LIBMULTIPATH_24.0.0 { +LIBMULTIPATH_25.0.0 { global: /* symbols referenced by multipath and multipathd */ add_foreign; @@ -134,6 +134,7 @@ global: libmp_get_version; libmp_get_multipath_config; libmp_dm_task_run; + libmp_mapinfo; libmp_put_multipath_config; libmp_udev_set_sync_support; libmultipath_exit; From patchwork Tue Jul 16 20:53:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734917 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C6DC7208BA for ; Tue, 16 Jul 2024 20:54:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163249; cv=none; b=s5qllQKklXIo6awyfVQeUl5SEClO+p+lEkoBblmGt/mp5f44iSfO/TkwD2WiF6sGR75LRmlH0ZEUJ9DyLx8B+jYu9XH4pEOF0GmtqzYiMS0Yc5+EGS7IuBnxUXrOZ7Kl6vL//uVlngspb97K63GVq4izPwW1GsDOoDu79kG6qEQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163249; c=relaxed/simple; bh=RqS5OkdukJ/6NixKEHYVlFKH5k3SPN1uZ1rTaDPfy6Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=RbTWGFGllTmZiY+KoJY7EgbohkATAXAIU6x7j+Ee9kLz/+JeEY4r/JXax4PEUE3iAYE8MsUObbEMcZeE02bKiiTEu0+UWEf1+Aqo08/v70/+7LlFwpU++ks9tIAJA4bb1HMRwtolZc24GotJj0H4MNc/5ZWMdUom96aZLAZ3VDk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=P19Ip/Zb; arc=none smtp.client-ip=209.85.167.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="P19Ip/Zb" Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-52e99060b41so6261852e87.2 for ; Tue, 16 Jul 2024 13:54:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163244; x=1721768044; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=0ojZN+hLTa0bKNhAR9crVk8ZyRoXR89FPSaOF6y6qOQ=; b=P19Ip/ZbGtKTYFIqQEveb49B4bbFYamema8DHJD+yPQOsgXFPX5FRTm6Gtxydsw1pd g7YaWUzLICLhoYMPHgDk58nwRBMncXO1rBTY/HQSvqgaMV+/OpU1grC0TV2lavLeBrOJ j9IEQjUUblCnkjUZx5wgitTJkprRtZGgQaPhtv2PtE1WLJHGgKZwVprY9HCUox04yvY9 JBlaWIh7Yorsh8lTpFSnqKR9l/Tzivi4yo7kA8/QxOYaqsIKGA5XiHh9CfNSG8g3a8dv eM+2IL7+iDP4XAa1m5NJfgmoJ1IKyDggtgplD2MzCsHkUfCe4SWzwhOWV6IzUMSualS+ 0zqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163244; x=1721768044; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0ojZN+hLTa0bKNhAR9crVk8ZyRoXR89FPSaOF6y6qOQ=; b=UuLWXKDne/cP1fQZpFR6pFYboFuSWuMhAPJrDGLiN3/TFTMXS+SZoDgj6kuQ1zc0tx 4p+DbDd6swnGmAEakBhb4emyxm3u07byho/4+pLrxkrm0FjiQm107s366GOge9QH8Y94 P90DygXZd2xP2y25rbd6ohqaW6ewQoNKcADbTIbppLhz+yTDF42/E24kS0db2q0ho6CP uDJsWQtlcF4hp0Bg5i2JJYG5hq2o+MhVmsS1szBUOD9C8FKypvdcbKHIWBp2C0iPbX0v UIepKN/8J6StK2jOxJBN94iLS3AkesD1KHSX1JHEWoNbVJC3ccUHj2EiMB9AOVfgimtP YX0Q== X-Gm-Message-State: AOJu0Ywn2e0T7/85iPdsyLkdwz5tA2nRUakteYVDocVDSVaJlmUgByuI Bitnq3z2BaQgifh2MQAT8HLvUsgd9fMLDfV6Bwo/33GdbbGNQ5SsrifLiwn0RnA= X-Google-Smtp-Source: AGHT+IFWXIb5w+UNrPdg9GzSJf2pNXCY7MalSa4OW7lqoUjh5Sv5nEpD2wKjHwteTf7kcn108pY3Mg== X-Received: by 2002:ac2:4e07:0:b0:52e:767a:ada7 with SMTP id 2adb3069b0e04-52edf02c24bmr2154345e87.50.1721163243536; Tue, 16 Jul 2024 13:54:03 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id a640c23a62f3a-a79bc7f1bffsm356582666b.135.2024.07.16.13.54.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:03 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 36/49] libmultipath: use libmp_mapinfo() in do_foreach_partmaps() Date: Tue, 16 Jul 2024 22:53:41 +0200 Message-ID: <20240716205344.146310-4-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Also, simplify the if statement a bit. drop is_mpath_part() and dm_type_match, because do_foreach_partmaps() was the last user of these functions. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmultipath/devmapper.c | 100 ++++++++++----------------------------- 1 file changed, 26 insertions(+), 74 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 1d41207..37d2578 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -948,60 +948,6 @@ out: return r; } -enum { - DM_TYPE_NOMATCH = 0, - DM_TYPE_MATCH, - /* more than 1 target */ - DM_TYPE_MULTI, - /* empty map */ - DM_TYPE_EMPTY, - DM_TYPE_ERR, -}; -static int dm_type_match(const char *name, char *type) -{ - struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL; - uint64_t start, length; - char *target_type = NULL; - char *params; - - if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE))) - return DM_TYPE_ERR; - - if (!dm_task_set_name(dmt, name)) - return DM_TYPE_ERR; - - if (!libmp_dm_task_run(dmt)) { - dm_log_error(3, DM_DEVICE_TABLE, dmt); - return DM_TYPE_ERR; - } - - /* Fetch 1st target */ - if (dm_get_next_target(dmt, NULL, &start, &length, - &target_type, ¶ms) != NULL) - /* multiple targets */ - return DM_TYPE_MULTI; - else if (!target_type) - return DM_TYPE_EMPTY; - else if (!strcmp(target_type, type)) - return DM_TYPE_MATCH; - else - return DM_TYPE_NOMATCH; -} - -static bool is_mpath_part(const char *part_name, const char *map_name) -{ - char part_uuid[DM_UUID_LEN], map_uuid[DM_UUID_LEN]; - - if (dm_get_dm_uuid(map_name, map_uuid) != DMP_OK - || !is_mpath_uuid(map_uuid)) - return false; - - if (dm_get_dm_uuid(part_name, part_uuid) != DMP_OK) - return false; - - return is_mpath_part_uuid(part_uuid, map_uuid); -} - int dm_is_mpath(const char *name) { char uuid[DM_UUID_LEN]; @@ -1443,7 +1389,7 @@ char *dm_mapname(int major, int minor) } static int -do_foreach_partmaps (const char * mapname, +do_foreach_partmaps (const char *mapname, int (*partmap_func)(const char *, void *), void *data) { @@ -1451,9 +1397,18 @@ do_foreach_partmaps (const char * mapname, char __attribute__((cleanup(cleanup_charp))) *params = NULL; struct dm_names *names; unsigned next = 0; - unsigned long long size; - char dev_t[32]; + char dev_t[BLK_DEV_SIZE]; char *p; + char map_uuid[DM_UUID_LEN]; + struct dm_info info; + + if (libmp_mapinfo(DM_MAP_BY_NAME, + (mapid_t) { .str = mapname }, + (mapinfo_t) { .uuid = map_uuid, .dmi = &info }) != DMP_OK) + return 1; + + if (safe_sprintf(dev_t, "%i:%i", info.major, info.minor)) + return 1; if (!(dmt = libmp_dm_task_create(DM_DEVICE_LIST))) return 1; @@ -1468,41 +1423,38 @@ do_foreach_partmaps (const char * mapname, /* this is perfectly valid */ return 0; - if (dm_dev_t(mapname, &dev_t[0], 32)) - return 1; - do { + char part_uuid[DM_UUID_LEN]; + if ( /* * if there is only a single "linear" target */ - (dm_type_match(names->name, TGT_PART) == DM_TYPE_MATCH) && - + libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_PART_ONLY, + (mapid_t) { .str = names->name }, + (mapinfo_t) { + .uuid = part_uuid, + .target = ¶ms, + }) == DMP_OK && /* * and the uuid of the target is a partition of the * uuid of the multipath device */ - is_mpath_part(names->name, mapname) && - - /* - * and we can fetch the map table from the kernel - */ - dm_get_map(names->name, &size, ¶ms) == DMP_OK && + is_mpath_part_uuid(part_uuid, map_uuid) && /* * and the table maps over the multipath map */ (p = strstr(params, dev_t)) && - !isdigit(*(p + strlen(dev_t))) - ) { - if (partmap_func(names->name, data) != 0) - return 1; - } + !isdigit(*(p + strlen(dev_t))) && + + (partmap_func(names->name, data) != 0)) + return 1; free(params); params = NULL; next = names->next; - names = (void *) names + next; + names = (void*) names + next; } while (next); return 0; From patchwork Tue Jul 16 20:53:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734916 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9F07954673 for ; Tue, 16 Jul 2024 20:54:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163249; cv=none; b=TnMSSS32LFxVu1dJCXYDXpfIM6kvBAEEEq7eFXA6VMcgrNHj2cSLMFUAcWa5oDEo2R8w7sVUo6ReKTgpkFfTztFkAp3B72WdgtYybgg/Oywk4ONsw8asEZH0JWArqvFpdRImcZI2H5NG/bSnEeAovHxTD+HiEZy+mTcFRxn9Rwo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163249; c=relaxed/simple; bh=GweVNAqcZa9BKq5VD8ofvk2pAhBaeBgRzh4dPQOvBbI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Zq2417TN7TaZAlsUr4H1KnKx8FXTiKEH9ittF9VAaDNeZqJfHl9tzUfYRAKohyqLgZQF8boPRZ9XNH6X6H1aeD9AAaoGuxbti3Jv8oVgjdWABp4cuDBAjqc2rWRfeIOHF8KEc/yMK30pmjzHLCUGRvznki8kXP72iMhHXoKWbmg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=KmupzFgd; arc=none smtp.client-ip=209.85.167.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="KmupzFgd" Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-52ea34ffcdaso6505174e87.1 for ; Tue, 16 Jul 2024 13:54:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163245; x=1721768045; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=EO+kKpFf+oG6lWy22jU3Kfjo0Qyea68mUaTXA7pruVo=; b=KmupzFgdRG5DlZr3wUH7ay6iePDaUhm0HIMKc7VldRL6/eaIAZwdYDAFlbxWtRyOJh D789GSw7i281ImjLJXhc585mGlo9jg5zCp81uDFY2NeotAnrU2HrsAayspj8hsGK/nEk HnbSf41/eVynaHhhQoLxmPLeBf6g2X9MKJO1qHK1gzzPrSSiWeBAJ1yhLvnyiQNz7ndY BYhofrgJuaOySIakjzeVL37p4C4LvMcJZJFz8eAVG+lUM8MqEElbdmiRdb+rEg9sFPn0 cDx5uMpZKcQOPKyfmhmLOXNr6i9ZVheHgj2Su4D3qzqaNShuMEoeE1IVdY9SCYYsp6KB zbng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163245; x=1721768045; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=EO+kKpFf+oG6lWy22jU3Kfjo0Qyea68mUaTXA7pruVo=; b=rJk5o2qySW9VJFMuG9l+OgIZc6bDZnM64FTNQp2VF7n5YNCmG4mPZNKTcpzWnm/gYs jIf5AuZD73xU4q777MFbvSfgWy+oJS71+HUtOzf2derusJ/H+GtJdmMEz/s0kaYWCLYR F586c3UjfthPJnbk+jdqX7NnGSJ2bPI6VvnvlMgiWib3JpBAEEzlDMOjZzgRIbiXTcMj XtlVPi9TaVG9emuFqUWsEm1TeQc/I/UacQ8d1urTOnaVkU0i62xIyzZQ/6xbf0iUxv8o FcPOlTtbGlfmoNwMB/zIjL8Ergyj1Jz0RrkoWdtcDfGYUasDUHrWlTNJka/WDUDYGK7t ej1A== X-Gm-Message-State: AOJu0YyTTYyYKR7xTNppO0gpyWVDF55i5trS/ePYSsaJ9QcgZNLBc8+3 ioKj1ctULEP4diFegkQMj366JgZbC8QuQ9JfFyWv6UGwOje3O4GwNW6DIewguNQ= X-Google-Smtp-Source: AGHT+IFdF0cecF7ATqUOZeBEVARs3IKwee8082XMGNmjIC7glIEWBPe5u8k5N//ClJmrT0iNz1flJQ== X-Received: by 2002:a05:6512:ea6:b0:52c:db4e:8dfc with SMTP id 2adb3069b0e04-52edf032eb5mr2083603e87.65.1721163244527; Tue, 16 Jul 2024 13:54:04 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id a640c23a62f3a-a79bc5b7edfsm361369066b.68.2024.07.16.13.54.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:04 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 42/49] multipathd: implement add_map_without_path() with libmp_mapinfo() Date: Tue, 16 Jul 2024 22:53:42 +0200 Message-ID: <20240716205344.146310-5-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Also, change the return value to int, as this is more expressive and the returned struct multipath isn't used by the caller. Also remove the call to sync_map_state() in ev_add_map(), which is redundant because add_map_without_path() would have called update_map() and thus sync_map_state() already. Note: this removes the call to remove_map() at the end of the function, which doesn't make sense anyway, because update_multipath_table() would not return error unless the table disassembly failed, in which case nothing would have been added the the mpvec or pathvec yet. It should be sufficient to just cleanup the local data structures when add_map_without_path() fails. Signed-off-by: Martin Wilck --- multipathd/main.c | 79 ++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/multipathd/main.c b/multipathd/main.c index 1e7a6ac..32011c9 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -707,51 +707,57 @@ fail: return 0; } -static struct multipath * -add_map_without_path (struct vectors *vecs, const char *alias) +static int add_map_without_path (struct vectors *vecs, const char *alias) { - struct multipath * mpp = alloc_multipath(); + struct multipath __attribute__((cleanup(cleanup_multipath_and_paths))) + *mpp = alloc_multipath(); + char __attribute__((cleanup(cleanup_charp))) *params = NULL; + char __attribute__((cleanup(cleanup_charp))) *status = NULL; struct config *conf; + char uuid[DM_UUID_LEN]; + int rc = DMP_ERR; - if (!mpp) - return NULL; - if (!alias) { - free(mpp); - return NULL; - } + if (!mpp || !(mpp->alias = strdup(alias))) + return DMP_ERR; - mpp->alias = strdup(alias); + if ((rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY, + (mapid_t) { .str = mpp->alias }, + (mapinfo_t) { + .uuid = uuid, + .dmi = &mpp->dmi, + .size = &mpp->size, + .target = ¶ms, + .status = &status, + })) != DMP_OK) + return rc; + + if (!is_mpath_uuid(uuid)) + return DMP_NO_MATCH; + else + strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid)); - if (dm_get_info(mpp->alias, &mpp->dmi) != DMP_OK) { - condlog(3, "%s: cannot access table", mpp->alias); - goto out; - } - if (!strlen(mpp->wwid) && - dm_get_wwid(mpp->alias, mpp->wwid, WWID_SIZE) != DMP_OK) { - condlog(3, "%s: cannot obtain WWID", mpp->alias); - goto out; - } if (!strlen(mpp->wwid)) condlog(1, "%s: adding map with empty WWID", mpp->alias); + conf = get_multipath_config(); mpp->mpe = find_mpe(conf->mptable, mpp->wwid); put_multipath_config(conf); - if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK) - goto out; + if ((rc = update_multipath_table__(mpp, vecs->pathvec, 0, params, status)) != DMP_OK) + return DMP_ERR; if (!vector_alloc_slot(vecs->mpvec)) - goto out; + return DMP_ERR; + vector_set_slot(vecs->mpvec, steal_ptr(mpp)); - vector_set_slot(vecs->mpvec, mpp); + /* + * We can't pass mpp here, steal_ptr() has just nullified it. + * vector_set_slot() just set the last slot, use that. + */ + if (update_map(VECTOR_LAST_SLOT(vecs->mpvec), vecs, 1) != 0) /* map removed */ + return DMP_ERR; - if (update_map(mpp, vecs, 1) != 0) /* map removed */ - return NULL; - - return mpp; -out: - remove_map(mpp, vecs->pathvec, vecs->mpvec); - return NULL; + return DMP_OK; } static int @@ -865,14 +871,9 @@ int ev_add_map (char * dev, const char * alias, struct vectors * vecs) { struct multipath * mpp; - int reassign_maps; + int reassign_maps, rc; struct config *conf; - if (dm_is_mpath(alias) != DM_IS_MPATH_YES) { - condlog(4, "%s: not a multipath map", alias); - return 0; - } - mpp = find_mp_by_alias(vecs->mpvec, alias); if (mpp) { @@ -910,10 +911,12 @@ ev_add_map (char * dev, const char * alias, struct vectors * vecs) /* * now we can register the map */ - if ((mpp = add_map_without_path(vecs, alias))) { - sync_map_state(mpp); + if ((rc = add_map_without_path(vecs, alias)) == DMP_OK) { condlog(2, "%s: devmap %s registered", alias, dev); return 0; + } else if (rc == DMP_NO_MATCH) { + condlog(4, "%s: not a multipath map", alias); + return 0; } else { condlog(2, "%s: ev_add_map failed", dev); return 1; From patchwork Tue Jul 16 20:53:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734918 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AAB8955887 for ; Tue, 16 Jul 2024 20:54:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163250; cv=none; b=oAcgd+MaSIaFugtbtpquenKMEeArtz6fCX5fKXXq63HApPQ9wFM7gY+R3nakHNVR5LsrCjzPpGLcIoxxJB2OzGSuX6QNapcDz9ZVlq9pZPcKZobjLz/4tmbdHlAvWYt7FhGJ0uELF5nhoQ8XLA15rP1E/g3ueLwXZhDdvqkr7JA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163250; c=relaxed/simple; bh=KBNzzxmp9Uwvhd7LoNNhnjnllY3aB3qc6HqzdfhXcEg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=JPOnIWpmeBx4ZTo2kOIsnfRdlF3UTVi1H6YCZYDRdEPrYHLG6tHlBvPGg4xshgty3PMxl/E4lPzq0pCQaEYG3B8PPnFH6m2Dmrfvfe/hv7JZIOyLrzyxypZ3YjEfE9efrZfGcsK7vTHRDMElxYWbwvyU3An66PsP/zeZT0jZdnE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=O+O90UrI; arc=none smtp.client-ip=209.85.208.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="O+O90UrI" Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-59f9f59b88cso1133579a12.3 for ; Tue, 16 Jul 2024 13:54:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163246; x=1721768046; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=3HPjIPC9OfQF0ldr5LtaR56w9OpZppFU3pyXpUE5ngQ=; b=O+O90UrI7pdK21sBanwVNgWaDqzOghRNCfAy/xLiKQgupdRV8TYqExP2BNLFMq9zCL SdtFSG+4C7HN1rJ3Wti30fYutVWh8djpEaN+mtpIElyQfiz7Qn3L3nDfC5QDcdNw80xk E+UzrcUtVX79zGjx4WRUwCmxtCbjcwTSonEawPpAysJN1NKWVAPX2U1HCe89O6Cwi1dn b2gtXme12jV1bHOQ34Yp0CNPTe5EL1N/YTeHUR89Y9jZtsg63yqxOuKX5Ep9auMYUx4X AXOT1PWBzlLWOlEBs12YlzSOfPkUdVetAEfiD2kFIPTa0mOCS/F4RQAjVRoQRaCWFLyK B5HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163246; x=1721768046; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3HPjIPC9OfQF0ldr5LtaR56w9OpZppFU3pyXpUE5ngQ=; b=bjjIv44Wz927PIutdb4yF9zb8UCgFVTZzflapcKg0ZmuYEGRRA1gsLPzJ1zC+2Gt6w nMjH3DlXuM8e1JRbf/GGRcZOftgllH+TKyTF43XlAnU2Y5lp51mmuDQQuuRnM+2qa//f L9M4oalToJIKXqMqfxjTsnMPFP/3IPFx0UgunInaquwpOR60XxV5O+rRfmD1Ak9aqnFh kU6VXlxthzdOE9jAvZGhlXaFPhEalXUmIPOrgCplo6Xf4j36lK8ttES0Bb0h1AR/Z7pS cyYXKlkp7PMaidvzv24uesX7Qv4T9BK9e41TZVS/x7kBXTT6965VqYTQg3QVRgwyMCJc lOVA== X-Gm-Message-State: AOJu0Yx7gRbgddAa9XEUBWsIGaRdxQ6gL9c80yf5LHz5TJTpMPym3ykB lv+AGAI/JOFz0/6dfoLdG/w1ojy8Tj2SSLJFOTmOER6iYBtwXC5JAabDONG/dn0H5ESEkbh5Gb7 S X-Google-Smtp-Source: AGHT+IG/HcupBLQp60I42pcZkYrJ9r8Pt7m2TD57sXAtHOO3L0NjkFKeWRFUYVcnFW9Voecrik7fKQ== X-Received: by 2002:a05:6402:510b:b0:57c:610a:6e7f with SMTP id 4fb4d7f45d1cf-59eeed2967emr2441488a12.11.1721163245741; Tue, 16 Jul 2024 13:54:05 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id 4fb4d7f45d1cf-59b223b3321sm5541465a12.0.2024.07.16.13.54.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:05 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 47/49] libmultipath: Move UUID check into libmp_pathinfo__() Date: Tue, 16 Jul 2024 22:53:43 +0200 Message-ID: <20240716205344.146310-6-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 We have a couple of callers that check whether the map UUID conforms to the multipath convention ("mpath-xyz"). Move this check into libmp_mapinfo__(). Add another flag MAPINFO_CHECK_UUID for this purpose. This allows to simplify some callers, which only fetched the UUID in order to check it. Note that the UUID check is orthogonal to MAPINFO_MPATH_ONLY, which tests the target type. We shold make sure that both tests are in agreement, but this is postponed to a later patch set. is_mpath_uuid() can now be converted to a static function. Also add some unit tests for the WWID check. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmpathpersist/mpath_persist_int.c | 11 +- libmultipath/devmapper.c | 52 +++---- libmultipath/devmapper.h | 3 +- libmultipath/libmultipath.version | 1 - multipath/main.c | 5 +- multipathd/main.c | 7 +- tests/mapinfo.c | 226 ++++++++++++++++++++++++++++ 7 files changed, 256 insertions(+), 49 deletions(-) diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c index 807415f..f4d9e7c 100644 --- a/libmpathpersist/mpath_persist_int.c +++ b/libmpathpersist/mpath_persist_int.c @@ -158,7 +158,7 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, struct multipath { int rc; struct stat info; - char alias[WWID_SIZE], uuid[DM_UUID_LEN]; + char alias[WWID_SIZE]; struct multipath *mpp; if (fstat(fd, &info) != 0){ @@ -171,14 +171,11 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, struct multipath } /* get alias from major:minor*/ - rc = libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY, + rc = libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, (mapid_t) { .devt = info.st_rdev }, - (mapinfo_t) { - .name = alias, - .uuid = uuid, - }); + (mapinfo_t) { .name = alias }); - if (rc == DMP_NO_MATCH || !is_mpath_uuid(uuid)) { + if (rc == DMP_NO_MATCH) { condlog(3, "%s: not a multipath device.", alias); return MPATH_PR_DMMP_ERROR; } else if (rc != DMP_OK) { diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index e96b83e..455905a 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -611,6 +611,11 @@ int dm_addmap_reload(struct multipath *mpp, char *params, int flush) return 0; } +static bool is_mpath_uuid(const char uuid[DM_UUID_LEN]) +{ + return !strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN); +} + bool has_dm_info(const struct multipath *mpp) { @@ -736,11 +741,17 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma * If error is returned, don't touch any output parameters. */ if ((info.name && !(name = dm_task_get_name(dmt))) - || (info.uuid && !(uuid = dm_task_get_uuid(dmt))) + || ((info.uuid || flags & MAPINFO_CHECK_UUID) + && !(uuid = dm_task_get_uuid(dmt))) || (info.status && !(tmp_status = strdup(params))) || (info.target && !tmp_target && !(tmp_target = strdup(params)))) return DMP_ERR; + if (flags & MAPINFO_CHECK_UUID && !is_mpath_uuid(uuid)) { + condlog(3, "%s: UUID mismatch: %s", fname__, uuid); + return DMP_NO_MATCH; + } + if (info.name) { strlcpy(info.name, name, WWID_SIZE); condlog(4, "%s: %s: name: \"%s\"", fname__, map_id, info.name); @@ -810,18 +821,6 @@ int libmp_mapinfo(int flags, mapid_t id, mapinfo_t info) libmp_map_identifier(flags, id, idbuf)); } -static int dm_get_dm_uuid(const char *mapname, char uuid[DM_UUID_LEN]) -{ - return libmp_mapinfo(DM_MAP_BY_NAME, - (mapid_t) { .str = mapname }, - (mapinfo_t) { .uuid = uuid }); -} - -bool is_mpath_uuid(const char uuid[DM_UUID_LEN]) -{ - return !strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN); -} - /** * dm_get_wwid(): return WWID for a multipath map * @returns: @@ -834,16 +833,14 @@ bool is_mpath_uuid(const char uuid[DM_UUID_LEN]) int dm_get_wwid(const char *name, char *uuid, int uuid_len) { char tmp[DM_UUID_LEN]; - int rc = dm_get_dm_uuid(name, tmp); + int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = name }, + (mapinfo_t) { .uuid = tmp }); if (rc != DMP_OK) return rc; - if (is_mpath_uuid(tmp)) - strlcpy(uuid, tmp + UUID_PREFIX_LEN, uuid_len); - else - return DMP_NO_MATCH; - + strlcpy(uuid, tmp + UUID_PREFIX_LEN, uuid_len); return DMP_OK; } @@ -861,16 +858,13 @@ static bool is_mpath_part_uuid(const char part_uuid[DM_UUID_LEN], int dm_is_mpath(const char *name) { - char uuid[DM_UUID_LEN]; - int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY, + int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, (mapid_t) { .str = name }, - (mapinfo_t) { .uuid = uuid }); + (mapinfo_t) { .uuid = NULL }); switch (rc) { case DMP_OK: - if (is_mpath_uuid(uuid)) - return DM_IS_MPATH_YES; - /* fallthrough */ + return DM_IS_MPATH_YES; case DMP_NOT_FOUND: case DMP_NO_MATCH: return DM_IS_MPATH_NO; @@ -961,14 +955,10 @@ int _dm_flush_map (const char *mapname, int flags, int retries) int queue_if_no_path = 0; int udev_flags = 0; char *params __attribute__((cleanup(cleanup_charp))) = NULL; - char uuid[DM_UUID_LEN]; - if (libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY, + if (libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, (mapid_t) { .str = mapname }, - (mapinfo_t) { - .uuid = uuid, - .target = ¶ms }) != DMP_OK - || !is_mpath_uuid(uuid)) + (mapinfo_t) { .target = ¶ms }) != DMP_OK) return DM_FLUSH_OK; /* nothing to do */ /* if the device currently has no partitions, do not diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index d0e20bf..7a551d9 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -57,6 +57,8 @@ enum __mapinfo_flags { /* Fail if target type is not "partition" (linear) */ MAPINFO_PART_ONLY = (1 << 9), __MAPINFO_TGT_TYPE = (MAPINFO_MPATH_ONLY | MAPINFO_PART_ONLY), + /* Fail if the UUID doesn't match the multipath UUID format */ + MAPINFO_CHECK_UUID = (1 << 10), }; typedef union libmp_map_identifier { @@ -138,7 +140,6 @@ enum { DM_IS_MPATH_ERR, }; -bool is_mpath_uuid(const char uuid[DM_UUID_LEN]); int dm_is_mpath(const char *name); enum { diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index 54b5a23..292a330 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -127,7 +127,6 @@ global: init_foreign; init_prio; io_err_stat_handle_pathfail; - is_mpath_uuid; is_path_valid; libmp_dm_task_create; libmp_get_version; diff --git a/multipath/main.c b/multipath/main.c index 0d989dc..4b19d2e 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -254,7 +254,7 @@ static int check_usable_paths(struct config *conf, if (pathvec == NULL) return r; - if (libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY, + if (libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, (mapid_t) { .devt = devt }, (mapinfo_t) { .name = mpp->alias, @@ -266,9 +266,6 @@ static int check_usable_paths(struct config *conf, }) != DMP_OK) return r; - if (!is_mpath_uuid(uuid)) - return r; - strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid)); if (update_multipath_table__(mpp, pathvec, 0, params, status) != DMP_OK) diff --git a/multipathd/main.c b/multipathd/main.c index 32011c9..833c1e2 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -720,7 +720,7 @@ static int add_map_without_path (struct vectors *vecs, const char *alias) if (!mpp || !(mpp->alias = strdup(alias))) return DMP_ERR; - if ((rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY, + if ((rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, (mapid_t) { .str = mpp->alias }, (mapinfo_t) { .uuid = uuid, @@ -731,10 +731,7 @@ static int add_map_without_path (struct vectors *vecs, const char *alias) })) != DMP_OK) return rc; - if (!is_mpath_uuid(uuid)) - return DMP_NO_MATCH; - else - strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid)); + strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid)); if (!strlen(mpp->wwid)) condlog(1, "%s: adding map with empty WWID", mpp->alias); diff --git a/tests/mapinfo.c b/tests/mapinfo.c index f3a8440..f7ad868 100644 --- a/tests/mapinfo.c +++ b/tests/mapinfo.c @@ -54,6 +54,16 @@ static const char MPATH_STATUS_01[] = "A 0 3 2 65:32 A 0 0 1 67:64 A 0 0 1 69:96 A 0 0 1 " "E 0 3 2 8:16 A 0 0 1 66:48 A 0 0 1 68:80 A 0 0 1 "; +static const char BAD_UUID_01[] = ""; +static const char BAD_UUID_02[] = "mpath3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_03[] = " mpath-3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_04[] = "-mpath-3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_05[] = "mpth-3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_06[] = "part1-mpath-3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_07[] = "mpath 3600a098038302d414b2b4d4453474f62"; +static const char BAD_UUID_08[] = "mpath"; +static const char BAD_UUID_09[] = "mpath-"; + char *__real_strdup(const char *str); char *__wrap_strdup(const char *str) { @@ -413,6 +423,208 @@ static void test_mapinfo_good_exists(void **state) assert_int_equal(rc, DMP_OK); } +static void test_mapinfo_bad_check_uuid_00(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, NULL); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_ERR); +} + +static void test_mapinfo_bad_check_uuid_01(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_01); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_02(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_02); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_03(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_03); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_04(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_04); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_05(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_05); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_06(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_06); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_07(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_07); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_08(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_08); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_NO_MATCH); +} + +static void test_mapinfo_bad_check_uuid_09(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, BAD_UUID_09); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_OK); +} + +static void test_mapinfo_good_check_uuid_01(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_OK); +} + +static void test_mapinfo_good_check_uuid_02(void **state) +{ + int rc; + char uuid[DM_UUID_LEN]; + + mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .uuid = uuid }); + assert_int_equal(rc, DMP_OK); +} + +static void test_mapinfo_good_check_uuid_03(void **state) +{ + int rc; + + mock_mapinfo_name_1(DM_DEVICE_STATUS, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + mock_dm_get_next_target(12345, TGT_MPATH, MPATH_STATUS_01, NULL); + will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01); + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .name = NULL }); + assert_int_equal(rc, DMP_OK); +} + +static void test_mapinfo_good_check_uuid_04(void **state) +{ + char __attribute__((cleanup(cleanup_charp))) *target = NULL; + int rc; + + mock_mapinfo_name_1(DM_DEVICE_TABLE, 1, "foo", 1, 1, 0); + WRAP_DM_TASK_GET_INFO(1); + WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01); + mock_dm_get_next_target(12345, TGT_MPATH, MPATH_STATUS_01, NULL); + will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01); + will_return(__wrap_strdup, 1); + + rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID, + (mapid_t) { .str = "foo", }, + (mapinfo_t) { .target = &target }); + assert_int_equal(rc, DMP_OK); +} + static void test_mapinfo_bad_set_uuid(void **state) { int rc; @@ -1126,6 +1338,20 @@ static int test_mapinfo(void) cmocka_unit_test(test_mapinfo_bad_get_info_03), cmocka_unit_test(test_mapinfo_bad_get_info_04), cmocka_unit_test(test_mapinfo_good_exists), + cmocka_unit_test(test_mapinfo_bad_check_uuid_00), + cmocka_unit_test(test_mapinfo_bad_check_uuid_01), + cmocka_unit_test(test_mapinfo_bad_check_uuid_02), + cmocka_unit_test(test_mapinfo_bad_check_uuid_03), + cmocka_unit_test(test_mapinfo_bad_check_uuid_04), + cmocka_unit_test(test_mapinfo_bad_check_uuid_05), + cmocka_unit_test(test_mapinfo_bad_check_uuid_06), + cmocka_unit_test(test_mapinfo_bad_check_uuid_07), + cmocka_unit_test(test_mapinfo_bad_check_uuid_08), + cmocka_unit_test(test_mapinfo_bad_check_uuid_09), + cmocka_unit_test(test_mapinfo_good_check_uuid_01), + cmocka_unit_test(test_mapinfo_good_check_uuid_02), + cmocka_unit_test(test_mapinfo_good_check_uuid_03), + cmocka_unit_test(test_mapinfo_good_check_uuid_04), cmocka_unit_test(test_mapinfo_bad_set_uuid), cmocka_unit_test(test_mapinfo_bad_set_dev_01), cmocka_unit_test(test_mapinfo_bad_set_dev_02), From patchwork Tue Jul 16 20:53:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 13734919 X-Patchwork-Delegate: christophe.varoqui@free.fr Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9739C54F95 for ; Tue, 16 Jul 2024 20:54:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163250; cv=none; b=hHxXGZXvqA5+Q4JUyIH2BqTfsqHudLKd2eluSlW3itQIlPN1p6yxY5rtSx0jexp+h1JWzMcp/r7HPnL7jF5eucugsWl4DFfGohWMyke5LXXqE+w2Yu1KCyaJP9S/rb+GBirEja6HI4036dRelbAYUQ7EQJ2a2/e+2K2tl+NECh4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721163250; c=relaxed/simple; bh=P3+BNHif6v8HpjvP6t7ikHMQ1mjMG+MSMGWRDKp1fus=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=jG1GyvCSrgdwUvxl6A4CR8O46KTH8T6exvJLK4lAKiqZVWQk550IhlH4hmS8hzliPRaFp38Cxu0ZGa8TKgjAx7Kt+DcUHXtHHlSG0gMkFCcWZW7OtH0T/pFsCkleVpTaEIl/F9w+cOdjwrNkjXkyH6uEkzNoLSKfQTaie6b5MiQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=CcnhvmlH; arc=none smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="CcnhvmlH" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-3678f36f154so3593953f8f.2 for ; Tue, 16 Jul 2024 13:54:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721163247; x=1721768047; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=uZlmDkSNB5/rTWFnOqgq7eKBBSYe3/9yGQfVskFcDZA=; b=CcnhvmlHOBwajQ3I/6adh61bLWjjrzG42YvfFYAfxuk5ZetXEamiuNMN9R1hjbnr/x szOjoePBDbkRnFjManYSIuPw+MCnNNgssBJ4GPzjhKrkMgGrcWZouxJFvgvKNZ07Scuf ij5s1CM87GqEB+RaDI1Hs63WD0VCLep8gdGvrK5UFmoIghgVBqTTdDv4Yhe71SgN3TQu qhBv7qfi8Z3LIsV0G8mJUmwep+YxVVYHpJQxddIzXx7BaqgIgVz3sY5X9H7BQPn+LqmH YfDH7yjqczUz4no5mvuRtNMQ9I+/ejqErIrpWIXDa4lWiqtynKSY1TE/i+T9TFvK0ydi zhZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721163247; x=1721768047; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uZlmDkSNB5/rTWFnOqgq7eKBBSYe3/9yGQfVskFcDZA=; b=uoVsZxHanXvnLQrY5+HNYszv3bWXMndJ1b7i8bhHBLja9cTwPDiS4GQDDWYHiLZiCY ZQvRcPqlNDxiBZXxzuJu0yJxpFeIrwIhBJW6m0Jb/ZIE3Fz3EWpNq9Y+874KILsTnXOY o5r5JC6nK+cKg5Y5ZYFK2Hp6uim6SVL+q+8y92rRHhVTiGNb6M6xYd3fMS95rJKBUv9l 7U8L1X2RLzNFCzZmjR0bRWuyvYhgWSScm606uVnOAjQasFwpzlymUNFBFV3t2fuBNBx3 ZRLPsXkGj1bL3x/B+ofikchH6Y01AXeMMbti7dsMEC16Adclx7AYIOn5Ohl8q6EISKrc W/dQ== X-Gm-Message-State: AOJu0Yz+Qwlnztk6xge834KCfiYB8pf5tDUTTLdEjeC1M8YJvraSacdB x4Z20ccEsdYJLVSwZXFHFE2w3d+FynnDZH5RQrTaTseGWeJQKCvFnppqWuLHSc0= X-Google-Smtp-Source: AGHT+IHBcivsKQMyy/xlTwaT3091kWdx/T5IsXhq6CKBaH4069k9/suTRfrIyjTUyUj0/yf7TL8Ypw== X-Received: by 2002:a5d:6143:0:b0:367:9877:750e with SMTP id ffacd0b85a97d-3682609b526mr2060241f8f.25.1721163246652; Tue, 16 Jul 2024 13:54:06 -0700 (PDT) Received: from localhost (p200300de37360a00d7e56139e90929dd.dip0.t-ipconnect.de. [2003:de:3736:a00:d7e5:6139:e909:29dd]) by smtp.gmail.com with UTF8SMTPSA id 4fb4d7f45d1cf-59b24a76ef7sm5473453a12.10.2024.07.16.13.54.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Jul 2024 13:54:06 -0700 (PDT) From: Martin Wilck X-Google-Original-From: Martin Wilck To: Christophe Varoqui , Benjamin Marzinski Cc: dm-devel@lists.linux.dev Subject: [PATCH v3 48/49] libmultipath: don't call do_foreach_partmaps() recursively Date: Tue, 16 Jul 2024 22:53:44 +0200 Message-ID: <20240716205344.146310-7-mwilck@suse.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240716205344.146310-1-mwilck@suse.com> References: <20240716205344.146310-1-mwilck@suse.com> Precedence: bulk X-Mailing-List: dm-devel@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 We've removed partition mappings recursively since 83fb936 ("Correctly remove logical partition maps"). This was wrong, because kpartx doesn't create logical partitions as mappings onto the extended partition. Rather, logical partitions are created by kpartx as mappings to the multipath device, and afaics, this has always been the case. Therefore, the loop in do_foreach_partmaps() will detect all partition mappings (primary, extended, and logical) without recursion. At least since 4059e42 ("libmultipath: fix partition detection"), the recursion has actually been pointless, because is_mpath_part() would never have returned "true" for a pair of two partition mappings (one representing an extended partition and one a logical partition). Avoiding the recursion has the additional benefit that the complexity of removing maps scales with N, where N is the number of dm devices, rather than with N^2. Also, it simplifies the code. Split partmap_in_use() into two separate functions, mpath_in_use() (to be called for multipath maps) and count_partitions(), which is called from do_foreach_partmaps(). Because do_foreach_partmaps() is now only legitimately called for multipath maps, quit early if called for another map type. Signed-off-by: Martin Wilck Reviewed-by: Benjamin Marzinski --- libmultipath/devmapper.c | 48 +++++++++++++++++++------------ libmultipath/devmapper.h | 2 +- libmultipath/libmultipath.version | 2 +- multipathd/main.c | 2 +- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 455905a..3b2e8ac 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -929,22 +929,37 @@ has_partmap(const char *name __attribute__((unused)), return 1; } -int -partmap_in_use(const char *name, void *data) +/* + * This will be called from mpath_in_use, for each partition. + * If the partition itself in use, returns 1 immediately, causing + * do_foreach_partmaps() to stop iterating and return 1. + * Otherwise, increases the partition count. + */ +static int count_partitions(const char *name, void *data) +{ + int *ret_count = (int *)data; + int open_count = dm_get_opencount(name); + + if (open_count) + return 1; + (*ret_count)++; + return 0; +} + +int mpath_in_use(const char *name) { - int part_count, *ret_count = (int *)data; int open_count = dm_get_opencount(name); - if (ret_count) - (*ret_count)++; - part_count = 0; if (open_count) { - if (do_foreach_partmaps(name, partmap_in_use, &part_count)) - return 1; - if (open_count != part_count) { - condlog(2, "%s: map in use", name); + int part_count = 0; + + if (do_foreach_partmaps(name, count_partitions, &part_count)) { + condlog(4, "%s: %s has open partitions", __func__, name); return 1; } + condlog(4, "%s: %s: %d openers, %d partitions", __func__, name, + open_count, part_count); + return open_count > part_count; } return 0; } @@ -968,7 +983,7 @@ int _dm_flush_map (const char *mapname, int flags, int retries) /* If you aren't doing a deferred remove, make sure that no * devices are in use */ - if (!(flags & DMFL_DEFERRED) && partmap_in_use(mapname, NULL)) + if (!(flags & DMFL_DEFERRED) && mpath_in_use(mapname)) return DM_FLUSH_BUSY; if ((flags & DMFL_SUSPEND) && @@ -1305,7 +1320,7 @@ do_foreach_partmaps (const char *mapname, char map_uuid[DM_UUID_LEN]; struct dm_info info; - if (libmp_mapinfo(DM_MAP_BY_NAME, + if (libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID, (mapid_t) { .str = mapname }, (mapinfo_t) { .uuid = map_uuid, .dmi = &info }) != DMP_OK) return 1; @@ -1372,12 +1387,9 @@ remove_partmap(const char *name, void *data) { struct remove_data *rd = (struct remove_data *)data; - if (dm_get_opencount(name)) { - dm_remove_partmaps(name, rd->flags); - if (!(rd->flags & DMFL_DEFERRED) && dm_get_opencount(name)) { - condlog(2, "%s: map in use", name); - return DM_FLUSH_BUSY; - } + if (!(rd->flags & DMFL_DEFERRED) && dm_get_opencount(name)) { + condlog(2, "%s: map in use", name); + return DM_FLUSH_BUSY; } condlog(4, "partition map %s removed", name); dm_device_remove(name, rd->flags); diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 7a551d9..f6d0017 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -150,7 +150,7 @@ enum { DM_FLUSH_BUSY, }; -int partmap_in_use(const char *name, void *data); +int mpath_in_use(const char *name); enum { DMFL_NONE = 0, diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version index 292a330..959f675 100644 --- a/libmultipath/libmultipath.version +++ b/libmultipath/libmultipath.version @@ -138,10 +138,10 @@ global: libmultipath_exit; libmultipath_init; load_config; + mpath_in_use; need_io_err_check; orphan_path; parse_prkey_flags; - partmap_in_use; pathcount; path_discovery; path_get_tpgs; diff --git a/multipathd/main.c b/multipathd/main.c index 833c1e2..13ed6d0 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -597,7 +597,7 @@ flush_map_nopaths(struct multipath *mpp, struct vectors *vecs) { return false; } if (mpp->flush_on_last_del == FLUSH_UNUSED && - partmap_in_use(mpp->alias, NULL) && is_queueing) { + mpath_in_use(mpp->alias) && is_queueing) { condlog(2, "%s: map in use and queueing, can't remove", mpp->alias); return false;