[Qemu-devel] [PATCH v3 1/6] block: add basic conversion api

[Qemu-devel] [PATCH v3 1/6] block: add basic conversion api

[Devin Nakamura]

add functions to block driver interface to support inplace image conversion

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block_int.h |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/block_int.h b/block_int.h
index 1e265d2..050ecf3 100644
--- a/block_int.h
+++ b/block_int.h
@@ -137,6 +137,76 @@ struct BlockDriver {
      */
     int (*bdrv_has_zero_init)(BlockDriverState *bs);
 
+    /* In-place image conversion */
+
+    /**
+     *
+     * @param bs      Basic Initialization done by bdrv_open_conversion_target()
+     *                Still need to set
+     * @param options Creation options.
+     * @return        Returns non-zero on failure.
+     */
+    int (*bdrv_open_conversion_target)(BlockDriverState *bs,
+        QEMUOptionParameter *options);
+
+    /**
+     * Gets a mapping in the image file.
+     *
+     * The function starts searching for a mapping at
+     * starting_guest_offset = guest_offset + contiguous_bytes
+     * @param bs[in]                   The image in which to find mapping.
+     * @param guest_offset[in,out]     On function entry used to calculate
+     *                                 starting search address.
+     *                                 On function exit contains the staring
+     *                                 guest offset of the mapping.
+     * @param host_offset[out]         The starting image file offset for the
+     *                                 mapping.
+     * @param contiguous_bytes[in,out] On function entry used to calculate
+     *                                 starting search address.
+     *                                 On function exit contains the number of
+     *                                 bytes for which this mapping is valid.
+     *                                 A value of 0 means there are no more
+     *                                 mappings in the image.
+     * @return                         Returns non-zero on error.
+     */
+    int (*bdrv_get_mapping)(BlockDriverState *bs, uint64_t *guest_offset,
+        uint64_t *host_offset, uint64_t *contiguous_bytes);
+
+    /**
+     * Sets a mapping in the image file.
+     *
+     * @param bs               Usualy opened with bdrv_open_conversion_target
+     * @param guest_offset     The starting guest offset of the mapping
+     *                         (in bytes)
+     * @param host_offset      The starting image offset of the mapping
+     *                         (in bytes)
+     * @param contiguous_bytes The number of bytes for which this mapping exists
+     * @return                 Returns non-zero on error
+     */
+    int (*bdrv_map)(BlockDriverState *bs, uint64_t guest_offset,
+        uint64_t host_offset, uint64_t contiguous_bytes);
+
+    /**
+     * Copies out the header of a conversion target
+     *
+     * Saves the current header for the image in a temporary file and overwrites
+     * it with the header for the new format (at the moment the header is
+     * assumed to be 1 sector)
+     *
+     * @param bs  Usualy opened with bdrv_open_conversion_target().
+     * @return    Returns non-zero on failure
+     */
+    int (*bdrv_copy_header) (BlockDriverState *bs);
+
+    /**
+     * Asks the block driver what options should be used to create a conversion
+     * target.
+     * @param options[out]
+     */
+    int (*bdrv_get_conversion_options)(BlockDriverState *bs,
+        QEMUOptionParameter **options);
+
+
     QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
1.7.6.rc1

[Qemu-devel] [PATCH v3 2/6] block: add bdrv_get_conversion_options()

[Devin Nakamura]

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   13 +++++++++++++
 block.h |    2 ++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 24a25d5..5c0ba2d 100644
--- a/block.c
+++ b/block.c
@@ -3018,3 +3018,16 @@ out:
 
     return ret;
 }
+
+int bdrv_get_conversion_options(BlockDriverState *bs,
+                                QEMUOptionParameter **options)
+{
+    if (!bs->drv) {
+        return -ENOENT;
+    }
+
+    if (!bs->drv->bdrv_get_conversion_options) {
+        return -ENOTSUP;
+    }
+    return bs->drv->bdrv_get_conversion_options(bs, options);
+}
diff --git a/block.h b/block.h
index 859d1d9..ca0af6d 100644
--- a/block.h
+++ b/block.h
@@ -250,6 +250,8 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
+int bdrv_get_conversion_options(BlockDriverState *bs,
+                                QEMUOptionParameter **options);
 typedef enum {
     BLKDBG_L1_UPDATE,
 
-- 
1.7.6.rc1

[Qemu-devel] [PATCH v3 3/6] block: add bdrv_open_conversion_target()

[Devin Nakamura]

Conflicts:

	block.h

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   31 +++++++++++++++++++++++++++++++
 block.h |    3 +++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 5c0ba2d..7d3bc67 100644
--- a/block.c
+++ b/block.c
@@ -3019,6 +3019,37 @@ out:
     return ret;
 }
 
+int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
+                                QEMUOptionParameter *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 = 0;
+    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, options);
+
+}
+
 int bdrv_get_conversion_options(BlockDriverState *bs,
                                 QEMUOptionParameter **options)
 {
diff --git a/block.h b/block.h
index ca0af6d..145608e 100644
--- a/block.h
+++ b/block.h
@@ -252,6 +252,9 @@ int bdrv_in_use(BlockDriverState *bs);
 
 int bdrv_get_conversion_options(BlockDriverState *bs,
                                 QEMUOptionParameter **options);
+int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
+                                QEMUOptionParameter *options,
+                                const char *target_fmt);
 typedef enum {
     BLKDBG_L1_UPDATE,
 
-- 
1.7.6.rc1

[Qemu-devel] [PATCH v3 4/6] block: add bdrv_get_mapping()

[Devin Nakamura]

Conflicts:

	block.h

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   29 +++++++++++++++++++++++++++++
 block.h |    2 ++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 7d3bc67..cda600b 100644
--- a/block.c
+++ b/block.c
@@ -3062,3 +3062,32 @@ int bdrv_get_conversion_options(BlockDriverState *bs,
     }
     return bs->drv->bdrv_get_conversion_options(bs, options);
 }
+
+/**
+ * Gets a mapping from an offset in the image to an offset within a file
+ *
+ * All offsets are in bytes. Functions starts its search at offset host_offset
+ * + count (offset within the image, not the file offset)
+ *
+ * @param guest_offset          used to calculate starting search location on
+ *                              function call. On return it is the starting
+ *                              offset of the mapping in the image.
+ * @param host_offset[out]      The starting file offset of the mapping.
+ * @param contiguous_bytes[out] The number of bytes for which this mapping is
+ *                              contiguous. If 0, there are no more mapppings in
+ *                              the image
+ */
+
+int bdrv_get_mapping(BlockDriverState *bs, uint64_t *guest_offset,
+                     uint64_t *host_offset, uint64_t *contiguous_bytes)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+    if (!drv->bdrv_get_mapping) {
+        return -ENOTSUP;
+    }
+    return drv->bdrv_get_mapping(bs, guest_offset, host_offset,
+        contiguous_bytes);
+}
diff --git a/block.h b/block.h
index 145608e..c40223d 100644
--- a/block.h
+++ b/block.h
@@ -255,6 +255,8 @@ int bdrv_get_conversion_options(BlockDriverState *bs,
 int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
                                 QEMUOptionParameter *options,
                                 const char *target_fmt);
+int bdrv_get_mapping(BlockDriverState *bs, uint64_t *guest_offset,
+                     uint64_t *host_offset, uint64_t *contiguous_bytes);
 typedef enum {
     BLKDBG_L1_UPDATE,
 
-- 
1.7.6.rc1

[Qemu-devel] [PATCH v3 5/6] block: add bdrv_map()

[Devin Nakamura]

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   14 ++++++++++++++
 block.h |    2 ++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index cda600b..86a4752 100644
--- a/block.c
+++ b/block.c
@@ -3091,3 +3091,17 @@ int bdrv_get_mapping(BlockDriverState *bs, uint64_t *guest_offset,
     return drv->bdrv_get_mapping(bs, guest_offset, host_offset,
         contiguous_bytes);
 }
+
+int bdrv_map(BlockDriverState *bs, uint64_t *guest_offset,
+             uint64_t *host_offset, uint64_t *contiguous_bytes)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+    if (!drv->bdrv_map) {
+        return -ENOTSUP;
+    }
+    return drv->bdrv_map(bs, guest_offset, host_offset,
+        contiguous_bytes);
+}
diff --git a/block.h b/block.h
index c40223d..baf6f10 100644
--- a/block.h
+++ b/block.h
@@ -257,6 +257,8 @@ int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
                                 const char *target_fmt);
 int bdrv_get_mapping(BlockDriverState *bs, uint64_t *guest_offset,
                      uint64_t *host_offset, uint64_t *contiguous_bytes);
+int bdrv_map(BlockDriverState *bs, uint64_t *guest_offset,
+             uint64_t *host_offset, uint64_t *contiguous_bytes);
 typedef enum {
     BLKDBG_L1_UPDATE,
 
-- 
1.7.6.rc1

[Qemu-devel] [PATCH v3 6/6] block: add bdrv_copy_header()

[Devin Nakamura]

Signed-off-by: Devin Nakamura <devin122@gmail.com>
---
 block.c |   12 ++++++++++++
 block.h |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 86a4752..0f38aff 100644
--- a/block.c
+++ b/block.c
@@ -3105,3 +3105,15 @@ int bdrv_map(BlockDriverState *bs, uint64_t *guest_offset,
     return drv->bdrv_map(bs, guest_offset, host_offset,
         contiguous_bytes);
 }
+
+int bdrv_copy_header(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+    if (!drv->bdrv_copy_header) {
+        return -ENOTSUP;
+    }
+    return drv->bdrv_copy_header(bs);
+}
diff --git a/block.h b/block.h
index baf6f10..169866c 100644
--- a/block.h
+++ b/block.h
@@ -259,6 +259,8 @@ int bdrv_get_mapping(BlockDriverState *bs, uint64_t *guest_offset,
                      uint64_t *host_offset, uint64_t *contiguous_bytes);
 int bdrv_map(BlockDriverState *bs, uint64_t *guest_offset,
              uint64_t *host_offset, uint64_t *contiguous_bytes);
+int bdrv_copy_header(BlockDriverState *bs);
+
 typedef enum {
     BLKDBG_L1_UPDATE,
 
-- 
1.7.6.rc1

Re: [Qemu-devel] [PATCH v3 1/6] block: add basic conversion api

[Kevin Wolf]

Am 13.07.2011 14:57, schrieb Devin Nakamura:
> add functions to block driver interface to support inplace image conversion
> 
> Signed-off-by: Devin Nakamura <devin122@gmail.com>
> ---
>  block_int.h |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/block_int.h b/block_int.h
> index 1e265d2..050ecf3 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -137,6 +137,76 @@ struct BlockDriver {
>       */
>      int (*bdrv_has_zero_init)(BlockDriverState *bs);
>  
> +    /* In-place image conversion */
> +
> +    /**
> +     *

I think this description is a bit short. :-)

> +     * @param bs      Basic Initialization done by bdrv_open_conversion_target()
> +     *                Still need to set

Need to set what?

> +     * @param options Creation options.
> +     * @return        Returns non-zero on failure.
> +     */
> +    int (*bdrv_open_conversion_target)(BlockDriverState *bs,
> +        QEMUOptionParameter *options);
> +
> +    /**
> +     * Gets a mapping in the image file.
> +     *
> +     * The function starts searching for a mapping at
> +     * starting_guest_offset = guest_offset + contiguous_bytes

Add a blank line here

> +     * @param bs[in]                   The image in which to find mapping.
> +     * @param guest_offset[in,out]     On function entry used to calculate
> +     *                                 starting search address.
> +     *                                 On function exit contains the staring
> +     *                                 guest offset of the mapping.

s/staring/starting/

> +     * @param host_offset[out]         The starting image file offset for the
> +     *                                 mapping.
> +     * @param contiguous_bytes[in,out] On function entry used to calculate
> +     *                                 starting search address.
> +     *                                 On function exit contains the number of
> +     *                                 bytes for which this mapping is valid.
> +     *                                 A value of 0 means there are no more
> +     *                                 mappings in the image.
> +     * @return                         Returns non-zero on error.
> +     */
> +    int (*bdrv_get_mapping)(BlockDriverState *bs, uint64_t *guest_offset,
> +        uint64_t *host_offset, uint64_t *contiguous_bytes);

I think it would be less surprising if the function worked like
bdrv_is_allocated(). That is, it doesn't search for mapped clusters, but
always returns information for exactly the given offset. It would return
if the range starting at the given offset is used or free.

If the caller wants to find the next existing mapping, he can just take
contiguous_bytes of the free region and add it to his current offset.

> +
> +    /**
> +     * Sets a mapping in the image file.
> +     *
> +     * @param bs               Usualy opened with bdrv_open_conversion_target

Is it required that the image was opened with
bdrv_open_conversion_target? If yes, say so in the comment. If no, no
reason to have it here.

> +     * @param guest_offset     The starting guest offset of the mapping
> +     *                         (in bytes)
> +     * @param host_offset      The starting image offset of the mapping
> +     *                         (in bytes)
> +     * @param contiguous_bytes The number of bytes for which this mapping exists
> +     * @return                 Returns non-zero on error
> +     */
> +    int (*bdrv_map)(BlockDriverState *bs, uint64_t guest_offset,
> +        uint64_t host_offset, uint64_t contiguous_bytes);

What happens if one of the offsets or contiguous_bytes is not aligned to
the cluster size?

> +
> +    /**
> +     * Copies out the header of a conversion target
> +     *
> +     * Saves the current header for the image in a temporary file and overwrites
> +     * it with the header for the new format (at the moment the header is
> +     * assumed to be 1 sector)
> +     *
> +     * @param bs  Usualy opened with bdrv_open_conversion_target().
> +     * @return    Returns non-zero on failure
> +     */
> +    int (*bdrv_copy_header) (BlockDriverState *bs);
> +
> +    /**
> +     * Asks the block driver what options should be used to create a conversion
> +     * target.
> +     * @param options[out]
> +     */
> +    int (*bdrv_get_conversion_options)(BlockDriverState *bs,
> +        QEMUOptionParameter **options);

No description for options? With this description, I'm not sure how this
function is meant to work and which QEMUOptionParameter list it uses.
The one of the source format or the one of the destination format?

Kevin