Re: Adjustments of NVDIMM devices and future data safety

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 30 Apr 2021 14:18:30 +0200
Milan Zamazal <mzamazal@redhat.com> wrote:

> Hi,
> 
> I work on NVDIMM support in oVirt/RHV, I think other virtualization
> management software built on top of QEMU may have similar concerns.
> 
> When a virtual NVDIMM device size is specified, it's not necessarily the
> eventual NVDIMM device size visible to the guest OS.  As seen in
> https://github.com/qemu/qemu/blob/v6.0.0/hw/mem/nvdimm.c#L117, QEMU
> makes some adjustments (other adjustments are performed by libvirt but
> that's a topic for a different forum):
> 
> - NVDIMM label size is subtracted from the NVDIMM size.
> 
> - NVDIMM label is pointed to a certain memory region.
> 
> - The remaining NVDIMM size is aligned down.
> 
> There are some related potential problems:
> 
> - If the alignment rules change in a future QEMU version, it may result
>   in a different device size visible to the guest (even if the requested
>   size remains the same) and cause trouble there up to data loss.
> 
> - If the layout on the backing device changes, e.g. a label placement,
>   then the stored data may become corrupt or inaccessible.

We usually tie ABI changes to machine versions, so if in future we decide to
change NVDIMM layout, we should preserve old layout for old machine types
(which is accomplished using compat mechanism)

> 
> - I'm not sure about the current QEMU version, but at least in previous
>   QEMU versions, the resulting size is important for memory hot plug.
>   The NVDIMM alignment size is smaller than the required regular memory
>   DIMM placement alignment.  If a VM contains an NVDIMM with the
>   resulting size not matching the DIMM placement requirements and a
>   memory hot plug is attempted then the hot plug fails because the DIMM
>   is mapped next to the end of the NVDIMM region, which is not
>   DIMM-aligned.


I vaguely recall that, start address of any hotplugged (NV)DIMM
is aligned on 1G boundary (only the very first versions of memory
hotplug used unaligned address). Described above situation shouldn't happen.

I'd try to fix alignment issues first if there is any before talking about
splitting label out.


> All this means:
> 
> - The requested NVDIMM size must be computed and specified carefully,
>   with attention to QEMU internal implementation.
> 
> - And because it depends on QEMU internal implementation, there is a
>   risk of malfunction or data loss when the same backing device with the
>   same parameters is used with a future QEMU version.

When making incompatible changes, we usually add a new property to enable them,
so normally situation when NVDIMM "with the same parameters" is used
should lead to old behaviour.
If we change 'default' values then as long as one uses versioned machine
type, old behaviour will be preserved with future QEMU.
However if one uses un-versioned/another machine type or enables new behavior,
QEMU doesn't guarantee any compatibility.


> As for labels, I was told NVDIMM labels might be put to regular files in
> future to avoid some problems.  Since label placement is not visible to
> the guest, such a change could be made transparently without disrupting
> access to the data.  (As long as the label data is transferred to the
> new location properly and undesirable resulting NVDIMM size changes are
> not induced by such a change.)

I think current approach resembles real nvdimm devices
(the only problem is that one has to configure size/label size,
where with real devices it's done by manufacturer).

if we add a dedicated property, It should be possible to split label out
into a separate file.
However I don't fancy carrying transparent migration from old format to
the new one in QEMU, I think it should be done by separate utility. So
that if users have access to backend + and know used label size,
they should be able split it.

Also I'm not sure that splitting label out is fixing anything, it just
replaces one set of rules how to set size/label (assuming there is one)
with another + user will have to manage 1 more backend (content + label).
 
> The primary point is still how to ensure that data kept on a backing
> device will remain accessible and safe in future QEMU versions and how
> to possibly avoid reliance on QEMU implementation details.  A big
> warning in the NVDIMM handling source code to keep backward
> compatibility (incl. memory hot plugs) and data safety on mind before
> making any changes there might be a reasonable minimum measure.
> Any additional ideas?  What do you think about it all?
we usually are trying to keep compatibility (versioned or new features
are disabled by default and user has to explicitly enable them)
when making breaking changes.
(and that is done without extra warnings in the code,
otherwise QEMU will be full of them).

> 
> Thank you,
> Milan
> 
>