Discussion:
[PATCH] Fix behavior for dot inside /proc/sys/ path
Petr Oros
2015-05-18 10:30:50 UTC
Permalink
When /proc/sys/ path contain dot (for example netif name) and config
file using * for filename tuna fail with traceback:

Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
Traceback (most recent call last):
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in __init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line 146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13, in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 80, in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''

This patch add support for escaping "." character.

Signed-off-by: Petr Oros <***@redhat.com>
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
@@ -30,6 +30,12 @@ class Config:
self.config[option] = value
self.cacheFileName = ''

+ def FileNameToConfigPath(self, filename):
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ def ConfigPathToFileName(self, configpath):
+ return configpath.replace(".", "/").replace("\\/", ".")
+
def updateDefault(self, filename):
if filename.replace("", "temp-direct-load.conf") != filename:
self.temp = ConfigParser.RawConfigParser()
@@ -154,9 +160,9 @@ class Config:
tempCfg = []
for index in range(len(oldTempCfg)):
if self.isFnString(oldTempCfg[index][0]):
- expanded = self.getFilesByFN("/proc/sys", oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys", self.ConfigPathToFileName(oldTempCfg[index][0]))
for index2 in range(len(expanded)):
- expandedData = (expanded[index2].replace("/", "."), oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]), oldTempCfg[index][1])
tempCfg.append(expandedData)
else:
tempCfg.append(oldTempCfg[index])
@@ -241,9 +247,9 @@ class Config:
def getSystemValue(self, filename):
filename = self.aliasToOriginal(filename)
try:
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
except IOError:
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s" %(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()

@@ -253,10 +259,10 @@ class Config:
if value == "" or old == value:
return 0
try:
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
except IOError:
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"), filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"), self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0

@@ -302,9 +308,9 @@ class Config:
snapcont = []
for index in range(len(snapcontPacked)):
if self.isFnString(snapcontPacked[index][0]):
- expanded = self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded = self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
for index2 in range(len(expanded)):
- expandedData = (expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
else:
snapcont.append(snapcontPacked[index])
@@ -348,7 +354,7 @@ class Config:
return msgStack
current = self.checkParser.items(option)
for opt,val in current:
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and len(self.getFilesByFN("/proc/sys/",opt.replace(".","/"))) == 0:
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt)) and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt))) == 0:
msgStack = "%s%s%s\n" % (msgStack, _("Warning: File not found: /proc/sys/"), opt)
self.empty = False
if self.empty:
@@ -367,7 +373,7 @@ class Config:
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
for opt,val in current:
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and len(self.getFilesByFN("/proc/sys/", opt.replace(".", "/"))) == 0:
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt)) and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt))) == 0:
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
except (ConfigParser.Error, IOError) as e:
--
2.4.1
Arnaldo Carvalho de Melo
2015-05-18 16:25:08 UTC
Permalink
Post by Petr Oros
When /proc/sys/ path contain dot (for example netif name) and config
Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in __init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line 146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13, in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 80, in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''
This patch add support for escaping "." character.
I guess this was in response to my report, right? So it would be nice to
have:

Reported-by: Arnaldo Carvalho de Melo <***@redhat.com>
Fixes: https://bugzilla.redhat.com/..../bz-number

I will review and test later, thanks for the patch!

- Arnaldo
Post by Petr Oros
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys", oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys", self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."), oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]), oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s" %(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"), filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"), self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded = self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded = self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData = (expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
msgStack = "%s%s%s\n" % (msgStack, _("Warning: File not found: /proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
John Kacur
2015-05-18 18:50:25 UTC
Permalink
Yeah, it was also in response to me pointing the bz out to him. I'm testing it as well, I would like to wait for your patch-review Arnaldo before
I grab this fix.

Thanks

John

----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
When /proc/sys/ path contain dot (for example netif name) and config
Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in __init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line
146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13,
in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 80, in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''
This patch add support for escaping "." character.
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
Post by Petr Oros
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."),
oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
Arnaldo Carvalho de Melo
2015-05-18 19:37:52 UTC
Permalink
Post by John Kacur
Yeah, it was also in response to me pointing the bz out to him. I'm testing it as well, I would like to wait for your patch-review Arnaldo before
I grab this fix.
----- Original Message -----
Post by Arnaldo Carvalho de Melo
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
... return configpath.replace(".", "/").replace("\\/", ".")
...
... return filename.replace(".", "\\.").replace("/", ".")
...
Post by John Kacur
Post by Arnaldo Carvalho de Melo
filename="/proc/sys/net/ipv4/neigh/tuna.1"
config=FileNameToConfigPath(None, filename)
print config
.proc.sys.net.ipv4.neigh.tuna\.1
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(config)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: ConfigPathToFileName() takes exactly 2 arguments (1 given)
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(None, config)
/proc/sys/net/ipv4/neigh/tuna.1
But then I don't know why that has to be a class method instead of a global
function, since it is not specific to that class, see how 'self' is not used at
all, and os potentially useful for other cases dealing with filenames as config
keys.

So I'd recommend to make those global functions, outside this class.

- Arnaldo
Post by John Kacur
Post by Arnaldo Carvalho de Melo
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."), oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
Petr Oros
2015-05-19 07:29:11 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by John Kacur
Yeah, it was also in response to me pointing the bz out to him. I'm
testing it as well, I would like to wait for your patch-review
Arnaldo before
I grab this fix.
----- Original Message -----
Post by Arnaldo Carvalho de Melo
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
... return configpath.replace(".", "/").replace("\\/", ".")
...
... return filename.replace(".", "\\.").replace("/", ".")
...
Post by John Kacur
Post by Arnaldo Carvalho de Melo
filename="/proc/sys/net/ipv4/neigh/tuna.1"
config=FileNameToConfigPath(None, filename)
print config
.proc.sys.net.ipv4.neigh.tuna\.1
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(config)
File "<stdin>", line 1, in <module>
TypeError: ConfigPathToFileName() takes exactly 2 arguments (1 given)
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(None, config)
/proc/sys/net/ipv4/neigh/tuna.1
But then I don't know why that has to be a class method instead of a global
function, since it is not specific to that class, see how 'self' is not used at
all, and os potentially useful for other cases dealing with filenames as config
keys.
So I'd recommend to make those global functions, outside this class.
- Arnaldo
It's conversion config <-> filesystem. Direct access is bad. Tuna can
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Just my opinion, if you still think, use as global is better, i will do
it.

Regards,
-Petr
Post by Arnaldo Carvalho de Melo
Post by John Kacur
Post by Arnaldo Carvalho de Melo
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".",
"\\.").replace("/", ".")
+
+ return configpath.replace(".",
"/").replace("\\/", ".")
+
if filename.replace("", "temp-direct
self.temp =
ConfigParser.RawConfigParser()
tempCfg = []
for index in
if
- expand
ed = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expand
ed = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
for
- > > > > expandedData = (expanded[index2].replace("/",
"."),
oldTempCfg[index][1])
+ > > > > expandedData =
(self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
Post by Petr Oros
tempCfg.append(expandedData)
tempCf
g.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" +
filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" +
self.ConfigPathToFileName(filename),
'r').read()
/proc/sys/%s" %(filename.replace(".",
"/")))
/proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" +
filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" +
self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to
file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to
file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
for index in
if
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(
".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcon
tPacked[index][0]))
for index2 in
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[ind
ex][1])
snapcont.appen
d(expandedData)
snapcont.append(snapco
ntPacked[index])
return msgStack
current =
self.checkParser.items(option)
- if not
os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not
os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/",
self.ConfigPathToFileName(opt)))
msgStack =
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('
categories', '#' + option, value)
current =
self.checkParser.items(option)
- if not
os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not
os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/",
self.ConfigPathToFileName(opt)))
self.checkPars
er.remove_option(option, opt)
self.checkPars
er.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
Arnaldo Carvalho de Melo
2015-05-19 14:10:58 UTC
Permalink
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by John Kacur
Yeah, it was also in response to me pointing the bz out to him. I'm
testing it as well, I would like to wait for your patch-review
Arnaldo before
I grab this fix.
----- Original Message -----
Post by Arnaldo Carvalho de Melo
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
... return configpath.replace(".", "/").replace("\\/", ".")
...
... return filename.replace(".", "\\.").replace("/", ".")
...
Post by John Kacur
Post by Arnaldo Carvalho de Melo
filename="/proc/sys/net/ipv4/neigh/tuna.1"
config=FileNameToConfigPath(None, filename)
print config
.proc.sys.net.ipv4.neigh.tuna\.1
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(config)
File "<stdin>", line 1, in <module>
TypeError: ConfigPathToFileName() takes exactly 2 arguments (1 given)
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(None, config)
/proc/sys/net/ipv4/neigh/tuna.1
But then I don't know why that has to be a class method instead of a global
function, since it is not specific to that class, see how 'self' is not used at
all, and os potentially useful for other cases dealing with filenames as config
keys.
So I'd recommend to make those global functions, outside this class.
It's conversion config <-> filesystem. Direct access is bad. Tuna can
Why is it bad?
Post by Petr Oros
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Can you think of one?
Post by Petr Oros
Just my opinion, if you still think, use as global is better, i will do
it.
- Arnaldo
Petr Oros
2015-05-19 15:20:21 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by John Kacur
Yeah, it was also in response to me pointing the bz out to him. I'm
testing it as well, I would like to wait for your patch-review
Arnaldo before
I grab this fix.
----- Original Message -----
Post by Arnaldo Carvalho de Melo
I guess this was in response to my report, right? So it would
be
nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
... return configpath.replace(".", "/").replace("\\/", ".")
...
... return filename.replace(".", "\\.").replace("/", ".")
...
Post by John Kacur
Post by Arnaldo Carvalho de Melo
filename="/proc/sys/net/ipv4/neigh/tuna.1"
config=FileNameToConfigPath(None, filename)
print config
.proc.sys.net.ipv4.neigh.tuna\.1
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(config)
File "<stdin>", line 1, in <module>
TypeError: ConfigPathToFileName() takes exactly 2 arguments (1 given)
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(None, config)
/proc/sys/net/ipv4/neigh/tuna.1
But then I don't know why that has to be a class method instead
of a
global
function, since it is not specific to that class, see how 'self'
is
not used at
all, and os potentially useful for other cases dealing with
filenames
as config
keys.
So I'd recommend to make those global functions, outside this class.
It's conversion config <-> filesystem. Direct access is bad. Tuna can
Why is it bad?
Post by Petr Oros
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Can you think of one?
These helpers are related to config class code. Python is object
oriented language and, i just don't see reason why move functions
somewhere into space. I created them for better outline and for
specific usage, not globally for some magic anywhere.

Regards,
-Petr
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Just my opinion, if you still think, use as global is better, i will do
it.
- Arnaldo
Arnaldo Carvalho de Melo
2015-05-19 19:34:27 UTC
Permalink
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by John Kacur
Yeah, it was also in response to me pointing the bz out to him. I'm
testing it as well, I would like to wait for your patch-review
Arnaldo before
I grab this fix.
----- Original Message -----
Post by Arnaldo Carvalho de Melo
I guess this was in response to my report, right? So it would
be
nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
... return configpath.replace(".", "/").replace("\\/", ".")
...
... return filename.replace(".", "\\.").replace("/", ".")
...
Post by John Kacur
Post by Arnaldo Carvalho de Melo
filename="/proc/sys/net/ipv4/neigh/tuna.1"
config=FileNameToConfigPath(None, filename)
print config
.proc.sys.net.ipv4.neigh.tuna\.1
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(config)
File "<stdin>", line 1, in <module>
TypeError: ConfigPathToFileName() takes exactly 2 arguments (1 given)
Post by John Kacur
Post by Arnaldo Carvalho de Melo
print ConfigPathToFileName(None, config)
/proc/sys/net/ipv4/neigh/tuna.1
But then I don't know why that has to be a class method instead
of a
global
function, since it is not specific to that class, see how 'self'
is
not used at
all, and os potentially useful for other cases dealing with
filenames
as config
keys.
So I'd recommend to make those global functions, outside this class.
It's conversion config <-> filesystem. Direct access is bad. Tuna can
Why is it bad?
Post by Petr Oros
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Can you think of one?
These helpers are related to config class code. Python is object
That is perfectly ok to leave them in that file then, just like lots of
other global functions in the tuna code.
Post by Petr Oros
oriented language and, i just don't see reason why move functions
They are not related to the config class code, if they were it would be
using self, no? I.e. are those methods dealing with internal state of
this class?

The fact that a language is object oriented doesn't preclude using
normal functions.
Post by Petr Oros
somewhere into space. I created them for better outline and for
specific usage, not globally for some magic anywhere.
There is no magic involved, its just logic, the code is not related to
the class as it doesn't touch its internal state.

- Arnaldo
Arnaldo Carvalho de Melo
2015-05-19 20:10:00 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
It's conversion config <-> filesystem. Direct access is bad. Tuna can
Why is it bad?
Post by Petr Oros
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Can you think of one?
These helpers are related to config class code. Python is object
That is perfectly ok to leave them in that file then, just like lots of
other global functions in the tuna code.
Post by Petr Oros
oriented language and, i just don't see reason why move functions
They are not related to the config class code, if they were it would be
using self, no? I.e. are those methods dealing with internal state of
this class?
The fact that a language is object oriented doesn't preclude using
normal functions.
Post by Petr Oros
somewhere into space. I created them for better outline and for
specific usage, not globally for some magic anywhere.
There is no magic involved, its just logic, the code is not related to
the class as it doesn't touch its internal state.
But if you insist that it be tied to the class, at least lets use the
right mechanism, static methods:

https://julien.danjou.info/blog/2013/guide-python-static-class-abstract-methods

-----------------------------------------------------------------------
Static methods

Static methods are a special case of methods. Sometimes, you'll write
code that belongs to a class, but that doesn't use the object itself at
all. For example:

class Pizza(object):
@staticmethod
def mix_ingredients(x, y):
return x + y

def cook(self):
return self.mix_ingredients(self.cheese, self.vegetables)

-----------------------------------------------------------------------

I.e. remove that self, is has no business being there, it is unused.

So it would be:

def FileNameToConfigPath(filename):
return filename.replace(".", "\\.").replace("/", ".")

def ConfigPathToFileName(configpath):
return configpath.replace(".", "/").replace("\\/", ".")

Testing here with this code:

---------------------------------------------------------------------------

[***@zoo ~]$ cat static_method.py
import sys

class Config:
def __init__(self, filename):
self.configpath = self.FileNameToConfigPath(filename)

@staticmethod
def FileNameToConfigPath(filename):
return filename.replace(".", "\\.").replace("/", ".")

@staticmethod
def ConfigPathToFileName(configpath):
return configpath.replace(".", "/").replace("\\/", ".")

def show(self):
print "config path=%s\n filename=%s" % (self.configpath, self.ConfigPathToFileName(self.configpath))

def main(argv):

cfg = Config(argv[1])
cfg.show()

if __name__ == '__main__':
main(sys.argv)
[***@zoo ~]$ python static_method.py /proc/sys/net/ipv4/conf/tuna.1
config path=.proc.sys.net.ipv4.conf.tuna\.1
filename=/proc/sys/net/ipv4/conf/tuna.1
[***@zoo ~]$

---------------------------------------------------------------------------

Then, as soon as, if ever, we need that function elsewhere, we either make it a
global function or we do some copy and paste and have a replica of this static
method, which would be a waste :-)

- Arnaldo
Petr Oros
2015-05-20 08:06:23 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
It's conversion config <-> filesystem. Direct access is bad.
Tuna
can
Why is it bad?
Post by Petr Oros
use config ONLY over this class. I don't know where can use this
conversion directly. Class method protect to potential bad use.
Can you think of one?
These helpers are related to config class code. Python is object
That is perfectly ok to leave them in that file then, just like lots of
other global functions in the tuna code.
Post by Petr Oros
oriented language and, i just don't see reason why move functions
They are not related to the config class code, if they were it would be
using self, no? I.e. are those methods dealing with internal state of
this class?
The fact that a language is object oriented doesn't preclude using
normal functions.
Post by Petr Oros
somewhere into space. I created them for better outline and for
specific usage, not globally for some magic anywhere.
There is no magic involved, its just logic, the code is not related to
the class as it doesn't touch its internal state.
But if you insist that it be tied to the class, at least lets use the
https://julien.danjou.info/blog/2013/guide-python-static-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
All methods in config have "self" param. Why do it for two new methods
different? What talk other people, when see staticmethod only on this
two places?

Regards,
-Petr
Post by Arnaldo Carvalho de Melo
---------------------------------------------------------------------
--
Static methods
Static methods are a special case of methods. Sometimes, you'll write
code that belongs to a class, but that doesn't use the object itself at
@staticmethod
return x + y
return self.mix_ingredients(self.cheese, self.vegetables)
---------------------------------------------------------------------
--
I.e. remove that self, is has no business being there, it is unused.
return filename.replace(".", "\\.").replace("/",
".")
return configpath.replace(".", "/").replace("\\/", ".")
---------------------------------------------------------------------
------
import sys
self.configpath =
self.FileNameToConfigPath(filename)
@staticmethod
return filename.replace(".", "\\.").replace("/", ".")
@staticmethod
return configpath.replace(".", "/").replace("\\/", ".")
print "config path=%s\n filename=%s" %
(self.configpath, self.ConfigPathToFileName(self.configpath))
cfg = Config(argv[1])
cfg.show()
main(sys.argv)
config path=.proc.sys.net.ipv4.conf.tuna\.1
filename=/proc/sys/net/ipv4/conf/tuna.1
---------------------------------------------------------------------
------
Then, as soon as, if ever, we need that function elsewhere, we either make it a
global function or we do some copy and paste and have a replica of this static
method, which would be a waste :-)
- Arnaldo
Arnaldo Carvalho de Melo
2015-05-20 13:31:03 UTC
Permalink
Post by Petr Oros
Em Tue, May 19, 2015 at 04:34:27PM -0300, Arnaldo Carvalho de Melo
But if you insist that it be tied to the class, at least lets use the
https://julien.danjou.info/blog/2013/guide-python-static-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
Fair question, it should be used where it makes sense, one never stops
learning :-)
Post by Petr Oros
All methods in config have "self" param. Why do it for two new methods
different? What talk other people, when see staticmethod only on this
Because it is not used.
Post by Petr Oros
two places?
That the other places where self is present but not used should use it?

I'll do an audit and do that change.

- Arnaldo
Petr Oros
2015-05-20 13:54:22 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Em Tue, May 19, 2015 at 04:34:27PM -0300, Arnaldo Carvalho de Melo
But if you insist that it be tied to the class, at least lets use the
https://julien.danjou.info/blog/2013/guide-python-static-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
Fair question, it should be used where it makes sense, one never stops
learning :-)
I understand, if you plan code refactoring ;)
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
All methods in config have "self" param. Why do it for two new methods
different? What talk other people, when see staticmethod only on this
Because it is not used.
Post by Petr Oros
two places?
That the other places where self is present but not used should use it?
After fast look, for example here :D :

checkTunedDaemon(self)
currentActiveProfile(self)
setCurrentActiveProfile(self)
getSystemValue(self)
isFnString(self, string)
getFilesByFN(self, string)

Regards,
-Petr
Post by Arnaldo Carvalho de Melo
I'll do an audit and do that change.
- Arnaldo
Arnaldo Carvalho de Melo
2015-05-20 13:57:18 UTC
Permalink
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Em Tue, May 19, 2015 at 04:34:27PM -0300, Arnaldo Carvalho de Melo
But if you insist that it be tied to the class, at least lets use the
https://julien.danjou.info/blog/2013/guide-python-static-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
Fair question, it should be used where it makes sense, one never
stops learning :-)
I understand, if you plan code refactoring ;)
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
All methods in config have "self" param. Why do it for two new methods
different? What talk other people, when see staticmethod only on this
Because it is not used.
Post by Petr Oros
two places?
That the other places where self is present but not used should use it?
checkTunedDaemon(self)
currentActiveProfile(self)
setCurrentActiveProfile(self)
getSystemValue(self)
isFnString(self, string)
getFilesByFN(self, string)
Ok, I'll come up with a patch for that, then do some testing, if nobody
does it first ;-)

- Arnaldo
John Kacur
2015-05-20 14:59:30 UTC
Permalink
----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Em Tue, May 19, 2015 at 04:34:27PM -0300, Arnaldo Carvalho de Melo
But if you insist that it be tied to the class, at least lets use the
https://julien.danjou.info/blog/2013/guide-python-static-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
Fair question, it should be used where it makes sense, one never
stops learning :-)
I understand, if you plan code refactoring ;)
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
All methods in config have "self" param. Why do it for two new methods
different? What talk other people, when see staticmethod only on this
Because it is not used.
Post by Petr Oros
two places?
That the other places where self is present but not used should use it?
checkTunedDaemon(self)
currentActiveProfile(self)
setCurrentActiveProfile(self)
getSystemValue(self)
isFnString(self, string)
getFilesByFN(self, string)
Ok, I'll come up with a patch for that, then do some testing, if nobody
does it first ;-)
But I'd like a minimal patch that fixes the one problem now, without having to worry about us
introducing a new problem.

Patches that improve inconsistencies in the code can go upstream and go through a round of testing first.

(risk management here)

Thanks

John
Petr Oros
2015-05-20 15:02:38 UTC
Permalink
Post by John Kacur
----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
Em Tue, May 19, 2015 at 04:34:27PM -0300, Arnaldo Carvalho
de
Melo
But if you insist that it be tied to the class, at least
lets use
the
https://julien.danjou.info/blog/2013/guide-python-static
-class
-abstract-methods
Ok, my last question. What about code consistency?
Found you anywhere staticmethod in tuna code?
Fair question, it should be used where it makes sense, one never
stops learning :-)
I understand, if you plan code refactoring ;)
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
All methods in config have "self" param. Why do it for two
new
methods
different? What talk other people, when see staticmethod only
on
this
Because it is not used.
Post by Petr Oros
two places?
That the other places where self is present but not used should
use
it?
checkTunedDaemon(self)
currentActiveProfile(self)
setCurrentActiveProfile(self)
getSystemValue(self)
isFnString(self, string)
getFilesByFN(self, string)
Ok, I'll come up with a patch for that, then do some testing, if nobody
does it first ;-)
But I'd like a minimal patch that fixes the one problem now, without
having to worry about us
introducing a new problem.
By this way is ok original patch from me ;)
Rest could be done "bulk" in future.

Regards,
-Petr
Post by John Kacur
Patches that improve inconsistencies in the code can go upstream and
go through a round of testing first.
(risk management here)
Thanks
John
Arnaldo Carvalho de Melo
2015-05-20 16:44:13 UTC
Permalink
Post by Petr Oros
Post by John Kacur
But I'd like a minimal patch that fixes the one problem now, without
having to worry about us
introducing a new problem.
By this way is ok original patch from me ;)
Rest could be done "bulk" in future.
Right, I can go with that, it is the wrong, suboptimal way to do it, but
hey, it works ;-)

- Arnaldo
John Kacur
2015-05-27 14:53:24 UTC
Permalink
So, I've incorporated this patch with this added.

Signed-off-by: Petr Oros <***@redhat.com>
Reported-by: Arnaldo Carvalho de Melo <***@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1178917
Signed-off-by: John Kacur <***@redhat.com>

Now, we've had a bit of discussion about refractoring.
That's going to happen for rhel7.2, so my question for Arnaldo and Petr is,
should I open a bz for the refractoring effort for rhel-7.3?

Thanks

John

----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
When /proc/sys/ path contain dot (for example netif name) and config
Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in __init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line
146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13,
in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 80, in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''
This patch add support for escaping "." character.
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
Post by Petr Oros
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."),
oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
John Kacur
2015-05-27 14:54:38 UTC
Permalink
Should read, that's NOT going to happen for rhel-7.2. sigh.

----- Original Message -----
Post by John Kacur
So, I've incorporated this patch with this added.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1178917
Now, we've had a bit of discussion about refractoring.
That's going to happen for rhel7.2, so my question for Arnaldo and Petr is,
should I open a bz for the refractoring effort for rhel-7.3?
Thanks
John
----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
When /proc/sys/ path contain dot (for example netif name) and config
Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in
__init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line
146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13,
in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line
80,
in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''
This patch add support for escaping "." character.
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
Post by Petr Oros
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."), oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not os.path.exists("/proc/sys/" +
self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not os.path.exists("/proc/sys/" +
self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
Arnaldo Carvalho de Melo
2015-05-27 15:36:31 UTC
Permalink
Post by John Kacur
So, I've incorporated this patch with this added.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1178917
Now, we've had a bit of discussion about refractoring.
That's going to happen for rhel7.2, so my question for Arnaldo and Petr is,
should I open a bz for the refractoring effort for rhel-7.3?
You may, but I don't think it is that necessary, should be done as
upstream work, then, at some point, we create a BZ for updating the RHEL
package to this new upstream version.

That, BTW, should incorporate patches Daniel Bristot sent, and some work
on allowing to refer to CPUs in nohz_full more as well as isolcpus, that
I did and tested but need to submit upstream.

Probably will collect all those patches, do the refactoring and then
either release the new upstream version or ask Jiri Kastner to do that.

- Arnaldo
Post by John Kacur
Thanks
John
----- Original Message -----
Post by Arnaldo Carvalho de Melo
Post by Petr Oros
When /proc/sys/ path contain dot (for example netif name) and config
Invalid item! file: /proc/sys/net/ipv6/conf/tuna/1/forwarding
File "/usr/bin/tuna", line 647, in <module>
main()
File "/usr/bin/tuna", line 641, in main
app = tuna_gui.main_gui(kthreads, uthreads, cpus_filtered)
File "/usr/lib/python2.7/site-packages/tuna/tuna_gui.py", line 64, in
__init__
self.profileview.init_default_file()
File "/usr/lib/python2.7/site-packages/tuna/gui/profileview.py", line
146, in init_default_file
self.commonview.updateCommonView()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 13,
in updateCommonView
self.setup()
File "/usr/lib/python2.7/site-packages/tuna/gui/commonview.py", line 80,
in setup
frameContent[catCntr]['texts'][contentCntr].set_value(int(self.config.ctlParams[catCntr][val]))
ValueError: invalid literal for int() with base 10: ''
This patch add support for escaping "." character.
I guess this was in response to my report, right? So it would be nice to
Fixes: https://bugzilla.redhat.com/..../bz-number
I will review and test later, thanks for the patch!
- Arnaldo
Post by Petr Oros
---
tuna/config.py | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tuna/config.py b/tuna/config.py
index 77cc4d8..02c730e 100644
--- a/tuna/config.py
+++ b/tuna/config.py
self.config[option] = value
self.cacheFileName = ''
+ return filename.replace(".", "\\.").replace("/", ".")
+
+ return configpath.replace(".", "/").replace("\\/", ".")
+
self.temp = ConfigParser.RawConfigParser()
tempCfg = []
- expanded = self.getFilesByFN("/proc/sys",
oldTempCfg[index][0].replace(".", "/"))
+ expanded = self.getFilesByFN("/proc/sys",
self.ConfigPathToFileName(oldTempCfg[index][0]))
- expandedData = (expanded[index2].replace("/", "."), oldTempCfg[index][1])
+ expandedData = (self.FileNameToConfigPath(expanded[index2]),
oldTempCfg[index][1])
tempCfg.append(expandedData)
tempCfg.append(oldTempCfg[index])
filename = self.aliasToOriginal(filename)
- buffer = open("/proc/sys/" + filename.replace(".", "/"), 'r').read()
+ buffer = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'r').read()
- print _("Invalid item! file: /proc/sys/%s" %(filename.replace(".", "/")))
+ print _("Invalid item! file: /proc/sys/%s"
%(self.ConfigPathToFileName(filename)))
return ""
return buffer.strip()
return 0
- fp = open("/proc/sys/" + filename.replace(".", "/"), 'w')
+ fp = open("/proc/sys/" + self.ConfigPathToFileName(filename), 'w')
fp.write(value)
- print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
filename.replace(".","/"), _("value:"), value)
+ print "%s%s %s %s" % (_("Cant write to file! path: /proc/sys/"),
self.ConfigPathToFileName(filename), _("value:"), value)
return -1
return 0
snapcont = []
- expanded =
self.getFilesByFN("/proc/sys",snapcontPacked[index][0].replace(".","/"))
+ expanded =
self.getFilesByFN("/proc/sys",self.ConfigPathToFileName(snapcontPacked[index][0]))
- expandedData =
(expanded[index2].replace("/","."),snapcontPacked[index][1])
+ expandedData =
(self.FileNameToConfigPath(expanded[index2]),snapcontPacked[index][1])
snapcont.append(expandedData)
snapcont.append(snapcontPacked[index])
return msgStack
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".","/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
/proc/sys/"), opt)
self.empty = False
self.checkParser.set('categories', '#' + option, value)
current = self.checkParser.items(option)
- if not os.path.exists("/proc/sys/" + opt.replace(".", "/")) and
+ if not os.path.exists("/proc/sys/" + self.ConfigPathToFileName(opt))
and len(self.getFilesByFN("/proc/sys/", self.ConfigPathToFileName(opt)))
self.checkParser.remove_option(option, opt)
self.checkParser.set(option, '#' + opt, val)
--
2.4.1
_______________________________________________
tuna-devel mailing list
https://lists.fedorahosted.org/mailman/listinfo/tuna-devel
Jiri Kastner
2015-05-28 12:12:01 UTC
Permalink
Post by Arnaldo Carvalho de Melo
Post by John Kacur
So, I've incorporated this patch with this added.
Now, we've had a bit of discussion about refractoring.
That's going to happen for rhel7.2, so my question for Arnaldo and Petr is,
should I open a bz for the refractoring effort for rhel-7.3?
You may, but I don't think it is that necessary, should be done as
upstream work, then, at some point, we create a BZ for updating the RHEL
package to this new upstream version.
That, BTW, should incorporate patches Daniel Bristot sent, and some work
on allowing to refer to CPUs in nohz_full more as well as isolcpus, that
I did and tested but need to submit upstream.
Probably will collect all those patches, do the refactoring and then
either release the new upstream version or ask Jiri Kastner to do that.
i'll be glad to do it, in fact i'm waiting for final verdict.

regarding daniel's patches i was not able to apply patches without losing
comment, signed-of-by and authourship. he wrote me, that you will have it
in repo.

daniel, peter, you should also push your tuna work to github.com :)

or, can you, please, arnaldo push them, even to separate branch, to your
tuna repo on korg?
Post by Arnaldo Carvalho de Melo
- Arnaldo
thanks
jiri

Loading...