diff options
author | Alex Elder <elder@inktank.com> | 2012-10-26 00:34:40 -0400 |
---|---|---|
committer | Alex Elder <elder@inktank.com> | 2012-10-30 09:34:28 -0400 |
commit | 41f38c2b2f8b66b176a0e548ef06294343a7bfa2 (patch) | |
tree | 6d577c2ac2a8a7b3fca878d9030dc9faa6226e4a | |
parent | f7760dad286829682a8d36f4563ab20a65732414 (diff) |
rbd: remove snapshots on error in rbd_add()
If rbd_dev_snaps_update() has ever been called for an rbd device
structure there could be snapshot structures on its snaps list.
In rbd_add(), this function is called but a subsequent error
path neglected to clean up any of these snapshots.
Add a call to rbd_remove_all_snaps() in the appropriate spot to
remedy this. Change a couple of error labels to be a little
clearer while there.
Drop the leading underscores from the function name; there's nothing
special about that function that they might signify. As suggested
in review, the leading underscores in __rbd_remove_snap_dev() have
been removed as well.
Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
-rw-r--r-- | drivers/block/rbd.c | 20 |
1 files changed, 11 insertions, 9 deletions
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index cc06c55875b9..c7681d46cf86 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c | |||
@@ -228,7 +228,7 @@ static int rbd_dev_snaps_update(struct rbd_device *rbd_dev); | |||
228 | static int rbd_dev_snaps_register(struct rbd_device *rbd_dev); | 228 | static int rbd_dev_snaps_register(struct rbd_device *rbd_dev); |
229 | 229 | ||
230 | static void rbd_dev_release(struct device *dev); | 230 | static void rbd_dev_release(struct device *dev); |
231 | static void __rbd_remove_snap_dev(struct rbd_snap *snap); | 231 | static void rbd_remove_snap_dev(struct rbd_snap *snap); |
232 | 232 | ||
233 | static ssize_t rbd_add(struct bus_type *bus, const char *buf, | 233 | static ssize_t rbd_add(struct bus_type *bus, const char *buf, |
234 | size_t count); | 234 | size_t count); |
@@ -1787,13 +1787,13 @@ static int rbd_read_header(struct rbd_device *rbd_dev, | |||
1787 | return ret; | 1787 | return ret; |
1788 | } | 1788 | } |
1789 | 1789 | ||
1790 | static void __rbd_remove_all_snaps(struct rbd_device *rbd_dev) | 1790 | static void rbd_remove_all_snaps(struct rbd_device *rbd_dev) |
1791 | { | 1791 | { |
1792 | struct rbd_snap *snap; | 1792 | struct rbd_snap *snap; |
1793 | struct rbd_snap *next; | 1793 | struct rbd_snap *next; |
1794 | 1794 | ||
1795 | list_for_each_entry_safe(snap, next, &rbd_dev->snaps, node) | 1795 | list_for_each_entry_safe(snap, next, &rbd_dev->snaps, node) |
1796 | __rbd_remove_snap_dev(snap); | 1796 | rbd_remove_snap_dev(snap); |
1797 | } | 1797 | } |
1798 | 1798 | ||
1799 | static void rbd_update_mapping_size(struct rbd_device *rbd_dev) | 1799 | static void rbd_update_mapping_size(struct rbd_device *rbd_dev) |
@@ -2146,7 +2146,7 @@ static bool rbd_snap_registered(struct rbd_snap *snap) | |||
2146 | return ret; | 2146 | return ret; |
2147 | } | 2147 | } |
2148 | 2148 | ||
2149 | static void __rbd_remove_snap_dev(struct rbd_snap *snap) | 2149 | static void rbd_remove_snap_dev(struct rbd_snap *snap) |
2150 | { | 2150 | { |
2151 | list_del(&snap->node); | 2151 | list_del(&snap->node); |
2152 | if (device_is_registered(&snap->dev)) | 2152 | if (device_is_registered(&snap->dev)) |
@@ -2569,7 +2569,7 @@ static int rbd_dev_snaps_update(struct rbd_device *rbd_dev) | |||
2569 | 2569 | ||
2570 | if (rbd_dev->mapping.snap_id == snap->id) | 2570 | if (rbd_dev->mapping.snap_id == snap->id) |
2571 | rbd_dev->mapping.snap_exists = false; | 2571 | rbd_dev->mapping.snap_exists = false; |
2572 | __rbd_remove_snap_dev(snap); | 2572 | rbd_remove_snap_dev(snap); |
2573 | dout("%ssnap id %llu has been removed\n", | 2573 | dout("%ssnap id %llu has been removed\n", |
2574 | rbd_dev->mapping.snap_id == snap->id ? | 2574 | rbd_dev->mapping.snap_id == snap->id ? |
2575 | "mapped " : "", | 2575 | "mapped " : "", |
@@ -3179,11 +3179,11 @@ static ssize_t rbd_add(struct bus_type *bus, | |||
3179 | /* no need to lock here, as rbd_dev is not registered yet */ | 3179 | /* no need to lock here, as rbd_dev is not registered yet */ |
3180 | rc = rbd_dev_snaps_update(rbd_dev); | 3180 | rc = rbd_dev_snaps_update(rbd_dev); |
3181 | if (rc) | 3181 | if (rc) |
3182 | goto err_out_header; | 3182 | goto err_out_probe; |
3183 | 3183 | ||
3184 | rc = rbd_dev_set_mapping(rbd_dev, snap_name); | 3184 | rc = rbd_dev_set_mapping(rbd_dev, snap_name); |
3185 | if (rc) | 3185 | if (rc) |
3186 | goto err_out_header; | 3186 | goto err_out_snaps; |
3187 | 3187 | ||
3188 | /* generate unique id: find highest unique id, add one */ | 3188 | /* generate unique id: find highest unique id, add one */ |
3189 | rbd_dev_id_get(rbd_dev); | 3189 | rbd_dev_id_get(rbd_dev); |
@@ -3247,7 +3247,9 @@ err_out_blkdev: | |||
3247 | unregister_blkdev(rbd_dev->major, rbd_dev->name); | 3247 | unregister_blkdev(rbd_dev->major, rbd_dev->name); |
3248 | err_out_id: | 3248 | err_out_id: |
3249 | rbd_dev_id_put(rbd_dev); | 3249 | rbd_dev_id_put(rbd_dev); |
3250 | err_out_header: | 3250 | err_out_snaps: |
3251 | rbd_remove_all_snaps(rbd_dev); | ||
3252 | err_out_probe: | ||
3251 | rbd_header_free(&rbd_dev->header); | 3253 | rbd_header_free(&rbd_dev->header); |
3252 | err_out_client: | 3254 | err_out_client: |
3253 | kfree(rbd_dev->header_name); | 3255 | kfree(rbd_dev->header_name); |
@@ -3345,7 +3347,7 @@ static ssize_t rbd_remove(struct bus_type *bus, | |||
3345 | goto done; | 3347 | goto done; |
3346 | } | 3348 | } |
3347 | 3349 | ||
3348 | __rbd_remove_all_snaps(rbd_dev); | 3350 | rbd_remove_all_snaps(rbd_dev); |
3349 | rbd_bus_del_dev(rbd_dev); | 3351 | rbd_bus_del_dev(rbd_dev); |
3350 | 3352 | ||
3351 | done: | 3353 | done: |