Patchwork [V11,07/17] block: add image info query function bdrv_query_image_info()

login
register
mail settings
Submitter Wayne Xia
Date April 2, 2013, 11:47 a.m.
Message ID <1364903250-10429-8-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/232975/
State New
Headers show

Comments

Wayne Xia - April 2, 2013, 11:47 a.m.
This patch adds function bdrv_query_image_info(), which will
retrieve image info in qmp object format. The implementation is
based on the code moved from qemu-img.c, but uses block layer
function to get snapshot info.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c         |   41 ++++++++++++++++++++++++++++++++++-------
 include/block/qapi.h |    6 +++---
 qemu-img.c           |    8 ++------
 3 files changed, 39 insertions(+), 16 deletions(-)
Markus Armbruster - April 10, 2013, 3:28 p.m.
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   This patch adds function bdrv_query_image_info(), which will
> retrieve image info in qmp object format. The implementation is
> based on the code moved from qemu-img.c, but uses block layer
> function to get snapshot info.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c         |   41 ++++++++++++++++++++++++++++++++++-------
>  include/block/qapi.h |    6 +++---
>  qemu-img.c           |    8 ++------
>  3 files changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 19d4d93..176a479 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>      return 0;
>  }
>  
> -void bdrv_collect_image_info(BlockDriverState *bs,
> -                             ImageInfo *info,
> -                             const char *filename)
> +/* return 0 on success, and @p_info will be set only on success. */
> +int bdrv_query_image_info(BlockDriverState *bs,
> +                          ImageInfo **p_info,
> +                          Error **errp)
>  {
>      uint64_t total_sectors;
> -    char backing_filename[1024];
> +    const char *backing_filename;
>      char backing_filename2[1024];
>      BlockDriverInfo bdi;
> +    int ret;
> +    Error *err = NULL;
> +    ImageInfo *info = g_new0(ImageInfo, 1);
>  
>      bdrv_get_geometry(bs, &total_sectors);
>  
> -    info->filename        = g_strdup(filename);
> +    info->filename        = g_strdup(bs->filename);
>      info->format          = g_strdup(bdrv_get_format_name(bs));
>      info->virtual_size    = total_sectors * 512;
>      info->actual_size     = bdrv_get_allocated_file_size(bs);
> @@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>          info->dirty_flag = bdi.is_dirty;
>          info->has_dirty_flag = true;
>      }
> -    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
> -    if (backing_filename[0] != '\0') {
> +    backing_filename = bs->backing_file;
> +    if (backing_filename && backing_filename[0] != '\0') {

backing_filename can't possibly be null here, because bs->backing_file
is an array.

>          info->backing_filename = g_strdup(backing_filename);
>          info->has_backing_filename = true;
>          bdrv_get_full_backing_filename(bs, backing_filename2,
> @@ -168,4 +172,27 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>              info->has_backing_filename_format = true;
>          }
>      }
> +
> +    ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
> +    switch (ret) {
> +    case 0:
> +        if (info->snapshots) {
> +            info->has_snapshots = true;
> +        }
> +        break;
> +    /* recoverable error */
> +    case -ENOMEDIUM:
> +        error_free(err);
> +        break;
> +    case -ENOTSUP:
> +        error_free(err);
> +        break;

Suggest

       case -ENOMEDIUM:
       case -ENOTSUP:
           /* no snapshots on this device */
           error_free(err);
           break;

for clarity.

> +    default:
> +        error_propagate(errp, err);
> +        qapi_free_ImageInfo(info);
> +        return ret;
> +    }
> +
> +    *p_info = info;
> +    return 0;
>  }
> diff --git a/include/block/qapi.h b/include/block/qapi.h
> index fe66053..2c62fdf 100644
> --- a/include/block/qapi.h
> +++ b/include/block/qapi.h
> @@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>                                    SnapshotInfoList **p_list,
>                                    bool vm_snapshot,
>                                    Error **errp);
> -void bdrv_collect_image_info(BlockDriverState *bs,
> -                             ImageInfo *info,
> -                             const char *filename);
> +int bdrv_query_image_info(BlockDriverState *bs,
> +                          ImageInfo **p_info,
> +                          Error **errp);
>  #endif
> diff --git a/qemu-img.c b/qemu-img.c
> index 261c277..1dd0a60 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1733,12 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
>              goto err;
>          }
>  
> -        info = g_new0(ImageInfo, 1);
> -        bdrv_collect_image_info(bs, info, filename);
> -        if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
> -                                           false, NULL) &&
> -            info->snapshots) {
> -            info->has_snapshots = true;
> +        if (bdrv_query_image_info(bs, &info, NULL)) {
> +            goto err;
>          }
>  
>          elem = g_new0(ImageInfoList, 1);
Wayne Xia - April 11, 2013, 5:59 a.m.
于 2013-4-10 23:28, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>>    This patch adds function bdrv_query_image_info(), which will
>> retrieve image info in qmp object format. The implementation is
>> based on the code moved from qemu-img.c, but uses block layer
>> function to get snapshot info.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   block/qapi.c         |   41 ++++++++++++++++++++++++++++++++++-------
>>   include/block/qapi.h |    6 +++---
>>   qemu-img.c           |    8 ++------
>>   3 files changed, 39 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 19d4d93..176a479 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>>       return 0;
>>   }
>>   
>> -void bdrv_collect_image_info(BlockDriverState *bs,
>> -                             ImageInfo *info,
>> -                             const char *filename)
>> +/* return 0 on success, and @p_info will be set only on success. */
>> +int bdrv_query_image_info(BlockDriverState *bs,
>> +                          ImageInfo **p_info,
>> +                          Error **errp)
>>   {
>>       uint64_t total_sectors;
>> -    char backing_filename[1024];
>> +    const char *backing_filename;
>>       char backing_filename2[1024];
>>       BlockDriverInfo bdi;
>> +    int ret;
>> +    Error *err = NULL;
>> +    ImageInfo *info = g_new0(ImageInfo, 1);
>>   
>>       bdrv_get_geometry(bs, &total_sectors);
>>   
>> -    info->filename        = g_strdup(filename);
>> +    info->filename        = g_strdup(bs->filename);
>>       info->format          = g_strdup(bdrv_get_format_name(bs));
>>       info->virtual_size    = total_sectors * 512;
>>       info->actual_size     = bdrv_get_allocated_file_size(bs);
>> @@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>>           info->dirty_flag = bdi.is_dirty;
>>           info->has_dirty_flag = true;
>>       }
>> -    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
>> -    if (backing_filename[0] != '\0') {
>> +    backing_filename = bs->backing_file;
>> +    if (backing_filename && backing_filename[0] != '\0') {
> 
> backing_filename can't possibly be null here, because bs->backing_file
> is an array.
> 
  OK, will remove it.

>>           info->backing_filename = g_strdup(backing_filename);
>>           info->has_backing_filename = true;
>>           bdrv_get_full_backing_filename(bs, backing_filename2,
>> @@ -168,4 +172,27 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>>               info->has_backing_filename_format = true;
>>           }
>>       }
>> +
>> +    ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
>> +    switch (ret) {
>> +    case 0:
>> +        if (info->snapshots) {
>> +            info->has_snapshots = true;
>> +        }
>> +        break;
>> +    /* recoverable error */
>> +    case -ENOMEDIUM:
>> +        error_free(err);
>> +        break;
>> +    case -ENOTSUP:
>> +        error_free(err);
>> +        break;
> 
> Suggest
> 
>         case -ENOMEDIUM:
>         case -ENOTSUP:
>             /* no snapshots on this device */
>             error_free(err);
>             break;
> 
> for clarity.
> 
  OK.

>> +    default:
>> +        error_propagate(errp, err);
>> +        qapi_free_ImageInfo(info);
>> +        return ret;
>> +    }
>> +
>> +    *p_info = info;
>> +    return 0;
>>   }
>> diff --git a/include/block/qapi.h b/include/block/qapi.h
>> index fe66053..2c62fdf 100644
>> --- a/include/block/qapi.h
>> +++ b/include/block/qapi.h
>> @@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>>                                     SnapshotInfoList **p_list,
>>                                     bool vm_snapshot,
>>                                     Error **errp);
>> -void bdrv_collect_image_info(BlockDriverState *bs,
>> -                             ImageInfo *info,
>> -                             const char *filename);
>> +int bdrv_query_image_info(BlockDriverState *bs,
>> +                          ImageInfo **p_info,
>> +                          Error **errp);
>>   #endif
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 261c277..1dd0a60 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1733,12 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
>>               goto err;
>>           }
>>   
>> -        info = g_new0(ImageInfo, 1);
>> -        bdrv_collect_image_info(bs, info, filename);
>> -        if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
>> -                                           false, NULL) &&
>> -            info->snapshots) {
>> -            info->has_snapshots = true;
>> +        if (bdrv_query_image_info(bs, &info, NULL)) {
>> +            goto err;
>>           }
>>   
>>           elem = g_new0(ImageInfoList, 1);
>

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 19d4d93..176a479 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -122,18 +122,22 @@  int bdrv_query_snapshot_info_list(BlockDriverState *bs,
     return 0;
 }
 
-void bdrv_collect_image_info(BlockDriverState *bs,
-                             ImageInfo *info,
-                             const char *filename)
+/* return 0 on success, and @p_info will be set only on success. */
+int bdrv_query_image_info(BlockDriverState *bs,
+                          ImageInfo **p_info,
+                          Error **errp)
 {
     uint64_t total_sectors;
-    char backing_filename[1024];
+    const char *backing_filename;
     char backing_filename2[1024];
     BlockDriverInfo bdi;
+    int ret;
+    Error *err = NULL;
+    ImageInfo *info = g_new0(ImageInfo, 1);
 
     bdrv_get_geometry(bs, &total_sectors);
 
-    info->filename        = g_strdup(filename);
+    info->filename        = g_strdup(bs->filename);
     info->format          = g_strdup(bdrv_get_format_name(bs));
     info->virtual_size    = total_sectors * 512;
     info->actual_size     = bdrv_get_allocated_file_size(bs);
@@ -150,8 +154,8 @@  void bdrv_collect_image_info(BlockDriverState *bs,
         info->dirty_flag = bdi.is_dirty;
         info->has_dirty_flag = true;
     }
-    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
-    if (backing_filename[0] != '\0') {
+    backing_filename = bs->backing_file;
+    if (backing_filename && backing_filename[0] != '\0') {
         info->backing_filename = g_strdup(backing_filename);
         info->has_backing_filename = true;
         bdrv_get_full_backing_filename(bs, backing_filename2,
@@ -168,4 +172,27 @@  void bdrv_collect_image_info(BlockDriverState *bs,
             info->has_backing_filename_format = true;
         }
     }
+
+    ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
+    switch (ret) {
+    case 0:
+        if (info->snapshots) {
+            info->has_snapshots = true;
+        }
+        break;
+    /* recoverable error */
+    case -ENOMEDIUM:
+        error_free(err);
+        break;
+    case -ENOTSUP:
+        error_free(err);
+        break;
+    default:
+        error_propagate(errp, err);
+        qapi_free_ImageInfo(info);
+        return ret;
+    }
+
+    *p_info = info;
+    return 0;
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index fe66053..2c62fdf 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -32,7 +32,7 @@  int bdrv_query_snapshot_info_list(BlockDriverState *bs,
                                   SnapshotInfoList **p_list,
                                   bool vm_snapshot,
                                   Error **errp);
-void bdrv_collect_image_info(BlockDriverState *bs,
-                             ImageInfo *info,
-                             const char *filename);
+int bdrv_query_image_info(BlockDriverState *bs,
+                          ImageInfo **p_info,
+                          Error **errp);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 261c277..1dd0a60 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1733,12 +1733,8 @@  static ImageInfoList *collect_image_info_list(const char *filename,
             goto err;
         }
 
-        info = g_new0(ImageInfo, 1);
-        bdrv_collect_image_info(bs, info, filename);
-        if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
-                                           false, NULL) &&
-            info->snapshots) {
-            info->has_snapshots = true;
+        if (bdrv_query_image_info(bs, &info, NULL)) {
+            goto err;
         }
 
         elem = g_new0(ImageInfoList, 1);