Hi Dan, These mostly look good, just a few minor comments below -
On Mon, 2019-01-14 at 20:24 -0800, Dan Williams wrote: > The kernel is implementing a '/sys/bus/dax' ABI to allow for alternate > device-DAX drivers to be bound to device instances. While the kernel > conversion to '/sys/bus/dax' does not effect the primary ndctl use case "kernel's conversion" or "kernel converting to" s/effect/affect/ > of putting namespaces into 'devdax' mode since that uses libnvdimm > namespace device relative paths, it does break current implementations > of 'ndctl list -X' and 'daxctl list'. It is also known to break fio and > some pmdk versions that explicitly reference "/sys/class/dax". > > In order to avoid userspace regressions the kernel can be configured to > maintain '/sys/class/dax' as the default ABI. However, once all > '/sys/class/dax' users have been converted, or removed from the > installation, an administrator can opt-in to the new '/sys/bus/dax' ABI. > The 'dax migrate-device-model' command installs a modprobe rule to > blacklist the dax_pmem_compat module and arrange for the dax_pmem module > to auto-load in response to the detection of device-DAX instances > emitted from the libnvdimm subsystem. > > Reported-by: Jeff Moyer <[email protected]> > Signed-off-by: Dan Williams <[email protected]> > --- > .gitignore | 1 > Documentation/daxctl/Makefile.am | 3 + > .../daxctl/daxctl-migrate-device-model.txt | 47 > ++++++++++++++++++++ > configure.ac | 5 ++ > daxctl/Makefile.am | 10 ++++ > daxctl/builtin.h | 1 > daxctl/daxctl.c | 1 > daxctl/lib/Makefile.am | 2 + > daxctl/lib/daxctl.conf | 2 + > daxctl/migrate.c | 41 +++++++++++++++++ > ndctl.spec.in | 1 > 11 files changed, 113 insertions(+), 1 deletion(-) > create mode 100644 Documentation/daxctl/daxctl-migrate-device-model.txt > create mode 100644 daxctl/lib/daxctl.conf > create mode 100644 daxctl/migrate.c > [..] > --- /dev/null > +++ b/daxctl/migrate.c > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2015-2018 Intel Corporation. All rights reserved. */ Copyright notice can be updated to 2019. This applies the one included by man pages as well, but that is unrelated and I can take care of that separately. > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <stdio.h> > +#include <errno.h> > +#include <fcntl.h> > +#include <daxctl/config.h> > +#include <daxctl/libdaxctl.h> > +#include <util/parse-options.h> > +#include <ccan/array_size/array_size.h> > + > +int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx) > +{ > + int i; > + static const struct option options[] = { > + OPT_END(), > + }; > + const char * const u[] = { > + "daxctl migrate-device-model", > + NULL > + }; > + > + argc = parse_options(argc, argv, options, u, 0); > + for (i = 0; i < argc; i++) > + error("unknown parameter \"%s\"\n", argv[i]); > + > + if (argc) > + usage_with_options(u, options); > + > + if (symlink(DAXCTL_MODPROBE_DATA, DAXCTL_MODPROBE_INSTALL) == 0) { > + fprintf(stderr, " Success: installed %s\n", > + DAXCTL_MODPROBE_INSTALL); > + return EXIT_SUCCESS; > + } > + > + error("failed to install %s: %s\n", DAXCTL_MODPROBE_INSTALL, > + strerror(errno)); 'Success' is capitalized, but 'failed' is lower case, could make them consistent. Most other messages seem to be lowercase, including the 'unknown parameter' one above. > + > + return EXIT_FAILURE; > +} > diff --git a/ndctl.spec.in b/ndctl.spec.in > index bc4d65c1f988..bc65a471a6d2 100644 > --- a/ndctl.spec.in > +++ b/ndctl.spec.in > @@ -126,6 +126,7 @@ make check > %license util/COPYING licenses/BSD-MIT licenses/CC0 > %{_bindir}/daxctl > %{_mandir}/man1/daxctl* > +%{_datadir}/daxctl/daxctl.conf > > %files -n LNAME > %defattr(-,root,root) > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
