On Mon, Jul 24, 2023 at 06:26:42PM +0100, Anthony PERARD wrote:
> On Thu, Jul 20, 2023 at 10:25:37AM +0200, Roger Pau Monne wrote:
> > ---
> > It would be nice to rename the json output field to 'cpu_policy'
> > instead of 'cpuid', so that it looks like:
> >
> > "cpu_policy": {
> > "cpuid": [
> > {
> > "leaf": 7,
> > "subleaf": 0,
> > "edx": "xx1xxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
> > },
> > {
> > "leaf": 1,
> > "ebx": "xxxxxxxxxxxxxxxx00010000xxxxxxxx"
> > }
> > }
> > }
> > ],
> > "msr": [
> > {
> > "index": 266,
> > "policy":
> > "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx1xx1x1"
> > }
> > ]
> > },
> >
> > Sadly I have no idea how to do that, and can be done in a followup
> > change anyway.
>
> I don't think that would be possible. I think that would mean renaming
> the field in "struct libxl_domain_build_info", and changing a fields
> name seems complicated.
I did wonder whether it would be possible to change the json field
name without changing the libxl_domain_build_info field name, but that
would need a lot of special casing AFAICT.
> > diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
> > index 3c8b2a72c0b8..68b797886642 100644
> > --- a/tools/libs/light/libxl_cpuid.c
> > +++ b/tools/libs/light/libxl_cpuid.c
> > @@ -592,17 +641,24 @@ int libxl__cpuid_policy_list_parse_json(libxl__gc *gc,
> > {
> > int i, size;
> > struct xc_xend_cpuid *l;
> > + struct xc_msr *msr;
> > + const libxl__json_object *co;
> > flexarray_t *array;
> >
> > - if (!libxl__json_object_is_array(o))
> > + if (!libxl__json_object_is_map(o))
> > return ERROR_FAIL;
>
> We still need to be able to migrate a VM from a previous release of Xen,
> and we are going to have an array instead of a map. Could you try to
> handle both the old and new format of "cpuid"? If we don't then the
> scenario "migrate then reboot" is going to fail to use the expected cpu
> policy.
>
> > diff --git a/tools/libs/light/libxl_types.idl
> > b/tools/libs/light/libxl_types.idl
> > index 9e48bb772646..887824fdd828 100644
> > --- a/tools/libs/light/libxl_types.idl
> > +++ b/tools/libs/light/libxl_types.idl
> > @@ -19,7 +19,7 @@ libxl_mac = Builtin("mac", json_parse_type="JSON_STRING",
> > passby=PASS_BY_REFEREN
> > libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY",
> > dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE,
> > check_default_fn="libxl_bitmap_is_empty",
> > copy_fn="libxl_bitmap_copy_alloc")
> > libxl_cpuid_policy_list = Builtin("cpuid_policy_list",
> > dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE,
> > - json_parse_type="JSON_ARRAY",
> > check_default_fn="libxl__cpuid_policy_is_empty",
> > + json_parse_type="JSON_MAP",
> > check_default_fn="libxl__cpuid_policy_is_empty",
>
> Maybe we should have json_parse_type as either "JSON_ARRAY | JSON_MAP"
> or just "JSON_ANY" since nothing beside libxl is supposed to do
> something with it, and when migrating from a previous version, we are
> going to have an array.
Yeah, we need to use JSON_ANY and then guess the version by whether
the object is an array or a map.
That should be easy to arrange, let me prepare v4.
Thanks, Roger.
