All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl/arm: fix guest type conversion
@ 2018-10-31 14:25 Wei Liu
  2018-10-31 14:51 ` Wei Liu
  2018-10-31 20:16 ` Julien Grall
  0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2018-10-31 14:25 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu

Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
changing the type field in c_info. This issue didn't surface until
ef72c93df9 which made creating PV guest on Arm unusable.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

Julien, can you give this a quick test? Put type='pv' in your Arm
guest xl cfg and note the difference before and after this patch.

This should fix libvirt breakage.
---
 tools/libxl/libxl_create.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4bb750e951..311957f87c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
         return ERROR_INVAL;
     }
 
+#if defined(__arm__) || defined(__aarch64__)
+    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
+        LOG(WARN, "Converting PV guest to PVH.");
+        LOG(WARN, "Arm guest are now PVH.");
+        LOG(WARN, "Please fix your configuration file/toolstack.");
+
+        c_info->type = LIBXL_DOMAIN_TYPE_PVH;
+    }
+#endif
+
     if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
         libxl_defbool_setdefault(&c_info->hap, true);
         libxl_defbool_setdefault(&c_info->oos, true);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/arm: fix guest type conversion
  2018-10-31 14:25 [PATCH] libxl/arm: fix guest type conversion Wei Liu
@ 2018-10-31 14:51 ` Wei Liu
  2018-10-31 20:16 ` Julien Grall
  1 sibling, 0 replies; 6+ messages in thread
From: Wei Liu @ 2018-10-31 14:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu

On Wed, Oct 31, 2018 at 02:25:45PM +0000, Wei Liu wrote:
> Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
> changing the type field in c_info. This issue didn't surface until
> ef72c93df9 which made creating PV guest on Arm unusable.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> 
> Julien, can you give this a quick test? Put type='pv' in your Arm
> guest xl cfg and note the difference before and after this patch.

I also managed to test this patch on an arndale. It worked.

> 
> This should fix libvirt breakage.
> ---
>  tools/libxl/libxl_create.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 4bb750e951..311957f87c 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>          return ERROR_INVAL;
>      }
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
> +        LOG(WARN, "Converting PV guest to PVH.");
> +        LOG(WARN, "Arm guest are now PVH.");
> +        LOG(WARN, "Please fix your configuration file/toolstack.");
> +
> +        c_info->type = LIBXL_DOMAIN_TYPE_PVH;
> +    }
> +#endif
> +
>      if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
>          libxl_defbool_setdefault(&c_info->hap, true);
>          libxl_defbool_setdefault(&c_info->oos, true);
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/arm: fix guest type conversion
  2018-10-31 14:25 [PATCH] libxl/arm: fix guest type conversion Wei Liu
  2018-10-31 14:51 ` Wei Liu
@ 2018-10-31 20:16 ` Julien Grall
  2018-11-01  8:59   ` Wei Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2018-10-31 20:16 UTC (permalink / raw)
  To: Wei Liu, xen-devel; +Cc: Stefano Stabellini, Ian Jackson

Hi Wei,

On 10/31/18 2:25 PM, Wei Liu wrote:
> Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
> changing the type field in c_info. This issue didn't surface until
> ef72c93df9 which made creating PV guest on Arm unusable.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> 
> Julien, can you give this a quick test? Put type='pv' in your Arm
> guest xl cfg and note the difference before and after this patch.

I tested on arm64 with and without the patch:

Tested-by: Julien Grall <julien.grall@arm.com>

> 
> This should fix libvirt breakage.
> ---
>   tools/libxl/libxl_create.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 4bb750e951..311957f87c 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>           return ERROR_INVAL;
>       }
>   
> +#if defined(__arm__) || defined(__aarch64__)
> +    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
> +        LOG(WARN, "Converting PV guest to PVH.");
> +        LOG(WARN, "Arm guest are now PVH.");
> +        LOG(WARN, "Please fix your configuration file/toolstack.");

We now end up to have this message printed twice. Do you see a use case 
where c_info->type is different from b_info->type? Do we need a check 
for that somewhere in the code?

> +
> +        c_info->type = LIBXL_DOMAIN_TYPE_PVH;
> +    }
> +#endif
> +
>       if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
>           libxl_defbool_setdefault(&c_info->hap, true);
>           libxl_defbool_setdefault(&c_info->oos, true);
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/arm: fix guest type conversion
  2018-10-31 20:16 ` Julien Grall
@ 2018-11-01  8:59   ` Wei Liu
  2018-11-01 10:26     ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2018-11-01  8:59 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Wei Liu, Ian Jackson

On Wed, Oct 31, 2018 at 08:16:52PM +0000, Julien Grall wrote:
> Hi Wei,
> 
> On 10/31/18 2:25 PM, Wei Liu wrote:
> > Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
> > changing the type field in c_info. This issue didn't surface until
> > ef72c93df9 which made creating PV guest on Arm unusable.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Julien Grall <julien.grall@arm.com>
> > 
> > Julien, can you give this a quick test? Put type='pv' in your Arm
> > guest xl cfg and note the difference before and after this patch.
> 
> I tested on arm64 with and without the patch:
> 
> Tested-by: Julien Grall <julien.grall@arm.com>
> 
> > 
> > This should fix libvirt breakage.
> > ---
> >   tools/libxl/libxl_create.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 4bb750e951..311957f87c 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
> >           return ERROR_INVAL;
> >       }
> > +#if defined(__arm__) || defined(__aarch64__)
> > +    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
> > +        LOG(WARN, "Converting PV guest to PVH.");
> > +        LOG(WARN, "Arm guest are now PVH.");
> > +        LOG(WARN, "Please fix your configuration file/toolstack.");
> 
> We now end up to have this message printed twice.

I thought we need some logging so I copied them here.

> Do you see a use case
> where c_info->type is different from b_info->type? Do we need a check for
> that somewhere in the code?

No, I don't think there will be cases where c_info->type is different
from b_info->type. The type in b_info is used as a key to the keyed
union. The normal way of using it is to call
libxl_domain_build_info_init_type with c_info->type (see
xl_parse.c:L1316).  I tried to follow that path as well, but the
plumbing became unwieldy.

Regarding adding a check, that's probably a good idea, but it is out of
scope of this patch.

Wei.

> 
> > +
> > +        c_info->type = LIBXL_DOMAIN_TYPE_PVH;
> > +    }
> > +#endif
> > +
> >       if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
> >           libxl_defbool_setdefault(&c_info->hap, true);
> >           libxl_defbool_setdefault(&c_info->oos, true);
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/arm: fix guest type conversion
  2018-11-01  8:59   ` Wei Liu
@ 2018-11-01 10:26     ` Julien Grall
  2018-11-01 10:38       ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2018-11-01 10:26 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Stefano Stabellini, Ian Jackson

Hi Wei,

On 01/11/2018 08:59, Wei Liu wrote:
> On Wed, Oct 31, 2018 at 08:16:52PM +0000, Julien Grall wrote:
>> Hi Wei,
>>
>> On 10/31/18 2:25 PM, Wei Liu wrote:
>>> Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
>>> changing the type field in c_info. This issue didn't surface until
>>> ef72c93df9 which made creating PV guest on Arm unusable.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Cc: Julien Grall <julien.grall@arm.com>
>>>
>>> Julien, can you give this a quick test? Put type='pv' in your Arm
>>> guest xl cfg and note the difference before and after this patch.
>>
>> I tested on arm64 with and without the patch:
>>
>> Tested-by: Julien Grall <julien.grall@arm.com>
>>
>>>
>>> This should fix libvirt breakage.
>>> ---
>>>    tools/libxl/libxl_create.c | 10 ++++++++++
>>>    1 file changed, 10 insertions(+)
>>>
>>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>>> index 4bb750e951..311957f87c 100644
>>> --- a/tools/libxl/libxl_create.c
>>> +++ b/tools/libxl/libxl_create.c
>>> @@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>>>            return ERROR_INVAL;
>>>        }
>>> +#if defined(__arm__) || defined(__aarch64__)
>>> +    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
>>> +        LOG(WARN, "Converting PV guest to PVH.");
>>> +        LOG(WARN, "Arm guest are now PVH.");
>>> +        LOG(WARN, "Please fix your configuration file/toolstack.");
>>
>> We now end up to have this message printed twice.
> 
> I thought we need some logging so I copied them here.

I think we can remove this logging if we rely on c_info->type == b_info->type.

> 
>> Do you see a use case
>> where c_info->type is different from b_info->type? Do we need a check for
>> that somewhere in the code?
> 
> No, I don't think there will be cases where c_info->type is different
> from b_info->type. The type in b_info is used as a key to the keyed
> union. The normal way of using it is to call
> libxl_domain_build_info_init_type with c_info->type (see
> xl_parse.c:L1316).  I tried to follow that path as well, but the
> plumbing became unwieldy.
> 
> Regarding adding a check, that's probably a good idea, but it is out of
> scope of this patch.

I wasn't suggested it for this patch :).

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/arm: fix guest type conversion
  2018-11-01 10:26     ` Julien Grall
@ 2018-11-01 10:38       ` Wei Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2018-11-01 10:38 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Wei Liu, Ian Jackson

On Thu, Nov 01, 2018 at 10:26:16AM +0000, Julien Grall wrote:
> Hi Wei,
> 
> On 01/11/2018 08:59, Wei Liu wrote:
> > On Wed, Oct 31, 2018 at 08:16:52PM +0000, Julien Grall wrote:
> > > Hi Wei,
> > > 
> > > On 10/31/18 2:25 PM, Wei Liu wrote:
> > > > Commit 359970fd8b ("tools/libxl: Switch Arm guest type to PVH") missed
> > > > changing the type field in c_info. This issue didn't surface until
> > > > ef72c93df9 which made creating PV guest on Arm unusable.
> > > > 
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > ---
> > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > Cc: Julien Grall <julien.grall@arm.com>
> > > > 
> > > > Julien, can you give this a quick test? Put type='pv' in your Arm
> > > > guest xl cfg and note the difference before and after this patch.
> > > 
> > > I tested on arm64 with and without the patch:
> > > 
> > > Tested-by: Julien Grall <julien.grall@arm.com>
> > > 
> > > > 
> > > > This should fix libvirt breakage.
> > > > ---
> > > >    tools/libxl/libxl_create.c | 10 ++++++++++
> > > >    1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > > > index 4bb750e951..311957f87c 100644
> > > > --- a/tools/libxl/libxl_create.c
> > > > +++ b/tools/libxl/libxl_create.c
> > > > @@ -35,6 +35,16 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
> > > >            return ERROR_INVAL;
> > > >        }
> > > > +#if defined(__arm__) || defined(__aarch64__)
> > > > +    if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
> > > > +        LOG(WARN, "Converting PV guest to PVH.");
> > > > +        LOG(WARN, "Arm guest are now PVH.");
> > > > +        LOG(WARN, "Please fix your configuration file/toolstack.");
> > > 
> > > We now end up to have this message printed twice.
> > 
> > I thought we need some logging so I copied them here.
> 
> I think we can remove this logging if we rely on c_info->type == b_info->type.

I will change the logging in libxl_arm.c.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-01 10:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 14:25 [PATCH] libxl/arm: fix guest type conversion Wei Liu
2018-10-31 14:51 ` Wei Liu
2018-10-31 20:16 ` Julien Grall
2018-11-01  8:59   ` Wei Liu
2018-11-01 10:26     ` Julien Grall
2018-11-01 10:38       ` Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.