Re: [Xen-devel] [RFC XEN PATCH v3 12/39] tools/xen-ndctl: add NVDIMM management util 'xen-ndctl'

[Dan Williams <dan.j.williams@xxxxxxxxx>]

On Mon, Sep 11, 2017 at 2:24 PM, Konrad Rzeszutek Wilk
<konrad.wilk@xxxxxxxxxx> wrote:
> On Mon, Sep 11, 2017 at 09:35:08AM -0700, Dan Williams wrote:
>> On Sun, Sep 10, 2017 at 10:39 PM, Haozhong Zhang
>> <haozhong.zhang@xxxxxxxxx> wrote:
>> > On 09/10/17 22:10 -0700, Dan Williams wrote:
>> >> On Sun, Sep 10, 2017 at 9:37 PM, Haozhong Zhang
>> >> <haozhong.zhang@xxxxxxxxx> wrote:
>> >> > The kernel NVDIMM driver and the traditional NVDIMM management
>> >> > utilities in Dom0 does not work now. 'xen-ndctl' is added as an
>> >> > alternatively, which manages NVDIMM via Xen hypercalls.
>> >> >
>> >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
>> >> > ---
>> >> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> >> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>> >> > ---
>> >> >  .gitignore             |   1 +
>> >> >  tools/misc/Makefile    |   4 ++
>> >> >  tools/misc/xen-ndctl.c | 172 
>> >> > +++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >  3 files changed, 177 insertions(+)
>> >> >  create mode 100644 tools/misc/xen-ndctl.c
>> >>
>> >> What about my offer to move this functionality into the upstream ndctl
>> >> utility [1]? I think it is thoroughly confusing that you are reusing
>> >> the name 'ndctl' and avoiding integration with the upstream ndctl
>> >> utility.
>> >>
>> >> [1]: https://patchwork.kernel.org/patch/9632865/
>> >
>> > I'm not object to integrate it with ndctl.
>> >
>> > My only concern is that the integration will introduces two types of
>> > user interface. The upstream ndctl works with the kernel driver and
>> > provides easily used *names* (e.g., namespace0.0, region0, nmem0,
>> > etc.) for user input. However, this version patchset hides NFIT from
>> > Dom0 (to simplify the first implementation), so the kernel driver does
>> > not work in Dom0, neither does ndctl. Instead, xen-ndctl has to use
>> > *the physical address* for users to specify their interested NVDIMM
>> > region, which is different from upstream ndctl.
>>
>> Ok, I think this means that xen-ndctl should be renamed (xen-nvdimm?)
>> so that the distinction between the 2 tools is clear.
>
> I think it makes much more sense to integrate this in the upstream
> version of ndctl. As surely in the future the ndctl will need to work
> with other OSes too? Such as FreeBSD?

I'm receptive to carrying Xen-specific enabling and / or a FreeBSD
compat layer in ndctl.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel