Re: dm_ioctl: Only send a change uevent when a resume ioctl changes the device.

[Mike Snitzer <snitzer@redhat.com>]

On Tue, Jan 26 2010 at 12:56pm -0500,
Dave Wysochanski <dwysocha@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@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;