[ovs-dev,v2] bugtool: Fix for Python3
diff mbox series

Message ID 5dc11a6204ee5c22bb40976a7079129da6acdec1.1584644593.git.tredaelli@redhat.com
State New
Headers show
Series
  • [ovs-dev,v2] bugtool: Fix for Python3
Related show

Commit Message

Timothy Redaelli March 19, 2020, 7:05 p.m. UTC
Currently ovs-bugtool tool doesn't start on Python 3.
This commit fixes ovs-bugtool to make it works on Python 3.

Replaced StringIO.StringIO with io.BytesIO since the script is
processing binary data.

Reported-at: https://bugzilla.redhat.com/1809241
Reported-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
---
Changes since v1:
  * Converted StringIO to BytesIO
  * Fix some other string/bytes conversion
---
 utilities/bugtool/ovs-bugtool.in | 45 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 23 deletions(-)

Comments

William Tu March 19, 2020, 7:20 p.m. UTC | #1
On Thu, Mar 19, 2020 at 12:05 PM Timothy Redaelli <tredaelli@redhat.com> wrote:
>
> Currently ovs-bugtool tool doesn't start on Python 3.
> This commit fixes ovs-bugtool to make it works on Python 3.
>
> Replaced StringIO.StringIO with io.BytesIO since the script is
> processing binary data.
>
> Reported-at: https://bugzilla.redhat.com/1809241
> Reported-by: Flavio Leitner <fbl@sysclose.org>
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---
> Changes since v1:
>   * Converted StringIO to BytesIO
>   * Fix some other string/bytes conversion
> ---

Thanks for sending out v2. Hit an error below:
~/ovs# python3
Python 3.5.2 (default, Oct  8 2019, 13:06:37)

~/ovs# ./utilities/bugtool/ovs-bugtool -y -s --output=tar.gz
--outfile=/tmp/t.tgz
Traceback (most recent call last):
  File "./utilities/bugtool/ovs-bugtool", line 1405, in <module>
    sys.exit(main())
  File "./utilities/bugtool/ovs-bugtool", line 717, in main
    collect_data()
  File "./utilities/bugtool/ovs-bugtool", line 388, in collect_data
    v['output'] = BytesIOmtime(s)
  File "./utilities/bugtool/ovs-bugtool", line 1395, in __init__
    BytesIO.__init__(self, buf)
TypeError: a bytes-like object is required, not 'str'

I think sometimes 's' is bytes type, sometimes 's' is a str type...
William
William Tu March 26, 2020, 12:33 a.m. UTC | #2
On Thu, Mar 19, 2020 at 12:20:19PM -0700, William Tu wrote:
> On Thu, Mar 19, 2020 at 12:05 PM Timothy Redaelli <tredaelli@redhat.com> wrote:
> >
> > Currently ovs-bugtool tool doesn't start on Python 3.
> > This commit fixes ovs-bugtool to make it works on Python 3.
> >
> > Replaced StringIO.StringIO with io.BytesIO since the script is
> > processing binary data.
> >
> > Reported-at: https://bugzilla.redhat.com/1809241
> > Reported-by: Flavio Leitner <fbl@sysclose.org>
> > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> > ---
> > Changes since v1:
> >   * Converted StringIO to BytesIO
> >   * Fix some other string/bytes conversion
> > ---
> 
> Thanks for sending out v2. Hit an error below:
> ~/ovs# python3
> Python 3.5.2 (default, Oct  8 2019, 13:06:37)
> 
> ~/ovs# ./utilities/bugtool/ovs-bugtool -y -s --output=tar.gz
> --outfile=/tmp/t.tgz
> Traceback (most recent call last):
>   File "./utilities/bugtool/ovs-bugtool", line 1405, in <module>
>     sys.exit(main())
>   File "./utilities/bugtool/ovs-bugtool", line 717, in main
>     collect_data()
>   File "./utilities/bugtool/ovs-bugtool", line 388, in collect_data
>     v['output'] = BytesIOmtime(s)
>   File "./utilities/bugtool/ovs-bugtool", line 1395, in __init__
>     BytesIO.__init__(self, buf)
> TypeError: a bytes-like object is required, not 'str'
> 
> I think sometimes 's' is bytes type, sometimes 's' is a str type...
> William

Hi Timothy,

How about adding this to your patch?
I tested it and works ok.

diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index c26c2be7a4eb..47f3c4629f70 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -385,7 +385,10 @@ def collect_data():
             except Exception as e:
                 s = str(e).encode()
             if check_space(cap, k, len(s)):
-                v['output'] = BytesIOmtime(s)
+                if isinstance(s, str):
+                    v['output'] = BytesIOmtime(s.encode())
+                else:
+                    v['output'] = BytesIOmtime(s)
 
 
 def main(argv=None):

Patch
diff mbox series

diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index e55bfc2ed..c26c2be7a 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -33,8 +33,7 @@ 
 # or func_output().
 #
 
-import StringIO
-import commands
+from io import BytesIO
 import fcntl
 import getopt
 import hashlib
@@ -48,7 +47,7 @@  import warnings
 import zipfile
 from select import select
 from signal import SIGTERM
-from subprocess import PIPE, Popen
+from subprocess import PIPE, Popen, check_output
 
 from xml.dom.minidom import getDOMImplementation, parse
 
@@ -348,7 +347,7 @@  def collect_data():
             cap = v['cap']
             if 'cmd_args' in v:
                 if 'output' not in v.keys():
-                    v['output'] = StringIOmtime()
+                    v['output'] = BytesIOmtime()
                 if v['repeat_count'] > 0:
                     if cap not in process_lists:
                         process_lists[cap] = []
@@ -373,20 +372,20 @@  def collect_data():
         if 'filename' in v and v['filename'].startswith('/proc/'):
             # proc files must be read into memory
             try:
-                f = open(v['filename'], 'r')
+                f = open(v['filename'], 'rb')
                 s = f.read()
                 f.close()
                 if check_space(cap, v['filename'], len(s)):
-                    v['output'] = StringIOmtime(s)
+                    v['output'] = BytesIOmtime(s)
             except:
                 pass
         elif 'func' in v:
             try:
                 s = v['func'](cap)
             except Exception as e:
-                s = str(e)
+                s = str(e).encode()
             if check_space(cap, k, len(s)):
-                v['output'] = StringIOmtime(s)
+                v['output'] = BytesIOmtime(s)
 
 
 def main(argv=None):
@@ -704,7 +703,7 @@  exclude those logs from the archive.
 
     # permit the user to filter out data
     # We cannot use iteritems, since we modify 'data' as we pass through
-    for (k, v) in sorted(data.items()):
+    for (k, v) in data.items():
         cap = v['cap']
         if 'filename' in v:
             key = k[0]
@@ -721,7 +720,7 @@  exclude those logs from the archive.
 
     # include inventory
     data['inventory.xml'] = {'cap': None,
-                        'output': StringIOmtime(make_inventory(data, subdir))}
+                        'output': BytesIOmtime(make_inventory(data, subdir))}
 
     # create archive
     if output_fd == -1:
@@ -782,7 +781,7 @@  def dump_scsi_hosts(cap):
 
 
 def module_info(cap):
-    output = StringIO.StringIO()
+    output = BytesIO()
     modules = open(PROC_MODULES, 'r')
     procs = []
 
@@ -806,7 +805,7 @@  def multipathd_topology(cap):
 
 
 def dp_list():
-    output = StringIO.StringIO()
+    output = BytesIO()
     procs = [ProcOutput([OVS_DPCTL, 'dump-dps'],
              caps[CAP_NETWORK_STATUS][MAX_TIME], output)]
 
@@ -828,7 +827,7 @@  def collect_ovsdb():
             if os.path.isfile(OPENVSWITCH_COMPACT_DB):
                 os.unlink(OPENVSWITCH_COMPACT_DB)
 
-            output = StringIO.StringIO()
+            output = BytesIO()
             max_time = 5
             procs = [ProcOutput(['ovsdb-tool', 'compact',
                                 OPENVSWITCH_CONF_DB, OPENVSWITCH_COMPACT_DB],
@@ -871,7 +870,7 @@  def fd_usage(cap):
 
 
 def dump_rdac_groups(cap):
-    output = StringIO.StringIO()
+    output = BytesIO()
     procs = [ProcOutput([MPPUTIL, '-a'], caps[cap][MAX_TIME], output)]
 
     run_procs([procs])
@@ -896,7 +895,7 @@  def load_plugins(just_capabilities=False, filter=None):
         for node in nodelist:
             if node.nodeType == node.TEXT_NODE:
                 rc += node.data
-        return rc.encode()
+        return rc
 
     def getBoolAttr(el, attr, default=False):
         ret = default
@@ -1037,7 +1036,7 @@  def make_tar(subdir, suffix, output_fd, output_file):
                     s = os.stat(v['filename'])
                     ti.mtime = s.st_mtime
                     ti.size = s.st_size
-                    tf.addfile(ti, open(v['filename']))
+                    tf.addfile(ti, open(v['filename'], 'rb'))
             except:
                 pass
     finally:
@@ -1095,12 +1094,12 @@  def make_inventory(inventory, subdir):
     s.setAttribute('date', time.strftime('%c'))
     s.setAttribute('hostname', platform.node())
     s.setAttribute('uname', ' '.join(platform.uname()))
-    s.setAttribute('uptime', commands.getoutput(UPTIME))
+    s.setAttribute('uptime', check_output(UPTIME).decode())
     document.getElementsByTagName(INVENTORY_XML_ROOT)[0].appendChild(s)
 
     map(lambda k_v: inventory_entry(document, subdir, k_v[0], k_v[1]),
         inventory.items())
-    return document.toprettyxml()
+    return document.toprettyxml().encode()
 
 
 def inventory_entry(document, subdir, k, v):
@@ -1301,7 +1300,7 @@  class ProcOutput(object):
             line = self.proc.stdout.readline()
         else:
             line = self.proc.stdout.read(self.bufsize)
-        if line == '':
+        if line == b'':
             # process exited
             self.proc.stdout.close()
             self.status = self.proc.wait()
@@ -1391,13 +1390,13 @@  def get_free_disk_space(path):
     return s.f_frsize * s.f_bfree
 
 
-class StringIOmtime(StringIO.StringIO):
-    def __init__(self, buf=''):
-        StringIO.StringIO.__init__(self, buf)
+class BytesIOmtime(BytesIO):
+    def __init__(self, buf=b''):
+        BytesIO.__init__(self, buf)
         self.mtime = time.time()
 
     def write(self, s):
-        StringIO.StringIO.write(self, s)
+        BytesIO.write(self, s)
         self.mtime = time.time()