All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: mreitz@redhat.com, qemu-devel@nongnu.org, famz@redhat.com,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
Date: Fri, 10 Jun 2016 16:39:29 -0600	[thread overview]
Message-ID: <575B41A1.5030702@redhat.com> (raw)
In-Reply-To: <1465574722-27656-6-git-send-email-kwolf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> We don't really want to go through the block layer in order to read from
> or write to the vmstate in a qcow2 image. Doing so required a few ugly
> hacks like saving and restoring the old image size (because writing to
> vmstate offsets would increase the image size) or disabling the "reads
> after EOF = zeroes" logic. When calling the right functions directly,
> these hacks aren't necessary any more.
> 
> Note that .bdrv_vmstate_load/save() return 0 instead of the number of
> bytes in case of success now.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 24 ++++--------------------
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 

> +++ b/block/qcow2.c
> @@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
>                                int64_t pos)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    int64_t total_sectors = bs->total_sectors;
> -    bool zero_beyond_eof = bs->zero_beyond_eof;
> -    int ret;
>  
>      BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
> -    bs->zero_beyond_eof = false;
> -    ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> -    bs->zero_beyond_eof = zero_beyond_eof;
> -
> -    /* bdrv_co_do_writev will have increased the total_sectors value to include
> -     * the VM state - the VM state is however not an actual part of the block
> -     * device, therefore, we need to restore the old value. */
> -    bs->total_sectors = total_sectors;
> -
> -    return ret;
> +    return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
> +                                    qiov->size, qiov, 0);
>  }

bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
have it yet.  Should you be asserting that it exists, and/or returning
an error if it does not?

>  
>  static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
>                                int64_t pos)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    bool zero_beyond_eof = bs->zero_beyond_eof;
> -    int ret;
>  
>      BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
> -    bs->zero_beyond_eof = false;
> -    ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
> -    bs->zero_beyond_eof = zero_beyond_eof;
> -
> -    return ret;
> +    return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
> +                                   qiov->size, qiov, 0);

Ditto.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2016-06-10 22:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
2016-06-10 20:50   ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored Kevin Wolf
2016-06-10 21:25   ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno Kevin Wolf
2016-06-10 21:29   ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
2016-06-10 22:33   ` Eric Blake
2016-06-16  8:49   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-10 16:05 ` [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly Kevin Wolf
2016-06-10 22:39   ` Eric Blake [this message]
2016-06-12  2:58     ` Fam Zheng
2016-06-13 12:36       ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof Kevin Wolf
2016-06-10 22:41   ` Eric Blake
2016-06-12  2:59 ` [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Fam Zheng
2016-06-16  8:54 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-16  9:33 ` [Qemu-devel] " Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=575B41A1.5030702@redhat.com \
    --to=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.