All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: "Tomáš Golembiovský" <tgolembi@redhat.com>
Cc: qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] raw-posix: add 'offset' and 'size' options
Date: Mon, 3 Oct 2016 12:11:46 +0100	[thread overview]
Message-ID: <20161003111146.GG13491@redhat.com> (raw)
In-Reply-To: <20161003130707.6fcfd9dc@fiorina>

On Mon, Oct 03, 2016 at 01:07:07PM +0200, Tomáš Golembiovský wrote:
> On Mon, 3 Oct 2016 11:52:59 +0100
> > 
> > > > > +
> > > > > +    if (((bs->drv != &bdrv_file) || !bs->read_only) &&    
> > > > 
> > > > Why the check against bdrv_file ?  
> > > 
> > > To limit it only to files. Maybe there is better way to do that? The
> > > devices have a nasty habit to change the size. Sure, this can happen to
> > > file too, e.g. if somebody truncates the file outside QEMU. But that's
> > > rather a bad behaviour. For devices changing the size may be perfectly
> > > valid operation, e.g. replacing CD in drive or card in a card reader.  
> > 
> > The raw driver is usable over any storage backend (file, rbd, iscsi,
> > etc, etc) and it is valid to want to use a offset/size parameter in
> > combination with any of them. So we should not restrict it to just
> > files.
> 
> The question is then, what to do when the underlying device changes? The
> size/offset may not be valid at that point anymore.

The underlying device shouldn't be changing in size without that change
going through the raw format driver

> > > > Why did you restrict it to read-only instead of adding the offset logic
> > > > to the write & truncate methods. IMHO if we're going to add this feature
> > > > we should make it work in all scenarios, not just do 1/2 the job.  
> > > 
> > > I still plan to do that, but I didn't feel confident enough to do it in
> > > the first run.
> > > 
> > > We need to decide what is the correct behaviour here first. Since the
> > > image we're working with is contained in some larger unit it cannot be
> > > resized freely. There are two option that come to my mind:
> > > 
> > > 1) block truncate/grow completely,
> > > 2) allow image to be truncated and grown only if the new size does not
> > >    go over the original size.
> > > 
> > > If we go with the second option then I have a another question. How
> > > strict is (or should be) qemu about the sizes being block aligned? I'm
> > > still little bit fuzzy on that. Somewhere it is assumed that the size is
> > > multiple of 512, sometimes qemu automatically rounds up if it isn't and
> > > sometimes it seems to me that the size can be arbitrary.  
> > 
> > We should not make assumptions about what is a valid or invalid usage,
> > as QEMU doesn't have enough knowledge to do that correctly.
> > 
> > IOW, we should just allow truncate with no restrictions. It is up to the
> > calling app to figure out whether truncate makes sense or not.
> 
> Maybe I'm missing something here. Can the truncate operation be somehow
> initiated from inside the VM, or is that only something that can be done
> from outside with 'qemu-img resize'?

Truncate is something run from the host - guests are certainly not allowed
to trigger truncate, as that would let you inflict a denial of service on
the host by consuming  disk space they should not be allowed.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

  reply	other threads:[~2016-10-03 11:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-02 19:13 [Qemu-devel] [PATCH] raw-posix: add 'offset' and 'size' options Tomáš Golembiovský
2016-10-03  8:52 ` Daniel P. Berrange
2016-10-03  9:20   ` Paolo Bonzini
2016-10-03 10:47     ` Tomáš Golembiovský
2016-10-03 11:20       ` Paolo Bonzini
2016-10-03 10:45   ` Tomáš Golembiovský
2016-10-03 10:52     ` Daniel P. Berrange
2016-10-03 11:07       ` Tomáš Golembiovský
2016-10-03 11:11         ` Daniel P. Berrange [this message]
2016-10-04  8:57         ` Kevin Wolf
2016-10-04  9:15           ` Daniel P. Berrange
2016-10-04 13:58             ` Eric Blake
2016-10-04 11:48           ` Tomáš Golembiovský
2016-10-03 15:28 ` Eric Blake
2016-10-04  9:24 ` Kevin Wolf
2016-10-04 10:12   ` Paolo Bonzini
2016-10-04 11:59     ` 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=20161003111146.GG13491@redhat.com \
    --to=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=tgolembi@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.