From: "Tomáš Golembiovský" <tgolembi@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-nbd: Add --image-size option
Date: Sun, 2 Oct 2016 21:33:44 +0200 [thread overview]
Message-ID: <20161002213344.5963994a@fiorina> (raw)
In-Reply-To: <7d43cbb2-6edf-78c0-89d0-09a58917ab56@redhat.com>
On Tue, 20 Sep 2016 13:45:08 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 20/09/2016 13:12, Daniel P. Berrange wrote:
> > On Tue, Sep 20, 2016 at 11:59:28AM +0200, Paolo Bonzini wrote:
> >>
> >>
> >> On 20/09/2016 11:41, Tomáš Golembiovský wrote:
> >>> When image is part of the file it makes sense to limit the length of the
> >>> image in the file. Otherwise it is assumed that the image spans to the
> >>> end of the file. This assumption may lead to reads/writes outside of the
> >>> image and thus lead to errors or data corruption.
> >>>
> >>> To limit the assumed image size new option is introduced.
> >>
> >> The patch makes sense, but I think the commit message is incorrect
> >> because this bug is already fixed by patch 1. Also, the option in the
> >> help is --device-size, not --image-size; I would just call it --size.
> >
> > I don't think it makes sense as a special case in qemu-nbd.
> >
> > It feels like this is better done by extending the 'raw'
> > block driver with 'offset' and 'length' parameters. You
> > can then layer the 'raw' block driver over any existing
> > QEMU blocker driver, anywhere in QEMU / tools that accept
> > a block device description. No need to add extra parameters
> > to any of the programs.
>
> This makes sense too (and then --offset and partitions can be
> implemented on top).
OK. Let's drop this change then. I have sent an initial attempt to
implement this in raw block driver here [1]. The change only covers
files opened in read-only mode for the moment so it will take a little
bit more effort before it can be used in qemu-nbd.
That being said, I believe the first [2] part of the series still should
be considered for merging. Unless there is something intrinsically wrong
with the patch. Once the necessary stuff is in the raw driver we can
change the related code in qemu-nbd.
Regards,
Tomas
[1] https://lists.nongnu.org/archive/html/qemu-block/2016-10/msg00008.html
[2] https://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg04554.html
--
Tomáš Golembiovský <tgolembi@redhat.com>
next prev parent reply other threads:[~2016-10-02 19:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-20 9:41 [Qemu-devel] [PATCH 2/2] qemu-nbd: Add --image-size option Tomáš Golembiovský
2016-09-20 9:59 ` Paolo Bonzini
2016-09-20 11:12 ` Daniel P. Berrange
2016-09-20 11:45 ` Paolo Bonzini
2016-10-02 19:33 ` Tomáš Golembiovský [this message]
2016-09-20 11:35 ` Tomáš Golembiovský
2016-09-20 12:45 ` Richard W.M. Jones
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=20161002213344.5963994a@fiorina \
--to=tgolembi@redhat.com \
--cc=berrange@redhat.com \
--cc=pbonzini@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).