Re: [Qemu-block] [Qemu-devel] [PATCH] raw-posix: add 'offset' and 'size' options

[Kevin Wolf]

Am 03.10.2016 um 13:07 hat Tomáš Golembiovský geschrieben:
> > > > > +    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.

Just to clear up some confusion here: There are the file/host_device/...
protocol drivers, which only access local files. These are implemented
in raw-posix.c, i.e. the file that this patch is touching. raw-win32.c
implements the same kind of file access for Windows.

And then there is the raw image format driver, which is raw_bsd.c. This
one is what is layered on top of any of the protocol drivers, including
raw-posix, raw-win32 and all the network protocols.

This is why implementing the feature in the raw format driver would make
it so much more useful. You could use it with any backend then.

> The question is then, what to do when the underlying device changes?
> The size/offset may not be valid at that point anymore.

A user who runs qemu on a block device must make sure that the device
makes sense while the VM is running. Second guessing is not qemu's job.

> > > > 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.

As long as there is only an offset, apply the offset and do the same
operation as usual.

If a size was specified, then maybe we should block the operation if it
requires changing the the size, i.e. it would behave the same as if we
were working on a real block device.

You could then still use bdrv_reopen() with new option values in order
to extend the visible 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.  

Traditionally, the qemu block layer worked in 512 byte units and you
still see places where this is true. But we're in a process of
converting things to use bytes everywhere (which saves a lot of
conversion back and forth and simplifies the code), and the basic
read/write functions in the primary block drivers (raw, qcow2, file) can
all work with byte granularity today.

Kevin