Re: [PATCH v2 2/3] hostmem-file: add readonly=on|off option

[Eduardo Habkost]

On Mon, Jan 04, 2021 at 03:42:23PM +0000, Stefan Hajnoczi wrote:
> On Mon, Dec 14, 2020 at 12:10:15PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Sep 2020 10:51:49 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > 
> > > Let -object memory-backend-file work on read-only files when the
> > > readonly=on option is given. This can be used to share the contents of a
> > > file between multiple guests while preventing them from consuming
> > > Copy-on-Write memory if guests dirty the pages, for example.
> > > 
> > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > cosmetic/style nits only
> > 
> > s/Object *o/Object *obj/
> > 
> > for consistency with the rest of the code in file.
> 
> Will fix.
> 
> > > @@ -153,6 +154,26 @@ static void file_memory_backend_set_pmem(Object *o, 
> > > bool value, Error **errp)
> > >      fb->is_pmem = value;
> > >  }
> > >  
> > > +static bool file_memory_backend_get_readonly(Object *o, Error **errp)
> > > +{
> > > +    return MEMORY_BACKEND_FILE(o)->readonly;
> > 
> > I thought using macro this way not acceptable and one should use
> > 
> > HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> > 
> > return fb->readonly;
> 
> I'm not sure where this is forbidden or why? I've updated the patch as
> suggested anyway.

I have a vague memory of seeing this documented somewhere, but I
can't find it anywhere in the QOM documentation or git log.

I don't think we need to make this a rule, though.

-- 
Eduardo