1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/5] Enable memory-backend-file.discard-data
  4. View patch

[libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible

Michal Privoznik posted 5 patches 7 years ago
There is a newer version of this series
[libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible
Posted by Michal Privoznik 7 years ago 
https://bugzilla.redhat.com/show_bug.cgi?id=1480668

The cases when we cannot enable this optimization are:
  1) nvdimms
  2) if memAccess='shared'

Otherwise it is safe to enable it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 527a35779d..b920f5c3e4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3121,6 +3121,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
                                   NULL) < 0)
             goto cleanup;
 
+        if (!mem->nvdimmPath && memAccess != VIR_DOMAIN_MEMORY_ACCESS_SHARED &&
+            virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
+            virJSONValueObjectAdd(props,
+                                  "B:discard-data", true,
+                                  NULL) < 0)
+            goto cleanup;
+
         switch (memAccess) {
         case VIR_DOMAIN_MEMORY_ACCESS_SHARED:
             if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible
Posted by Daniel P. Berrangé 7 years ago 
On Tue, Apr 17, 2018 at 01:16:42PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> 
> The cases when we cannot enable this optimization are:
>   1) nvdimms
>   2) if memAccess='shared'
> 
> Otherwise it is safe to enable it.

This is basically saying that if memAccess==private, then no person/app cares
about the contents of the memory-backend-file storage after QEMU has exited,
because (implicitly) whatever data was in that storage is not going to be used
again. While I accept that's going to be a common case, I'm wary of saying that
is a 100% safe/valid assumption for every user.

I tend think it should require an explicit opt-in to turn on this behaviour


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible
Posted by Peter Krempa 7 years ago 
On Tue, Apr 17, 2018 at 13:18:02 +0100, Daniel Berrange wrote:
> On Tue, Apr 17, 2018 at 01:16:42PM +0200, Michal Privoznik wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> > 
> > The cases when we cannot enable this optimization are:
> >   1) nvdimms
> >   2) if memAccess='shared'
> > 
> > Otherwise it is safe to enable it.
> 
> This is basically saying that if memAccess==private, then no person/app cares
> about the contents of the memory-backend-file storage after QEMU has exited,
> because (implicitly) whatever data was in that storage is not going to be used
> again. While I accept that's going to be a common case, I'm wary of saying that
> is a 100% safe/valid assumption for every user.
> 
> I tend think it should require an explicit opt-in to turn on this behaviour

Well, we used that behaviour prior to storing the backing files in a
"predictible" path as the file was unlinked right after creating it
(so that some memory management software can do some magic behind our
backs with that) so I don't think anybody would depend on the described
behaviour.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible
Posted by Daniel P. Berrangé 7 years ago 
On Tue, Apr 17, 2018 at 02:57:50PM +0200, Peter Krempa wrote:
> On Tue, Apr 17, 2018 at 13:18:02 +0100, Daniel Berrange wrote:
> > On Tue, Apr 17, 2018 at 01:16:42PM +0200, Michal Privoznik wrote:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> > > 
> > > The cases when we cannot enable this optimization are:
> > >   1) nvdimms
> > >   2) if memAccess='shared'
> > > 
> > > Otherwise it is safe to enable it.
> > 
> > This is basically saying that if memAccess==private, then no person/app cares
> > about the contents of the memory-backend-file storage after QEMU has exited,
> > because (implicitly) whatever data was in that storage is not going to be used
> > again. While I accept that's going to be a common case, I'm wary of saying that
> > is a 100% safe/valid assumption for every user.
> > 
> > I tend think it should require an explicit opt-in to turn on this behaviour
> 
> Well, we used that behaviour prior to storing the backing files in a
> "predictible" path as the file was unlinked right after creating it
> (so that some memory management software can do some magic behind our
> backs with that) so I don't think anybody would depend on the described
> behaviour.

Hmm, I guess that's true.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] qemu: Enable memory-backend-file.discard-data whenever possible
Posted by Eduardo Habkost 7 years ago 
On Tue, Apr 17, 2018 at 01:16:42PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> 
> The cases when we cannot enable this optimization are:
>   1) nvdimms
>   2) if memAccess='shared'

The specific use case for discard-data=on uses share=on, see:
https://bugzilla.redhat.com/show_bug.cgi?id=1460848#c4

It looks like this will require a explicit XML element, after
all.


> 
> Otherwise it is safe to enable it.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 527a35779d..b920f5c3e4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3121,6 +3121,13 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>                                    NULL) < 0)
>              goto cleanup;
>  
> +        if (!mem->nvdimmPath && memAccess != VIR_DOMAIN_MEMORY_ACCESS_SHARED &&
> +            virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
> +            virJSONValueObjectAdd(props,
> +                                  "B:discard-data", true,
> +                                  NULL) < 0)
> +            goto cleanup;
> +
>          switch (memAccess) {
>          case VIR_DOMAIN_MEMORY_ACCESS_SHARED:
>              if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
> -- 
> 2.16.1
> 

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.