[PATCH] rbd: fix module sysfs setup/teardown code

[PATCH] rbd: fix module sysfs setup/teardown code

[Alex Elder]

Once rbd_bus_type is registered, it allows an "add" operation via
the /sys/bus/rbd/add bus attribute, and adding a new rbd device that
way establishes a connection between the device and rbd_root_dev.
But rbd_root_dev is not registered until after the rbd_bus_type
registration is complete.  This could (in principle anyway) result
in an invalid state.

Since rbd_root_dev has no tie to rbd_bus_type we can reorder these
two initializations and never be faced with this scenario.

In addition, unregister the device in the event the bus registration
fails at module init time.

Signed-off-by: Alex Elder <elder@dreamhost.com>
---
  drivers/block/rbd.c |    8 +++++---
  1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index bc539cd..b7c9af5 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
  {
  	int ret;

-	ret = bus_register(&rbd_bus_type);
+	ret = device_register(&rbd_root_dev);
  	if (ret < 0)
  		return ret;

-	ret = device_register(&rbd_root_dev);
+	ret = bus_register(&rbd_bus_type);
+	if (ret)
+		device_unregister(&rbd_root_dev);

  	return ret;
  }

  static void rbd_sysfs_cleanup(void)
  {
-	device_unregister(&rbd_root_dev);
  	bus_unregister(&rbd_bus_type);
+	device_unregister(&rbd_root_dev);
  }

  int __init rbd_init(void)
-- 
1.7.5.4

Re: [PATCH] rbd: fix module sysfs setup/teardown code

[Yehuda Sadeh Weinraub]

On Tue, Feb 28, 2012 at 8:00 PM, Alex Elder <elder@dreamhost.com> wrote:
>
> Once rbd_bus_type is registered, it allows an "add" operation via
> the /sys/bus/rbd/add bus attribute, and adding a new rbd device that
> way establishes a connection between the device and rbd_root_dev.
> But rbd_root_dev is not registered until after the rbd_bus_type
> registration is complete.  This could (in principle anyway) result
> in an invalid state.
>
> Since rbd_root_dev has no tie to rbd_bus_type we can reorder these
> two initializations and never be faced with this scenario.
>
> In addition, unregister the device in the event the bus registration
> fails at module init time.
>
> Signed-off-by: Alex Elder <elder@dreamhost.com>
> ---
>  drivers/block/rbd.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index bc539cd..b7c9af5 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
>  {
>        int ret;
>
> -       ret = bus_register(&rbd_bus_type);
> +       ret = device_register(&rbd_root_dev);
>        if (ret < 0)
>                return ret;
>
> -       ret = device_register(&rbd_root_dev);
> +       ret = bus_register(&rbd_bus_type);
> +       if (ret)

For consistency if (ret < 0) or change the previous one.

> +               device_unregister(&rbd_root_dev);
>
>        return ret;
>  }
>
>  static void rbd_sysfs_cleanup(void)
>  {
> -       device_unregister(&rbd_root_dev);
>        bus_unregister(&rbd_bus_type);
> +       device_unregister(&rbd_root_dev);
>  }
>
>  int __init rbd_init(void)
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rbd: fix module sysfs setup/teardown code

[Alex Elder]

On 02/29/2012 12:25 PM, Yehuda Sadeh Weinraub wrote:
> For consistency if (ret<  0) or change the previous one.

OK.  I will update.	-Alex

Re: [PATCH] rbd: fix module sysfs setup/teardown code

[Sage Weil]

Reviewed-by: Sage Weil <sage@newdream.net>

On Tue, 28 Feb 2012, Alex Elder wrote:

> Once rbd_bus_type is registered, it allows an "add" operation via
> the /sys/bus/rbd/add bus attribute, and adding a new rbd device that
> way establishes a connection between the device and rbd_root_dev.
> But rbd_root_dev is not registered until after the rbd_bus_type
> registration is complete.  This could (in principle anyway) result
> in an invalid state.
> 
> Since rbd_root_dev has no tie to rbd_bus_type we can reorder these
> two initializations and never be faced with this scenario.
> 
> In addition, unregister the device in the event the bus registration
> fails at module init time.
> 
> Signed-off-by: Alex Elder <elder@dreamhost.com>
> ---
>  drivers/block/rbd.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index bc539cd..b7c9af5 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
>  {
>  	int ret;
> 
> -	ret = bus_register(&rbd_bus_type);
> +	ret = device_register(&rbd_root_dev);
>  	if (ret < 0)
>  		return ret;
> 
> -	ret = device_register(&rbd_root_dev);
> +	ret = bus_register(&rbd_bus_type);
> +	if (ret)
> +		device_unregister(&rbd_root_dev);
> 
>  	return ret;
>  }
> 
>  static void rbd_sysfs_cleanup(void)
>  {
> -	device_unregister(&rbd_root_dev);
>  	bus_unregister(&rbd_bus_type);
> +	device_unregister(&rbd_root_dev);
>  }
> 
>  int __init rbd_init(void)
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>