Linux Device Mapper development
 help / color / mirror / Atom feed
From: "Benjamin Marzinski" <bmarzins@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: dm-devel@redhat.com
Subject: Re: [PATCH RESEND 0/4] multipath-tools: fixes for path wwid detection and path change uevents
Date: Fri, 5 Oct 2018 18:07:04 -0500	[thread overview]
Message-ID: <20181005230704.GH3172@octiron.msp.redhat.com> (raw)
In-Reply-To: <7aa1314c9095e3315e02525ec2e005030898a9b4.camel@suse.com>

On Tue, Oct 02, 2018 at 11:02:50PM +0200, Martin Wilck wrote:
> Hi Ben, Christophe,
> 
> I found a problem with this old patch series of mine.
> 
> while I am still pondering about your latest series, it occured to me
> that my patch "get_uid: don't quit prematurely without udev" is against
> the spirit of your "retrigger uevents to try and get the uid through
> udev" patch from 2016 (688aa22b).
> 
> "get_uid: don't quit prematurely without udev" has been merged a while
> ago (08d42ee6). Should it be reverted??
> 
> Pro: the patch is against the "udev first" philosophy and subverts the
> retrigger logic.
> Con: with the patch, we'll be able to retrieve WWIDs more quickly in
> some situations, as we don't have to wait for udev.
> 
> Either way (but more likely with my patch), it may happen that we
> retrieve a WWID from sysfs or elsewhere first, and from udev later.
> These WWIDs may not necessarily match, and a "WWID changed" problem may
> occur.
> 
> Please tell me what you think.

I'm having a hard time seeing how get_uid() could get called on a path
without a udev device attached in multipathd. It can happen because of
update_paths() in multipath, for paths that don't get discovered when
searching for paths, but do belong to an existing multipath device. I
don't think you patch will hurt anything here. So, unless I'm missing
some code path in multipathd, I don't see a problem with this patch.

In fact, we should probably make scsi_uid_fallback() skip (or at least
always pass) the retrigger check if we aren't in the daemon, so we
always try the failback when we run multipath.

-Ben

> 
> Martin
> 
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> 

  reply	other threads:[~2018-10-05 23:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-07 23:21 [PATCH RESEND 0/4] multipath-tools: fixes for path wwid detection and path change uevents Martin Wilck
2018-03-07 23:21 ` [PATCH RESEND 1/4] libmultipath: get_uid: check VPD pages for SCSI only Martin Wilck
2018-03-07 23:21 ` [PATCH RESEND 2/4] libmultipath: get_uid: don't quit prematurely without udev Martin Wilck
2018-03-07 23:21 ` [PATCH RESEND 3/4] libmultipath: uev_update_path: always warn if WWID changed Martin Wilck
2018-03-07 23:21 ` [PATCH RESEND 4/4] libmultipath: uev_update_path: update path properties Martin Wilck
2018-03-14 17:40 ` [PATCH RESEND 0/4] multipath-tools: fixes for path wwid detection and path change uevents Benjamin Marzinski
2018-10-02 21:02   ` Martin Wilck
2018-10-05 23:07     ` Benjamin Marzinski [this message]
2018-10-08  8:26       ` Martin Wilck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181005230704.GH3172@octiron.msp.redhat.com \
    --to=bmarzins@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=mwilck@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox