From patchwork Mon Feb 5 17:07:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 10200975 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id F1BB26056A for ; Mon, 5 Feb 2018 17:09:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E1E1128708 for ; Mon, 5 Feb 2018 17:09:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D68BD2871F; Mon, 5 Feb 2018 17:09:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4D72928708 for ; Mon, 5 Feb 2018 17:09:03 +0000 (UTC) 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 mx1.redhat.com (Postfix) with ESMTPS id 949565B2F6; Mon, 5 Feb 2018 17:09:01 +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 0FF315D6B4; Mon, 5 Feb 2018 17:09:00 +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 2AD1418033D9; Mon, 5 Feb 2018 17:08:57 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w15H7aJS031764 for ; Mon, 5 Feb 2018 12:07:36 -0500 Received: by smtp.corp.redhat.com (Postfix) id AFD3F5C556; Mon, 5 Feb 2018 17:07:36 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from mx1.redhat.com (ext-mx03.extmail.prod.ext.phx2.redhat.com [10.5.110.27]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A61025C544 for ; Mon, 5 Feb 2018 17:07:34 +0000 (UTC) Received: from prv3-mh.provo.novell.com (prv3-mh.provo.novell.com [137.65.250.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D2EC883F46 for ; Mon, 5 Feb 2018 17:07:32 +0000 (UTC) Received: from hro119.suse.de (prv-ext-foundry1int.gns.novell.com [137.65.251.240]) by prv3-mh.provo.novell.com with ESMTP (TLS encrypted); Mon, 05 Feb 2018 10:07:17 -0700 Message-ID: <1517850433.14196.63.camel@suse.com> From: Martin Wilck To: Julian Andres Klode Date: Mon, 05 Feb 2018 18:07:13 +0100 In-Reply-To: <20180205154622.axv3cv5wxot3hmbx@jak-x230> References: <20180205085831.23923-1-julian.klode@canonical.com> <1517826733.14196.3.camel@suse.com> <20180205104500.n5rrw6nqzamwijvl@jak-x230> <1517838787.14196.10.camel@suse.com> <20180205144121.27ruwyewpkm3a4kd@jak-x230> <1517843520.14196.45.camel@suse.com> <20180205154622.axv3cv5wxot3hmbx@jak-x230> Mime-Version: 1.0 X-Greylist: Sender passed SPF test, Sender IP whitelisted by DNSRBL, ACL 207 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 05 Feb 2018 17:07:33 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 05 Feb 2018 17:07:33 +0000 (UTC) for IP:'137.65.250.26' DOMAIN:'prv3-mh.provo.novell.com' HELO:'prv3-mh.provo.novell.com' FROM:'mwilck@suse.com' RCPT:'' X-RedHat-Spam-Score: -2.301 (RCVD_IN_DNSWL_MED, SPF_PASS) 137.65.250.26 prv3-mh.provo.novell.com 137.65.250.26 prv3-mh.provo.novell.com X-Scanned-By: MIMEDefang 2.78 on 10.5.110.27 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: dm-devel@redhat.com Cc: dm-devel@redhat.com Subject: Re: [dm-devel] [PATCH] kpartx: Improve finding loopback device by file 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: , 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-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 05 Feb 2018 17:09:02 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 2018-02-05 at 16:46 +0100, Julian Andres Klode wrote: > On Mon, Feb 05, 2018 at 04:12:00PM +0100, Martin Wilck wrote: > > > > Can we agree on the following: > > > > 1 if realpath (filename) results in error, abort > > OK > > > > 2 if realpath(lo_name) results in ENODEV and filename matches > > lo_name, > > remove loop device > > 3 if realpath(lo_name) results in another error code, skip it > > 4 remove if realpath(filename) matches realpath(lo_name) > > > This seems like a lot of ifs. It might be easier to just path if > realpath(path) fails (as realpath(1) does), something like: > > > char *loopname = loopinfo.lo_name; > if (realpath(loopinfo.lo_name, rloopfilename) != NULL) > loopname = rloopfilename; > > if (0 == strcmp(filename, loopinfo.lo_name) ||my 0 == strcmp(rfilename, loopname))) { > found = realpath(path, NULL); > break; > } > > > That should solve all problems and produce useful results with not > a lot of logic. I've reproduced your issue. I can see that the current behavior is wrong. This may be paranoid, but I really want to avoid false positives - kpartx removing mappings it isn't supposed to remove. Therefore I'd like to avoid compare by "filename" and "loopname". I think it's possible. IMO it would be better to have kpartx use the realpath while *creating* the partition mapping already: That would make sure that kpartx can delete loop devices created by itself, which is what we want. IMO it's sufficient to solve your issue. The -ENODEV case is more tricky, your patch doesn't solve it. If you do kpartx -a /some/image rm -f /some/image kpartx -d /some/image kpartx -d fails, and that's unrelated to the realpath code (it fails in stat(device) already). Honestly, I don't think we need to support this scenario. Setting up a loop device and then deleting the backing file is shooting oneself in the foot. Martin --- a/kpartx/kpartx.c +++ b/kpartx/kpartx.c @@ -341,7 +341,7 @@ main(int argc, char **argv){ if (!loopdev) { loopdev = find_unused_loop_device(); - if (set_loop(loopdev, device, 0, &ro)) { + if (set_loop(loopdev, rpath, 0, &ro)) { fprintf(stderr, "can't set up loop\n"); exit (1); }