Re: [Qemu-devel] [PATCH 0/5] hostmem-file: Add "persistent" option

[Zack Cornelius]

The share=on attribute can also be used in the case of a pre-allocated file, 
created outside qemu, to be used as a memory backing, using the MAP_SHARED flag 
to allow data to be written out to the file in the case of host memory 
pressure. In this case, as the file was created outside qemu, it would also be 
inappropriate to unlink the file.

In Kove's specific use case, we have a file on a virtual file system, which is 
allocated from and backed by a hardware device. In this case, unlink isn't 
available, but MADV_REMOVE or FALLOC_FL_PUNCH_HOLE will prevent qemu flushing 
unneeded data to the backing device.

--
Zack

----- Original Message -----
> From: "Eduardo Habkost" <address@hidden>
> To: "Daniel P. Berrange" <address@hidden>
> Cc: "Paolo Bonzini" <address@hidden>, "Zack Cornelius" <address@hidden>, 
> address@hidden, "Igor
> Mammedov" <address@hidden>, "Dr. David Alan Gilbert" <address@hidden>
> Sent: Monday, August 14, 2017 6:40:35 AM
> Subject: Re: [Qemu-devel] [PATCH 0/5] hostmem-file: Add "persistent" option

> On Mon, Aug 14, 2017 at 10:39:40AM +0100, Daniel P. Berrange wrote:
>> On Fri, Aug 11, 2017 at 03:15:59PM -0300, Eduardo Habkost wrote:
>> > On Fri, Aug 11, 2017 at 05:44:55PM +0100, Daniel P. Berrange wrote:
>> > > On Fri, Aug 11, 2017 at 01:33:00PM -0300, Eduardo Habkost wrote:
>> > > > CCing Zack Cornelius.
>> > > > 
>> > > > On Wed, Jun 14, 2017 at 05:29:55PM -0300, Eduardo Habkost wrote:
>> > > > > This series adds a new "persistent" option to
>> > > > > memory-backend-file.  The new option it will be useful if
>> > > > > somebody is sharing RAM contents on a file using share=on, but
>> > > > > don't need it to be flushed to disk when QEMU exits.
>> > > > > 
>> > > > > Internally, it will trigger a madvise(MADV_REMOVE) or
>> > > > > fallocate(FALLOC_FL_PUNCH_HOLE) call when the memory backend is
>> > > > > destroyed.
>> > > > > 
>> > > > > To make we actually trigger the new code when QEMU exits, the
>> > > > > first patch in the series ensures we destroy all user-created
>> > > > > objects when exiting QEMU.
>> > > > 
>> > > > So, before sending a new version of this, we need to clarify one
>> > > > thing: why exactly unlink()+close() wouldn't be enough to avoid
>> > > > having data unnecessarily flushed to the backing store and make
>> > > > the new option unnecessary?
>> > > 
>> > > If the backend file is shared between processes, unlinking
>> > > it feels bad - you're assuming no /future/ process wants to
>> > > attach to the file. Also if QEMU aborts for any reason, the
>> > > cleanup code is never going to run
>> > 
>> > If mem-path is a directory, QEMU will create a file, open() it
>> > and unlink() it immediately.
>> > 
>> > This solves the problem of not running the cleanup code when QEMU
>> > aborts (which is not solved by the madvise() method).
>> > 
>> > > 
>> > > > I would expect close() to not write any data unnecessarily if
>> > > > there are no remaining references to the file.  Why/when this is
>> > > > not the case?
>> > > 
>> > > Isn't the unlink() delayed until such time as *all* open handles
>> > > on that file are closed. If so, it seems that if 2 processes
>> > > have the file open, and one closes it, it is still valid for the
>> > > kernel to want to flush data out to the backing store if it needs
>> > > to free up working memory consumed by i/o cache.
>> > > 
>> > > If this wasn't the case, then one process could write 20 GB of data,
>> > > unlink + close the file, and that 20 GB would never be able to be
>> > > purge from I/O cache for as long as another process had that FD
>> > > open. That would be pretty bad denial of sevice for memory management
>> > > system.
>> > 
>> > I'm assuming QEMU is the only process opening the file.  Are
>> > there use cases where 1) there's a need for another process to
>> > the keep the file open; but 2) there's no need for the data on
>> > the file to be kept?
>> 
>> Saying only QEMU opens the file contradicts what is implied
>> by the original commit message:
>> 
>>   > > > This series adds a new "persistent" option to
>>   > > > memory-backend-file.  The new option it will be useful if
>>   > > > somebody is sharing RAM contents on a file using share=on, but
>>   > > > don't need it to be flushed to disk when QEMU exits.
>> 
>> AFAIK, the whole point of share=on, is so that non-QEMU processes
>> can open the file to get access to the guest RAM region, eg the
>> vhostuser backend process.  Why would you ever use share=on, if
>> QEMU is the only process opening the file ?
> 
> That's exactly what I'm asking about: if the use case I tried to
> describe in the cover letter really makes sense.
> 
> The cover letter tried to describe a theoretical scenario where
> the new flag would make a difference.  Now I'm trying to
> understand why/when exactly this would happen in practice.
> 
> --
> Eduardo