[dm-devel] dm_ioctl: Only send a change uevent when a resume ioctl changes the device.
Dave Wysochanski
dwysocha at redhat.com
Tue Jan 26 18:48:34 UTC 2010
On Tue, 2010-01-26 at 13:09 -0500, Mike Snitzer wrote:
> On Tue, Jan 26 2010 at 12:56pm -0500,
> Dave Wysochanski <dwysocha at redhat.com> wrote:
>
> > Resume ioctls may result in no changes to the device and thus they
> > should not generate change uevents. For example, if a device is
> > not suspended when a resume ioctl occurs, we should not send a
> > uevent.
> >
> > Signed-off-by: Dave Wysochanski <dwysocha at redhat.com>
> > ---
> > drivers/md/dm-ioctl.c | 12 ++++++++----
> > 1 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > index 1d66932..0c1cf53 100644
> > --- a/drivers/md/dm-ioctl.c
> > +++ b/drivers/md/dm-ioctl.c
> > @@ -851,6 +851,7 @@ static int do_suspend(struct dm_ioctl *param)
> >
> > static int do_resume(struct dm_ioctl *param)
> > {
> > + int send_uevent = 0;
> > int r = 0;
> > unsigned suspend_flags = DM_SUSPEND_LOCKFS_FLAG;
> > struct hash_cell *hc;
> > @@ -895,18 +896,21 @@ static int do_resume(struct dm_ioctl *param)
> > set_disk_ro(dm_disk(md), 0);
> > else
> > set_disk_ro(dm_disk(md), 1);
> > + send_uevent = 1;
> > }
> > -
> > - if (dm_suspended_md(md))
> > + if (dm_suspended_md(md)) {
> > r = dm_resume(md);
> > + send_uevent = 1;
> > + }
> >
> > if (old_map)
> > dm_table_destroy(old_map);
> >
> > - if (!r) {
> > + if (send_uevent)
> > dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr);
> > +
> > + if (!r)
> > r = __dev_status(md, param);
> > - }
> >
> > dm_put(md);
> > return r;
>
> Seems you're sending the uevent even if resume failed. Is that intended?
>
> Also, can dm_kobject_uevent() come before dm_table_destroy()?
>
> If so this would be a bit simpler:
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 1d66932..e3cf568 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -897,16 +897,17 @@ static int do_resume(struct dm_ioctl *param)
> set_disk_ro(dm_disk(md), 1);
> }
>
> - if (dm_suspended_md(md))
> + if (dm_suspended_md(md)) {
> r = dm_resume(md);
> + if (!r)
> + dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr);
> + }
>
> if (old_map)
> dm_table_destroy(old_map);
>
> - if (!r) {
> - dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr);
> + if (!r)
> r = __dev_status(md, param);
> - }
>
> dm_put(md);
> return r;
>
> --
> dm-devel mailing list
> dm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
Nice simplification Mike, as discussed on IRC.
Ack.
More information about the dm-devel
mailing list