[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] tools/libxl: Switch Arm guest type to PVH
Hi Roger, Sorry for the late reply. On 09/03/2018 03:40 PM, Roger Pau Monné wrote: On Mon, Sep 03, 2018 at 12:15:16PM +0100, Julien Grall wrote:On 03/09/18 12:09, Julien Grall wrote:On 23/08/18 08:58, Roger Pau Monné wrote:On Wed, Aug 22, 2018 at 06:48:05PM +0100, Julien Grall wrote:+ + b_info->type = LIBXL_DOMAIN_TYPE_PVH; + + /* + * They only fie ld in u.pv that matters on Arm are: kernel, cmdline, + * ramdisk. + */ + + if (!b_info->kernel && b_info->u.pv.kernel) + b_info->kernel = b_info->u.pv.kernel; + + if (!b_info->ramdisk && b_info->u.pv.ramdisk) + b_info->ramdisk = b_info->u.pv.ramdisk; + + if (!b_info->cmdline && b_info->u.pv.cmdline) + b_info->cmdline = b_info->u.pv.cmdline; + + /* Reset b_info->u.pvh to default values */ + memset(&b_info->u.pvh, 0, sizeof(b_info->u.pvh));I'm afraid that's not correct. The default values for u.pvh are set by libxl__domain_build_info_setdefault.I thought that this should be covered by the switch right after the call of libxl__arch_domain_build_info_setdefault. Did I miss anything?Oh right, libxl__arch_domain_build_info_setdefault is called by libxl__domain_build_info_setdefault.What I wanted to do here is resetting the union to 0 so you don't get data mangled by the pv fields.Another possible option I think would be to mark those fields as deprecated in the IDL, and libxl__domain_build_info_copy_deprecated will take care of copying them to the new place. In fact I think all guest types should be using the top level kernel, ramdisk and cmdline fields.I will have a look at it.I'm not specially comfortable with changing the guest type in the middle of libxl__domain_build_info_setdefault, but I also don't have a much better suggestion apart from using the deprecation helper.I forgot to answer to this bit. I don't think the deprecation helper will do all the jobs. There are still PV specific parameters: slack_memkb, features, e820_host.Those can be left inside the PV sub-struct and shouldn't be marked as deprecated.Those are not necessary for Arm, if you don't zero them then you will not initialize the PVH structure with default values. How do you suggest to handle them?But I guess ARM doesn't use any of those fields (or else they should be moved to the pvh sub-struct anyway)? Those fields should not be used by Arm. Looking at the current fields in pv, they all should be zeroed. However, I am not sure if we can assume that will always be the case.If we can assume it, then I think it would just be sufficient to have b_info->type = LIBXL_DOMAIN_TYPE_PVH in Arm. Any thoughts? Cheers, In which case allowing the user to set them in the first place seems like an error to me. Agreed. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |