Re: [Qemu-devel] [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area

[Stefan Hajnoczi <stefanha@redhat.com>]

On Wed, Aug 26, 2015 at 06:40:26PM +0800, Xiao Guangrong wrote:
> On 08/26/2015 12:03 AM, Stefan Hajnoczi wrote:
> >On Fri, Aug 14, 2015 at 10:52:01PM +0800, Xiao Guangrong wrote:
> >
> >>+    if (fd < 0) {
> >>+        error_setg(errp, "can not open %s", nvdimm->file);
> >
> >s/can not/cannot/
> >
> >>+        return;
> >>+    }
> >>+
> >>+    size = get_file_size(fd);
> >>+    buf = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> >
> >I guess the user will want to choose between MAP_SHARED and MAP_PRIVATE.
> >This can be added in the future.
> 
> Good idea, it will allow guest to write data but discards its content after it
> exits. Will implement O_RDONLY + MAP_PRIVATE in the near future.

Great.

> >>+        goto do_unmap;
> >>+    }
> >>+
> >>+    nvdimm->device_index = new_device_index();
> >>+    sprintf(name, "NVDIMM-%d", nvdimm->device_index);
> >>+    memory_region_init_ram_ptr(&nvdimm->mr, OBJECT(dev), name, nvdimm_size,
> >>+                               buf);
> >
> >How is the autogenerated name used?
> >
> >Why not just use "pc-nvdimm.memory"?
> 
> Ah. Just for debug proposal :) and i am not sure if a name used for multiple
> MRs (MemoryRegion) is a good idea.

Other devices use a constant name too (git grep
memory_region_init_ram_ptr) so it seems to be okay.  The unique thing is
the OBJECT(dev) which differs for each NVDIMM instance.