Patchwork [RFC,03/24] block: add bdrv_open_conversion_target()

login
register
mail settings
Submitter Devin Nakamura
Date July 29, 2011, 4:49 a.m.
Message ID <1311914994-20482-4-git-send-email-devin122@gmail.com>
Download mbox | patch
Permalink /patch/107329/
State New
Headers show

Comments

Devin Nakamura - July 29, 2011, 4:49 a.m.
Conflicts:

	block.h

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   32 ++++++++++++++++++++++++++++++++
 block.h |    4 ++++
 2 files changed, 36 insertions(+), 0 deletions(-)
Kevin Wolf - Aug. 1, 2011, 1:42 p.m.
Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Conflicts:
> 
> 	block.h

You can probably remove this note. ;-)

> 
> Signed-off-by: Devin Nakamura <devin122@gmail.com>
> ---
>  block.c |   32 ++++++++++++++++++++++++++++++++
>  block.h |    4 ++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 4503b7b..9530577 100644
> --- a/block.c
> +++ b/block.c
> @@ -3038,6 +3038,38 @@ out:
>      return ret;
>  }
>  
> +int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
> +                                BlockConversionOptions *drv_options,
> +                                QEMUOptionParameter *usr_options,
> +                                const char *target_fmt)
> +{
> +    BlockDriver *drv;
> +    BlockDriverState *bss;
> +
> +    drv = bdrv_find_format(target_fmt);
> +    if (!drv) {
> +        return -ENOENT;
> +    }
> +
> +    if (!drv->bdrv_open_conversion_target) {
> +        return -ENOTSUP;
> +    }
> +
> +    *bs = bdrv_new("");
> +    bss = *bs;
> +    bss->file = file;
> +    bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS;
> +    bss->encrypted = 0;
> +    bss->valid_key = 0;
> +    bss->open_flags = 0;
> +    /* buffer_alignment defaulted to 512, drivers can change this value */
> +    bss->buffer_alignment = 512;
> +    bss->opaque = qemu_mallocz(drv->instance_size);
> +    bss->drv = drv;
> +    return drv->bdrv_open_conversion_target(bss, drv_options, usr_options);
> +
> +}

How big are the differences really to bdrv_open_common? Have you checked
if you could reuse it? It looks to me as if you're just handling fewer
cases and could achieve the same by passing the right flags to
bdrv_open_common. The only real difference is drv->bdrv_open vs.
drv->bdrv_open_conversion_target, which could probably be handled by
another flag.

The problem with keeping it separate is that it makes it easy to change
bdrv_open without changing bdrv_open_conversion_target. Most people
touching the code won't use in-place conversion very often, so they
won't notice any breakage in their testing.

Kevin
Stefan Hajnoczi - Aug. 2, 2011, 8:57 a.m.
On Fri, Jul 29, 2011 at 12:49:33AM -0400, Devin Nakamura wrote:
> +int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
> +                                BlockConversionOptions *drv_options,
> +                                QEMUOptionParameter *usr_options,
> +                                const char *target_fmt)
> +{
> +    BlockDriver *drv;
> +    BlockDriverState *bss;
> +
> +    drv = bdrv_find_format(target_fmt);
> +    if (!drv) {
> +        return -ENOENT;
> +    }
> +
> +    if (!drv->bdrv_open_conversion_target) {
> +        return -ENOTSUP;
> +    }
> +
> +    *bs = bdrv_new("");
> +    bss = *bs;
> +    bss->file = file;
> +    bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS;
> +    bss->encrypted = 0;
> +    bss->valid_key = 0;
> +    bss->open_flags = 0;
> +    /* buffer_alignment defaulted to 512, drivers can change this value */
> +    bss->buffer_alignment = 512;
> +    bss->opaque = qemu_mallocz(drv->instance_size);
> +    bss->drv = drv;
> +    return drv->bdrv_open_conversion_target(bss, drv_options, usr_options);

Normally a function that fails does not leave anything allocated.
Please delete bss and *bs = NULL before returning an error.

Stefan

Patch

diff --git a/block.c b/block.c
index 4503b7b..9530577 100644
--- a/block.c
+++ b/block.c
@@ -3038,6 +3038,38 @@  out:
     return ret;
 }
 
+int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
+                                BlockConversionOptions *drv_options,
+                                QEMUOptionParameter *usr_options,
+                                const char *target_fmt)
+{
+    BlockDriver *drv;
+    BlockDriverState *bss;
+
+    drv = bdrv_find_format(target_fmt);
+    if (!drv) {
+        return -ENOENT;
+    }
+
+    if (!drv->bdrv_open_conversion_target) {
+        return -ENOTSUP;
+    }
+
+    *bs = bdrv_new("");
+    bss = *bs;
+    bss->file = file;
+    bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS;
+    bss->encrypted = 0;
+    bss->valid_key = 0;
+    bss->open_flags = 0;
+    /* buffer_alignment defaulted to 512, drivers can change this value */
+    bss->buffer_alignment = 512;
+    bss->opaque = qemu_mallocz(drv->instance_size);
+    bss->drv = drv;
+    return drv->bdrv_open_conversion_target(bss, drv_options, usr_options);
+
+}
+
 int bdrv_get_conversion_options(BlockDriverState *bs,
                                 BlockConversionOptions *options)
 {
diff --git a/block.h b/block.h
index ad7e5ea..662bae7 100644
--- a/block.h
+++ b/block.h
@@ -255,6 +255,10 @@  typedef struct BlockConversionOptions BlockConversionOptions;
 
 int bdrv_get_conversion_options(BlockDriverState *bs,
                                 BlockConversionOptions *options);
+int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
+                                BlockConversionOptions *drv_options,
+                                QEMUOptionParameter *usr_options,
+                                const char *target_fmt);
 typedef enum {
     BLKDBG_L1_UPDATE,