All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] block-raw: Allow pread beyond the end of growable images
Date: Wed, 1 Jul 2009 13:37:26 +0200	[thread overview]
Message-ID: <20090701113726.GD10455@lst.de> (raw)
In-Reply-To: <4A4B2777.3050303@redhat.com>

On Wed, Jul 01, 2009 at 11:08:07AM +0200, Kevin Wolf wrote:
> raw-win32 is actually a good question, I haven't tested that one. I
> think nbd is used as a protocol rather than the format. qcow1/2 should
> work, don't know for other formats like VMDK.
> 
> What is biting us here is that nobody has ever specified what the block
> driver functions are supposed to do. They exist because they are in the
> struct, their parameters have names that give a rough idea about their
> meaning and that's it. Who cares about special cases?
> 
> That said, while implementing the the fix in bdrv_pwrite is going to be
> ugly, we could do it in bdrv_read. Maybe this is the best approach.

I have looked a bit at this area and it's even uglier than I expected.

First problem is our use of the growable flag - it's overloaded for two
different purposes:  First we use it to allow growing files if they are
protocols and opened through bdrv_file_open.  Which already seems broken
for host devices, but I'll need to test it.

Second inside qcow2 it allows reading/writing past any image internally
if bdrv_pwrite/bdrv_pread are called via
qcow_put_buffer/qcow_get_buffer.  I think we'd be much more future-proof
if we make these flags to aio_read/aio_write to allow writing past the
image size.

Also none of this currently is easily testable as we don't expose
the growable flag through qemu-io.

And last not least I really hate how we tie up writing the VM metadata
into the block/image code.   I think we should also allow writing it
into an external file with no ties to the image to allow offline
migration or savevm even on a plain file or more important LVM volume.

      reply	other threads:[~2009-07-01 11:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-26 17:51 [Qemu-devel] [PATCH] block-raw: Allow pread beyond the end of growable images Kevin Wolf
2009-06-30 18:09 ` Christoph Hellwig
2009-07-01  7:37   ` Kevin Wolf
2009-07-01  8:56     ` Christoph Hellwig
2009-07-01  9:08       ` Kevin Wolf
2009-07-01 11:37         ` Christoph Hellwig [this message]

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=20090701113726.GD10455@lst.de \
    --to=hch@lst.de \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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.