https://bugzilla.redhat.com/show_bug.cgi?id=1480668

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

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f864350bd5..67350719aa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3148,6 +3148,12 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
                                   NULL) < 0)
             goto cleanup;
 
+        if (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

On Thu, Apr 12, 2018 at 18:39:52 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f864350bd5..67350719aa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3148,6 +3148,12 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>                                    NULL) < 0)
>              goto cleanup;
>  
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
> +            virJSONValueObjectAdd(props,
> +                                  "B:discard-data", true,

This code path is used also for NVDIMMs, and since the documentaion for
the option states that:

    The new option can be used to indicate that the file contents can
    be destroyed and don't need to be flushed to disk when QEMU exits
    or when the memory backend object is removed.

I'm not sure whether this is right.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 04/17/2018 10:05 AM, Peter Krempa wrote:
> On Thu, Apr 12, 2018 at 18:39:52 +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/qemu/qemu_command.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index f864350bd5..67350719aa 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3148,6 +3148,12 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>>                                    NULL) < 0)
>>              goto cleanup;
>>  
>> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
>> +            virJSONValueObjectAdd(props,
>> +                                  "B:discard-data", true,
> 
> This code path is used also for NVDIMMs, and since the documentaion for
> the option states that:
> 
>     The new option can be used to indicate that the file contents can
>     be destroyed and don't need to be flushed to disk when QEMU exits
>     or when the memory backend object is removed.
> 
> I'm not sure whether this is right.
> 

Okay, I'll make it configurable via domain XML. However, I'm struggling
to come up with useful design. Perhaps we can have <discardData/>
element under <memoryBacking/>?

  <memoryBacking>
    <discardData/>
  </memoryBacking>

and in order to be able to fine tune this per nvdimm/RAM modules we can
have:

  <memory model='nvdimm'>
    <source ../>
    <target ../>
    <discardData/>
  </memory>

However, this would allow the element even for cases when
memory-backing-ram is used (or the old way of configuration without any
memory-backend-* at all).

Michal

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

On Tue, Apr 17, 2018 at 10:49:39 +0200, Michal Privoznik wrote:
> On 04/17/2018 10:05 AM, Peter Krempa wrote:
> > On Thu, Apr 12, 2018 at 18:39:52 +0200, Michal Privoznik wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1480668
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/qemu/qemu_command.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> >> index f864350bd5..67350719aa 100644
> >> --- a/src/qemu/qemu_command.c
> >> +++ b/src/qemu/qemu_command.c
> >> @@ -3148,6 +3148,12 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
> >>                                    NULL) < 0)
> >>              goto cleanup;
> >>  
> >> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD) &&
> >> +            virJSONValueObjectAdd(props,
> >> +                                  "B:discard-data", true,
> > 
> > This code path is used also for NVDIMMs, and since the documentaion for
> > the option states that:
> > 
> >     The new option can be used to indicate that the file contents can
> >     be destroyed and don't need to be flushed to disk when QEMU exits
> >     or when the memory backend object is removed.
> > 
> > I'm not sure whether this is right.
> > 
> 
> Okay, I'll make it configurable via domain XML. However, I'm struggling
> to come up with useful design. Perhaps we can have <discardData/>
> element under <memoryBacking/>?
> 
>   <memoryBacking>
>     <discardData/>
>   </memoryBacking>
> 
> and in order to be able to fine tune this per nvdimm/RAM modules we can
> have:
> 
>   <memory model='nvdimm'>
>     <source ../>
>     <target ../>
>     <discardData/>
>   </memory>
> 
> However, this would allow the element even for cases when
> memory-backing-ram is used (or the old way of configuration without any
> memory-backend-* at all).

Well, I think you need to think more about how this is going to be used.
I think that for NVDIMMs we should never use that option, but for normal
memory dimms we should always use it. I don't really think that having
it configurable would add any benefit. I just wanted to point out that
for NVDIMMS which are supposed to be persistent we should not destroy
the data.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list