[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,

On 10/01/2018 05:37 PM, Roger Pau Monné wrote:
On Mon, Oct 01, 2018 at 04:33:45PM +0100, Julien Grall wrote:

On 10/01/2018 04:27 PM, Roger Pau Monné wrote:
On Tue, Sep 25, 2018 at 08:36:36PM +0100, Julien Grall wrote:
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

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

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,

Those can be left inside the PV sub-struct and shouldn't be marked as

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?

If we go down this route for ARM I think you should add to xl.cfg that
the `type` option only applies to x86, and that there's only one guest
type on ARM. IMO it's best to not use type if there's only one type
available, so that it can be intruded later on ARM if required.

That's not what we discussed before. The idea is to default Arm guest to PVH
for xl. But "type=" would still be available.

We still have to support other type as other toolstack (e.g libvirt) may use
PV for Arm. So the idea here is libxl will check if it was PV and convert to

In an earlier reply, you said that we can use deprecated to copy most of the
options over. However, can we always assume the pv.* fields will always be
zero after the value was copied over? If not, then we would have to memset

Yes, the field is cleared/disposed after the value is copied over, see
libxl_C_type_copy_deprecated in gentypes.py.

That's answer part of my question :). AFAIU, nothing promise that a default value will always be 0 (see libxl_devid). I am concerned that someone may decide to add such field in either PV or PVH in the future.

Anyway, I think I find a way to address that, I will resend a new version later today.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.