| Message ID | 20200916095150.755714-3-stefanha@redhat.com |
|---|---|
| State | Accepted |
| Commit | 86635aa4e9d627d5142b81c57a33dd1f36627d07 |
| Headers | show |
| Series | nvdimm: read-only file support | expand |
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. > --- > backends/hostmem-file.c | 26 +++++++++++++++++++++++++- > qemu-options.hx | 5 ++++- > 2 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index dffdf142e0..da585e4300 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -31,6 +31,7 @@ struct HostMemoryBackendFile { > uint64_t align; > bool discard_data; > bool is_pmem; > + bool readonly; > }; > > static void > @@ -58,7 +59,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > backend->size, fb->align, > (backend->share ? RAM_SHARED : 0) | > (fb->is_pmem ? RAM_PMEM : 0), > - fb->mem_path, false, errp); > + fb->mem_path, fb->readonly, errp); > g_free(name); > #endif > } > @@ -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; > +} > + > +static void file_memory_backend_set_readonly(Object *o, bool value, > + Error **errp) > +{ > + HostMemoryBackend *backend = MEMORY_BACKEND(o); > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); > + > + if (host_memory_backend_mr_inited(backend)) { > + error_setg(errp, "cannot change property 'readonly' of %s.", > + object_get_typename(o)); > + return; > + } > + > + fb->readonly = value; > +} > + > static void file_backend_unparent(Object *obj) > { > HostMemoryBackend *backend = MEMORY_BACKEND(obj); > @@ -184,6 +205,9 @@ file_backend_class_init(ObjectClass *oc, void *data) > NULL, NULL); > object_class_property_add_bool(oc, "pmem", > file_memory_backend_get_pmem, file_memory_backend_set_pmem); > + object_class_property_add_bool(oc, "readonly", > + file_memory_backend_get_readonly, > + file_memory_backend_set_readonly); > } > > static void file_backend_instance_finalize(Object *o) > diff --git a/qemu-options.hx b/qemu-options.hx > index b0f020594e..3dfaaddd62 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4369,7 +4369,7 @@ SRST > they are specified. Note that the 'id' property must be set. These > objects are placed in the '/objects' path. > > - ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align`` > + ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align,readonly=on|off`` > Creates a memory file backend object, which can be used to back > the guest RAM with huge pages. > > @@ -4452,6 +4452,9 @@ SRST > 4.15) and the filesystem of ``mem-path`` mounted with DAX > option. > > + The ``readonly`` option specifies whether the backing file is opened > + read-only or read-write (default). > + > ``-object memory-backend-ram,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave`` > Creates a memory backend object, which can be used to back the > guest RAM. Memory backend objects offer more control than the
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. Stefan
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
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index dffdf142e0..da585e4300 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -31,6 +31,7 @@ struct HostMemoryBackendFile { uint64_t align; bool discard_data; bool is_pmem; + bool readonly; }; static void @@ -58,7 +59,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) backend->size, fb->align, (backend->share ? RAM_SHARED : 0) | (fb->is_pmem ? RAM_PMEM : 0), - fb->mem_path, false, errp); + fb->mem_path, fb->readonly, errp); g_free(name); #endif } @@ -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; +} + +static void file_memory_backend_set_readonly(Object *o, bool value, + Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(o); + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); + + if (host_memory_backend_mr_inited(backend)) { + error_setg(errp, "cannot change property 'readonly' of %s.", + object_get_typename(o)); + return; + } + + fb->readonly = value; +} + static void file_backend_unparent(Object *obj) { HostMemoryBackend *backend = MEMORY_BACKEND(obj); @@ -184,6 +205,9 @@ file_backend_class_init(ObjectClass *oc, void *data) NULL, NULL); object_class_property_add_bool(oc, "pmem", file_memory_backend_get_pmem, file_memory_backend_set_pmem); + object_class_property_add_bool(oc, "readonly", + file_memory_backend_get_readonly, + file_memory_backend_set_readonly); } static void file_backend_instance_finalize(Object *o) diff --git a/qemu-options.hx b/qemu-options.hx index b0f020594e..3dfaaddd62 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4369,7 +4369,7 @@ SRST they are specified. Note that the 'id' property must be set. These objects are placed in the '/objects' path. - ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align`` + ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align,readonly=on|off`` Creates a memory file backend object, which can be used to back the guest RAM with huge pages. @@ -4452,6 +4452,9 @@ SRST 4.15) and the filesystem of ``mem-path`` mounted with DAX option. + The ``readonly`` option specifies whether the backing file is opened + read-only or read-write (default). + ``-object memory-backend-ram,id=id,merge=on|off,dump=on|off,share=on|off,prealloc=on|off,size=size,host-nodes=host-nodes,policy=default|preferred|bind|interleave`` Creates a memory backend object, which can be used to back the guest RAM. Memory backend objects offer more control than the