removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
always going to be there anyway, switch to a less specific but more
compatible construct.
Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Sadly this is only the tip of the iceberg. Some minimum version of the
json-c library is apparently needed for the toolstack to build, but no
minimum version is being checked for.
--- a/tools/libs/light/gentypes.py
+++ b/tools/libs/light/gentypes.py
@@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
s += "int rc;\n"
sub_scope_object = "jso_sub_1"
else:
- sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
+ sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
if isinstance(ty, idl.Array):
if parent is None:On Thu, Oct 16, 2025 at 01:22:30PM +0200, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
>
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Thanks,
--
Anthony PERARD
On 21.10.2025 16:51, Anthony PERARD wrote:
> On Thu, Oct 16, 2025 at 01:22:30PM +0200, Jan Beulich wrote:
>> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
>> always going to be there anyway, switch to a less specific but more
>> compatible construct.
>>
>> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Anthony PERARD <anthony.perard@vates.tech>
Thanks, but: So you prefer this variant over Jason's?
Jan
On 10/16/25 1:22 PM, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
>
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich<jbeulich@suse.com>
> ---
> Sadly this is only the tip of the iceberg. Some minimum version of the
> json-c library is apparently needed for the toolstack to build, but no
> minimum version is being checked for.
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
Thanks.
~ Oleksii
>
> --- a/tools/libs/light/gentypes.py
> +++ b/tools/libs/light/gentypes.py
> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
> s += "int rc;\n"
> sub_scope_object = "jso_sub_1"
> else:
> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
> + sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
>
> if isinstance(ty, idl.Array):
> if parent is None:
On 16/10/2025 12:22 pm, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
>
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Sadly this is only the tip of the iceberg. Some minimum version of the
> json-c library is apparently needed for the toolstack to build, but no
> minimum version is being checked for.
Well, this is why we have release candidates, and a bug queue.
>
> --- a/tools/libs/light/gentypes.py
> +++ b/tools/libs/light/gentypes.py
> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
> s += "int rc;\n"
> sub_scope_object = "jso_sub_1"
> else:
> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
> + sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
This isn't quite an equivalent change. You want:
def removeprefix(s, p): # Py < 3.9 compat
if s.startswith(p):
return s[len(p):]
return s
at the top level somewhere, and to call removeprefix(scope_object,
"jso_sub_") here.
Sadly, because string is a builtin type, you can't make the 3.9 syntax
work on older versions of python by extending the string type.
~Andrew
On 16.10.2025 13:45, Andrew Cooper wrote:
> On 16/10/2025 12:22 pm, Jan Beulich wrote:
>> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
>> always going to be there anyway, switch to a less specific but more
>> compatible construct.
>>
>> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Sadly this is only the tip of the iceberg. Some minimum version of the
>> json-c library is apparently needed for the toolstack to build, but no
>> minimum version is being checked for.
>
> Well, this is why we have release candidates, and a bug queue.
>
>>
>> --- a/tools/libs/light/gentypes.py
>> +++ b/tools/libs/light/gentypes.py
>> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
>> s += "int rc;\n"
>> sub_scope_object = "jso_sub_1"
>> else:
>> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
>> + sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
>
> This isn't quite an equivalent change.
Yes, as said in the description.
> You want:
>
> def removeprefix(s, p): # Py < 3.9 compat
> if s.startswith(p):
> return s[len(p):]
> return s
>
> at the top level somewhere, and to call removeprefix(scope_object,
> "jso_sub_") here.
I first thought of doing something like this, but then didn't really see why
we would need such. If the prefix is anything else, the original construct
wouldn't work anyway (I expect an exception would be raised unless the incoming
string was itself consisting of only digits).
Jan
removeprefix is only added in Python 3.9.
Instead of the prefix removal, switch to passing in a "depth" parameter,
and incrementing it for each level.
There is a slight change in the generated _libxl_types.c. instances of
KeyedUnion increment depth without outputing any code. The net result
is some cases where jso_sub_1 is followed by jso_sub_3. As an example:
_libxl_types.c
_libxl_types.c
@@ -5535,12 +5535,12 @@
if (!jso_sub_1)
goto out;
if (!libxl__string_is_default(&p->u.pty.path)) {
- json_object *jso_sub_2 = NULL;
- rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
+ json_object *jso_sub_3 = NULL;
+ rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
if (rc)
goto out;
- if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
- json_object_put(jso_sub_2);
+ if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
+ json_object_put(jso_sub_3);
goto out;
}
}
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Here's an alternative approach to workaround removeprefix.
tools/libs/light/gentypes.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
index 006bea170a..0e45c04f49 100644
--- a/tools/libs/light/gentypes.py
+++ b/tools/libs/light/gentypes.py
@@ -377,15 +377,16 @@ def get_default_expr(f, nparent, fexpr):
return "%s" % fexpr
# For json-c gen_json functions
-def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
+def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
s = ""
if parent is None:
s += "json_object *jso;\n"
s += "int rc;\n"
- sub_scope_object = "jso_sub_1"
+ depth = 1
else:
- sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
+ depth += 1
+ sub_scope_object = "jso_sub_%d" % depth
if isinstance(ty, idl.Array):
if parent is None:
raise Exception("Array type must have a parent")
@@ -398,7 +399,8 @@ def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "
s += " json_object *%s;\n" % (sub_scope_object)
# remove some indent, it's over indented at least in one case libxl_vcpu_sched_params_gen_json
s += libxl_C_type_gen_jso(ty.elem_type, v+"[i]",
- indent + " ", parent, sub_scope_object)
+ indent + " ", parent, sub_scope_object,
+ depth)
s += " if (json_object_array_add(%s, %s)) {\n" % (scope_object, sub_scope_object)
s += " json_object_put(%s);\n" % (sub_scope_object)
s += " goto out;\n"
@@ -417,7 +419,7 @@ def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "
(nparent,fexpr) = ty.member(v, f, parent is None)
s += "case %s:\n" % f.enumname
if f.type is not None:
- s += libxl_C_type_gen_jso(f.type, fexpr, indent + " ", nparent, scope_object)
+ s += libxl_C_type_gen_jso(f.type, fexpr, indent + " ", nparent, scope_object, depth)
else:
s += " %s = json_object_new_object();\n" % (scope_object)
s += " if (!%s)\n" % (scope_object)
@@ -433,7 +435,7 @@ def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "
default_expr = get_default_expr(f, nparent, fexpr)
s += "if (%s) {\n" % default_expr
s += " json_object *%s = NULL;\n" % (sub_scope_object)
- s += libxl_C_type_gen_jso(f.type, fexpr, " ", nparent, sub_scope_object)
+ s += libxl_C_type_gen_jso(f.type, fexpr, " ", nparent, sub_scope_object, depth)
s += libxl_C_type_gen_jso_map_key(f, nparent, " ", scope_object, sub_scope_object)
s += "}\n"
--
2.51.0On Thu, Oct 16, 2025 at 10:06:13PM -0400, Jason Andryuk wrote:
> removeprefix is only added in Python 3.9.
>
> Instead of the prefix removal, switch to passing in a "depth" parameter,
> and incrementing it for each level.
>
> There is a slight change in the generated _libxl_types.c. instances of
> KeyedUnion increment depth without outputing any code. The net result
> is some cases where jso_sub_1 is followed by jso_sub_3. As an example:
>
> _libxl_types.c
> _libxl_types.c
> @@ -5535,12 +5535,12 @@
> if (!jso_sub_1)
> goto out;
> if (!libxl__string_is_default(&p->u.pty.path)) {
> - json_object *jso_sub_2 = NULL;
> - rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
> + json_object *jso_sub_3 = NULL;
> + rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
> if (rc)
> goto out;
> - if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
> - json_object_put(jso_sub_2);
> + if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
> + json_object_put(jso_sub_3);
> goto out;
> }
> }
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Here's an alternative approach to workaround removeprefix.
Yeah, this version is less obscure about what's going on. Let's go for
it.
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
> tools/libs/light/gentypes.py | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
> index 006bea170a..0e45c04f49 100644
> --- a/tools/libs/light/gentypes.py
> +++ b/tools/libs/light/gentypes.py
> @@ -377,15 +377,16 @@ def get_default_expr(f, nparent, fexpr):
> return "%s" % fexpr
>
> # For json-c gen_json functions
> -def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
> +def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
> s = ""
> if parent is None:
> s += "json_object *jso;\n"
> s += "int rc;\n"
> - sub_scope_object = "jso_sub_1"
> + depth = 1
> else:
> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
> + depth += 1
We could simply do `depth += 1` regardless of the value of parent, it
would have the same effect, since depth start at 0. We just need to
create a new variable name that is different from the one created by the
callers (`depth=random.randrange(9999, 99999999)` actually works :-D,
but no guaranty of having different values). Anyway, the code is fine as
is, no need to make last minute edit.
Cheers,
--
Anthony PERARD
On 21/10/2025 5:23 pm, Anthony PERARD wrote:
> On Thu, Oct 16, 2025 at 10:06:13PM -0400, Jason Andryuk wrote:
>> removeprefix is only added in Python 3.9.
>>
>> Instead of the prefix removal, switch to passing in a "depth" parameter,
>> and incrementing it for each level.
>>
>> There is a slight change in the generated _libxl_types.c. instances of
>> KeyedUnion increment depth without outputing any code. The net result
>> is some cases where jso_sub_1 is followed by jso_sub_3. As an example:
>>
>> _libxl_types.c
>> _libxl_types.c
>> @@ -5535,12 +5535,12 @@
>> if (!jso_sub_1)
>> goto out;
>> if (!libxl__string_is_default(&p->u.pty.path)) {
>> - json_object *jso_sub_2 = NULL;
>> - rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
>> + json_object *jso_sub_3 = NULL;
>> + rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
>> if (rc)
>> goto out;
>> - if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
>> - json_object_put(jso_sub_2);
>> + if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
>> + json_object_put(jso_sub_3);
>> goto out;
>> }
>> }
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> Here's an alternative approach to workaround removeprefix.
> Yeah, this version is less obscure about what's going on. Let's go for
> it.
>
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Thanks. I'll take this version, and take the liberty of assuming that
the Release Ack is transferable to whichever solution the maintainers
prefer in the end.
>
>> tools/libs/light/gentypes.py | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
>> index 006bea170a..0e45c04f49 100644
>> --- a/tools/libs/light/gentypes.py
>> +++ b/tools/libs/light/gentypes.py
>> @@ -377,15 +377,16 @@ def get_default_expr(f, nparent, fexpr):
>> return "%s" % fexpr
>>
>> # For json-c gen_json functions
>> -def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
>> +def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
>> s = ""
>> if parent is None:
>> s += "json_object *jso;\n"
>> s += "int rc;\n"
>> - sub_scope_object = "jso_sub_1"
>> + depth = 1
>> else:
>> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
>> + depth += 1
> We could simply do `depth += 1` regardless of the value of parent, it
> would have the same effect, since depth start at 0.
That makes the code even more simple, because it takes out the else.
The net hunk is:
@@ -377,15 +377,14 @@ def get_default_expr(f, nparent, fexpr):
return "%s" % fexpr
# For json-c gen_json functions
-def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
+def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
s = ""
if parent is None:
s += "json_object *jso;\n"
s += "int rc;\n"
- sub_scope_object = "jso_sub_1"
- else:
- sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
+ depth += 1
+ sub_scope_object = "jso_sub_%d" % depth
if isinstance(ty, idl.Array):
if parent is None:
raise Exception("Array type must have a parent")
~Andrew
On 2025-10-21 12:31, Andrew Cooper wrote:
> On 21/10/2025 5:23 pm, Anthony PERARD wrote:
>> On Thu, Oct 16, 2025 at 10:06:13PM -0400, Jason Andryuk wrote:
>>> removeprefix is only added in Python 3.9.
>>>
>>> Instead of the prefix removal, switch to passing in a "depth" parameter,
>>> and incrementing it for each level.
>>>
>>> There is a slight change in the generated _libxl_types.c. instances of
>>> KeyedUnion increment depth without outputing any code. The net result
>>> is some cases where jso_sub_1 is followed by jso_sub_3. As an example:
>>>
>>> _libxl_types.c
>>> _libxl_types.c
>>> @@ -5535,12 +5535,12 @@
>>> if (!jso_sub_1)
>>> goto out;
>>> if (!libxl__string_is_default(&p->u.pty.path)) {
>>> - json_object *jso_sub_2 = NULL;
>>> - rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
>>> + json_object *jso_sub_3 = NULL;
>>> + rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
>>> if (rc)
>>> goto out;
>>> - if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
>>> - json_object_put(jso_sub_2);
>>> + if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
>>> + json_object_put(jso_sub_3);
>>> goto out;
>>> }
>>> }
>>>
>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>> ---
>>> Here's an alternative approach to workaround removeprefix.
>> Yeah, this version is less obscure about what's going on. Let's go for
>> it.
>>
>> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
>
> Thanks. I'll take this version, and take the liberty of assuming that
> the Release Ack is transferable to whichever solution the maintainers
> prefer in the end.
>
>>
>>> tools/libs/light/gentypes.py | 14 ++++++++------
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
>>> index 006bea170a..0e45c04f49 100644
>>> --- a/tools/libs/light/gentypes.py
>>> +++ b/tools/libs/light/gentypes.py
>>> @@ -377,15 +377,16 @@ def get_default_expr(f, nparent, fexpr):
>>> return "%s" % fexpr
>>>
>>> # For json-c gen_json functions
>>> -def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
>>> +def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
>>> s = ""
>>> if parent is None:
>>> s += "json_object *jso;\n"
>>> s += "int rc;\n"
>>> - sub_scope_object = "jso_sub_1"
>>> + depth = 1
>>> else:
>>> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
>>> + depth += 1
>> We could simply do `depth += 1` regardless of the value of parent, it
>> would have the same effect, since depth start at 0.
>
> That makes the code even more simple, because it takes out the else.
> The net hunk is:
>
> @@ -377,15 +377,14 @@ def get_default_expr(f, nparent, fexpr):
> return "%s" % fexpr
>
> # For json-c gen_json functions
> -def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso"):
> +def libxl_C_type_gen_jso(ty, v, indent = " ", parent = None, scope_object = "jso", depth = 0):
> s = ""
> if parent is None:
> s += "json_object *jso;\n"
> s += "int rc;\n"
> - sub_scope_object = "jso_sub_1"
> - else:
> - sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
>
> + depth += 1
> + sub_scope_object = "jso_sub_%d" % depth
> if isinstance(ty, idl.Array):
> if parent is None:
> raise Exception("Array type must have a parent")
This works for me, thanks - I even tried it at some point while tracking
down the jso_sub_2/3 difference from KeyedUnion. I should have gone
back to it :)
Regards,
Jason
On 17.10.2025 04:06, Jason Andryuk wrote:
> removeprefix is only added in Python 3.9.
>
> Instead of the prefix removal, switch to passing in a "depth" parameter,
> and incrementing it for each level.
>
> There is a slight change in the generated _libxl_types.c. instances of
> KeyedUnion increment depth without outputing any code. The net result
> is some cases where jso_sub_1 is followed by jso_sub_3. As an example:
>
> _libxl_types.c
> _libxl_types.c
> @@ -5535,12 +5535,12 @@
> if (!jso_sub_1)
> goto out;
> if (!libxl__string_is_default(&p->u.pty.path)) {
> - json_object *jso_sub_2 = NULL;
> - rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
> + json_object *jso_sub_3 = NULL;
> + rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
> if (rc)
> goto out;
> - if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
> - json_object_put(jso_sub_2);
> + if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
> + json_object_put(jso_sub_3);
> goto out;
> }
> }
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Here's an alternative approach to workaround removeprefix.
Fine with me; it's really Anthony's call. (He'll be back on Monday, aiui.)
Jan
© 2016 - 2025 Red Hat, Inc.