diff mbox

: set up rbd snapshot handling

Message ID CAF3hT9Cu0BZQfrgCpBRRaPHyfSy3V-A=jJLe3qGr4UoFMcniOQ@mail.gmail.com
State New
Headers show

Commit Message

Gregory Farnum Jan. 10, 2012, 8:01 p.m. UTC 
rbd: wire up snapshot removal and rollback functionality

Signed-off-by: Greg Farnum <gregory.farnum@dreamhost.com>
Reviewed-by: Josh Durgin <josh.durgin@dreamhost.com>
---
 block/rbd.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

Comments

Stefan Hajnoczi Jan. 11, 2012, 9:58 a.m. UTC | #1 
On Tue, Jan 10, 2012 at 8:01 PM, Gregory Farnum
<gregory.farnum@dreamhost.com> wrote:
> +static int qemu_rbd_snap_remove(BlockDriverState *bs,
> +                                const char *snapshot_name)
> +{
> +    BDRVRBDState *s = bs->opaque;
> +    int r;
> +
> +    r = rbd_snap_remove(s->image, snapshot_name);
> +    if (r < 0) {
> +        error_report("failed to remove snap: %s", strerror(-r));
> +        return r;

There's no need to report an error message here.  This function should
return -errno and let the caller decide how to show the error to the
user.  If you look at callers in the codebase they already print an
equivalent error message.

Stefan
Christoph Hellwig Jan. 11, 2012, 10 a.m. UTC | #2 
> +static int qemu_rbd_snap_remove(BlockDriverState *bs,
> +                                const char *snapshot_name)
> +{
> +    BDRVRBDState *s = bs->opaque;
> +    int r;
> +
> +    r = rbd_snap_remove(s->image, snapshot_name);

> +    r = rbd_snap_rollback(s->image, snapshot_name);

Have these functions been available since day 1 in librbd or should they
get a version checks like the cache flush call?
Gregory Farnum Jan. 11, 2012, 7:48 p.m. UTC | #3 
On Wed, Jan 11, 2012 at 1:58 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Jan 10, 2012 at 8:01 PM, Gregory Farnum
> <gregory.farnum@dreamhost.com> wrote:
>> +static int qemu_rbd_snap_remove(BlockDriverState *bs,
>> +                                const char *snapshot_name)
>> +{
>> +    BDRVRBDState *s = bs->opaque;
>> +    int r;
>> +
>> +    r = rbd_snap_remove(s->image, snapshot_name);
>> +    if (r < 0) {
>> +        error_report("failed to remove snap: %s", strerror(-r));
>> +        return r;
>
> There's no need to report an error message here.  This function should
> return -errno and let the caller decide how to show the error to the
> user.  If you look at callers in the codebase they already print an
> equivalent error message.
>
> Stefan

Oh yep, guess I was a little too formulaic. Resend in a moment...

On Wed, Jan 11, 2012 at 2:00 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> +static int qemu_rbd_snap_remove(BlockDriverState *bs,
>> +                                const char *snapshot_name)
>> +{
>> +    BDRVRBDState *s = bs->opaque;
>> +    int r;
>> +
>> +    r = rbd_snap_remove(s->image, snapshot_name);
>
>> +    r = rbd_snap_rollback(s->image, snapshot_name);
>
> Have these functions been available since day 1 in librbd or should they
> get a version checks like the cache flush call?

Day 1. :)
diff mbox

Patch

diff --git a/block/rbd.c b/block/rbd.c
index 7a2384c..f52c1ca 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -787,6 +787,36 @@  static int qemu_rbd_snap_create(BlockDriverState *bs,
     return 0;
 }

+static int qemu_rbd_snap_remove(BlockDriverState *bs,
+                                const char *snapshot_name)
+{
+    BDRVRBDState *s = bs->opaque;
+    int r;
+
+    r = rbd_snap_remove(s->image, snapshot_name);
+    if (r < 0) {
+        error_report("failed to remove snap: %s", strerror(-r));
+        return r;
+    }
+
+    return 0;
+}
+
+static int qemu_rbd_snap_rollback(BlockDriverState *bs,
+                                  const char *snapshot_name)
+{
+    BDRVRBDState *s = bs->opaque;
+    int r;
+
+    r = rbd_snap_rollback(s->image, snapshot_name);
+    if (r < 0) {
+        error_report("failed to rollback to snap: %s", strerror(-r));
+        return r;
+    }
+
+    return 0;
+}
+
 static int qemu_rbd_snap_list(BlockDriverState *bs,
                               QEMUSnapshotInfo **psn_tab)
 {
@@ -860,7 +890,9 @@  static BlockDriver bdrv_rbd = {
     .bdrv_co_flush_to_disk  = qemu_rbd_co_flush,

     .bdrv_snapshot_create   = qemu_rbd_snap_create,
+    .bdrv_snapshot_delete   = qemu_rbd_snap_remove,
     .bdrv_snapshot_list     = qemu_rbd_snap_list,
+    .bdrv_snapshot_goto     = qemu_rbd_snap_rollback,
 };

 static void bdrv_rbd_init(void)