* [PATCH] nvmetcli: ANA configuration support
@ 2018-07-27 11:08 Hannes Reinecke
2018-10-06 12:43 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2018-07-27 11:08 UTC (permalink / raw)
Add support for ANA configuration.
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
nvmet/__init__.py | 2 +-
nvmet/nvme.py | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
nvmetcli | 61 +++++++++++++++++++++++++++++++++-
3 files changed, 160 insertions(+), 2 deletions(-)
diff --git a/nvmet/__init__.py b/nvmet/__init__.py
index 6542d87..ca05de4 100644
--- a/nvmet/__init__.py
+++ b/nvmet/__init__.py
@@ -1 +1 @@
-from .nvme import Root, Subsystem, Namespace, Port, Host, Referral
+from .nvme import Root, Subsystem, Namespace, Port, Host, Referral, ANAGroup
diff --git a/nvmet/nvme.py b/nvmet/nvme.py
index 8253ea9..8909bd8 100644
--- a/nvmet/nvme.py
+++ b/nvmet/nvme.py
@@ -566,6 +566,24 @@ class Namespace(CFSNode):
def _get_nsid(self):
return self._nsid
+ def _get_grpid(self):
+ self._check_self()
+ _grpid = 0
+ path = "%s/ana_grpid" % self.path
+ if os.path.isfile(path):
+ with open(path, 'r') as file_fd:
+ _grpid = int(file_fd.read().strip())
+ return _grpid
+
+ def set_grpid(self, grpid):
+ self._check_self()
+ path = "%s/ana_grpid" % self.path
+ if os.path.isfile(path):
+ with open(path, 'w') as file_fd:
+ file_fd.write(str(grpid))
+
+ grpid = property(_get_grpid, doc="Get the ANA Group ID.")
+
subsystem = property(_get_subsystem,
doc="Get the parent Subsystem object.")
nsid = property(_get_nsid, doc="Get the NSID as an int.")
@@ -589,10 +607,12 @@ class Namespace(CFSNode):
return
ns._setup_attrs(n, err_func)
+ ns.set_grpid(int(n['ana_grpid']))
def dump(self):
d = super(Namespace, self).dump()
d['nsid'] = self.nsid
+ d['ana_grpid'] = self.grpid
return d
@@ -652,6 +672,8 @@ class Port(CFSNode):
self._check_self()
for s in self.subsystems:
self.remove_subsystem(s)
+ for a in self.ana_groups:
+ a.delete()
for r in self.referrals:
r.delete()
super(Port, self).delete()
@@ -664,6 +686,14 @@ class Port(CFSNode):
referrals = property(_list_referrals,
doc="Get the list of Referrals for this Port.")
+ def _list_ana_groups(self):
+ self._check_self()
+ for d in os.listdir("%s/ana_groups/" % self._path):
+ yield ANAGroup(self, int(d), 'lookup')
+
+ ana_groups = property(_list_ana_groups,
+ doc="Get the list of ANA Groups for this Port.")
+
@classmethod
def setup(cls, root, n, err_func):
'''
@@ -685,6 +715,8 @@ class Port(CFSNode):
port._setup_attrs(n, err_func)
for s in n.get('subsystems', []):
port.add_subsystem(s)
+ for a in n.get('ana_groups', []):
+ ANAGroup.setup(port, a, err_func)
for r in n.get('referrals', []):
Referral.setup(port, r, err_func)
@@ -692,6 +724,7 @@ class Port(CFSNode):
d = super(Port, self).dump()
d['portid'] = self.portid
d['subsystems'] = self.subsystems
+ d['ana_groups'] = [a.dump() for a in self.ana_groups]
d['referrals'] = [r.dump() for r in self.referrals]
return d
@@ -747,6 +780,72 @@ class Referral(CFSNode):
return d
+class ANAGroup(CFSNode):
+ '''
+ This is an interface to a NVMe ANA Group in configFS.
+ '''
+
+ MAX_GRPID = 128
+
+ def __repr__(self):
+ return "<ANA Group %d>" % self.grpid
+
+ def __init__(self, port, grpid, mode='any'):
+ super(ANAGroup, self).__init__()
+
+ if grpid is None:
+ if mode == 'lookup':
+ raise CFSError("Need grpid for lookup")
+
+ grpids = [n.grpid for n in port.ana_groups]
+ for index in xrange(2, self.MAX_GRPID + 1):
+ if index not in grpids:
+ grpid = index
+ break
+ if grpid is None:
+ raise CFSError("All ANA Group IDs 1-%d in use" % self.MAX_GRPID)
+ else:
+ grpid = int(grpid)
+ if grpid < 1 or grpid > self.MAX_GRPID:
+ raise CFSError("GRPID %d must be 1 to %d" % (grpid, self.MAX_GRPID))
+
+ self.attr_groups = ['ana']
+ self._port = port
+ self._grpid = grpid
+ self._path = "%s/ana_groups/%d" % (self._port.path, self.grpid)
+ self._create_in_cfs(mode)
+
+ def _get_grpid(self):
+ return self._grpid
+
+ grpid = property(_get_grpid, doc="Get the ANA Group ID.")
+
+ @classmethod
+ def setup(cls, port, n, err_func):
+ '''
+ Set up an ANA Group object based upon n dict, from saved config.
+ Guard against missing or bad dict items, but keep going.
+ Call 'err_func' for each error.
+ '''
+
+ if 'grpid' not in n:
+ err_func("'grpid' not defined for ANA Group")
+ return
+
+ try:
+ a = ANAGroup(port, n['grpid'])
+ except CFSError as e:
+ err_func("Could not create ANA Group object: %s" % e)
+ return
+
+ a._setup_attrs(n, err_func)
+
+ def dump(self):
+ d = super(ANAGroup, self).dump()
+ d['grpid'] = self.grpid
+ return d
+
+
class Host(CFSNode):
'''
This is an interface to a NVMe Host in configFS.
diff --git a/nvmetcli b/nvmetcli
index 6b102a2..0193b66 100755
--- a/nvmetcli
+++ b/nvmetcli
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
'''
Frontend to access to the NVMe target configfs hierarchy
@@ -258,6 +258,16 @@ class UINamespaceNode(UINode):
raise configshell.ExecutionError(
"The Namespace could not be disabled.")
+ def ui_command_grpid(self, grpid):
+ '''
+ Sets the ANA Group ID of the current Namespace to I{grpid}
+ '''
+ try:
+ self.cfnode.set_grpid(grpid)
+ except Exception as e:
+ raise configshell.ExecutionError(
+ "Failed to set ANA Group ID for this Namespace.")
+
def summary(self):
info = []
info.append("path=" + self.cfnode.get_attr("device", "path"))
@@ -267,6 +277,8 @@ class UINamespaceNode(UINode):
ns_nguid = self.cfnode.get_attr("device", "nguid")
if ngiud_set(ns_nguid):
info.append("nguid=" + ns_nguid)
+ if self.cfnode.grpid != 0:
+ info.append("grpid=" + str(self.cfnode.grpid))
info.append("enabled" if self.cfnode.get_enable() else "disabled")
ns_enabled = self.cfnode.get_enable()
return (", ".join(info), True if ns_enabled == 1 else ns_enabled)
@@ -378,6 +390,7 @@ class UIPortNode(UINode):
def __init__(self, parent, cfnode):
UINode.__init__(self, str(cfnode.portid), parent, cfnode)
UIPortSubsystemsNode(self)
+ UIANAGroupsNode(self)
UIReferralsNode(self)
def summary(self):
@@ -539,6 +552,52 @@ class UIReferralNode(UINode):
"The Referral could not be disabled.")
+class UIANAGroupsNode(UINode):
+ def __init__(self, parent):
+ UINode.__init__(self, 'ana_groups', parent)
+
+ def refresh(self):
+ self._children = set([])
+ for a in self.parent.cfnode.ana_groups:
+ UIANAGroupNode(self, a)
+
+ def ui_command_create(self, grpid):
+ '''
+ Creates a new ANA Group.
+
+ SEE ALSO
+ ========
+ B{delete}
+ '''
+ a = nvme.ANAGroup(self.parent.cfnode, grpid, mode='create')
+ UIANAGroupNode(self, a)
+
+ def ui_command_delete(self, grpid):
+ '''
+ Deletes the ANA Group with the specified I{name}.
+
+ SEE ALSO
+ ========
+ B{create}
+ '''
+ a = nvme.ANAGroup(self.parent.cfnode, grpid, mode='lookup')
+ a.delete()
+ self.refresh()
+
+
+class UIANAGroupNode(UINode):
+ ui_desc_ana = {
+ 'state' : ('string', 'ANA state'),
+ }
+
+ def __init__(self, parent, cfnode):
+ UINode.__init__(self, str(cfnode.grpid), parent, cfnode)
+
+ def summary(self):
+ info = []
+ info.append("state=" + self.cfnode.get_attr("ana", "state"))
+ return (", ".join(info), True)
+
class UIHostsNode(UINode):
def __init__(self, parent):
UINode.__init__(self, 'hosts', parent)
--
2.13.7
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] nvmetcli: ANA configuration support
2018-07-27 11:08 [PATCH] nvmetcli: ANA configuration support Hannes Reinecke
@ 2018-10-06 12:43 ` Christoph Hellwig
2018-10-06 13:09 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2018-10-06 12:43 UTC (permalink / raw)
I just wanted to apply this in preparation of a new nvmetcli release
once Linux 4.19 is released with ANA support, but the code does not
seem to work well on kernel without multipath support. E.g.:
Traceback (most recent call last):
File "./nvmetcli", line 735, in <module>
main()
File "./nvmetcli", line 717, in main
funcs[sys.argv[1]](savefile)
File "./nvmetcli", line 684, in clear
nvme.Root().clear_existing()
File "/root/nvmetcli/nvmet/nvme.py", line 315, in clear_existing
p.delete()
File "/root/nvmetcli/nvmet/nvme.py", line 676, in delete
a.delete()
File "/root/nvmetcli/nvmet/nvme.py", line 200, in delete
os.rmdir(self.path)
PermissionError: [Errno 1] Operation not permitted: '/sys/kernel/config/nvmet/ports/2/ana_groups/1'
The new ANA code will probably need similar exception handling as the
inline_data_size attribute.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] nvmetcli: ANA configuration support
2018-10-06 12:43 ` Christoph Hellwig
@ 2018-10-06 13:09 ` Hannes Reinecke
2018-10-17 7:11 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2018-10-06 13:09 UTC (permalink / raw)
On 10/6/18 2:43 PM, Christoph Hellwig wrote:
> I just wanted to apply this in preparation of a new nvmetcli release
> once Linux 4.19 is released with ANA support, but the code does not
> seem to work well on kernel without multipath support. E.g.:
>
> Traceback (most recent call last):
> File "./nvmetcli", line 735, in <module>
> main()
> File "./nvmetcli", line 717, in main
> funcs[sys.argv[1]](savefile)
> File "./nvmetcli", line 684, in clear
> nvme.Root().clear_existing()
> File "/root/nvmetcli/nvmet/nvme.py", line 315, in clear_existing
> p.delete()
> File "/root/nvmetcli/nvmet/nvme.py", line 676, in delete
> a.delete()
> File "/root/nvmetcli/nvmet/nvme.py", line 200, in delete
> os.rmdir(self.path)
> PermissionError: [Errno 1] Operation not permitted: '/sys/kernel/config/nvmet/ports/2/ana_groups/1'
>
> The new ANA code will probably need similar exception handling as the
> inline_data_size attribute.
>
Okay, will be fixing it up.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] nvmetcli: ANA configuration support
2018-10-06 13:09 ` Hannes Reinecke
@ 2018-10-17 7:11 ` Christoph Hellwig
2018-11-08 8:48 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2018-10-17 7:11 UTC (permalink / raw)
On Sat, Oct 06, 2018@03:09:20PM +0200, Hannes Reinecke wrote:
> > The new ANA code will probably need similar exception handling as the
> > inline_data_size attribute.
> >
> Okay, will be fixing it up.
Any chance to get a fixed version in the next days? I'd like to cut
a new release with it not too long after the 4.19 release so that people
can easily use the ANA code.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] nvmetcli: ANA configuration support
2018-10-17 7:11 ` Christoph Hellwig
@ 2018-11-08 8:48 ` Christoph Hellwig
2018-11-08 8:59 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2018-11-08 8:48 UTC (permalink / raw)
On Wed, Oct 17, 2018@09:11:18AM +0200, Christoph Hellwig wrote:
> On Sat, Oct 06, 2018@03:09:20PM +0200, Hannes Reinecke wrote:
> > > The new ANA code will probably need similar exception handling as the
> > > inline_data_size attribute.
> > >
> > Okay, will be fixing it up.
>
> Any chance to get a fixed version in the next days? I'd like to cut
> a new release with it not too long after the 4.19 release so that people
> can easily use the ANA code.
Do you still plan to resubmit this code?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] nvmetcli: ANA configuration support
2018-11-08 8:48 ` Christoph Hellwig
@ 2018-11-08 8:59 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2018-11-08 8:59 UTC (permalink / raw)
On 11/8/18 9:48 AM, Christoph Hellwig wrote:
> On Wed, Oct 17, 2018@09:11:18AM +0200, Christoph Hellwig wrote:
>> On Sat, Oct 06, 2018@03:09:20PM +0200, Hannes Reinecke wrote:
>>>> The new ANA code will probably need similar exception handling as the
>>>> inline_data_size attribute.
>>>>
>>> Okay, will be fixing it up.
>>
>> Any chance to get a fixed version in the next days? I'd like to cut
>> a new release with it not too long after the 4.19 release so that people
>> can easily use the ANA code.
>
> Do you still plan to resubmit this code?
>
Yes, I'm just testing the changes.
Should be ready by the end of the week.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH nvmetcli] ANA configuration support
@ 2018-06-07 9:57 Hannes Reinecke
2018-06-07 10:00 ` Johannes Thumshirn
0 siblings, 1 reply; 10+ messages in thread
From: Hannes Reinecke @ 2018-06-07 9:57 UTC (permalink / raw)
Add support for ANA configuration to nvmetcli.
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
nvmet/nvme.py | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
nvmetcli | 59 +++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
diff --git a/nvmet/nvme.py b/nvmet/nvme.py
index 8253ea9..baf972c 100644
--- a/nvmet/nvme.py
+++ b/nvmet/nvme.py
@@ -566,6 +566,24 @@ class Namespace(CFSNode):
def _get_nsid(self):
return self._nsid
+ def _get_grpid(self):
+ self._check_self()
+ _grpid = 0
+ path = "%s/ana_grpid" % self.path
+ if os.path.isfile(path):
+ with open(path, 'r') as file_fd:
+ _grpid = int(file_fd.read().strip())
+ return _grpid
+
+ def set_grpid(self, grpid):
+ self._check_self()
+ path = "%s/ana_grpid" % self.path
+ if os.path.isfile(path):
+ with open(path, 'w') as file_fd:
+ file_fd.write(str(grpid))
+
+ grpid = property(_get_grpid, doc="Get the ANA Group ID.")
+
subsystem = property(_get_subsystem,
doc="Get the parent Subsystem object.")
nsid = property(_get_nsid, doc="Get the NSID as an int.")
@@ -589,10 +607,12 @@ class Namespace(CFSNode):
return
ns._setup_attrs(n, err_func)
+ ns.set_grpid(int(n['ana_grpid']))
def dump(self):
d = super(Namespace, self).dump()
d['nsid'] = self.nsid
+ d['ana_grpid'] = self.grpid
return d
@@ -652,6 +672,8 @@ class Port(CFSNode):
self._check_self()
for s in self.subsystems:
self.remove_subsystem(s)
+ for a in self.ana_groups:
+ a.delete()
for r in self.referrals:
r.delete()
super(Port, self).delete()
@@ -664,6 +686,14 @@ class Port(CFSNode):
referrals = property(_list_referrals,
doc="Get the list of Referrals for this Port.")
+ def _list_ana_groups(self):
+ self._check_self()
+ for d in os.listdir("%s/ana_groups/" % self._path):
+ yield ANA_Group(self, int(d), 'lookup')
+
+ ana_groups = property(_list_ana_groups,
+ doc="Get the list of ANA Groups for this Port.")
+
@classmethod
def setup(cls, root, n, err_func):
'''
@@ -685,6 +715,8 @@ class Port(CFSNode):
port._setup_attrs(n, err_func)
for s in n.get('subsystems', []):
port.add_subsystem(s)
+ for a in n.get('ana_groups', []):
+ ANA_Group.setup(port, a, err_func)
for r in n.get('referrals', []):
Referral.setup(port, r, err_func)
@@ -692,6 +724,7 @@ class Port(CFSNode):
d = super(Port, self).dump()
d['portid'] = self.portid
d['subsystems'] = self.subsystems
+ d['ana_groups'] = [a.dump() for a in self.ana_groups]
d['referrals'] = [r.dump() for r in self.referrals]
return d
@@ -747,6 +780,72 @@ class Referral(CFSNode):
return d
+class ANA_Group(CFSNode):
+ '''
+ This is an interface to a NVMe ANA Group in configFS.
+ '''
+
+ MAX_GRPID = 128
+
+ def __repr__(self):
+ return "<ANA Group %d>" % self.grpid
+
+ def __init__(self, port, grpid, mode='any'):
+ super(ANA_Group, self).__init__()
+
+ if grpid is None:
+ if mode == 'lookup':
+ raise CFSError("Need grpid for lookup")
+
+ grpids = [n.grpid for n in port.ana_groups]
+ for index in xrange(2, self.MAX_GRPID + 1):
+ if index not in grpids:
+ grpid = index
+ break
+ if grpid is None:
+ raise CFSError("All ANA Group IDs 1-%d in use" % self.MAX_GRPID)
+ else:
+ grpid = int(grpid)
+ if grpid < 1 or grpid > self.MAX_GRPID:
+ raise CFSError("GRPID %d must be 1 to %d" % (grpid, self.MAX_GRPID))
+
+ self.attr_groups = ['ana']
+ self._port = port
+ self._grpid = grpid
+ self._path = "%s/ana_groups/%d" % (self._port.path, self.grpid)
+ self._create_in_cfs(mode)
+
+ def _get_grpid(self):
+ return self._grpid
+
+ grpid = property(_get_grpid, doc="Get the ANA Group ID.")
+
+ @classmethod
+ def setup(cls, port, n, err_func):
+ '''
+ Set up an ANA Group object based upon n dict, from saved config.
+ Guard against missing or bad dict items, but keep going.
+ Call 'err_func' for each error.
+ '''
+
+ if 'grpid' not in n:
+ err_func("'grpid' not defined for ANA Group")
+ return
+
+ try:
+ a = ANA_Group(port, n['grpid'])
+ except CFSError as e:
+ err_func("Could not create ANA Group object: %s" % e)
+ return
+
+ a._setup_attrs(n, err_func)
+
+ def dump(self):
+ d = super(ANA_Group, self).dump()
+ d['grpid'] = self.grpid
+ return d
+
+
class Host(CFSNode):
'''
This is an interface to a NVMe Host in configFS.
diff --git a/nvmetcli b/nvmetcli
index 6b102a2..5e4bd16 100755
--- a/nvmetcli
+++ b/nvmetcli
@@ -258,6 +258,16 @@ class UINamespaceNode(UINode):
raise configshell.ExecutionError(
"The Namespace could not be disabled.")
+ def ui_command_grpid(self, grpid):
+ '''
+ Sets the ANA Group ID of the current Namespace to I{grpid}
+ '''
+ try:
+ self.cfnode.set_grpid(grpid)
+ except Exception as e:
+ raise configshell.ExecutionError(
+ "Failed to set ANA Group ID for this Namespace.")
+
def summary(self):
info = []
info.append("path=" + self.cfnode.get_attr("device", "path"))
@@ -267,6 +277,8 @@ class UINamespaceNode(UINode):
ns_nguid = self.cfnode.get_attr("device", "nguid")
if ngiud_set(ns_nguid):
info.append("nguid=" + ns_nguid)
+ if self.cfnode.grpid != 0:
+ info.append("grpid=" + str(self.cfnode.grpid))
info.append("enabled" if self.cfnode.get_enable() else "disabled")
ns_enabled = self.cfnode.get_enable()
return (", ".join(info), True if ns_enabled == 1 else ns_enabled)
@@ -378,6 +390,7 @@ class UIPortNode(UINode):
def __init__(self, parent, cfnode):
UINode.__init__(self, str(cfnode.portid), parent, cfnode)
UIPortSubsystemsNode(self)
+ UIANAGroupsNode(self)
UIReferralsNode(self)
def summary(self):
@@ -539,6 +552,52 @@ class UIReferralNode(UINode):
"The Referral could not be disabled.")
+class UIANAGroupsNode(UINode):
+ def __init__(self, parent):
+ UINode.__init__(self, 'ana_groups', parent)
+
+ def refresh(self):
+ self._children = set([])
+ for a in self.parent.cfnode.ana_groups:
+ UIANAGroupNode(self, a)
+
+ def ui_command_create(self, grpid):
+ '''
+ Creates a new ANA Group.
+
+ SEE ALSO
+ ========
+ B{delete}
+ '''
+ a = nvme.ANA_Group(self.parent.cfnode, grpid, mode='create')
+ UIANAGroupNode(self, a)
+
+ def ui_command_delete(self, grpid):
+ '''
+ Deletes the ANA Group with the specified I{name}.
+
+ SEE ALSO
+ ========
+ B{create}
+ '''
+ a = nvme.ANA_Group(self.parent.cfnode, grpid, mode='lookup')
+ a.delete()
+ self.refresh()
+
+
+class UIANAGroupNode(UINode):
+ ui_desc_ana = {
+ 'state' : ('string', 'ANA state'),
+ }
+
+ def __init__(self, parent, cfnode):
+ UINode.__init__(self, str(cfnode.grpid), parent, cfnode)
+
+ def summary(self):
+ info = []
+ info.append("state=" + self.cfnode.get_attr("ana", "state"))
+ return (", ".join(info), True)
+
class UIHostsNode(UINode):
def __init__(self, parent):
UINode.__init__(self, 'hosts', parent)
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH nvmetcli] ANA configuration support
2018-06-07 9:57 [PATCH nvmetcli] " Hannes Reinecke
@ 2018-06-07 10:00 ` Johannes Thumshirn
2018-06-07 12:12 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2018-06-07 10:00 UTC (permalink / raw)
On Thu, Jun 07, 2018@11:57:09AM +0200, Hannes Reinecke wrote:
> +class ANA_Group(CFSNode):
Please use ANAGroup to be consistent with the rest and Python's PEP8
guidelines.
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH nvmetcli] ANA configuration support
2018-06-07 10:00 ` Johannes Thumshirn
@ 2018-06-07 12:12 ` Christoph Hellwig
2018-06-07 12:38 ` Hannes Reinecke
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2018-06-07 12:12 UTC (permalink / raw)
On Thu, Jun 07, 2018@12:00:35PM +0200, Johannes Thumshirn wrote:
> On Thu, Jun 07, 2018@11:57:09AM +0200, Hannes Reinecke wrote:
> > +class ANA_Group(CFSNode):
>
> Please use ANAGroup to be consistent with the rest and Python's PEP8
> guidelines.
Agreed. Please run the code through pep8 and pylint in general. There
are a few warnings, especially from the latter, but nothing crazy
should be introduced.
Also we'll need to cover the ANA code in the unit tests.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH nvmetcli] ANA configuration support
2018-06-07 12:12 ` Christoph Hellwig
@ 2018-06-07 12:38 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2018-06-07 12:38 UTC (permalink / raw)
On Thu, 7 Jun 2018 14:12:45 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Jun 07, 2018@12:00:35PM +0200, Johannes Thumshirn wrote:
> > On Thu, Jun 07, 2018 at 11:57:09AM +0200, Hannes Reinecke wrote:
> > > +class ANA_Group(CFSNode):
> >
> > Please use ANAGroup to be consistent with the rest and Python's PEP8
> > guidelines.
>
> Agreed. Please run the code through pep8 and pylint in general.
> There are a few warnings, especially from the latter, but nothing
> crazy should be introduced.
>
> Also we'll need to cover the ANA code in the unit tests.
>
No problem. First time for me to send a substantial python patch, so I
guess I have still a learning curve here.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-08 8:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 11:08 [PATCH] nvmetcli: ANA configuration support Hannes Reinecke
2018-10-06 12:43 ` Christoph Hellwig
2018-10-06 13:09 ` Hannes Reinecke
2018-10-17 7:11 ` Christoph Hellwig
2018-11-08 8:48 ` Christoph Hellwig
2018-11-08 8:59 ` Hannes Reinecke
-- strict thread matches above, loose matches on Subject: below --
2018-06-07 9:57 [PATCH nvmetcli] " Hannes Reinecke
2018-06-07 10:00 ` Johannes Thumshirn
2018-06-07 12:12 ` Christoph Hellwig
2018-06-07 12:38 ` Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.