[Qemu-devel] [PATCH 6/6] qed: add bdrv_qed_copy_header()

[Qemu-devel] [PATCH 6/6] qed: add bdrv_qed_copy_header()

[Devin Nakamura]

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

diff --git a/block/qed.c b/block/qed.c
index 341cf9d..caecdff 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1586,6 +1586,20 @@ static int bdrv_qed_map(BlockDriverState *bs, uint64_t guest_offset,
     return 0;
 }
 
+static int bdrv_qed_copy_header(BlockDriverState *bs, char* filename)
+{
+    BlockDriverState *backup;
+    uint8_t buffer[512];
+    bdrv_create_file(filename, NULL);
+    bdrv_file_open(&backup, filename, BDRV_O_RDWR);
+    bdrv_read(bs->file, 0, buffer, 1); /*TODO: check return code*/
+    bdrv_write(backup, 0, buffer, 1); /*TODO: check return code*/
+    bdrv_close(backup);
+
+    qed_write_header_sync(bs->opaque);
+    return 0;
+}
+
 static QEMUOptionParameter qed_create_options[] = {
     {
         .name = BLOCK_OPT_SIZE,
@@ -1636,6 +1650,7 @@ static BlockDriver bdrv_qed = {
     .bdrv_open_conversion_target = bdrv_qed_open_conversion_target,
     .bdrv_get_mapping         = bdrv_qed_get_mapping,
     .bdrv_map                 = bdrv_qed_map,
+    .bdrv_copy_header            = bdrv_qed_copy_header,
 };
 
 static void bdrv_qed_init(void)
-- 
1.7.6.rc1

Re: [Qemu-devel] [PATCH 6/6] qed: add bdrv_qed_copy_header()

[Kevin Wolf]

Am 12.09.2011 16:59, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura <devin122@gmail.com>
> ---
>  block/qed.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/block/qed.c b/block/qed.c
> index 341cf9d..caecdff 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1586,6 +1586,20 @@ static int bdrv_qed_map(BlockDriverState *bs, uint64_t guest_offset,
>      return 0;
>  }
>  
> +static int bdrv_qed_copy_header(BlockDriverState *bs, char* filename)
> +{
> +    BlockDriverState *backup;
> +    uint8_t buffer[512];
> +    bdrv_create_file(filename, NULL);
> +    bdrv_file_open(&backup, filename, BDRV_O_RDWR);
> +    bdrv_read(bs->file, 0, buffer, 1); /*TODO: check return code*/
> +    bdrv_write(backup, 0, buffer, 1); /*TODO: check return code*/

Come on, checking return values isn't much more work than adding a TODO
comment.

> +    bdrv_close(backup);
> +
> +    qed_write_header_sync(bs->opaque);
> +    return 0;
> +}

This is probably right, but it makes me wonder if we are sure that QED
never tries to write out the header before bdrv_qed_copy_header() is
called? In that case we would have destroyed the original format before
completing the conversion.

I would feel more comfortable if BDRVQEDState had a field header_offset
that would be set to a temporary location by
bdrv_qed_open_conversion_target and reset to 0 by bdrv_qed_copy_header.

Kevin