All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rajnoha <prajnoha@redhat.com>
To: Martin Wilck <mwilck@suse.com>
Cc: dm-devel@redhat.com, lvm-devel@redhat.com,
	Hannes Reinecke <hare@suse.de>,
	Zdenek Kabelac <zkabelac@redhat.com>
Subject: Re: [PATCH RESEND 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"
Date: Tue, 17 Apr 2018 11:48:48 +0200	[thread overview]
Message-ID: <cb503a94-3993-013a-2bd1-4c3a143d055c@redhat.com> (raw)
In-Reply-To: <20180416185341.8588-3-mwilck@suse.com>

On 04/16/2018 08:53 PM, Martin Wilck wrote:
> The current logic that avoids setting SYSTEMD_ALIAS and SYSTEMD_WANTS
> on "change" events is flawed in the default "systemd background job"
> configuration. For systemd, it's important that device properties don't
> change spuriously.
> 
> If an "add" event starts lvm2-pvscan@.service for a device, and a
> "change" event follows, removing SYSTEMD_ALIAS and SYSTEMD_WANTS from the
> udev db, information about unit dependencies between the device and the
> pvscan service can be lost in systemd, in particular if the daemon
> configuration is reloaded.
> 
> Steps to reproduce problem:
> 
> - create a device with an LVM PV
> - remove device
> - add device (generates "add" and "change" uevents for the device)
>   (at this point SYSTEMD_ALIAS and SYSTEMD_WANTS are clear in udev db)
> - systemctl daemon-reload
>   (systemd reloads udev db)
> - vgchange -a n
> - remove device
> 
> => the lvm2-pvscan@.service for the device is still active although the
> device is gone.
> 
> - add device again
> 
> => the PV is not detected, because systemd sees the lvm2-pvscan@.service
> as active and thus doesn't restart it.
> 
> The original purpose of this logic was to avoid volumes being scanned
> over and over again. With systemd background jobs, that isn't necessary,
> because systemd will not restart the job as long as it's active.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  udev/69-dm-lvm-metad.rules.in | 56 +++++++++++++++++++++++++++++++------------
>  udev/Makefile.in              |  4 +++-
>  2 files changed, 44 insertions(+), 16 deletions(-)
> 

Thanks for the patch! I wasn't aware that systemd is reading those
variables again from udev db on reload - nice catch! Applied:

https://sourceware.org/git/?p=lvm2.git;a=commit;h=7a7b8a7778aace88c967ee8285485c494ce1f3f8

-- 
Peter

--
lvm-devel mailing list
lvm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel

WARNING: multiple messages have this Message-ID (diff)
From: Peter Rajnoha <prajnoha@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH RESEND 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"
Date: Tue, 17 Apr 2018 11:48:48 +0200	[thread overview]
Message-ID: <cb503a94-3993-013a-2bd1-4c3a143d055c@redhat.com> (raw)
In-Reply-To: <20180416185341.8588-3-mwilck@suse.com>

On 04/16/2018 08:53 PM, Martin Wilck wrote:
> The current logic that avoids setting SYSTEMD_ALIAS and SYSTEMD_WANTS
> on "change" events is flawed in the default "systemd background job"
> configuration. For systemd, it's important that device properties don't
> change spuriously.
> 
> If an "add" event starts lvm2-pvscan at .service for a device, and a
> "change" event follows, removing SYSTEMD_ALIAS and SYSTEMD_WANTS from the
> udev db, information about unit dependencies between the device and the
> pvscan service can be lost in systemd, in particular if the daemon
> configuration is reloaded.
> 
> Steps to reproduce problem:
> 
> - create a device with an LVM PV
> - remove device
> - add device (generates "add" and "change" uevents for the device)
>   (at this point SYSTEMD_ALIAS and SYSTEMD_WANTS are clear in udev db)
> - systemctl daemon-reload
>   (systemd reloads udev db)
> - vgchange -a n
> - remove device
> 
> => the lvm2-pvscan at .service for the device is still active although the
> device is gone.
> 
> - add device again
> 
> => the PV is not detected, because systemd sees the lvm2-pvscan at .service
> as active and thus doesn't restart it.
> 
> The original purpose of this logic was to avoid volumes being scanned
> over and over again. With systemd background jobs, that isn't necessary,
> because systemd will not restart the job as long as it's active.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  udev/69-dm-lvm-metad.rules.in | 56 +++++++++++++++++++++++++++++++------------
>  udev/Makefile.in              |  4 +++-
>  2 files changed, 44 insertions(+), 16 deletions(-)
> 

Thanks for the patch! I wasn't aware that systemd is reading those
variables again from udev db on reload - nice catch! Applied:

https://sourceware.org/git/?p=lvm2.git;a=commit;h=7a7b8a7778aace88c967ee8285485c494ce1f3f8

-- 
Peter



  reply	other threads:[~2018-04-17  9:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16 18:53 [PATCH RESEND 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
2018-04-16 18:53 ` Martin Wilck
2018-04-16 18:53 ` [PATCH RESEND 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule Martin Wilck
2018-04-16 18:53   ` Martin Wilck
2018-04-17  9:46   ` Peter Rajnoha
2018-04-17  9:46     ` Peter Rajnoha
2018-04-16 18:53 ` [PATCH RESEND 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" Martin Wilck
2018-04-16 18:53   ` Martin Wilck
2018-04-17  9:48   ` Peter Rajnoha [this message]
2018-04-17  9:48     ` Peter Rajnoha

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=cb503a94-3993-013a-2bd1-4c3a143d055c@redhat.com \
    --to=prajnoha@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=hare@suse.de \
    --cc=lvm-devel@redhat.com \
    --cc=mwilck@suse.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.