diff mbox

[V2,2/4] qemu-nbd: support internal snapshot export

Message ID 1379842791-3776-3-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia Sept. 22, 2013, 9:39 a.m. UTC 
Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

Comments

Paolo Bonzini Sept. 23, 2013, 10:25 a.m. UTC | #1 
Il 22/09/2013 11:39, Wenchao Xia ha scritto:
> Now it is possible to directly export an internal snapshot, which
> can be used to probe the snapshot's contents without qemu-img
> convert.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 53 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..e450d04 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -20,6 +20,7 @@
>  #include "block/block.h"
>  #include "block/nbd.h"
>  #include "qemu/main-loop.h"
> +#include "block/snapshot.h"
>  
>  #include <stdarg.h>
>  #include <stdio.h>
> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>      }
>  }
>  
> +#define SNAPSHOT_OPT_ID         "id"
> +#define SNAPSHOT_OPT_NAME       "name"
> +
> +static QEMUOptionParameter snapshot_options[] = {
> +    {
> +        .name = SNAPSHOT_OPT_ID,
> +        .type = OPT_STRING,
> +        .help = "snapshot id"
> +    },
> +    {
> +        .name = SNAPSHOT_OPT_NAME,
> +        .type = OPT_STRING,
> +        .help = "snapshot name"
> +    },
> +    { NULL }
> +};

I think whatever mechanism you use here to pick a snapshot id or name
should be implemented in qemu-img too.

Also, I think QEMUOptionParameter is being phased out.

>  int main(int argc, char **argv)
>  {
>      BlockDriverState *bs;
> @@ -315,7 +333,10 @@ int main(int argc, char **argv)
>      char *device = NULL;
>      int port = NBD_DEFAULT_PORT;
>      off_t fd_size;
> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
> +    QEMUOptionParameter *sn_param = NULL;
> +    const QEMUOptionParameter *sn_param_id, *sn_param_name;
> +    const char *sn_id = NULL, *sn_name = NULL;
> +    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
>      struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> @@ -328,6 +349,7 @@ int main(int argc, char **argv)
>          { "connect", 1, NULL, 'c' },
>          { "disconnect", 0, NULL, 'd' },
>          { "snapshot", 0, NULL, 's' },
> +        { "snapshot-load", 1, NULL, 'l' },

Please call this option "load-snapshot".

Paolo

>          { "nocache", 0, NULL, 'n' },
>          { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>  #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>                  errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>              }
>              break;
> +        case 'l':
> +            sn_param = parse_option_parameters(optarg,
> +                                               snapshot_options, sn_param);
> +            if (!sn_param) {
> +                errx(EXIT_FAILURE,
> +                     "Invalid snapshot-load options '%s'", optarg);
> +            }
> +            /* fall through */
>          case 'r':
>              nbdflags |= NBD_FLAG_READ_ONLY;
>              flags &= ~BDRV_O_RDWR;
> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>              error_get_pretty(local_err));
>      }
>  
> +    if (sn_param) {
> +        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
> +        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
> +        if (sn_param_id) {
> +            sn_id = sn_param_id->value.s;
> +        }
> +        if (sn_param_name) {
> +            sn_name = sn_param_name->value.s;
> +        }
> +        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
> +        if (ret < 0) {
> +            errno = -ret;
> +            err(EXIT_FAILURE,
> +                "Failed to load snapshot, reason:\n%s",
> +                error_get_pretty(local_err));
> +        }
> +    }
> +
>      fd_size = bdrv_getlength(bs);
>  
>      if (partition != -1) {
> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>          unlink(sockpath);
>      }
>  
> +    if (sn_param) {
> +        free_option_parameters(sn_param);
> +    }
> +
>      if (device) {
>          void *ret;
>          pthread_join(client_thread, &ret);
>
Wayne Xia Sept. 24, 2013, 2:56 a.m. UTC | #2 
于 2013/9/23 18:25, Paolo Bonzini 写道:
> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>> Now it is possible to directly export an internal snapshot, which
>> can be used to probe the snapshot's contents without qemu-img
>> convert.
>>
>> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>> ---
>>   qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 53 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index c26c98e..e450d04 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -20,6 +20,7 @@
>>   #include "block/block.h"
>>   #include "block/nbd.h"
>>   #include "qemu/main-loop.h"
>> +#include "block/snapshot.h"
>>
>>   #include<stdarg.h>
>>   #include<stdio.h>
>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>>       }
>>   }
>>
>> +#define SNAPSHOT_OPT_ID         "id"
>> +#define SNAPSHOT_OPT_NAME       "name"
>> +
>> +static QEMUOptionParameter snapshot_options[] = {
>> +    {
>> +        .name = SNAPSHOT_OPT_ID,
>> +        .type = OPT_STRING,
>> +        .help = "snapshot id"
>> +    },
>> +    {
>> +        .name = SNAPSHOT_OPT_NAME,
>> +        .type = OPT_STRING,
>> +        .help = "snapshot name"
>> +    },
>> +    { NULL }
>> +};
> I think whatever mechanism you use here to pick a snapshot id or name
> should be implemented in qemu-img too.
qemu-img already pick up snapshot by mixed id and name, do you like to 
add a new
interface like the above(Keep old interface untouched for compatiablity)?

> Also, I think QEMUOptionParameter is being phased out.
>
Is QemuOptsList the recommanded method?

>>   int main(int argc, char **argv)
>>   {
>>       BlockDriverState *bs;
>> @@ -315,7 +333,10 @@ int main(int argc, char **argv)
>>       char *device = NULL;
>>       int port = NBD_DEFAULT_PORT;
>>       off_t fd_size;
>> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
>> +    QEMUOptionParameter *sn_param = NULL;
>> +    const QEMUOptionParameter *sn_param_id, *sn_param_name;
>> +    const char *sn_id = NULL, *sn_name = NULL;
>> +    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
>>       struct option lopt[] = {
>>           { "help", 0, NULL, 'h' },
>>           { "version", 0, NULL, 'V' },
>> @@ -328,6 +349,7 @@ int main(int argc, char **argv)
>>           { "connect", 1, NULL, 'c' },
>>           { "disconnect", 0, NULL, 'd' },
>>           { "snapshot", 0, NULL, 's' },
>> +        { "snapshot-load", 1, NULL, 'l' },
> Please call this option "load-snapshot".
>
OK.


> Paolo
>
>>           { "nocache", 0, NULL, 'n' },
>>           { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>>   #ifdef CONFIG_LINUX_AIO
>> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>>                   errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>>               }
>>               break;
>> +        case 'l':
>> +            sn_param = parse_option_parameters(optarg,
>> +                                               snapshot_options, sn_param);
>> +            if (!sn_param) {
>> +                errx(EXIT_FAILURE,
>> +                     "Invalid snapshot-load options '%s'", optarg);
>> +            }
>> +            /* fall through */
>>           case 'r':
>>               nbdflags |= NBD_FLAG_READ_ONLY;
>>               flags&= ~BDRV_O_RDWR;
>> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>>               error_get_pretty(local_err));
>>       }
>>
>> +    if (sn_param) {
>> +        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
>> +        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
>> +        if (sn_param_id) {
>> +            sn_id = sn_param_id->value.s;
>> +        }
>> +        if (sn_param_name) {
>> +            sn_name = sn_param_name->value.s;
>> +        }
>> +        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name,&local_err);
>> +        if (ret<  0) {
>> +            errno = -ret;
>> +            err(EXIT_FAILURE,
>> +                "Failed to load snapshot, reason:\n%s",
>> +                error_get_pretty(local_err));
>> +        }
>> +    }
>> +
>>       fd_size = bdrv_getlength(bs);
>>
>>       if (partition != -1) {
>> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>>           unlink(sockpath);
>>       }
>>
>> +    if (sn_param) {
>> +        free_option_parameters(sn_param);
>> +    }
>> +
>>       if (device) {
>>           void *ret;
>>           pthread_join(client_thread,&ret);
>>
Paolo Bonzini Sept. 24, 2013, 8:11 a.m. UTC | #3 
Il 24/09/2013 04:56, Wenchao Xia ha scritto:
> 于 2013/9/23 18:25, Paolo Bonzini 写道:
>> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>>> Now it is possible to directly export an internal snapshot, which
>>> can be used to probe the snapshot's contents without qemu-img
>>> convert.
>>>
>>> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   qemu-nbd.c |   54
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   1 files changed, 53 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>>> index c26c98e..e450d04 100644
>>> --- a/qemu-nbd.c
>>> +++ b/qemu-nbd.c
>>> @@ -20,6 +20,7 @@
>>>   #include "block/block.h"
>>>   #include "block/nbd.h"
>>>   #include "qemu/main-loop.h"
>>> +#include "block/snapshot.h"
>>>
>>>   #include<stdarg.h>
>>>   #include<stdio.h>
>>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>>>       }
>>>   }
>>>
>>> +#define SNAPSHOT_OPT_ID         "id"
>>> +#define SNAPSHOT_OPT_NAME       "name"
>>> +
>>> +static QEMUOptionParameter snapshot_options[] = {
>>> +    {
>>> +        .name = SNAPSHOT_OPT_ID,
>>> +        .type = OPT_STRING,
>>> +        .help = "snapshot id"
>>> +    },
>>> +    {
>>> +        .name = SNAPSHOT_OPT_NAME,
>>> +        .type = OPT_STRING,
>>> +        .help = "snapshot name"
>>> +    },
>>> +    { NULL }
>>> +};
>> I think whatever mechanism you use here to pick a snapshot id or name
>> should be implemented in qemu-img too.
> qemu-img already pick up snapshot by mixed id and name, do you like to
> add a new
> interface like the above(Keep old interface untouched for compatiablity)?

Yes, please.  And also implement the "mixed" method here.

>> Also, I think QEMUOptionParameter is being phased out.
>>
> Is QemuOptsList the recommanded method?

Yes.

Paolo
diff mbox

Patch

diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..e450d04 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@ 
 #include "block/block.h"
 #include "block/nbd.h"
 #include "qemu/main-loop.h"
+#include "block/snapshot.h"
 
 #include <stdarg.h>
 #include <stdio.h>
@@ -304,6 +305,23 @@  static void nbd_accept(void *opaque)
     }
 }
 
+#define SNAPSHOT_OPT_ID         "id"
+#define SNAPSHOT_OPT_NAME       "name"
+
+static QEMUOptionParameter snapshot_options[] = {
+    {
+        .name = SNAPSHOT_OPT_ID,
+        .type = OPT_STRING,
+        .help = "snapshot id"
+    },
+    {
+        .name = SNAPSHOT_OPT_NAME,
+        .type = OPT_STRING,
+        .help = "snapshot name"
+    },
+    { NULL }
+};
+
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
@@ -315,7 +333,10 @@  int main(int argc, char **argv)
     char *device = NULL;
     int port = NBD_DEFAULT_PORT;
     off_t fd_size;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+    QEMUOptionParameter *sn_param = NULL;
+    const QEMUOptionParameter *sn_param_id, *sn_param_name;
+    const char *sn_id = NULL, *sn_name = NULL;
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -328,6 +349,7 @@  int main(int argc, char **argv)
         { "connect", 1, NULL, 'c' },
         { "disconnect", 0, NULL, 'd' },
         { "snapshot", 0, NULL, 's' },
+        { "snapshot-load", 1, NULL, 'l' },
         { "nocache", 0, NULL, 'n' },
         { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
 #ifdef CONFIG_LINUX_AIO
@@ -428,6 +450,14 @@  int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
             }
             break;
+        case 'l':
+            sn_param = parse_option_parameters(optarg,
+                                               snapshot_options, sn_param);
+            if (!sn_param) {
+                errx(EXIT_FAILURE,
+                     "Invalid snapshot-load options '%s'", optarg);
+            }
+            /* fall through */
         case 'r':
             nbdflags |= NBD_FLAG_READ_ONLY;
             flags &= ~BDRV_O_RDWR;
@@ -581,6 +611,24 @@  int main(int argc, char **argv)
             error_get_pretty(local_err));
     }
 
+    if (sn_param) {
+        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
+        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
+        if (sn_param_id) {
+            sn_id = sn_param_id->value.s;
+        }
+        if (sn_param_name) {
+            sn_name = sn_param_name->value.s;
+        }
+        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
+        if (ret < 0) {
+            errno = -ret;
+            err(EXIT_FAILURE,
+                "Failed to load snapshot, reason:\n%s",
+                error_get_pretty(local_err));
+        }
+    }
+
     fd_size = bdrv_getlength(bs);
 
     if (partition != -1) {
@@ -641,6 +689,10 @@  int main(int argc, char **argv)
         unlink(sockpath);
     }
 
+    if (sn_param) {
+        free_option_parameters(sn_param);
+    }
+
     if (device) {
         void *ret;
         pthread_join(client_thread, &ret);