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

[Kevin Wolf <kwolf@redhat.com>]

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