Skip to contentSkip to conversation

libxl/arm: fix guest type conversion

PATCH Superseded Wei Liu 2 participants 6 messages Oct 31, 2018 – Nov 1, 2018

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Wei Liu

author
#0 lore raw

PATCH 1/1 libxl/arm: fix guest type conversion

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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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(+)
tools/libxl/libxl_create.c +10 -0
index 4bb750e951..311957f87c 100644
@@ -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);
Signature · 7 lines
2.11.0


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#1 lore raw
Quoted 11 lines · 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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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.

Quoted 30 lines

This should fix libvirt breakage.

---
 tools/libxl/libxl_create.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/libxl/libxl_create.c +10 -0
index 4bb750e951..311957f87c 100644
@@ -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);
Signature · 1 line
2.11.0

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Tested-by on v1 1/1
#2 lore raw
In reply to Wei Liu

Hi Wei,

Quoted 11 lines · 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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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 <[email protected]>
Quoted 11 lines

This should fix libvirt breakage.

---
  tools/libxl/libxl_create.c | 10 ++++++++++
  1 file changed, 10 insertions(+)
tools/libxl/libxl_create.c +0 -0
index 4bb750e951..311957f87c 100644

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc, · line 42

Quoted 8 lines
tools/libxl/libxl_create.c +5 -0
@@ -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?

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc, · line 50

Quoted 9 lines
tools/libxl/libxl_create.c +5 -0
+
+        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,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#3 lore raw
Quoted 25 lines · Julien Grall

Hi Wei,

Quoted 11 lines · 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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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 <[email protected]>
Quoted 6 lines

This should fix libvirt breakage.

---
  tools/libxl/libxl_create.c | 10 ++++++++++
  1 file changed, 10 insertions(+)
Quoted 14 lines

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc, · line 41

Quoted 12 lines
tools/libxl/libxl_create.c +5 -0
index 4bb750e951..311957f87c 100644
@@ -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.

Quoted 3 lines

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.

Quoted 15 lines

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc,

Quoted 10 lines
tools/libxl/libxl_create.c +5 -0
+
+        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,

Signature · 1 line
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

#4 lore raw

Hi Wei,

Quoted 41 lines · Wei Liu
Quoted 25 lines · Julien Grall

Hi Wei,

Quoted 11 lines · 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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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 <[email protected]>
Quoted 6 lines

This should fix libvirt breakage.

---
   tools/libxl/libxl_create.c | 10 ++++++++++
   1 file changed, 10 insertions(+)
Quoted 14 lines

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc, · line 41

Quoted 12 lines
tools/libxl/libxl_create.c +5 -0
index 4bb750e951..311957f87c 100644
@@ -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.

Quoted 1 line

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.

Quoted 12 lines
Quoted 4 lines

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.

Quoted 2 lines

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,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#5 lore raw
Quoted 44 lines · Julien Grall

Hi Wei,

Quoted 41 lines · Wei Liu
Quoted 39 lines · Julien Grall

Hi Wei,

Quoted 11 lines · 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 <[email protected]>

---
Cc: Stefano Stabellini <[email protected]>
Cc: Julien Grall <[email protected]>

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 <[email protected]>
Quoted 11 lines

This should fix libvirt breakage.

---
   tools/libxl/libxl_create.c | 10 ++++++++++
   1 file changed, 10 insertions(+)
tools/libxl/libxl_create.c +0 -0
index 4bb750e951..311957f87c 100644

tools/libxl/libxl_create.c int libxl__domain_create_info_setdefault(libxl__gc *gc, · line 41

Quoted 7 lines
tools/libxl/libxl_create.c +5 -0
@@ -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.

Quoted 3 lines
Quoted 1 line

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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel