Re: [PATCH 2/9] migration: Fix file migration with fdset

[Peter Xu]

On Wed, May 08, 2024 at 09:02:16AM +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2024 at 12:23:51PM -0400, Peter Xu wrote:
> > On Fri, Apr 26, 2024 at 11:20:35AM -0300, Fabiano Rosas wrote:
> > > When the migration using the "file:" URI was implemented, I don't
> > > think any of us noticed that if you pass in a file name with the
> > > format "/dev/fdset/N", this allows a file descriptor to be passed in
> > > to QEMU and that behaves just like the "fd:" URI. So the "file:"
> > > support has been added without regard for the fdset part and we got
> > > some things wrong.
> > > 
> > > The first issue is that we should not truncate the migration file if
> > > we're allowing an fd + offset. We need to leave the file contents
> > > untouched.
> > 
> > I'm wondering whether we can use fallocate() instead on the ranges so that
> > we always don't open() with O_TRUNC.  Before that..  could you remind me
> > why do we need to truncate in the first place?  I definitely missed
> > something else here too.
> 
> You're mixing distinct concepts here. fallocate makes a file region
> non-sparse, while O_TRUNC removes all existing allocation, making it
> sparse if we write at non-contiguous offsets. I don't think we would
> want to call fallocate, since we /want/ a sparse file so that we
> don't needlessly store large regions of all-zeros as RAM maps.

I meant fallocate() with FALLOC_FL_PUNCH_HOLE.  But now I think it'll be
good we avoid both.

Thanks,

-- 
Peter Xu