| Message ID | alpine.LFD.2.21.1903252158090.3080@austen3.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | python3 issues | expand |
On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > I have been testing the python3 changes committed to xen and found a few > issues. There are a couple of ocaml python build scripts that don't work > for me with python3, and I needed a few fixes to get pygrub to work, > mostly due to the change from strings to bytes. I am attaching the patch I > put together in testing to get these things to work to illustrate where > the problems are and in case it is useful to others, though I believe at > least some of it isn't compatible with python2. Most Python code can be easily made compatible with both Python 2 and Python 3 with the right constructs. > --- xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py.orig 2019-03-06 14:42:49.000000000 +0000 > +++ xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py 2019-03-13 21:33:59.805930989 +0000 > @@ -86,14 +87,14 @@ > def autogen_header(open_comment, close_comment): > s = open_comment + " AUTO-GENERATED FILE DO NOT EDIT " + close_comment + "\n" > s += open_comment + " autogenerated by \n" > - s += reduce(lambda x,y: x + " ", range(len(open_comment + " ")), "") > + s += reduce(lambda x,y: x + " ", list(range(len(open_comment + " "))), "") > s += "%s" % " ".join(sys.argv) > s += "\n " + close_comment + "\n\n" > return s > > if __name__ == '__main__': > if len(sys.argv) < 3: > - print >>sys.stderr, "Usage: genlevels.py <mli> <ml> <c-inc>" > + print("Usage: genlevels.py <mli> <ml> <c-inc>", file=sys.stderr) > sys.exit(1) > > levels, olevels = read_levels() The print() function notation is required for Python 3, but has a bit of trouble with Python 2. Python 2 though does have support. At the top of the relevant files add "from __future__ import print_function" and the print() will work in Python 2. I'm unsure of the status of the other constructs in this patch.
On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > I have been testing the python3 changes committed to xen and found a few > issues. There are a couple of ocaml python build scripts that don't work > for me with python3, and I needed a few fixes to get pygrub to work, > mostly due to the change from strings to bytes. I am attaching the patch I > put together in testing to get these things to work to illustrate where > the problems are and in case it is useful to others, though I believe at > least some of it isn't compatible with python2. My fault. Somehow all my local testing and project's CIs failed to catch these files. Thanks for fixing these. I will turn it into a proper patch, put your SoB there and submit it. > > Michael Young > --- xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py.orig 2019-03-06 14:42:49.000000000 +0000 > +++ xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py 2019-03-13 21:33:59.805930989 +0000 > @@ -1,6 +1,7 @@ > #!/usr/bin/python > > import sys > +from functools import reduce We should have from __future__ import print_function here to be python2 compatible. > > def read_levels(): > f = open('../../../libs/toollog/include/xentoollog.h', 'r') > @@ -86,14 +87,14 @@ > def autogen_header(open_comment, close_comment): > s = open_comment + " AUTO-GENERATED FILE DO NOT EDIT " + close_comment + "\n" > s += open_comment + " autogenerated by \n" > - s += reduce(lambda x,y: x + " ", range(len(open_comment + " ")), "") > + s += reduce(lambda x,y: x + " ", list(range(len(open_comment + " "))), "") I don't think list is required here. reduce should work with generator just fine. > s += "%s" % " ".join(sys.argv) > s += "\n " + close_comment + "\n\n" > return s > > if __name__ == '__main__': > if len(sys.argv) < 3: > - print >>sys.stderr, "Usage: genlevels.py <mli> <ml> <c-inc>" > + print("Usage: genlevels.py <mli> <ml> <c-inc>", file=sys.stderr) > sys.exit(1) > > levels, olevels = read_levels() > --- xen-4.12.0-rc5/tools/ocaml/libs/xl/genwrap.py.orig 2019-03-06 14:42:49.000000000 +0000 > +++ xen-4.12.0-rc5/tools/ocaml/libs/xl/genwrap.py 2019-03-13 21:34:00.674962832 +0000 > @@ -3,6 +3,7 @@ > import sys,os > > import idl > +from functools import reduce Same here as above. > if ty.init_fn is not None: > --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 > +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 > @@ -230,10 +230,10 @@ > def _get_default(self): > return self._default > def _set_default(self, val): > - if val == "saved": > + if val == "saved" or not val.isdecimal(): > self._default = 0 > else: > - self._default = val > + self._default = int(val) > > if self._default < 0: > raise ValueError("default must be positive number") > --- xen-4.12.0-rc6/tools/pygrub/src/pygrub.orig 2019-03-24 22:44:05.503582025 +0000 > +++ xen-4.12.0-rc6/tools/pygrub/src/pygrub 2019-03-24 22:48:24.446113809 +0000 > @@ -457,7 +457,7 @@ > # limit read size to avoid pathological cases > buf = f.read(FS_READ_MAX) > del f > - self.cf.parse(buf) > + self.cf.parse(buf.decode()) Hmm... This could be a bit problematic for 2 compatibility. I will need some time to check the documents. Wei.
On Tue, Mar 26, 2019 at 01:16:35PM +0000, Wei Liu wrote: > On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > > if ty.init_fn is not None: > > --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 > > +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 > > @@ -230,10 +230,10 @@ > > def _get_default(self): > > return self._default > > def _set_default(self, val): > > - if val == "saved": > > + if val == "saved" or not val.isdecimal(): Your change suggested there could be a non-decimal string that is not "saved" -- is this really needed? Wei. > > self._default = 0 > > else: > > - self._default = val > > + self._default = int(val)
On Tue, 26 Mar 2019, Wei Liu wrote: > On Tue, Mar 26, 2019 at 01:16:35PM +0000, Wei Liu wrote: > > On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > > > if ty.init_fn is not None: > > > --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 > > > +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 > > > @@ -230,10 +230,10 @@ > > > def _get_default(self): > > > return self._default > > > def _set_default(self, val): > > > - if val == "saved": > > > + if val == "saved" or not val.isdecimal(): > > Your change suggested there could be a non-decimal string that is not > "saved" -- is this really needed? It is getting set to ${next_entry} presumably from the clause if [ "${next_entry}" ] ; then set default="${next_entry}" set next_entry= save_env next_entry set boot_once=true else set default="${saved_entry}" fi in the grub.cfg file giving the error File "/usr/lib64/python3.7/site-packages/grub/GrubConf.py", line 239, in _set_default if self._default < 0: TypeError: '<' not supported between instances of 'str' and 'int' I didn't see this with python 2 before the patch so I assume python3 is more fussy. Michael Young
On 3/26/19 7:18 PM, M A Young wrote: > On Tue, 26 Mar 2019, Wei Liu wrote: > >> On Tue, Mar 26, 2019 at 01:16:35PM +0000, Wei Liu wrote: >>> On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: >>>> if ty.init_fn is not None: >>>> --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 >>>> +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 >>>> @@ -230,10 +230,10 @@ >>>> def _get_default(self): >>>> return self._default >>>> def _set_default(self, val): >>>> - if val == "saved": >>>> + if val == "saved" or not val.isdecimal(): >> >> Your change suggested there could be a non-decimal string that is not >> "saved" -- is this really needed? > > It is getting set to ${next_entry} presumably from the clause > > if [ "${next_entry}" ] ; then > set default="${next_entry}" > set next_entry= > save_env next_entry > set boot_once=true > else > set default="${saved_entry}" > fi > > in the grub.cfg file giving the error > > File "/usr/lib64/python3.7/site-packages/grub/GrubConf.py", line 239, in > _set_default > if self._default < 0: > TypeError: '<' not supported between instances of 'str' and 'int' > > I didn't see this with python 2 before the patch so I assume python3 is > more fussy. Comparison is also used for sorting. In Python 2, numeric values always sort before strings. So, for example: == sorted(['a','b','c', 1, 2, 3]) [1, 2, 3, 'a', 'b', 'c'] However, this behavior also easily leads to bugs, for example when someone forgets to convert strings that hold numeric values to actual numbers and still compares things, which silently introduces unintended behavior. So, the above if self._default < 0 will just always be False if self._default is a string. I didn't read the context of these lines, but this looks like an actual bug currently. Python 3 no longer allows comparing string and int, because it doesn't make sense. == sorted([1,2,3,'a','b','c']) Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: unorderable types: str() < int() So I guess that if the contents are expected to be "saved" or a number as string like "1", "2", then: 1. check for "saved" 2. try: int(val) except: blah 3. etc Also, python 2 does not have isdecimal() on strings. Hans
On 3/26/19 2:16 PM, Wei Liu wrote: > On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: >> [...] >> --- xen-4.12.0-rc6/tools/pygrub/src/pygrub.orig 2019-03-24 22:44:05.503582025 +0000 >> +++ xen-4.12.0-rc6/tools/pygrub/src/pygrub 2019-03-24 22:48:24.446113809 +0000 >> @@ -457,7 +457,7 @@ >> # limit read size to avoid pathological cases >> buf = f.read(FS_READ_MAX) >> del f >> - self.cf.parse(buf) >> + self.cf.parse(buf.decode()) > > Hmm... This could be a bit problematic for 2 compatibility. I will need > some time to check the documents. The exact moment when you're typing .decode() or .encode() to try fix a python2/3 problem, you're entering a world of pain (tm). [insert big lebowski walter pointing gun gif here] Some of the python 2 and 3 compatibility things can be fixed easily by importing things from the future, but when dealing with strings and bytes, you're entering the "is it really worth it" department. In python 2, strings are bytes, and there's some added duct tape available to do something with unicode and utf-8. In python 3, strings are unicode and bytes are bytes, which is sooooooo much more convenient. If the strings have to be stored in some location that stores bytes, you can encode them. If you have some incoming byte stream, you can decode it. I don't really have recommendations to make both of them work with the same lines of code since I tried and gave up when doing it myself. What you're looking at is how the in-memory storage for a 'string' is done and how to connect input and output to that, while looking at python 2 and 3 functions with the same name, doing different things in a different context. There is no sane way to do this that works with python 2 and 3. Hans
On Tue, Mar 26, 2019 at 10:06:48PM +0100, Hans van Kranenburg wrote: > On 3/26/19 7:18 PM, M A Young wrote: > > On Tue, 26 Mar 2019, Wei Liu wrote: > > > >> On Tue, Mar 26, 2019 at 01:16:35PM +0000, Wei Liu wrote: > >>> On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > >>>> if ty.init_fn is not None: > >>>> --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 > >>>> +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 > >>>> @@ -230,10 +230,10 @@ > >>>> def _get_default(self): > >>>> return self._default > >>>> def _set_default(self, val): > >>>> - if val == "saved": > >>>> + if val == "saved" or not val.isdecimal(): > >> > >> Your change suggested there could be a non-decimal string that is not > >> "saved" -- is this really needed? > > > > It is getting set to ${next_entry} presumably from the clause > > > > if [ "${next_entry}" ] ; then > > set default="${next_entry}" > > set next_entry= > > save_env next_entry > > set boot_once=true > > else > > set default="${saved_entry}" > > fi > > > > in the grub.cfg file giving the error > > > > File "/usr/lib64/python3.7/site-packages/grub/GrubConf.py", line 239, in > > _set_default > > if self._default < 0: > > TypeError: '<' not supported between instances of 'str' and 'int' > > > > I didn't see this with python 2 before the patch so I assume python3 is > > more fussy. > > Comparison is also used for sorting. > > In Python 2, numeric values always sort before strings. So, for example: > > == sorted(['a','b','c', 1, 2, 3]) > [1, 2, 3, 'a', 'b', 'c'] > > However, this behavior also easily leads to bugs, for example when > someone forgets to convert strings that hold numeric values to actual > numbers and still compares things, which silently introduces unintended > behavior. > > So, the above if self._default < 0 will just always be False if > self._default is a string. I didn't read the context of these lines, but > this looks like an actual bug currently. > > Python 3 no longer allows comparing string and int, because it doesn't > make sense. > > == sorted([1,2,3,'a','b','c']) > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > TypeError: unorderable types: str() < int() Thanks for the detailed explanation! I think the original behaviour is not very nice. Python lacking type annotation doesn't help either. > > So I guess that if the contents are expected to be "saved" or a number > as string like "1", "2", then: > > 1. check for "saved" > 2. try: int(val) except: blah > 3. etc I think a better way is to use string conversation all the way -- it doesn't make sense for an API to return string sometimes and integers sometimes. I will give it a stab. Wei.
On Tue, Mar 26, 2019 at 10:06:48PM +0100, Hans van Kranenburg wrote: > > Python 3 no longer allows comparing string and int, because it doesn't > make sense. > > == sorted([1,2,3,'a','b','c']) > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > TypeError: unorderable types: str() < int() > > So I guess that if the contents are expected to be "saved" or a number > as string like "1", "2", then: > > 1. check for "saved" > 2. try: int(val) except: blah > 3. etc > Actually I think you scheme here is better. Relying on string comparison seems to be error-prone. Wei.
On Mon, 1 Apr 2019, Wei Liu wrote: > On Tue, Mar 26, 2019 at 10:06:48PM +0100, Hans van Kranenburg wrote: > > > > Python 3 no longer allows comparing string and int, because it doesn't > > make sense. > > > > == sorted([1,2,3,'a','b','c']) > > Traceback (most recent call last): > > File "<stdin>", line 1, in <module> > > TypeError: unorderable types: str() < int() > > > > So I guess that if the contents are expected to be "saved" or a number > > as string like "1", "2", then: > > > > 1. check for "saved" > > 2. try: int(val) except: blah > > 3. etc > > > > Actually I think you scheme here is better. Relying on string comparison > seems to be error-prone. > > Wei. I tested with the patch --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 @@ -233,7 +233,10 @@ if val == "saved": self._default = 0 else: - self._default = val + try: + self._default = int(val) + except ValueError: + self._default = 0 if self._default < 0: raise ValueError("default must be positive number") which works for me but should probably have some logging added for debug purposes. Michael Young
On Mon, Apr 01, 2019 at 10:59:05AM +0100, M A Young wrote: > > > On Mon, 1 Apr 2019, Wei Liu wrote: > > > On Tue, Mar 26, 2019 at 10:06:48PM +0100, Hans van Kranenburg wrote: > > > > > > Python 3 no longer allows comparing string and int, because it doesn't > > > make sense. > > > > > > == sorted([1,2,3,'a','b','c']) > > > Traceback (most recent call last): > > > File "<stdin>", line 1, in <module> > > > TypeError: unorderable types: str() < int() > > > > > > So I guess that if the contents are expected to be "saved" or a number > > > as string like "1", "2", then: > > > > > > 1. check for "saved" > > > 2. try: int(val) except: blah > > > 3. etc > > > > > > > Actually I think you scheme here is better. Relying on string comparison > > seems to be error-prone. > > > > Wei. > > I tested with the patch > > --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 > 22:44:05.502581989 +0000 > +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 > 22:49:14.025934786 +0000 > @@ -233,7 +233,10 @@ > if val == "saved": > self._default = 0 > else: > - self._default = val > + try: > + self._default = int(val) > + except ValueError: > + self._default = 0 > > if self._default < 0: > raise ValueError("default must be positive number") > > which works for me but should probably have some logging added for debug > purposes. Ha, I have the exact same snippet in my patch. :-) Wei. > > Michael Young
On Wed, Mar 27, 2019 at 01:36:10AM +0100, Hans van Kranenburg wrote: > On 3/26/19 2:16 PM, Wei Liu wrote: > > On Mon, Mar 25, 2019 at 10:20:05PM +0000, YOUNG, MICHAEL A. wrote: > >> [...] > >> --- xen-4.12.0-rc6/tools/pygrub/src/pygrub.orig 2019-03-24 22:44:05.503582025 +0000 > >> +++ xen-4.12.0-rc6/tools/pygrub/src/pygrub 2019-03-24 22:48:24.446113809 +0000 > >> @@ -457,7 +457,7 @@ > >> # limit read size to avoid pathological cases > >> buf = f.read(FS_READ_MAX) > >> del f > >> - self.cf.parse(buf) > >> + self.cf.parse(buf.decode()) > > > > Hmm... This could be a bit problematic for 2 compatibility. I will need > > some time to check the documents. > > The exact moment when you're typing .decode() or .encode() to try fix a > python2/3 problem, you're entering a world of pain (tm). > > [insert big lebowski walter pointing gun gif here] > > Some of the python 2 and 3 compatibility things can be fixed easily by > importing things from the future, but when dealing with strings and > bytes, you're entering the "is it really worth it" department. > > In python 2, strings are bytes, and there's some added duct tape > available to do something with unicode and utf-8. > > In python 3, strings are unicode and bytes are bytes, which is sooooooo > much more convenient. If the strings have to be stored in some location > that stores bytes, you can encode them. If you have some incoming byte > stream, you can decode it. > > I don't really have recommendations to make both of them work with the > same lines of code since I tried and gave up when doing it myself. > > What you're looking at is how the in-memory storage for a 'string' is > done and how to connect input and output to that, while looking at > python 2 and 3 functions with the same name, doing different things in > a different context. There is no sane way to do this that works with > python 2 and 3. I have tested the following snippet: from __future__ import print_function import sys s = b'1234' if sys.version_info[0] < 3: print(s) else: print(s.decode()) It seems to be the easiest way to solve this issue. Wei. > > Hans
--- xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py.orig 2019-03-06 14:42:49.000000000 +0000 +++ xen-4.12.0-rc5/tools/ocaml/libs/xentoollog/genlevels.py 2019-03-13 21:33:59.805930989 +0000 @@ -1,6 +1,7 @@ #!/usr/bin/python import sys +from functools import reduce def read_levels(): f = open('../../../libs/toollog/include/xentoollog.h', 'r') @@ -86,14 +87,14 @@ def autogen_header(open_comment, close_comment): s = open_comment + " AUTO-GENERATED FILE DO NOT EDIT " + close_comment + "\n" s += open_comment + " autogenerated by \n" - s += reduce(lambda x,y: x + " ", range(len(open_comment + " ")), "") + s += reduce(lambda x,y: x + " ", list(range(len(open_comment + " "))), "") s += "%s" % " ".join(sys.argv) s += "\n " + close_comment + "\n\n" return s if __name__ == '__main__': if len(sys.argv) < 3: - print >>sys.stderr, "Usage: genlevels.py <mli> <ml> <c-inc>" + print("Usage: genlevels.py <mli> <ml> <c-inc>", file=sys.stderr) sys.exit(1) levels, olevels = read_levels() --- xen-4.12.0-rc5/tools/ocaml/libs/xl/genwrap.py.orig 2019-03-06 14:42:49.000000000 +0000 +++ xen-4.12.0-rc5/tools/ocaml/libs/xl/genwrap.py 2019-03-13 21:34:00.674962832 +0000 @@ -3,6 +3,7 @@ import sys,os import idl +from functools import reduce # typename -> ( ocaml_type, c_from_ocaml, ocaml_from_c ) builtins = { @@ -78,7 +79,7 @@ elif isinstance(ty,idl.Array): return "%s array" % ocaml_type_of(ty.elem_type) elif isinstance(ty,idl.Builtin): - if not builtins.has_key(ty.typename): + if ty.typename not in builtins: raise NotImplementedError("Unknown Builtin %s (%s)" % (ty.typename, type(ty))) typename,_,_ = builtins[ty.typename] if not typename: @@ -251,7 +252,7 @@ else: s += "\texternal default : ctx -> %sunit -> t = \"stub_libxl_%s_init\"\n" % (union_args, ty.rawname) - if functions.has_key(ty.rawname): + if ty.rawname in functions: for name,args in functions[ty.rawname]: s += "\texternal %s : " % name s += " -> ".join(args) @@ -278,7 +279,7 @@ else: s += "%s = Int_val(%s);" % (c, o) elif isinstance(ty,idl.Builtin): - if not builtins.has_key(ty.typename): + if ty.typename not in builtins: raise NotImplementedError("Unknown Builtin %s (%s)" % (ty.typename, type(ty))) _,fn,_ = builtins[ty.typename] if not fn: @@ -375,7 +376,7 @@ else: s += "%s = Val_int(%s);" % (o, c) elif isinstance(ty,idl.Builtin): - if not builtins.has_key(ty.typename): + if ty.typename not in builtins: raise NotImplementedError("Unknown Builtin %s (%s)" % (ty.typename, type(ty))) _,_,fn = builtins[ty.typename] if not fn: @@ -513,14 +514,14 @@ def autogen_header(open_comment, close_comment): s = open_comment + " AUTO-GENERATED FILE DO NOT EDIT " + close_comment + "\n" s += open_comment + " autogenerated by \n" - s += reduce(lambda x,y: x + " ", range(len(open_comment + " ")), "") + s += reduce(lambda x,y: x + " ", list(range(len(open_comment + " "))), "") s += "%s" % " ".join(sys.argv) s += "\n " + close_comment + "\n\n" return s if __name__ == '__main__': if len(sys.argv) < 4: - print >>sys.stderr, "Usage: genwrap.py <idl> <mli> <ml> <c-inc>" + print("Usage: genwrap.py <idl> <mli> <ml> <c-inc>", file=sys.stderr) sys.exit(1) (_,types) = idl.parse(sys.argv[1]) @@ -533,7 +534,7 @@ for t in blacklist: if t not in [ty.rawname for ty in types]: - print "unknown type %s in blacklist" % t + print("unknown type %s in blacklist" % t) types = [ty for ty in types if not ty.rawname in blacklist] @@ -564,7 +565,7 @@ cinc.write("\n") cinc.write(gen_Val_ocaml(ty)) cinc.write("\n") - if functions.has_key(ty.rawname): + if ty.rawname in functions: cinc.write(gen_c_stub_prototype(ty, functions[ty.rawname])) cinc.write("\n") if ty.init_fn is not None: --- xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py.orig 2019-03-24 22:44:05.502581989 +0000 +++ xen-4.12.0-rc6/tools/pygrub/src/GrubConf.py 2019-03-24 22:49:14.025934786 +0000 @@ -230,10 +230,10 @@ def _get_default(self): return self._default def _set_default(self, val): - if val == "saved": + if val == "saved" or not val.isdecimal(): self._default = 0 else: - self._default = val + self._default = int(val) if self._default < 0: raise ValueError("default must be positive number") --- xen-4.12.0-rc6/tools/pygrub/src/pygrub.orig 2019-03-24 22:44:05.503582025 +0000 +++ xen-4.12.0-rc6/tools/pygrub/src/pygrub 2019-03-24 22:48:24.446113809 +0000 @@ -457,7 +457,7 @@ # limit read size to avoid pathological cases buf = f.read(FS_READ_MAX) del f - self.cf.parse(buf) + self.cf.parse(buf.decode()) def image_index(self): if isinstance(self.cf.default, int): @@ -960,5 +960,5 @@ ostring = format_simple(bootcfg["kernel"], bootcfg["ramdisk"], args, "\0") sys.stdout.flush() - os.write(fd, ostring) + os.write(fd, ostring.encode())