On 01/11/18 20:06 +0000, Dr. David Alan Gilbert wrote:
> * Haozhong Zhang (haozhong.zh...@intel.com) wrote:
> > This option controls whether QEMU mmap(2) the memory backend file with
> > MAP_SYNC flag, which can fully guarantee the guest write persistence
> > to the backend, if MAP_SYNC flag is supported by the host kernel
> > (Linux kernel 4.15 and later) and the backend is a file supporting
> > DAX (e.g., file on ext4/xfs file system mounted with '-o dax').
> >
> > It can take one of following values:
> > - on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or
> > 'share=off', QEMU will abort
> > - off: never pass MAP_SYNC to mmap(2)
> > - auto (default): if MAP_SYNC is supported and 'share=on', work as if
> > 'sync=on'; otherwise, work as if 'sync=off'
>
> I don't know anything about MAP_SYNC but see two minor comments below:
>
> > Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com>
> > Sugguested-by: Eduardo Habkost <ehabk...@redhat.com>
> ^ typo!
>
[..]
> > +static void file_memory_backend_set_sync(
> > + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp)
> > +{
> > + HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj);
> > + Error *local_err = NULL;
> > + OnOffAuto value;
> > +
> > + if (host_memory_backend_mr_inited(backend)) {
> > + error_setg(&local_err, "cannot change property value");
>
> Please make this error clearer; if I was helping someone and saw
> this error message in a log, I wouldn't know what type of property it
> was on what device. Please at least include something saying it's
> memory backend related and sync related; even just including a %s and
> __func__ would help; even better if you include the name of the object
> so if you had multiple backend files and one failed you could see which
> one.
I'll fix the typo, and include the backend id and the property name in
the error messages in the next version.
Thanks,
Haozhong
