From patchwork Tue May 19 04:57:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556909 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AD771913 for ; Tue, 19 May 2020 04:57:36 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6A9C2206D4 for ; Tue, 19 May 2020 04:57:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="cix/nuMk" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A9C2206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864255; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=dbo9sBCLG23y2wlS24QM5oDDZGwR0IlqFKbtooaH3lE=; b=cix/nuMk9uhSkLAgvfNzU822mtSYkHAQkd76HrheVGQysogTjjOME4yvWkhiCQvQsCl6hm riG8ZXkm2Kjp/1vdm+ct6nBt+ODbYxNyzJ6zZCNK06ro3ETFFrwK6STOA85PY2baai48A5 UZVbcCHsCowjxLIj1qeJYreGhINTW2k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-336-UbPy1serP1K1Oe6PIvoFtg-1; Tue, 19 May 2020 00:57:32 -0400 X-MC-Unique: UbPy1serP1K1Oe6PIvoFtg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 96D701800D42; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7CC126297D; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 54AA51809557; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vEqM019134 for ; Tue, 19 May 2020 00:57:14 -0400 Received: by smtp.corp.redhat.com (Postfix) id BEB2D60C05; Tue, 19 May 2020 04:57:14 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 16B2B60BF3; Tue, 19 May 2020 04:57:12 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vAhY001405; Mon, 18 May 2020 23:57:10 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4v9ms001404; Mon, 18 May 2020 23:57:09 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:03 -0500 Message-Id: <1589864228-1363-2-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck Subject: [dm-devel] [PATCH v2 1/6] libmultipath: make libmp_dm_init optional X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Move dm_initialized out of libmp_dm_task_create(), and add a function skip_libmp_dm_init() so that users of libmultipath can skip initializing device-mapper. This is needed for other programs that use libmultipath (or a library that depends on it) but want to control how device-mapper is set up. Also make dm_prereq a global function. Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/devmapper.c | 17 +++++++++++++---- libmultipath/devmapper.h | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 13a1cf53..7ed494a1 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -33,6 +33,8 @@ #define MAX_WAIT 5 #define LOOPS_PER_SEC 5 +static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT; + static int dm_conf_verbosity; #ifdef LIBDM_API_DEFERRED @@ -229,7 +231,7 @@ dm_tgt_prereq (unsigned int *ver) return 1; } -static int dm_prereq(unsigned int *v) +int dm_prereq(unsigned int *v) { if (dm_lib_prereq()) return 1; @@ -243,7 +245,7 @@ void libmp_udev_set_sync_support(int on) libmp_dm_udev_sync = !!on; } -void libmp_dm_init(void) +static void libmp_dm_init(void) { struct config *conf; int verbosity; @@ -262,11 +264,18 @@ void libmp_dm_init(void) dm_udev_set_sync_support(libmp_dm_udev_sync); } +static void _do_skip_libmp_dm_init(void) +{ +} + +void skip_libmp_dm_init(void) +{ + pthread_once(&dm_initialized, _do_skip_libmp_dm_init); +} + struct dm_task* libmp_dm_task_create(int task) { - static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT; - pthread_once(&dm_initialized, libmp_dm_init); return dm_task_create(task); } diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 7557a86b..17fc9faf 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -28,7 +28,8 @@ #define UUID_PREFIX_LEN (sizeof(UUID_PREFIX) - 1) void dm_init(int verbosity); -void libmp_dm_init(void); +int dm_prereq(unsigned int *v); +void skip_libmp_dm_init(void); void libmp_udev_set_sync_support(int on); struct dm_task *libmp_dm_task_create(int task); int dm_drv_version (unsigned int * version); From patchwork Tue May 19 04:57:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556901 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E7AD2618 for ; Tue, 19 May 2020 04:57:25 +0000 (UTC) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A4456206D4 for ; Tue, 19 May 2020 04:57:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LQPfdxLh" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A4456206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864244; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=wjtqtLFGRPZ0srzNTn1Ng8smGQRwH9hDH7rZTIyo7UQ=; b=LQPfdxLhDn7sOeCMzMepX26e00eDPFp2eeCsR3UdtthcnAkTjlYlMGeYsR+Gg1CjtfB9fY YgBTOM6m4qnj+b9GNxjpVdORUaPk9DdBgs4c1HoUoNL9eCxljA+wlVn+/LYEMT+ZdttQ2B ZEadvOpnP2pQzj01ehQ0m9Q4gUw9k9g= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-187-4OuCvSLYOs-gCqMAjpEACQ-1; Tue, 19 May 2020 00:57:22 -0400 X-MC-Unique: 4OuCvSLYOs-gCqMAjpEACQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D820F8005AA; Tue, 19 May 2020 04:57:17 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BB5AC60BF3; Tue, 19 May 2020 04:57:17 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 97B5F4ED3C; Tue, 19 May 2020 04:57:17 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vGpQ019146 for ; Tue, 19 May 2020 00:57:16 -0400 Received: by smtp.corp.redhat.com (Postfix) id 7617F1002394; Tue, 19 May 2020 04:57:16 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9347110013D9; Tue, 19 May 2020 04:57:13 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vCtW001409; Mon, 18 May 2020 23:57:12 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4vB5k001408; Mon, 18 May 2020 23:57:11 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:04 -0500 Message-Id: <1589864228-1363-3-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck Subject: [dm-devel] [PATCH v2 2/6] libmultipath: make sysfs_is_multipathed able to return wwid X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com sysfs_is_multipathed reads the wwid of the dm device holding a path to check if its a multipath device. Add code to optinally set pp->wwid to that wwid. This will be used by a future patch. Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/sysfs.c | 24 +++++++++++++++++++----- libmultipath/sysfs.h | 2 +- multipath/main.c | 7 ++++--- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c index 62ec2ed7..12a82d95 100644 --- a/libmultipath/sysfs.c +++ b/libmultipath/sysfs.c @@ -295,7 +295,7 @@ static int select_dm_devs(const struct dirent *di) return fnmatch("dm-*", di->d_name, FNM_FILE_NAME) == 0; } -bool sysfs_is_multipathed(const struct path *pp) +bool sysfs_is_multipathed(struct path *pp, bool set_wwid) { char pathbuf[PATH_MAX]; struct scandir_result sr; @@ -325,7 +325,7 @@ bool sysfs_is_multipathed(const struct path *pp) for (i = 0; i < r && !found; i++) { long fd; int nr; - char uuid[6]; + char uuid[WWID_SIZE + UUID_PREFIX_LEN]; if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s/dm/uuid", di[i]->d_name)) @@ -339,12 +339,26 @@ bool sysfs_is_multipathed(const struct path *pp) pthread_cleanup_push(close_fd, (void *)fd); nr = read(fd, uuid, sizeof(uuid)); - if (nr == sizeof(uuid) && !memcmp(uuid, "mpath-", sizeof(uuid))) + if (nr > (int)UUID_PREFIX_LEN && + !memcmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN)) found = true; else if (nr < 0) { - condlog(1, "%s: error reading from %s: %s", - __func__, pathbuf, strerror(errno)); + condlog(1, "%s: error reading from %s: %m", + __func__, pathbuf); } + if (found && set_wwid) { + nr -= UUID_PREFIX_LEN; + memcpy(pp->wwid, uuid + UUID_PREFIX_LEN, nr); + if (nr == WWID_SIZE) { + condlog(4, "%s: overflow while reading from %s", + __func__, pathbuf); + pp->wwid[0] = '\0'; + } else { + pp->wwid[nr] = '\0'; + strchop(pp->wwid); + } + } + pthread_cleanup_pop(1); } pthread_cleanup_pop(1); diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h index 9ae30b39..72b39ab2 100644 --- a/libmultipath/sysfs.h +++ b/libmultipath/sysfs.h @@ -14,5 +14,5 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name, unsigned char * value, size_t value_len); int sysfs_get_size (struct path *pp, unsigned long long * size); int sysfs_check_holders(char * check_devt, char * new_devt); -bool sysfs_is_multipathed(const struct path *pp); +bool sysfs_is_multipathed(struct path *pp, bool set_wwid); #endif diff --git a/multipath/main.c b/multipath/main.c index 78822ee1..f25b8693 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -640,7 +640,8 @@ configure (struct config *conf, enum mpath_cmds cmd, * Shortcut for find_multipaths smart: * Quick check if path is already multipathed. */ - if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0))) { + if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0), + false)) { r = RTVL_YES; goto print_valid; } @@ -749,8 +750,8 @@ configure (struct config *conf, enum mpath_cmds cmd, /* * Check if we raced with multipathd */ - r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0)) ? - RTVL_YES : RTVL_NO; + r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0), + false) ? RTVL_YES : RTVL_NO; } goto print_valid; } From patchwork Tue May 19 04:57:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556913 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 42FF8913 for ; Tue, 19 May 2020 04:57:39 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E102E206D4 for ; Tue, 19 May 2020 04:57:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="ROtZ9FWN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E102E206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864257; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=yeoXGo/IAtt0GAK/bB5ZkqW3uRgUpS7AaezHPwH3syI=; b=ROtZ9FWNKO49MjZz+cI5xwvjvwbrfVbYCLZJmlPPgH/fZtzRa4o+xUUCRqiaza+Ep6SR6o +1OTc8UQCPAZqc1bHJJWaCy9xrpBqzboC1OVmp+5h2y4vlK3PmW8vzFPbLTuMhszQ+kcfj LbWlgmRFgqSMAA45hhY9/ndtKUt4F8s= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-140-g-f93McPP-OpXxJZBUjqUA-1; Tue, 19 May 2020 00:57:33 -0400 X-MC-Unique: g-f93McPP-OpXxJZBUjqUA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5B874107ACF7; Tue, 19 May 2020 04:57:29 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3C48C1001B2B; Tue, 19 May 2020 04:57:29 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 170264ED59; Tue, 19 May 2020 04:57:29 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vH6K019161 for ; Tue, 19 May 2020 00:57:17 -0400 Received: by smtp.corp.redhat.com (Postfix) id DA7E719C4F; Tue, 19 May 2020 04:57:17 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 18200196AE; Tue, 19 May 2020 04:57:15 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vDMG001413; Mon, 18 May 2020 23:57:13 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4vD0q001412; Mon, 18 May 2020 23:57:13 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:05 -0500 Message-Id: <1589864228-1363-4-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck Subject: [dm-devel] [PATCH v2 3/6] multipath: centralize validation code X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com This code pulls the multipath path validation code out of configure(), and puts it into its own function, check_path_valid(). This function calls a new libmultipath function, is_path_valid() to check just path requested. This seperation exists so that is_path_valid() can be reused by future code. This code will give almost the same answer as the existing code, with the exception that now, if a device is currently multipathed, it will always be a valid multipath path. Signed-off-by: Benjamin Marzinski --- libmultipath/Makefile | 3 +- libmultipath/devmapper.c | 45 ++++++ libmultipath/devmapper.h | 1 + libmultipath/structs.h | 24 +--- libmultipath/valid.c | 118 ++++++++++++++++ libmultipath/valid.h | 42 ++++++ libmultipath/wwids.c | 10 +- multipath/main.c | 296 +++++++++++++++++---------------------- 8 files changed, 344 insertions(+), 195 deletions(-) create mode 100644 libmultipath/valid.c create mode 100644 libmultipath/valid.h diff --git a/libmultipath/Makefile b/libmultipath/Makefile index ad690a49..436f479d 100644 --- a/libmultipath/Makefile +++ b/libmultipath/Makefile @@ -47,7 +47,8 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \ switchgroup.o uxsock.o print.o alias.o log_pthread.o \ log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \ lock.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \ - io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o + io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o \ + valid.o all: $(LIBS) diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 7ed494a1..27d52398 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -770,6 +770,51 @@ out: return r; } +/* + * Return + * 1 : map with uuid exists + * 0 : map with uuid doesn't exist + * -1 : error + */ +int +dm_map_present_by_uuid(const char *uuid) +{ + struct dm_task *dmt; + struct dm_info info; + char prefixed_uuid[WWID_SIZE + UUID_PREFIX_LEN]; + int r = -1; + + if (!uuid || uuid[0] == '\0') + return 0; + + if (safe_sprintf(prefixed_uuid, UUID_PREFIX "%s", uuid)) + goto out; + + if (!(dmt = dm_task_create(DM_DEVICE_INFO))) + goto out; + + dm_task_no_open_count(dmt); + + if (!dm_task_set_uuid(dmt, prefixed_uuid)) + goto out_task; + + if (!dm_task_run(dmt)) + goto out_task; + + if (!dm_task_get_info(dmt, &info)) + goto out_task; + + r = !!info.exists; + +out_task: + dm_task_destroy(dmt); +out: + if (r < 0) + condlog(3, "%s: dm command failed in %s: %s", uuid, + __FUNCTION__, strerror(errno)); + return r; +} + static int dm_dev_t (const char * mapname, char * dev_t, int len) { diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 17fc9faf..5ed7edc5 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -39,6 +39,7 @@ int dm_simplecmd_noflush (int, const char *, uint16_t); int dm_addmap_create (struct multipath *mpp, char *params); int dm_addmap_reload (struct multipath *mpp, char *params, int flush); int dm_map_present (const char *); +int dm_map_present_by_uuid(const char *uuid); int dm_get_map(const char *, unsigned long long *, char *); int dm_get_status(const char *, char *); int dm_type(const char *, char *); diff --git a/libmultipath/structs.h b/libmultipath/structs.h index 9bd39eb1..d69bc2e9 100644 --- a/libmultipath/structs.h +++ b/libmultipath/structs.h @@ -101,29 +101,13 @@ enum yes_no_undef_states { YNU_YES, }; -#define _FIND_MULTIPATHS_F (1 << 1) -#define _FIND_MULTIPATHS_I (1 << 2) -#define _FIND_MULTIPATHS_N (1 << 3) -/* - * _FIND_MULTIPATHS_F must have the same value as YNU_YES. - * Generate a compile time error if that isn't the case. - */ -extern char ___error1___[-(_FIND_MULTIPATHS_F != YNU_YES)]; - -#define find_multipaths_on(conf) \ - (!!((conf)->find_multipaths & _FIND_MULTIPATHS_F)) -#define ignore_wwids_on(conf) \ - (!!((conf)->find_multipaths & _FIND_MULTIPATHS_I)) -#define ignore_new_devs_on(conf) \ - (!!((conf)->find_multipaths & _FIND_MULTIPATHS_N)) - enum find_multipaths_states { FIND_MULTIPATHS_UNDEF = YNU_UNDEF, FIND_MULTIPATHS_OFF = YNU_NO, - FIND_MULTIPATHS_ON = _FIND_MULTIPATHS_F, - FIND_MULTIPATHS_GREEDY = _FIND_MULTIPATHS_I, - FIND_MULTIPATHS_SMART = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_I, - FIND_MULTIPATHS_STRICT = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_N, + FIND_MULTIPATHS_ON = YNU_YES, + FIND_MULTIPATHS_GREEDY, + FIND_MULTIPATHS_SMART, + FIND_MULTIPATHS_STRICT, __FIND_MULTIPATHS_LAST, }; diff --git a/libmultipath/valid.c b/libmultipath/valid.c new file mode 100644 index 00000000..456b1f6e --- /dev/null +++ b/libmultipath/valid.c @@ -0,0 +1,118 @@ +/* + Copyright (c) 2020 Benjamin Marzinski, IBM + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License + as published by the Free Software Foundation; either version 2 + of the License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + */ +#include +#include +#include + +#include "vector.h" +#include "config.h" +#include "debug.h" +#include "util.h" +#include "devmapper.h" +#include "discovery.h" +#include "wwids.h" +#include "sysfs.h" +#include "blacklist.h" +#include "mpath_cmd.h" +#include "valid.h" + +int +is_path_valid(const char *name, struct config *conf, struct path *pp, + bool check_multipathd) +{ + int r; + int fd; + + if (!pp || !name || !conf) + return PATH_IS_ERROR; + + if (conf->find_multipaths <= FIND_MULTIPATHS_UNDEF || + conf->find_multipaths >= __FIND_MULTIPATHS_LAST) + return PATH_IS_ERROR; + + if (safe_sprintf(pp->dev, "%s", name)) + return PATH_IS_ERROR; + + if (sysfs_is_multipathed(pp, true)) { + if (pp->wwid[0] == '\0') + return PATH_IS_ERROR; + return PATH_IS_VALID_NO_CHECK; + } + + /* + * "multipath -u" may be run before the daemon is started. In this + * case, systemd might own the socket but might delay multipathd + * startup until some other unit (udev settle!) has finished + * starting. With many LUNs, the listen backlog may be exceeded, which + * would cause connect() to block. This causes udev workers calling + * "multipath -u" to hang, and thus creates a deadlock, until "udev + * settle" times out. To avoid this, call connect() in non-blocking + * mode here, and take EAGAIN as indication for a filled-up systemd + * backlog. + */ + + if (check_multipathd) { + fd = __mpath_connect(1); + if (fd < 0) { + if (errno != EAGAIN && !systemd_service_enabled(name)) { + condlog(3, "multipathd not running or enabled"); + return PATH_IS_NOT_VALID; + } + } else + mpath_disconnect(fd); + } + + pp->udev = udev_device_new_from_subsystem_sysname(udev, "block", name); + if (!pp->udev) + return PATH_IS_ERROR; + + r = pathinfo(pp, conf, DI_SYSFS | DI_WWID | DI_BLACKLIST); + if (r == PATHINFO_SKIPPED) + return PATH_IS_NOT_VALID; + else if (r) + return PATH_IS_ERROR; + + if (pp->wwid[0] == '\0') + return PATH_IS_NOT_VALID; + + if (pp->udev && pp->uid_attribute && + filter_property(conf, pp->udev, 3, pp->uid_attribute) > 0) + return PATH_IS_NOT_VALID; + + r = is_failed_wwid(pp->wwid); + if (r != WWID_IS_NOT_FAILED) { + if (r == WWID_IS_FAILED) + return PATH_IS_NOT_VALID; + return PATH_IS_ERROR; + } + + if (conf->find_multipaths == FIND_MULTIPATHS_GREEDY) + return PATH_IS_VALID; + + if (check_wwids_file(pp->wwid, 0) == 0) + return PATH_IS_VALID_NO_CHECK; + + if (dm_map_present_by_uuid(pp->wwid) == 1) + return PATH_IS_VALID; + + /* all these act like FIND_MULTIPATHS_STRICT for finding if a + * path is valid */ + if (conf->find_multipaths != FIND_MULTIPATHS_SMART) + return PATH_IS_NOT_VALID; + + return PATH_IS_MAYBE_VALID; +} diff --git a/libmultipath/valid.h b/libmultipath/valid.h new file mode 100644 index 00000000..ce1c7cbf --- /dev/null +++ b/libmultipath/valid.h @@ -0,0 +1,42 @@ +/* + Copyright (c) 2020 Benjamin Marzinski, IBM + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License + as published by the Free Software Foundation; either version 2 + of the License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + */ +#ifndef _VALID_H +#define _VALID_H + +/* + * PATH_IS_VALID_NO_CHECK is returned when multipath should claim + * the path, regardless of whether is has been released to systemd + * already. + * PATH_IS_VALID is returned by is_path_valid, when the path is + * valid only if it hasn't been released to systemd already. + * PATH_IS_MAYBE_VALID is returned when the the path would be valid + * if other paths with the same wwid existed. It is up to the caller + * to check for these other paths. + */ +enum is_path_valid_result { + PATH_IS_ERROR = -1, + PATH_IS_NOT_VALID, + PATH_IS_VALID, + PATH_IS_VALID_NO_CHECK, + PATH_IS_MAYBE_VALID, + PATH_MAX_VALID_RESULT, /* only for bounds checking */ +}; + +int is_path_valid(const char *name, struct config *conf, struct path *pp, + bool check_multipathd); + +#endif /* _VALID_D */ diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c index fab6fc8f..397f6e54 100644 --- a/libmultipath/wwids.c +++ b/libmultipath/wwids.c @@ -289,19 +289,19 @@ out: int should_multipath(struct path *pp1, vector pathvec, vector mpvec) { - int i, ignore_new_devs, find_multipaths; + int i, find_multipaths; struct path *pp2; struct config *conf; conf = get_multipath_config(); - ignore_new_devs = ignore_new_devs_on(conf); - find_multipaths = find_multipaths_on(conf); + find_multipaths = conf->find_multipaths; put_multipath_config(conf); - if (!find_multipaths && !ignore_new_devs) + if (find_multipaths == FIND_MULTIPATHS_OFF || + find_multipaths == FIND_MULTIPATHS_GREEDY) return 1; condlog(4, "checking if %s should be multipathed", pp1->dev); - if (!ignore_new_devs) { + if (find_multipaths != FIND_MULTIPATHS_STRICT) { char tmp_wwid[WWID_SIZE]; struct multipath *mp = find_mp_by_wwid(mpvec, pp1->wwid); diff --git a/multipath/main.c b/multipath/main.c index f25b8693..5eac757a 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -63,21 +63,18 @@ #include "propsel.h" #include "time-util.h" #include "file.h" +#include "valid.h" int logsink; struct udev *udev; struct config *multipath_conf; /* - * Return values of configure(), print_cmd_valid(), and main(). - * RTVL_{YES,NO} are synonyms for RTVL_{OK,FAIL} for the CMD_VALID_PATH case. + * Return values of configure(), check_path_valid(), and main(). */ enum { RTVL_OK = 0, - RTVL_YES = RTVL_OK, RTVL_FAIL = 1, - RTVL_NO = RTVL_FAIL, - RTVL_MAYBE, /* only used internally, never returned */ RTVL_RETRY, /* returned by configure(), not by main() */ }; @@ -271,9 +268,6 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid) continue; } - if (cmd == CMD_VALID_PATH) - continue; - dm_get_map(mpp->alias, &mpp->size, params); condlog(3, "params = %s", params); dm_get_status(mpp->alias, status); @@ -493,10 +487,11 @@ static int print_cmd_valid(int k, const vector pathvec, struct timespec until; struct path *pp; - if (k != RTVL_YES && k != RTVL_NO && k != RTVL_MAYBE) - return RTVL_NO; + if (k != PATH_IS_VALID && k != PATH_IS_NOT_VALID && + k != PATH_IS_MAYBE_VALID) + return PATH_IS_NOT_VALID; - if (k == RTVL_MAYBE) { + if (k == PATH_IS_MAYBE_VALID) { /* * Caller ensures that pathvec[0] is the path to * examine. @@ -506,7 +501,7 @@ static int print_cmd_valid(int k, const vector pathvec, wait = find_multipaths_check_timeout( pp, pp->find_multipaths_timeout, &until); if (wait != FIND_MULTIPATHS_WAITING) - k = RTVL_NO; + k = PATH_IS_NOT_VALID; } else if (pathvec != NULL && (pp = VECTOR_SLOT(pathvec, 0))) wait = find_multipaths_check_timeout(pp, 0, &until); if (wait == FIND_MULTIPATHS_WAITING) @@ -515,9 +510,9 @@ static int print_cmd_valid(int k, const vector pathvec, else if (wait == FIND_MULTIPATHS_WAIT_DONE) printf("FIND_MULTIPATHS_WAIT_UNTIL=\"0\"\n"); printf("DM_MULTIPATH_DEVICE_PATH=\"%d\"\n", - k == RTVL_MAYBE ? 2 : k == RTVL_YES ? 1 : 0); + k == PATH_IS_MAYBE_VALID ? 2 : k == PATH_IS_VALID ? 1 : 0); /* Never return RTVL_MAYBE */ - return k == RTVL_NO ? RTVL_NO : RTVL_YES; + return k == PATH_IS_NOT_VALID ? PATH_IS_NOT_VALID : PATH_IS_VALID; } /* @@ -550,7 +545,6 @@ configure (struct config *conf, enum mpath_cmds cmd, int di_flag = 0; char * refwwid = NULL; char * dev = NULL; - bool released = released_to_systemd(); /* * allocate core vectors to store paths and multipaths @@ -575,7 +569,7 @@ configure (struct config *conf, enum mpath_cmds cmd, cmd != CMD_REMOVE_WWID && (filter_devnode(conf->blist_devnode, conf->elist_devnode, dev) > 0)) { - goto print_valid; + goto out; } /* @@ -583,14 +577,10 @@ configure (struct config *conf, enum mpath_cmds cmd, * failing the translation is fatal (by policy) */ if (devpath) { - int failed = get_refwwid(cmd, devpath, dev_type, - pathvec, &refwwid); + get_refwwid(cmd, devpath, dev_type, pathvec, &refwwid); if (!refwwid) { condlog(4, "%s: failed to get wwid", devpath); - if (failed == 2 && cmd == CMD_VALID_PATH) - goto print_valid; - else - condlog(3, "scope is null"); + condlog(3, "scope is null"); goto out; } if (cmd == CMD_REMOVE_WWID) { @@ -616,53 +606,6 @@ configure (struct config *conf, enum mpath_cmds cmd, goto out; } condlog(3, "scope limited to %s", refwwid); - /* If you are ignoring the wwids file and find_multipaths is - * set, you need to actually check if there are two available - * paths to determine if this path should be multipathed. To - * do this, we put off the check until after discovering all - * the paths. - * Paths listed in the wwids file are always considered valid. - */ - if (cmd == CMD_VALID_PATH) { - if (is_failed_wwid(refwwid) == WWID_IS_FAILED) { - r = RTVL_NO; - goto print_valid; - } - if ((!find_multipaths_on(conf) && - ignore_wwids_on(conf)) || - check_wwids_file(refwwid, 0) == 0) - r = RTVL_YES; - if (!ignore_wwids_on(conf)) - goto print_valid; - /* At this point, either r==0 or find_multipaths_on. */ - - /* - * Shortcut for find_multipaths smart: - * Quick check if path is already multipathed. - */ - if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0), - false)) { - r = RTVL_YES; - goto print_valid; - } - - /* - * DM_MULTIPATH_DEVICE_PATH=="0" means that we have - * been called for this device already, and have - * released it to systemd. Unless the device is now - * already multipathed (see above), we can't try to - * grab it, because setting SYSTEMD_READY=0 would - * cause file systems to be unmounted. - * Leave DM_MULTIPATH_DEVICE_PATH="0". - */ - if (released) { - r = RTVL_NO; - goto print_valid; - } - if (r == RTVL_YES) - goto print_valid; - /* find_multipaths_on: Fall through to path detection */ - } } /* @@ -703,59 +646,6 @@ configure (struct config *conf, enum mpath_cmds cmd, goto out; } - if (cmd == CMD_VALID_PATH) { - struct path *pp; - int fd; - - /* This only happens if find_multipaths and - * ignore_wwids is set, and the path is not in WWIDs - * file, not currently multipathed, and has - * never been released to systemd. - * If there is currently a multipath device matching - * the refwwid, or there is more than one path matching - * the refwwid, then the path is valid */ - if (VECTOR_SIZE(curmp) != 0) { - r = RTVL_YES; - goto print_valid; - } else if (VECTOR_SIZE(pathvec) > 1) - r = RTVL_YES; - else - r = RTVL_MAYBE; - - /* - * If opening the path with O_EXCL fails, the path - * is in use (e.g. mounted during initramfs processing). - * We know that it's not used by dm-multipath. - * We may not set SYSTEMD_READY=0 on such devices, it - * might cause systemd to umount the device. - * Use O_RDONLY, because udevd would trigger another - * uevent for close-after-write. - * - * The O_EXCL check is potentially dangerous, because it may - * race with other tasks trying to access the device. Therefore - * this code is only executed if the path hasn't been released - * to systemd earlier (see above). - * - * get_refwwid() above stores the path we examine in slot 0. - */ - pp = VECTOR_SLOT(pathvec, 0); - fd = open(udev_device_get_devnode(pp->udev), - O_RDONLY|O_EXCL); - if (fd >= 0) - close(fd); - else { - condlog(3, "%s: path %s is in use: %s", - __func__, pp->dev, - strerror(errno)); - /* - * Check if we raced with multipathd - */ - r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0), - false) ? RTVL_YES : RTVL_NO; - } - goto print_valid; - } - if (cmd != CMD_CREATE && cmd != CMD_DRY_RUN) { r = RTVL_OK; goto out; @@ -768,10 +658,6 @@ configure (struct config *conf, enum mpath_cmds cmd, conf->force_reload, cmd); r = rc == CP_RETRY ? RTVL_RETRY : rc == CP_OK ? RTVL_OK : RTVL_FAIL; -print_valid: - if (cmd == CMD_VALID_PATH) - r = print_cmd_valid(r, pathvec, conf); - out: if (refwwid) FREE(refwwid); @@ -782,6 +668,112 @@ out: return r; } +static int +check_path_valid(const char *name, struct config *conf, bool is_uevent) +{ + int fd, r = PATH_IS_ERROR; + struct path *pp = NULL; + vector pathvec = NULL; + + pp = alloc_path(); + if (!pp) + return RTVL_FAIL; + + r = is_path_valid(name, conf, pp, is_uevent); + if (r <= PATH_IS_ERROR || r >= PATH_MAX_VALID_RESULT) + goto fail; + + /* set path values if is_path_valid() didn't */ + if (!pp->udev) + pp->udev = udev_device_new_from_subsystem_sysname(udev, "block", + name); + if (!pp->udev) + goto fail; + + if (!strlen(pp->dev_t)) { + dev_t devt = udev_device_get_devnum(pp->udev); + if (major(devt) == 0 && minor(devt) == 0) + goto fail; + snprintf(pp->dev_t, BLK_DEV_SIZE, "%d:%d", major(devt), + minor(devt)); + } + + pathvec = vector_alloc(); + if (!pathvec) + goto fail; + + if (store_path(pathvec, pp) != 0) { + free_path(pp); + goto fail; + } + + if ((r == PATH_IS_VALID || r == PATH_IS_MAYBE_VALID) && + released_to_systemd()) + r = PATH_IS_NOT_VALID; + + /* This state is only used to skip the released_to_systemd() check */ + if (r == PATH_IS_VALID_NO_CHECK) + r = PATH_IS_VALID; + + if (r != PATH_IS_MAYBE_VALID) + goto out; + + /* + * If opening the path with O_EXCL fails, the path + * is in use (e.g. mounted during initramfs processing). + * We know that it's not used by dm-multipath. + * We may not set SYSTEMD_READY=0 on such devices, it + * might cause systemd to umount the device. + * Use O_RDONLY, because udevd would trigger another + * uevent for close-after-write. + * + * The O_EXCL check is potentially dangerous, because it may + * race with other tasks trying to access the device. Therefore + * this code is only executed if the path hasn't been released + * to systemd earlier (see above). + */ + fd = open(udev_device_get_devnode(pp->udev), O_RDONLY|O_EXCL); + if (fd >= 0) + close(fd); + else { + condlog(3, "%s: path %s is in use: %m", __func__, pp->dev); + /* Check if we raced with multipathd */ + if (sysfs_is_multipathed(pp, false)) + r = PATH_IS_VALID; + else + r = PATH_IS_NOT_VALID; + goto out; + } + + /* For find_multipaths = SMART, if there is more than one path + * matching the refwwid, then the path is valid */ + if (path_discovery(pathvec, DI_SYSFS | DI_WWID) < 0) + goto fail; + filter_pathvec(pathvec, pp->wwid); + if (VECTOR_SIZE(pathvec) > 1) + r = PATH_IS_VALID; + else + r = PATH_IS_MAYBE_VALID; + +out: + r = print_cmd_valid(r, pathvec, conf); + free_pathvec(pathvec, FREE_PATHS); + /* + * multipath -u must exit with status 0, otherwise udev won't + * import its output. + */ + if (!is_uevent && r == PATH_IS_NOT_VALID) + return RTVL_FAIL; + return RTVL_OK; + +fail: + if (pathvec) + free_pathvec(pathvec, FREE_PATHS); + else + free_path(pp); + return RTVL_FAIL; +} + static int get_dev_type(char *dev) { struct stat buf; @@ -863,32 +855,6 @@ out: return r; } -static int test_multipathd_socket(void) -{ - int fd; - /* - * "multipath -u" may be run before the daemon is started. In this - * case, systemd might own the socket but might delay multipathd - * startup until some other unit (udev settle!) has finished - * starting. With many LUNs, the listen backlog may be exceeded, which - * would cause connect() to block. This causes udev workers calling - * "multipath -u" to hang, and thus creates a deadlock, until "udev - * settle" times out. To avoid this, call connect() in non-blocking - * mode here, and take EAGAIN as indication for a filled-up systemd - * backlog. - */ - - fd = __mpath_connect(1); - if (fd == -1) { - if (errno == EAGAIN) - condlog(3, "daemon backlog exceeded"); - else - return 0; - } else - close(fd); - return 1; -} - int main (int argc, char *argv[]) { @@ -972,7 +938,11 @@ main (int argc, char *argv[]) conf->force_reload = FORCE_RELOAD_YES; break; case 'i': - conf->find_multipaths |= _FIND_MULTIPATHS_I; + if (conf->find_multipaths == FIND_MULTIPATHS_ON || + conf->find_multipaths == FIND_MULTIPATHS_STRICT) + conf->find_multipaths = FIND_MULTIPATHS_SMART; + else if (conf->find_multipaths == FIND_MULTIPATHS_OFF) + conf->find_multipaths = FIND_MULTIPATHS_GREEDY; break; case 't': r = dump_config(conf, NULL, NULL) ? RTVL_FAIL : RTVL_OK; @@ -1070,15 +1040,10 @@ main (int argc, char *argv[]) condlog(0, "the -c option requires a path to check"); goto out; } - if (cmd == CMD_VALID_PATH && - dev_type == DEV_UEVENT) { - if (!test_multipathd_socket()) { - condlog(3, "%s: daemon is not running", dev); - if (!systemd_service_enabled(dev)) { - r = print_cmd_valid(RTVL_NO, NULL, conf); - goto out; - } - } + if (cmd == CMD_VALID_PATH) { + char * name = convert_dev(dev, (dev_type == DEV_DEVNODE)); + r = check_path_valid(name, conf, dev_type == DEV_UEVENT); + goto out; } if (cmd == CMD_REMOVE_WWID && !dev) { @@ -1142,13 +1107,6 @@ out: cleanup_prio(); cleanup_checkers(); - /* - * multipath -u must exit with status 0, otherwise udev won't - * import its output. - */ - if (cmd == CMD_VALID_PATH && dev_type == DEV_UEVENT && r == RTVL_NO) - r = RTVL_OK; - if (dev_type == DEV_UEVENT) closelog(); From patchwork Tue May 19 04:57:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556903 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 13D5C618 for ; Tue, 19 May 2020 04:57:28 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B827D206D4 for ; Tue, 19 May 2020 04:57:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="NwYFOIkV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B827D206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864246; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=9lKgY8UUoFF2yaPl9ayfzjvXFZOosDyV71NVBKFHkkk=; b=NwYFOIkVZx8kVxZJmRK6rdcX6FlEKPxXdb55jTefohvuZFUjxaFDzC6B5g9yCUiXxIV6uk 4g2UYShPAQ2N0xTpJ18yq9VfML9cdo60roAYkkaLYYnUQh2+H8AVKhZpgNUg3R9AhME/A5 9jyBK8iLvTof72gUyugZ5CeETNmc9P8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-303-A3OPmGZjPCy2p6RlCbTknA-1; Tue, 19 May 2020 00:57:23 -0400 X-MC-Unique: A3OPmGZjPCy2p6RlCbTknA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 181311009441; Tue, 19 May 2020 04:57:19 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F031E7055D; Tue, 19 May 2020 04:57:18 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id C972B1809547; Tue, 19 May 2020 04:57:18 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vH9k019160 for ; Tue, 19 May 2020 00:57:17 -0400 Received: by smtp.corp.redhat.com (Postfix) id DA853397; Tue, 19 May 2020 04:57:17 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9EF45398; Tue, 19 May 2020 04:57:16 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vFuQ001417; Mon, 18 May 2020 23:57:15 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4vEML001416; Mon, 18 May 2020 23:57:14 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:06 -0500 Message-Id: <1589864228-1363-5-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck Subject: [dm-devel] [PATCH v2 4/6] Unit tests for is_path_valid() X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Signed-off-by: Benjamin Marzinski --- tests/Makefile | 4 +- tests/valid.c | 486 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 489 insertions(+), 1 deletion(-) create mode 100644 tests/valid.c diff --git a/tests/Makefile b/tests/Makefile index 77ff3249..7fc261c3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -13,7 +13,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \ LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \ - alias directio + alias directio valid .SILENT: $(TESTS:%=%.o) .PRECIOUS: $(TESTS:%=%-test) @@ -50,6 +50,8 @@ vpd-test_OBJDEPS := ../libmultipath/discovery.o vpd-test_LIBDEPS := -ludev -lpthread -ldl alias-test_TESTDEPS := test-log.o alias-test_LIBDEPS := -lpthread -ldl +valid-test_OBJDEPS := ../libmultipath/valid.o +valid-test_LIBDEPS := -ludev -lpthread -ldl ifneq ($(DIO_TEST_DEV),) directio-test_LIBDEPS := -laio endif diff --git a/tests/valid.c b/tests/valid.c new file mode 100644 index 00000000..693c72c5 --- /dev/null +++ b/tests/valid.c @@ -0,0 +1,486 @@ +/* + * Copyright (c) 2020 Benjamin Marzinski, Redhat + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include "globals.c" +#include "util.h" +#include "discovery.h" +#include "wwids.h" +#include "blacklist.h" +#include "valid.h" + +int test_fd; +struct udev_device { + int unused; +} test_udev; + +bool __wrap_sysfs_is_multipathed(struct path *pp, bool set_wwid) +{ + bool is_multipathed = mock_type(bool); + assert_non_null(pp); + assert_int_not_equal(strlen(pp->dev), 0); + if (is_multipathed && set_wwid) + strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE); + return is_multipathed; +} + +int __wrap___mpath_connect(int nonblocking) +{ + bool connected = mock_type(bool); + assert_int_equal(nonblocking, 1); + if (connected) + return test_fd; + errno = mock_type(int); + return -1; +} + +int __wrap_systemd_service_enabled(const char *dev) +{ + return (int)mock_type(bool); +} + +/* There's no point in checking the return value here */ +int __wrap_mpath_disconnect(int fd) +{ + assert_int_equal(fd, test_fd); + return 0; +} + +struct udev_device *__wrap_udev_device_new_from_subsystem_sysname(struct udev *udev, const char *subsystem, const char *sysname) +{ + bool passed = mock_type(bool); + assert_string_equal(sysname, mock_ptr_type(char *)); + if (passed) + return &test_udev; + return NULL; +} + +int __wrap_pathinfo(struct path *pp, struct config *conf, int mask) +{ + int ret = mock_type(int); + assert_string_equal(pp->dev, mock_ptr_type(char *)); + assert_int_equal(mask, DI_SYSFS | DI_WWID | DI_BLACKLIST); + if (ret == PATHINFO_OK) { + pp->uid_attribute = "ID_TEST"; + strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE); + } else + memset(pp->wwid, 0, WWID_SIZE); + return ret; +} + +int __wrap_filter_property(struct config *conf, struct udev_device *udev, + int lvl, const char *uid_attribute) +{ + int ret = mock_type(int); + assert_string_equal(uid_attribute, "ID_TEST"); + return ret; +} + +int __wrap_is_failed_wwid(const char *wwid) +{ + int ret = mock_type(int); + assert_string_equal(wwid, mock_ptr_type(char *)); + return ret; +} + +int __wrap_check_wwids_file(char *wwid, int write_wwid) +{ + bool passed = mock_type(bool); + assert_int_equal(write_wwid, 0); + assert_string_equal(wwid, mock_ptr_type(char *)); + if (passed) + return 0; + else + return -1; +} + +int __wrap_dm_map_present_by_uuid(const char *uuid) +{ + int ret = mock_type(int); + assert_string_equal(uuid, mock_ptr_type(char *)); + return ret; +} + +enum { + STAGE_IS_MULTIPATHED, + STAGE_CHECK_MULTIPATHD, + STAGE_GET_UDEV_DEVICE, + STAGE_PATHINFO, + STAGE_FILTER_PROPERTY, + STAGE_IS_FAILED, + STAGE_CHECK_WWIDS, + STAGE_UUID_PRESENT, +}; + +enum { + CHECK_MPATHD_RUNNING, + CHECK_MPATHD_EAGAIN, + CHECK_MPATHD_ENABLED, + CHECK_MPATHD_SKIP, +}; + +/* setup the test to continue past the given stage in is_path_valid() */ +static void setup_passing(char *name, char *wwid, unsigned int check_multipathd, + unsigned int stage) +{ + will_return(__wrap_sysfs_is_multipathed, false); + if (stage == STAGE_IS_MULTIPATHED) + return; + if (check_multipathd == CHECK_MPATHD_RUNNING) + will_return(__wrap___mpath_connect, true); + else if (check_multipathd == CHECK_MPATHD_EAGAIN) { + will_return(__wrap___mpath_connect, false); + will_return(__wrap___mpath_connect, EAGAIN); + } else if (check_multipathd == CHECK_MPATHD_ENABLED) { + will_return(__wrap___mpath_connect, false); + will_return(__wrap___mpath_connect, ECONNREFUSED); + will_return(__wrap_systemd_service_enabled, true); + } + /* nothing for CHECK_MPATHD_SKIP */ + if (stage == STAGE_CHECK_MULTIPATHD) + return; + will_return(__wrap_udev_device_new_from_subsystem_sysname, true); + will_return(__wrap_udev_device_new_from_subsystem_sysname, + name); + if (stage == STAGE_GET_UDEV_DEVICE) + return; + will_return(__wrap_pathinfo, PATHINFO_OK); + will_return(__wrap_pathinfo, name); + will_return(__wrap_pathinfo, wwid); + if (stage == STAGE_PATHINFO) + return; + will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_EXCEPT); + if (stage == STAGE_FILTER_PROPERTY) + return; + will_return(__wrap_is_failed_wwid, WWID_IS_NOT_FAILED); + will_return(__wrap_is_failed_wwid, wwid); + if (stage == STAGE_IS_FAILED) + return; + will_return(__wrap_check_wwids_file, false); + will_return(__wrap_check_wwids_file, wwid); + if (stage == STAGE_CHECK_WWIDS) + return; + will_return(__wrap_dm_map_present_by_uuid, 0); + will_return(__wrap_dm_map_present_by_uuid, wwid); +} + +static void test_bad_arguments(void **state) +{ + struct path pp; + char too_long[FILE_NAME_SIZE + 1]; + + memset(&pp, 0, sizeof(pp)); + /* test NULL pointers */ + assert_int_equal(is_path_valid("test", &conf, NULL, true), + PATH_IS_ERROR); + assert_int_equal(is_path_valid("test", NULL, &pp, true), + PATH_IS_ERROR); + assert_int_equal(is_path_valid(NULL, &conf, &pp, true), + PATH_IS_ERROR); + /* test undefined find_multipaths */ + conf.find_multipaths = FIND_MULTIPATHS_UNDEF; + assert_int_equal(is_path_valid("test", &conf, &pp, true), + PATH_IS_ERROR); + /* test name too long */ + memset(too_long, 'x', sizeof(too_long)); + too_long[sizeof(too_long) - 1] = '\0'; + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + assert_int_equal(is_path_valid(too_long, &conf, &pp, true), + PATH_IS_ERROR); +} + +static void test_sysfs_is_multipathed(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test_wwid"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + /* test for already existing multiapthed device */ + will_return(__wrap_sysfs_is_multipathed, true); + will_return(__wrap_sysfs_is_multipathed, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_VALID_NO_CHECK); + assert_string_equal(pp.dev, name); + assert_string_equal(pp.wwid, wwid); + /* test for wwid device with empty wwid */ + will_return(__wrap_sysfs_is_multipathed, true); + will_return(__wrap_sysfs_is_multipathed, ""); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_ERROR); +} + +static void test_check_multipathd(void **state) +{ + struct path pp; + char *name = "test"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + /* test failed check to see if multipathd is active */ + will_return(__wrap_sysfs_is_multipathed, false); + will_return(__wrap___mpath_connect, false); + will_return(__wrap___mpath_connect, ECONNREFUSED); + will_return(__wrap_systemd_service_enabled, false); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_NOT_VALID); + assert_string_equal(pp.dev, name); + /* test pass because service is enabled. fail getting udev */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, NULL, CHECK_MPATHD_ENABLED, STAGE_CHECK_MULTIPATHD); + will_return(__wrap_udev_device_new_from_subsystem_sysname, false); + will_return(__wrap_udev_device_new_from_subsystem_sysname, + name); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_ERROR); + assert_string_equal(pp.dev, name); + /* test pass because connect returned EAGAIN. fail getting udev */ + setup_passing(name, NULL, CHECK_MPATHD_EAGAIN, STAGE_CHECK_MULTIPATHD); + will_return(__wrap_udev_device_new_from_subsystem_sysname, false); + will_return(__wrap_udev_device_new_from_subsystem_sysname, + name); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_ERROR); + /* test pass because connect succeeded. fail getting udev */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, NULL, CHECK_MPATHD_RUNNING, STAGE_CHECK_MULTIPATHD); + will_return(__wrap_udev_device_new_from_subsystem_sysname, false); + will_return(__wrap_udev_device_new_from_subsystem_sysname, + name); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_ERROR); + assert_string_equal(pp.dev, name); +} + +static void test_pathinfo(void **state) +{ + struct path pp; + char *name = "test"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + /* Test pathinfo blacklisting device */ + setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE); + will_return(__wrap_pathinfo, PATHINFO_SKIPPED); + will_return(__wrap_pathinfo, name); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + /* Test pathinfo failing */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE); + will_return(__wrap_pathinfo, PATHINFO_FAILED); + will_return(__wrap_pathinfo, name); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_ERROR); + /* Test blank wwid */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE); + will_return(__wrap_pathinfo, PATHINFO_OK); + will_return(__wrap_pathinfo, name); + will_return(__wrap_pathinfo, ""); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); +} + +static void test_filter_property(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + /* test blacklist property */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO); + will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); + /* test missing property */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO); + will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_MISSING); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + /* test MATCH_NOTHING fail on is_failed_wwid */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO); + will_return(__wrap_filter_property, MATCH_NOTHING); + will_return(__wrap_is_failed_wwid, WWID_IS_FAILED); + will_return(__wrap_is_failed_wwid, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); +} + +static void test_is_failed_wwid(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + /* Test wwid failed */ + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_FILTER_PROPERTY); + will_return(__wrap_is_failed_wwid, WWID_IS_FAILED); + will_return(__wrap_is_failed_wwid, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); + /* test is_failed_wwid error */ + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_FILTER_PROPERTY); + will_return(__wrap_is_failed_wwid, WWID_FAILED_ERROR); + will_return(__wrap_is_failed_wwid, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_ERROR); +} + +static void test_greedy(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + /* test greedy success with checking multipathd */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_GREEDY; + setup_passing(name, wwid, CHECK_MPATHD_RUNNING, STAGE_IS_FAILED); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); + /* test greedy success without checking multiapthd */ + memset(&pp, 0, sizeof(pp)); + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_IS_FAILED); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_VALID); +} + +static void test_check_wwids(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + setup_passing(name, wwid, CHECK_MPATHD_EAGAIN, STAGE_IS_FAILED); + will_return(__wrap_check_wwids_file, true); + will_return(__wrap_check_wwids_file, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_VALID_NO_CHECK); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); +} + +static void test_check_uuid_present(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + setup_passing(name, wwid, CHECK_MPATHD_ENABLED, STAGE_CHECK_WWIDS); + will_return(__wrap_dm_map_present_by_uuid, 1); + will_return(__wrap_dm_map_present_by_uuid, wwid); + assert_int_equal(is_path_valid(name, &conf, &pp, true), + PATH_IS_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); +} + + +static void test_find_multipaths(void **state) +{ + struct path pp; + char *name = "test"; + char *wwid = "test-wwid"; + + /* test find_multipaths = FIND_MULTIPATHS_STRICT */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_STRICT; + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); + /* test find_multipaths = FIND_MULTIPATHS_OFF */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_OFF; + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + /* test find_multipaths = FIND_MULTIPATHS_ON */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_ON; + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_NOT_VALID); + /* test find_multipaths = FIND_MULTIPATHS_SMART */ + memset(&pp, 0, sizeof(pp)); + conf.find_multipaths = FIND_MULTIPATHS_SMART; + setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT); + assert_int_equal(is_path_valid(name, &conf, &pp, false), + PATH_IS_MAYBE_VALID); + assert_string_equal(pp.dev, name); + assert_ptr_equal(pp.udev, &test_udev); + assert_string_equal(pp.wwid, wwid); +} + +int test_valid(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_bad_arguments), + cmocka_unit_test(test_sysfs_is_multipathed), + cmocka_unit_test(test_check_multipathd), + cmocka_unit_test(test_pathinfo), + cmocka_unit_test(test_filter_property), + cmocka_unit_test(test_is_failed_wwid), + cmocka_unit_test(test_greedy), + cmocka_unit_test(test_check_wwids), + cmocka_unit_test(test_check_uuid_present), + cmocka_unit_test(test_find_multipaths), + }; + return cmocka_run_group_tests(tests, NULL, NULL); +} + +int main(void) +{ + int ret = 0; + ret += test_valid(); + return ret; +} From patchwork Tue May 19 04:57:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556911 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B8DED618 for ; Tue, 19 May 2020 04:57:38 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7B6DA206D4 for ; Tue, 19 May 2020 04:57:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="WZzZOkP7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7B6DA206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864257; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=iAS5pudMyNaVapQQzkk9bgacLxAauabAkIid/Cu1fkE=; b=WZzZOkP74q+anW36nqD3h1ziQ1n+4QEkEMWEbMk0UXLy/3aavwu5K5T7qsJG1h3KXgqpdx WNF0b9CbgL882ScXqd7t0F1hs63jjFhxUka//pM+NwWXQ/DHVEFNvihv/M4fYyx8IM3rix HAD3U+pXO5aBTyHXkpBs59GEwMjcliw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-147-953JKuX3OLe7XkaXyIEKZg-1; Tue, 19 May 2020 00:57:35 -0400 X-MC-Unique: 953JKuX3OLe7XkaXyIEKZg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 17BA88005AA; Tue, 19 May 2020 04:57:31 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F31B06EA0C; Tue, 19 May 2020 04:57:30 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id CC1C4180954D; Tue, 19 May 2020 04:57:30 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vKLU019197 for ; Tue, 19 May 2020 00:57:20 -0400 Received: by smtp.corp.redhat.com (Postfix) id D783C7055D; Tue, 19 May 2020 04:57:20 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 34163579A6; Tue, 19 May 2020 04:57:18 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vGIC001421; Mon, 18 May 2020 23:57:16 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4vGP0001420; Mon, 18 May 2020 23:57:16 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:07 -0500 Message-Id: <1589864228-1363-6-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck Subject: [dm-devel] [PATCH v2 5/6] libmultipath: simplify failed wwid code X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com The (is|mark|unmark)_failed_wwid code is needlessly complicated. Locking a file is necssary if multiple processes could otherwise be writing to it at the same time. That is not the case with the failed_wwids files. They can simply be empty files in a directory. Even with all the locking in place, two processes accessing or modifying a file at the same time will still race. And even without the locking, if two processes try to access or modify a file at the same time, they will both see a reasonable result, and will leave the files in a valid state afterwards. Instead of doing all the locking work (which made it necessary to write a file, even just to check if a file existed), simply check for files with lstat(), create them with open(), and remove them with unlink(). Reviewed-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/wwids.c | 131 ++++++++++++++++++------------------------- 1 file changed, 56 insertions(+), 75 deletions(-) diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c index 397f6e54..da55924b 100644 --- a/libmultipath/wwids.c +++ b/libmultipath/wwids.c @@ -6,6 +6,7 @@ #include #include #include +#include #include "util.h" #include "checkers.h" @@ -348,113 +349,93 @@ remember_wwid(char *wwid) } static const char shm_dir[] = MULTIPATH_SHM_BASE "failed_wwids"; -static const char shm_lock[] = ".lock"; -static const char shm_header[] = "multipath shm lock file, don't edit"; -static char _shm_lock_path[sizeof(shm_dir)+sizeof(shm_lock)]; -static const char *shm_lock_path = &_shm_lock_path[0]; -static void init_shm_paths(void) +static void print_failed_wwid_result(const char * msg, const char *wwid, int r) { - snprintf(_shm_lock_path, sizeof(_shm_lock_path), - "%s/%s", shm_dir, shm_lock); + switch(r) { + case WWID_FAILED_ERROR: + condlog(1, "%s: %s: %m", msg, wwid); + return; + case WWID_IS_FAILED: + case WWID_IS_NOT_FAILED: + condlog(4, "%s: %s is %s", msg, wwid, + r == WWID_IS_FAILED ? "failed" : "good"); + return; + case WWID_FAILED_CHANGED: + condlog(3, "%s: %s", msg, wwid); + } } -static pthread_once_t shm_path_once = PTHREAD_ONCE_INIT; - -static int multipath_shm_open(bool rw) +int is_failed_wwid(const char *wwid) { - int fd; - int can_write; - - pthread_once(&shm_path_once, init_shm_paths); - fd = open_file(shm_lock_path, &can_write, shm_header); + struct stat st; + char path[PATH_MAX]; + int r; - if (fd >= 0 && rw && !can_write) { - close(fd); - condlog(1, "failed to open %s for writing", shm_dir); + if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { + condlog(1, "%s: path name overflow", __func__); return -1; } - return fd; -} - -static void multipath_shm_close(void *arg) -{ - long fd = (long)arg; + if (lstat(path, &st) == 0) + r = WWID_IS_FAILED; + else if (errno == ENOENT) + r = WWID_IS_NOT_FAILED; + else + r = WWID_FAILED_ERROR; - close(fd); - unlink(shm_lock_path); + print_failed_wwid_result("is_failed", wwid, r); + return r; } -static int _failed_wwid_op(const char *wwid, bool rw, - int (*func)(const char *), const char *msg) +int mark_failed_wwid(const char *wwid) { char path[PATH_MAX]; - long lockfd; - int r = -1; + int r, fd; if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { condlog(1, "%s: path name overflow", __func__); return -1; } - - lockfd = multipath_shm_open(rw); - if (lockfd == -1) + if (ensure_directories_exist(path, 0700) < 0) { + condlog(1, "%s: can't setup directories", __func__); return -1; + } - pthread_cleanup_push(multipath_shm_close, (void *)lockfd); - r = func(path); - pthread_cleanup_pop(1); - - if (r == WWID_FAILED_ERROR) - condlog(1, "%s: %s: %s", msg, wwid, strerror(errno)); - else if (r == WWID_FAILED_CHANGED) - condlog(3, "%s: %s", msg, wwid); - else if (!rw) - condlog(4, "%s: %s is %s", msg, wwid, - r == WWID_IS_FAILED ? "failed" : "good"); + fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR); + if (fd >= 0) { + close(fd); + r = WWID_FAILED_CHANGED; + } else if (errno == EEXIST) + r = WWID_FAILED_UNCHANGED; + else + r = WWID_FAILED_ERROR; + print_failed_wwid_result("mark_failed", wwid, r); return r; } -static int _is_failed(const char *path) +int unmark_failed_wwid(const char *wwid) { - struct stat st; + char path[PATH_MAX]; + int r; - if (lstat(path, &st) == 0) - return WWID_IS_FAILED; + if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { + condlog(1, "%s: path name overflow", __func__); + return -1; + } + + if (unlink(path) == 0) + r = WWID_FAILED_CHANGED; else if (errno == ENOENT) - return WWID_IS_NOT_FAILED; + r = WWID_FAILED_UNCHANGED; else - return WWID_FAILED_ERROR; -} - -static int _mark_failed(const char *path) -{ - /* Called from _failed_wwid_op: we know that shm_lock_path exists */ - if (_is_failed(path) == WWID_IS_FAILED) - return WWID_FAILED_UNCHANGED; - return (link(shm_lock_path, path) == 0 ? WWID_FAILED_CHANGED : - WWID_FAILED_ERROR); -} + r = WWID_FAILED_ERROR; -static int _unmark_failed(const char *path) -{ - if (_is_failed(path) == WWID_IS_NOT_FAILED) - return WWID_FAILED_UNCHANGED; - return (unlink(path) == 0 ? WWID_FAILED_CHANGED : WWID_FAILED_ERROR); -} - -#define declare_failed_wwid_op(op, rw) \ -int op ## _wwid(const char *wwid) \ -{ \ - return _failed_wwid_op(wwid, (rw), _ ## op, #op); \ + print_failed_wwid_result("unmark_failed", wwid, r); + return r; } -declare_failed_wwid_op(is_failed, false) -declare_failed_wwid_op(mark_failed, true) -declare_failed_wwid_op(unmark_failed, true) - int remember_cmdline_wwid(void) { FILE *f = NULL; From patchwork Tue May 19 04:57:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Marzinski X-Patchwork-Id: 11556907 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D37AB913 for ; Tue, 19 May 2020 04:57:35 +0000 (UTC) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 94F42206D4 for ; Tue, 19 May 2020 04:57:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Hhp8eAf3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 94F42206D4 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dm-devel-bounces@redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589864254; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=1X59tQRdRWRjBr93goQo1Sh4gzrkYwIXlCvMkKYrW8E=; b=Hhp8eAf3gSsY7VSesbl19/QxfPdRggdDScoLSkRZBbZR8KZ/uloMPXN1hSrOtqKLnRblX+ O2WDdTc+Mqc56vn2I354tzkespZJaKwlVUwZ5vyqOAIwyDNa7Uz0l7m6XIGssiNc8YG63R lAviFw//eaz8YKySh8yPe5oa1+4kpxY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-413-VDrt9abUOZeVQzEFragsEw-1; Tue, 19 May 2020 00:57:32 -0400 X-MC-Unique: VDrt9abUOZeVQzEFragsEw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9264A460; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 78C4F81C05; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 5608A4ED5B; Tue, 19 May 2020 04:57:27 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04J4vJIQ019183 for ; Tue, 19 May 2020 00:57:19 -0400 Received: by smtp.corp.redhat.com (Postfix) id D92BF5D9E2; Tue, 19 May 2020 04:57:19 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from octiron.msp.redhat.com (octiron.msp.redhat.com [10.15.80.209]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C3AEA5D9DD; Tue, 19 May 2020 04:57:19 +0000 (UTC) Received: from octiron.msp.redhat.com (localhost.localdomain [127.0.0.1]) by octiron.msp.redhat.com (8.14.9/8.14.9) with ESMTP id 04J4vIQT001425; Mon, 18 May 2020 23:57:18 -0500 Received: (from bmarzins@localhost) by octiron.msp.redhat.com (8.14.9/8.14.9/Submit) id 04J4vHD7001424; Mon, 18 May 2020 23:57:17 -0500 From: Benjamin Marzinski To: Christophe Varoqui Date: Mon, 18 May 2020 23:57:08 -0500 Message-Id: <1589864228-1363-7-git-send-email-bmarzins@redhat.com> In-Reply-To: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> References: <1589864228-1363-1-git-send-email-bmarzins@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: dm-devel@redhat.com Cc: device-mapper development , Martin Wilck , Martin Wilck Subject: [dm-devel] [PATCH v2 6/6] libmultipath: use atomic linkat() in mark_failed_wwid() X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com From: Martin Wilck This keeps (almost) the simplicity of the previous patch, while making sure that the return value of mark_failed_wwid() (WWID_FAILED_CHANGED vs. WWID_FAILED_UNCHANGED) is correct, even if several processes access this WWID at the same time. Signed-off-by: Martin Wilck Signed-off-by: Benjamin Marzinski --- libmultipath/wwids.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c index da55924b..c7a16636 100644 --- a/libmultipath/wwids.c +++ b/libmultipath/wwids.c @@ -374,7 +374,7 @@ int is_failed_wwid(const char *wwid) if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { condlog(1, "%s: path name overflow", __func__); - return -1; + return WWID_FAILED_ERROR; } if (lstat(path, &st) == 0) @@ -390,27 +390,43 @@ int is_failed_wwid(const char *wwid) int mark_failed_wwid(const char *wwid) { - char path[PATH_MAX]; - int r, fd; + char tmpfile[WWID_SIZE + 2 * sizeof(long) + 1]; + int r = WWID_FAILED_ERROR, fd, dfd; - if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { - condlog(1, "%s: path name overflow", __func__); - return -1; + dfd = open(shm_dir, O_RDONLY|O_DIRECTORY); + if (dfd == -1 && errno == ENOENT) { + char path[sizeof(shm_dir) + 2]; + + /* arg for ensure_directories_exist() must not end with "/" */ + safe_sprintf(path, "%s/_", shm_dir); + ensure_directories_exist(path, 0700); + dfd = open(shm_dir, O_RDONLY|O_DIRECTORY); } - if (ensure_directories_exist(path, 0700) < 0) { - condlog(1, "%s: can't setup directories", __func__); - return -1; + if (dfd == -1) { + condlog(1, "%s: can't setup %s: %m", __func__, shm_dir); + return WWID_FAILED_ERROR; } - fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR); - if (fd >= 0) { + safe_sprintf(tmpfile, "%s.%lx", wwid, (long)getpid()); + fd = openat(dfd, tmpfile, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR); + if (fd >= 0) close(fd); + else + goto out_closedir; + + if (linkat(dfd, tmpfile, dfd, wwid, 0) == 0) r = WWID_FAILED_CHANGED; - } else if (errno == EEXIST) + else if (errno == EEXIST) r = WWID_FAILED_UNCHANGED; else r = WWID_FAILED_ERROR; + if (unlinkat(dfd, tmpfile, 0) == -1) + condlog(2, "%s: failed to unlink %s/%s: %m", + __func__, shm_dir, tmpfile); + +out_closedir: + close(dfd); print_failed_wwid_result("mark_failed", wwid, r); return r; } @@ -422,7 +438,7 @@ int unmark_failed_wwid(const char *wwid) if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { condlog(1, "%s: path name overflow", __func__); - return -1; + return WWID_FAILED_ERROR; } if (unlink(path) == 0)