diff mbox

[v5,17/22] blkverify: Allow command-line configuration

Message ID 1386954633-28905-18-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Dec. 13, 2013, 5:10 p.m. UTC 
Introduce the "test" and "raw" options for specifying images.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/blkverify.c | 59 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 20 deletions(-)

Comments

Kevin Wolf Dec. 13, 2013, 8:46 p.m. UTC | #1 
Am 13.12.2013 um 18:10 hat Max Reitz geschrieben:
> Introduce the "test" and "raw" options for specifying images.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/blkverify.c | 59 ++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/block/blkverify.c b/block/blkverify.c
> index c6eb287..8bf81b5 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c
> @@ -116,13 +116,46 @@ static QemuOptsList runtime_opts = {
>      },
>  };
>  
> +static int open_image(BlockDriverState **pbs, const char *fname, QDict *options,
> +                      const char *bdref_key, int flags, Error **errp)
> +{
> +    QDict *image_options;
> +    int ret;
> +    char *bdref_key_dot;
> +
> +    bdref_key_dot = g_strdup_printf("%s.", bdref_key);
> +    qdict_extract_subqdict(options, &image_options, bdref_key_dot);
> +    g_free(bdref_key_dot);
> +
> +    if (fname) {
> +        /* If a filename is given, use bdrv_open() in order to use the correct
> +           block driver (instead of just opening the raw image). */
> +
> +        if (qdict_get_try_str(options, bdref_key)) {
> +            error_setg(errp, "Cannot reference an existing block device while "
> +                       "giving a filename");
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +
> +        *pbs = bdrv_new("");
> +        ret = bdrv_open(*pbs, fname, image_options, flags, NULL, errp);
> +    } else {
> +        ret = bdrv_file_open(pbs, NULL, qdict_get_try_str(options, bdref_key),
> +                             image_options, flags, errp);
> +    }
> +
> +fail:
> +    qdict_del(options, bdref_key);
> +    return ret;
> +}
> +
>  static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
>                            Error **errp)
>  {
>      BDRVBlkverifyState *s = bs->opaque;
>      QemuOpts *opts;
>      Error *local_err = NULL;
> -    const char *filename, *raw;
>      int ret;
>  
>      opts = qemu_opts_create_nofail(&runtime_opts);
> @@ -133,33 +166,19 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
>          goto fail;
>      }
>  
> -    /* Parse the raw image filename */
> -    raw = qemu_opt_get(opts, "x-raw");
> -    if (raw == NULL) {
> -        error_setg(errp, "Could not retrieve raw image filename");
> -        ret = -EINVAL;
> -        goto fail;
> -    }
> -
> -    ret = bdrv_file_open(&bs->file, raw, NULL, NULL, flags, &local_err);
> +    /* Open the raw file */
> +    ret = open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, "raw",
> +                     flags, &local_err);

This changes the behaviour: We now do format probing, and probably
insert a raw block driver between blkverify and the protocol driver.

If you had a generalised open_image() that is shared with blkdebug, you
could specify no probing here, and enable probing for the main image.

Kevin
Max Reitz Dec. 14, 2013, 12:22 a.m. UTC | #2 
On 13.12.2013 21:46, Kevin Wolf wrote:
> Am 13.12.2013 um 18:10 hat Max Reitz geschrieben:
>> Introduce the "test" and "raw" options for specifying images.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/blkverify.c | 59 ++++++++++++++++++++++++++++++++++++-------------------
>>   1 file changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/block/blkverify.c b/block/blkverify.c
>> index c6eb287..8bf81b5 100644
>> --- a/block/blkverify.c
>> +++ b/block/blkverify.c
>> @@ -116,13 +116,46 @@ static QemuOptsList runtime_opts = {
>>       },
>>   };
>>   
>> +static int open_image(BlockDriverState **pbs, const char *fname, QDict *options,
>> +                      const char *bdref_key, int flags, Error **errp)
>> +{
>> +    QDict *image_options;
>> +    int ret;
>> +    char *bdref_key_dot;
>> +
>> +    bdref_key_dot = g_strdup_printf("%s.", bdref_key);
>> +    qdict_extract_subqdict(options, &image_options, bdref_key_dot);
>> +    g_free(bdref_key_dot);
>> +
>> +    if (fname) {
>> +        /* If a filename is given, use bdrv_open() in order to use the correct
>> +           block driver (instead of just opening the raw image). */
>> +
>> +        if (qdict_get_try_str(options, bdref_key)) {
>> +            error_setg(errp, "Cannot reference an existing block device while "
>> +                       "giving a filename");
>> +            ret = -EINVAL;
>> +            goto fail;
>> +        }
>> +
>> +        *pbs = bdrv_new("");
>> +        ret = bdrv_open(*pbs, fname, image_options, flags, NULL, errp);
>> +    } else {
>> +        ret = bdrv_file_open(pbs, NULL, qdict_get_try_str(options, bdref_key),
>> +                             image_options, flags, errp);
>> +    }
>> +
>> +fail:
>> +    qdict_del(options, bdref_key);
>> +    return ret;
>> +}
>> +
>>   static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
>>                             Error **errp)
>>   {
>>       BDRVBlkverifyState *s = bs->opaque;
>>       QemuOpts *opts;
>>       Error *local_err = NULL;
>> -    const char *filename, *raw;
>>       int ret;
>>   
>>       opts = qemu_opts_create_nofail(&runtime_opts);
>> @@ -133,33 +166,19 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
>>           goto fail;
>>       }
>>   
>> -    /* Parse the raw image filename */
>> -    raw = qemu_opt_get(opts, "x-raw");
>> -    if (raw == NULL) {
>> -        error_setg(errp, "Could not retrieve raw image filename");
>> -        ret = -EINVAL;
>> -        goto fail;
>> -    }
>> -
>> -    ret = bdrv_file_open(&bs->file, raw, NULL, NULL, flags, &local_err);
>> +    /* Open the raw file */
>> +    ret = open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, "raw",
>> +                     flags, &local_err);
> This changes the behaviour: We now do format probing, and probably
> insert a raw block driver between blkverify and the protocol driver.
>
> If you had a generalised open_image() that is shared with blkdebug, you
> could specify no probing here, and enable probing for the main image.

My main problem with a shared open_image() was finding a nice global 
name. ;-)

I think I'll add a flag for now. With that flag, we could easily make it 
global, but finding a nice name still remains a problem.

Max
diff mbox

Patch

diff --git a/block/blkverify.c b/block/blkverify.c
index c6eb287..8bf81b5 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,13 +116,46 @@  static QemuOptsList runtime_opts = {
     },
 };
 
+static int open_image(BlockDriverState **pbs, const char *fname, QDict *options,
+                      const char *bdref_key, int flags, Error **errp)
+{
+    QDict *image_options;
+    int ret;
+    char *bdref_key_dot;
+
+    bdref_key_dot = g_strdup_printf("%s.", bdref_key);
+    qdict_extract_subqdict(options, &image_options, bdref_key_dot);
+    g_free(bdref_key_dot);
+
+    if (fname) {
+        /* If a filename is given, use bdrv_open() in order to use the correct
+           block driver (instead of just opening the raw image). */
+
+        if (qdict_get_try_str(options, bdref_key)) {
+            error_setg(errp, "Cannot reference an existing block device while "
+                       "giving a filename");
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        *pbs = bdrv_new("");
+        ret = bdrv_open(*pbs, fname, image_options, flags, NULL, errp);
+    } else {
+        ret = bdrv_file_open(pbs, NULL, qdict_get_try_str(options, bdref_key),
+                             image_options, flags, errp);
+    }
+
+fail:
+    qdict_del(options, bdref_key);
+    return ret;
+}
+
 static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
                           Error **errp)
 {
     BDRVBlkverifyState *s = bs->opaque;
     QemuOpts *opts;
     Error *local_err = NULL;
-    const char *filename, *raw;
     int ret;
 
     opts = qemu_opts_create_nofail(&runtime_opts);
@@ -133,33 +166,19 @@  static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    /* Parse the raw image filename */
-    raw = qemu_opt_get(opts, "x-raw");
-    if (raw == NULL) {
-        error_setg(errp, "Could not retrieve raw image filename");
-        ret = -EINVAL;
-        goto fail;
-    }
-
-    ret = bdrv_file_open(&bs->file, raw, NULL, NULL, flags, &local_err);
+    /* Open the raw file */
+    ret = open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, "raw",
+                     flags, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto fail;
     }
 
     /* Open the test file */
-    filename = qemu_opt_get(opts, "x-image");
-    if (filename == NULL) {
-        error_setg(errp, "Could not retrieve test image filename");
-        ret = -EINVAL;
-        goto fail;
-    }
-
-    s->test_file = bdrv_new("");
-    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
+    ret = open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options,
+                     "test", flags, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        bdrv_unref(s->test_file);
         s->test_file = NULL;
         goto fail;
     }