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

Stefan Hajnoczi posted 3 patches 5 years, 4 months ago
There is a newer version of this series
[PATCH v2 2/3] hostmem-file: add readonly=on|off option
Posted by Stefan Hajnoczi 5 years, 4 months ago
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>
---
 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;
+}
+
+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
-- 
2.26.2

Re: [PATCH v2 2/3] hostmem-file: add readonly=on|off option
Posted by Igor Mammedov 5 years, 1 month ago
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


Re: [PATCH v2 2/3] hostmem-file: add readonly=on|off option
Posted by Stefan Hajnoczi 5 years, 1 month ago
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
Re: [PATCH v2 2/3] hostmem-file: add readonly=on|off option
Posted by Eduardo Habkost 5 years, 1 month ago
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