All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rajnoha <prajnoha@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: Franck Bui <fbui@suse.de>,
	lvm-devel@redhat.com, dm-devel@redhat.com,
	David Teigland <teigland@redhat.com>,
	Zdenek Kabelac <zkabelac@redhat.com>,
	Zdenek Kabelac <zdenek.kabelac@gmail.com>,
	Heming Zhao <heming.zhao@suse.com>
Subject: Re: [dm-devel] [PATCH] udev: create symlinks and watch even in suspended state
Date: Mon, 31 Jan 2022 14:33:54 +0100	[thread overview]
Message-ID: <20220131133354.vfomn5gdlgrhue4g@alatyr-rpi.brq.redhat.com> (raw)
In-Reply-To: <712b54a06fa1b13f9ac92a00d7b121979c43d31c.camel@suse.com>

On Sat 29 Jan 2022 21:46, Martin Wilck wrote:
> On Sat, 2022-01-29 at 21:05 +0100, Zdenek Kabelac wrote:
> > Dne 29. 01. 22 v 0:21 Martin Wilck napsal(a):
> > 
> > > 
> > > AFAICS my patch eliminates the window for this error entirely.
> > 
> > 
> > Ok now I see that there is already skip for DM_SUSPENDED
> > and you patch likely only tries to preserve some existing state of
> > links.
> > 
> > I'll need to get in touch with Peter here.
> > 
> > I guess the idea behind was to avoid read of device that will be
> > resumed and 
> > will automatically get a new event - and suspened device itself
> > cannot change 
> 
> Thank you!
> 
> > - but that fact it's been loosing existing state was missed - I'm
> > wondering 
> > why this was not seen as problem before.
> 
> One reason is that we're now starting multipathd earlier. This has a
> lot of advantages, but it reveals problems that were hidden behind the
> "After=systemd-udev-settle.service" dependency of mulltipathd before.

Hi!

(just discussed this with Zdenek too)

The patch makes sense to me!

We added all the DM_UDEV_PRIMARY_SOURCE_FLAG and related for exactly
such cases where we need to take the existing values already scanned
in previous event, main use-case being the trigger at boot. We just
didn't cover the 13-dm-disk.rules with the same logic regarding the
suspended state to keep the symlinks - I didn't think it would cause
issues (because, usually, after suspend, we anticipate incoming
resume where the device is scanned again).

But yes, if temporarily losing the symlink causes issues, your patch
solves that (Zdenek will push that upstream).

(There's one more situation with checking the suspended state for the
pvscan we have in 69-dm-lvm...rules, but that's lvm-specific, we'll
patch that one...)

Thanks!

-- 
Peter

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


WARNING: multiple messages have this Message-ID (diff)
From: Peter Rajnoha <prajnoha@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH] udev: create symlinks and watch even in suspended state
Date: Mon, 31 Jan 2022 14:33:54 +0100	[thread overview]
Message-ID: <20220131133354.vfomn5gdlgrhue4g@alatyr-rpi.brq.redhat.com> (raw)
In-Reply-To: <712b54a06fa1b13f9ac92a00d7b121979c43d31c.camel@suse.com>

On Sat 29 Jan 2022 21:46, Martin Wilck wrote:
> On Sat, 2022-01-29 at 21:05 +0100, Zdenek Kabelac wrote:
> > Dne 29. 01. 22 v 0:21 Martin Wilck napsal(a):
> > 
> > > 
> > > AFAICS my patch eliminates the window for this error entirely.
> > 
> > 
> > Ok now I see that there is already skip for DM_SUSPENDED
> > and you patch likely only tries to preserve some existing state of
> > links.
> > 
> > I'll need to get in touch with Peter here.
> > 
> > I guess the idea behind was to avoid read of device that will be
> > resumed and 
> > will automatically get a new event - and suspened device itself
> > cannot change 
> 
> Thank you!
> 
> > - but that fact it's been loosing existing state was missed - I'm
> > wondering 
> > why this was not seen as problem before.
> 
> One reason is that we're now starting multipathd earlier. This has a
> lot of advantages, but it reveals problems that were hidden behind the
> "After=systemd-udev-settle.service" dependency of mulltipathd before.

Hi!

(just discussed this with Zdenek too)

The patch makes sense to me!

We added all the DM_UDEV_PRIMARY_SOURCE_FLAG and related for exactly
such cases where we need to take the existing values already scanned
in previous event, main use-case being the trigger at boot. We just
didn't cover the 13-dm-disk.rules with the same logic regarding the
suspended state to keep the symlinks - I didn't think it would cause
issues (because, usually, after suspend, we anticipate incoming
resume where the device is scanned again).

But yes, if temporarily losing the symlink causes issues, your patch
solves that (Zdenek will push that upstream).

(There's one more situation with checking the suspended state for the
pvscan we have in 69-dm-lvm...rules, but that's lvm-specific, we'll
patch that one...)

Thanks!

-- 
Peter



  reply	other threads:[~2022-01-31 13:34 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-28 13:42 [dm-devel] [PATCH] udev: create symlinks and watch even in suspended state mwilck
2022-01-28 13:42 ` mwilck
2022-01-28 15:33 ` [dm-devel] " Zdenek Kabelac
2022-01-28 15:33   ` Zdenek Kabelac
2022-01-28 15:57   ` [dm-devel] " Martin Wilck
2022-01-28 15:57     ` Martin Wilck
2022-01-28 16:02     ` [dm-devel] " Martin Wilck
2022-01-28 16:02       ` Martin Wilck
2022-01-28 17:47       ` [dm-devel] " Zdenek Kabelac
2022-01-28 17:47         ` Zdenek Kabelac
2022-01-28 18:46         ` [dm-devel] " Martin Wilck
2022-01-28 18:46           ` Martin Wilck
2022-01-28 21:06           ` [dm-devel] " Zdenek Kabelac
2022-01-28 21:06             ` Zdenek Kabelac
2022-01-28 23:21             ` [dm-devel] " Martin Wilck
2022-01-28 23:21               ` Martin Wilck
2022-01-29 20:05               ` [dm-devel] " Zdenek Kabelac
2022-01-29 20:05                 ` Zdenek Kabelac
2022-01-29 20:46                 ` [dm-devel] " Martin Wilck
2022-01-29 20:46                   ` Martin Wilck
2022-01-31 13:33                   ` Peter Rajnoha [this message]
2022-01-31 13:33                     ` Peter Rajnoha
2022-02-01  8:40                     ` [dm-devel] " Martin Wilck
2022-02-01  8:40                       ` Martin Wilck
2022-02-01 10:11                       ` [dm-devel] " Zdenek Kabelac
2022-02-01 10:11                         ` Zdenek Kabelac
2022-02-01 10:55                         ` [dm-devel] " Martin Wilck
2022-02-01 10:55                           ` Martin Wilck
2022-02-01 10:55                       ` [dm-devel] " Peter Rajnoha
2022-02-01 10:55                         ` Peter Rajnoha
2022-02-01 11:11                         ` [dm-devel] " Martin Wilck
2022-02-01 11:11                           ` 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=20220131133354.vfomn5gdlgrhue4g@alatyr-rpi.brq.redhat.com \
    --to=prajnoha@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=fbui@suse.de \
    --cc=heming.zhao@suse.com \
    --cc=lvm-devel@redhat.com \
    --cc=mwilck@suse.com \
    --cc=teigland@redhat.com \
    --cc=zdenek.kabelac@gmail.com \
    --cc=zkabelac@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.