Linux Device Mapper development
 help / color / mirror / Atom feed
From: Martin Wilck <mwilck@suse.com>
To: Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com, David Bond <dbond@suse.com>
Subject: Re: [dm-devel] [PATCH 04/21] libmultipath: never allocate an alias that's already taken
Date: Thu, 07 Sep 2023 16:22:33 +0200	[thread overview]
Message-ID: <3660a822f53efbeab83dbdf19b035c6a40c86163.camel@suse.com> (raw)
In-Reply-To: <15ea4cca19b287bab47bd212218cb66bad0acdda.camel@suse.com>

On Thu, 2023-09-07 at 15:33 +0200, Martin Wilck wrote:
> On Thu, 2023-09-07 at 09:24 +0200, Martin Wilck wrote:
> > On Wed, 2023-09-06 at 17:42 -0500, Benjamin Marzinski wrote:
> > > On Fri, Sep 01, 2023 at 08:02:17PM +0200, mwilck@suse.com wrote:
> > > 
> > > 
> > > Again, unless I'm overlooking something, I don't think we need to
> > > check if the alias is already taken here. Since we know that a
> > > device
> > > already exists with alias_old and the correct WWID, as long as
> > > alias_old
> > > is a valid user_friendly_name we can just use it.
> > 
> > Similar reasoning as above. We could perhaps remove these checks,
> > but
> > we'd need to replace them by comments explaining why this condition
> > can't occur.
> > 
> > We could (and maybe should) move the call to find_existing_alias()
> > from
> > add_map_with_path() to get_user_friendly_alias(), so that we have
> > the
> > entire alias logic in a single place. The mpp->alias_old field
> > would
> > then only be used for ACT_RENAME.
> 
> Well, if we do this, we would need to pass vecs->mpvec to
> get_user_friendly_alias(), which means that we could use it also for
> the alias_already_taken() check. It's not exactly the same because in
> one case we look at internal state and in the other case at kernel
> state. OTOH, we do trust that the two are in agreement, right? And we
> derive alias_old from the mpvec today, anyway.

Ok, this doesn't work. The kernel device mapper mandates no naming
conventions for map aliases / names, except that they're unique.
The mpvec contains only aliases of multipath maps. But it's not
forbidden that some non-multipath map be called "mpathc".

OTOH, if we have positive evidence that a given WWID has an alias
mpathX assigned, we do know that no other map can have this same name.
It just comes down to whether we trust the mpvec to correctly represent
kernel state, and I suppose we have to, anyway.

Sorry for the spam. I'll just remove these these checks, and the
corresponding test cases.

Martin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


  reply	other threads:[~2023-09-07 14:23 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-01 18:02 [dm-devel] [PATCH 00/21] multipath-tools: user-friendly names rework mwilck
2023-09-01 18:02 ` [dm-devel] [PATCH 01/21] libmultipath: sysfs_set_scsi_tmo: do nothing for ACT_DRY_RUN mwilck
2023-09-06 22:42   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 02/21] libmultipath: add alias_already_taken() mwilck
2023-09-06 22:42   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 03/21] libmultipath: unify use_existing_alias() and get_user_friendly_alias() mwilck
2023-09-06 22:42   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 04/21] libmultipath: never allocate an alias that's already taken mwilck
2023-09-06 22:42   ` Benjamin Marzinski
2023-09-07  7:24     ` Martin Wilck
2023-09-07 13:33       ` Martin Wilck
2023-09-07 14:22         ` Martin Wilck [this message]
2023-09-01 18:02 ` [dm-devel] [PATCH 05/21] libmultipath: lookup_binding: add comment about the algorithm mwilck
2023-09-06 22:43   ` Benjamin Marzinski
2023-09-07  8:22     ` Martin Wilck
2023-09-01 18:02 ` [dm-devel] [PATCH 06/21] multipath-tools test: simplify debugging for condlog mismatch mwilck
2023-09-06 22:43   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 07/21] multipath-tools tests: add tests for get_user_friendly_alias() mwilck
2023-09-06 22:43   ` Benjamin Marzinski
2023-09-07  7:55     ` Martin Wilck
2023-09-01 18:02 ` [dm-devel] [PATCH 08/21] multipath-tools test: consistent use of macros in alias test mwilck
2023-09-06 22:43   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 09/21] multipath-tools tests: convert mock_{failed, used}_alias to macros mwilck
2023-09-06 22:44   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 10/21] multipath-tools test: use mock_bindings_file() consistently mwilck
2023-09-06 22:43   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 11/21] libmultipath: add global variable for current bindings mwilck
2023-09-06 22:44   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 12/21] libmultipath: rename fix_bindings_file() to update_bindings_file() mwilck
2023-09-06 22:44   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 13/21] libmultipath: alias.c: move bindings related code up mwilck
2023-09-06 22:44   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 14/21] libmultipath: update_bindings_file: take filename argument mwilck
2023-09-06 22:45   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 15/21] libmultipath: update_bindings_file: use a single write() mwilck
2023-09-06 22:45   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 16/21] libmultipath: update_bindings_file: don't log temp file name mwilck
2023-09-06 22:45   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 17/21] libmultipath: alias.c: factor out read_binding() mwilck
2023-09-06 22:45   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 18/21] libmultipath: keep bindings in memory mwilck
2023-09-06 22:47   ` Benjamin Marzinski
2023-09-07 10:30     ` Martin Wilck
2023-09-07 19:14       ` Benjamin Marzinski
2023-09-07 20:02         ` Benjamin Marzinski
2023-09-07 20:43           ` Martin Wilck
2023-09-08 17:22             ` Benjamin Marzinski
2023-09-11  6:25               ` Martin Wilck
2023-09-11 14:47                 ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 19/21] multipath-tools tests: fix alias tests mwilck
2023-09-06 22:47   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 20/21] libmultipath: dm_get_uuid(): return emtpy UUID for non-existing maps mwilck
2023-09-06 22:47   ` Benjamin Marzinski
2023-09-01 18:02 ` [dm-devel] [PATCH 21/21] libmultipath: adapt to new semantics of dm_get_uuid() mwilck
2023-09-06 22:47   ` Benjamin Marzinski

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=3660a822f53efbeab83dbdf19b035c6a40c86163.camel@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=dbond@suse.com \
    --cc=dm-devel@redhat.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