[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] block/parallels: Don't update header until
From: |
Denis V. Lunev |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] block/parallels: Don't update header until the first actual write |
Date: |
Fri, 27 Oct 2017 14:13:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 10/27/2017 10:57 AM, Jeff Cody wrote:
> The on disk image format 'inuse' header field is updated blindly if the
> image is opened RDWR. This can cause problems if the QEMU runstate is
> set to INMIGRATE, at which point the underlying file is set to INACTIVE.
> This causes an assert in bdrv_co_pwritev().
>
> Do something similar to what is done in VHDX; latch the first write, and
> update the header the first time we modify the file.
>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
> block/parallels.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index fed199eccd..c560e2fcf2 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -100,6 +100,8 @@ typedef struct BDRVParallelsState {
> unsigned int tracks;
>
> unsigned int off_multiplier;
> +
> + bool first_write_latch;
> } BDRVParallelsState;
>
>
> @@ -317,6 +319,16 @@ static coroutine_fn int
> parallels_co_writev(BlockDriverState *bs,
> QEMUIOVector hd_qiov;
> int ret = 0;
>
> + if (s->first_write_latch) {
> + s->first_write_latch = false;
> + qemu_co_mutex_lock(&s->lock);
> + ret = parallels_update_header(bs);
> + qemu_co_mutex_unlock(&s->lock);
> + }
> + if (ret < 0) {
> + return ret;
> + }
> +
> qemu_iovec_init(&hd_qiov, qiov->niov);
>
> while (nb_sectors > 0) {
> @@ -416,6 +428,9 @@ static int parallels_check(BlockDriverState *bs,
> BdrvCheckResult *res,
> /* parallels_close will do the job right */
> res->corruptions_fixed++;
> s->header_unclean = false;
> + /* set that a write has occurred, so that parallels_close() will
> + * update the inuse field in the header */
> + s->first_write_latch = false;
> }
> }
>
> @@ -597,6 +612,8 @@ static int parallels_open(BlockDriverState *bs, QDict
> *options, int flags,
> Error *local_err = NULL;
> char *buf;
>
> + s->first_write_latch = true;
> +
> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
> false, errp);
> if (!bs->file) {
> @@ -710,10 +727,6 @@ static int parallels_open(BlockDriverState *bs, QDict
> *options, int flags,
>
> if (flags & BDRV_O_RDWR) {
> s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
> - ret = parallels_update_header(bs);
> - if (ret < 0) {
> - goto fail;
> - }
> }
>
> s->bat_dirty_block = 4 * getpagesize();
> @@ -741,7 +754,9 @@ static void parallels_close(BlockDriverState *bs)
> {
> BDRVParallelsState *s = bs->opaque;
>
> - if (bs->open_flags & BDRV_O_RDWR) {
> + /* Only need to update the header, if we ever actually wrote to the
> + * image at all */
> + if ((bs->open_flags & BDRV_O_RDWR) && !s->first_write_latch) {
> s->header->inuse = 0;
> parallels_update_header(bs);
> }
Reviewed-by: Denis V. Lunev <address@hidden>