From patchwork Sun May 4 18:42:29 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christophe Varoqui X-Patchwork-Id: 4110751 X-Patchwork-Delegate: christophe.varoqui@free.fr Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A0D899F1E1 for ; Sun, 4 May 2014 18:46:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5BF7E201F2 for ; Sun, 4 May 2014 18:46:54 +0000 (UTC) Received: from mx6-phx2.redhat.com (mx6-phx2.redhat.com [209.132.183.39]) by mail.kernel.org (Postfix) with ESMTP id 61BA6201D5 for ; Sun, 4 May 2014 18:46:52 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx6-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s44IgXqL010755; Sun, 4 May 2014 14:42:34 -0400 Received: from int-mx02.intmail.prod.int.phx2.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 s44IgWnm013174 for ; Sun, 4 May 2014 14:42:32 -0400 Received: from mx1.redhat.com (ext-mx11.extmail.prod.ext.phx2.redhat.com [10.5.110.16]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s44IgWQE002795 for ; Sun, 4 May 2014 14:42:32 -0400 Received: from mail-ie0-f174.google.com (mail-ie0-f174.google.com [209.85.223.174]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s44IgUh0005268 for ; Sun, 4 May 2014 14:42:30 -0400 Received: by mail-ie0-f174.google.com with SMTP id ar20so6944420iec.5 for ; Sun, 04 May 2014 11:42:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=0EhtLMXBlYu/oLHZUQU/rnDrLxjPygV1fu08A0XmsFY=; b=RU2vXlmUMxjYfFs0lDfgsvYvgbl5fE8kNGIOzfX22UsEOgMjWRDKO15IV9qiK3yicc PuRxvQY2XNdkYWojSF5Lfyo17Sp8CA9QqzgIJ0Cu7aVChgkC/ZY2r4fAoXN/kCwgDLHf FIlpysfKGwETOCiASdAqkt2D64uCQp/xjqd64y5l+gnmt4BR96VzK/I7Xk1E2JCaRme/ P5fKi3E5Y4lWwO5GNUHjtZUVavJo50OGvVaGcOeKwZ0YwgHlyo2WkaRIoX8RdqpKT5w9 h1m+6qyO9LnWByWg0nSufb0NxurRjOTsliuju7xBVvsPsTOQ7p4BptFPr5h7jg+tkBGw yvYA== MIME-Version: 1.0 X-Received: by 10.50.60.72 with SMTP id f8mr18804652igr.20.1399228949979; Sun, 04 May 2014 11:42:29 -0700 (PDT) Received: by 10.64.230.198 with HTTP; Sun, 4 May 2014 11:42:29 -0700 (PDT) In-Reply-To: References: <536670DF.2040605@suse.de> <5366778D.90109@redhat.com> Date: Sun, 4 May 2014 20:42:29 +0200 X-Google-Sender-Auth: y_ltHb1MgFxZKTiKPUSC6rY_zpU Message-ID: From: Christophe Varoqui To: dgilbert@interlog.com X-RedHat-Spam-Score: -2.698 (BAYES_00, DCC_REPUT_13_19, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, SPF_PASS, URIBL_BLOCKED) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-Scanned-By: MIMEDefang 2.68 on 10.5.110.16 X-loop: dm-devel@redhat.com Cc: device-mapper development Subject: Re: [dm-devel] sg_persist triggers block kernel event ??? X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development 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-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, HTML_MESSAGE,MIME_HTML_MOSTLY,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Doug, would this patch address the Linux kernel backward-compatibility concern you raised ? Note, you would have to substitute the 3000 version by the appropriate version that introduced support for prin commands submitted on a open-ro device ... return SG_LIB_FILE_ERROR; } Best regards, Christophe Varoqui www.opensvc.com On Sun, May 4, 2014 at 8:01 PM, Christophe Varoqui < christophe.varoqui@opensvc.com> wrote: > Indeed, I'd rather let udev do its work on legitimate device changes. > Problem is, this change event caused by sg_persist prin commands in not > legitimate : no device change can be caused by interrogating the disk pr > status. > > Doug informed me newer kernels accept prin commands inside a > open-ro=>close sequence, which I now verified by testing with the patch I > sent ... but older kernels do not. My patch is thus not backward compatible > as-is. > > Any advice on how to treat this backward-compatibility issue ? > > For information, here's a more recent version of the patch I sent to Doug > earlier to address not-Linux platform behaviour stability : > > --- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200 > +++ sg_persist.c 2014-05-04 08:41:29.482017943 +0200 > @@ -1029,6 +1029,8 @@ > struct sg_simple_inquiry_resp inq_resp; > const char * cp; > struct opts_t opts; > + int omode = 0; > + const char *omode_desc = "rw"; > > memset(&opts, 0, sizeof(opts)); > opts.prin = 1; > @@ -1292,10 +1294,17 @@ > sg_cmds_close_device(sg_fd); > } > > - if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */, > +#ifdef SG_LIB_LINUX > + if (opts.prin) { > + omode = 1; > + omode_desc = "ro"; > + } > +#endif > + > + if ((sg_fd = sg_cmds_open_device(device_name, omode, > opts.verbose)) < 0) { > - pr2serr("sg_persist: error opening file (rw): %s: %s\n", > device_name, > - safe_strerror(-sg_fd)); > + pr2serr("sg_persist: error opening file (%s): %s: %s\n", > omode_desc, > + device_name, safe_strerror(-sg_fd)); > return SG_LIB_FILE_ERROR; > } > > Regards, > Christophe Varoqui > www.opensvc.com > > > > On Sun, May 4, 2014 at 7:23 PM, Zdenek Kabelac wrote: > >> Dne 4.5.2014 18:54, Hannes Reinecke napsal(a): >> >> On 05/04/2014 01:34 AM, Christophe Varoqui wrote: >>> >>>> It seems sg_persist is doing an "open rw => close" for --in commands, >>>> causing a kernel change-event. >>>> >>> Yep. >>> >>> Look for 'watch' in the udev rules, that's precisely what it's doing. >>> >>> (Bloody annoying if you ask me. Generally I recommend to remove that >>> thing >>> from the rules). >>> >> >> When watch rule is disabled/removed in udev rules - your udev db becomes >> invalid when i.e. you run command like 'mkfs' - since the udev db will >> not be updated to list information about newly formatted filesystem. >> >> Of course there are many cases where disabling watch rule makes sense >> (i.e. you export lots of disks to virtual guests) - but unless you are >> familiar with udev and you know what you are doing - think twice before >> disabling. >> >> Zdenek >> >> -- >> dm-devel mailing list >> dm-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/dm-devel >> > > --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel --- sg_persist.c.orig 2014-05-04 01:10:01.987981956 +0200 +++ sg_persist.c 2014-05-04 20:44:34.665292548 +0200 @@ -16,6 +16,7 @@ #include #include #include +#include #define __STDC_FORMAT_MACROS 1 #include @@ -26,6 +27,7 @@ #include "sg_lib.h" #include "sg_cmds_basic.h" #include "sg_cmds_extra.h" +#include "sg_io_linux.h" static const char * version_str = "0.44 20140202"; @@ -1029,6 +1031,8 @@ struct sg_simple_inquiry_resp inq_resp; const char * cp; struct opts_t opts; + int omode = 0; + const char *omode_desc = "rw"; memset(&opts, 0, sizeof(opts)); opts.prin = 1; @@ -1292,10 +1296,31 @@ sg_cmds_close_device(sg_fd); } - if ((sg_fd = sg_cmds_open_device(device_name, 0 /* rw */, +#ifdef SG_LIB_LINUX + /* + * PRIN commands do not provoke device changes, so we should avoid to + * open the device read-write so that the Linux kernel does not generate + * a change event. + * Older kernel do not support PRIN commands submission on a read-only + * opened device, so don't try in this case. + */ + int v; + if (opts.prin) { + sg_fd = sg_cmds_open_device(device_name, 1, opts.verbose); + if (sg_fd >= 0) { + if (ioctl(sg_fd, SG_GET_VERSION_NUM, &v) >= 0 && v >= 30000) { + omode = 1; + omode_desc = "ro"; + } + sg_cmds_close_device(sg_fd); + } + } +#endif + + if ((sg_fd = sg_cmds_open_device(device_name, omode, opts.verbose)) < 0) { - pr2serr("sg_persist: error opening file (rw): %s: %s\n", device_name, - safe_strerror(-sg_fd)); + pr2serr("sg_persist: error opening file (%s): %s: %s\n", omode_desc, + device_name, safe_strerror(-sg_fd));