[libvirt] [PATCH 10/10] xenconfig: add support for type="pvh"

Marek Marczykowski-Górecki posted 10 patches 6 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH 10/10] xenconfig: add support for type="pvh"
Posted by Marek Marczykowski-Górecki 6 years, 11 months ago
Handle PVH domain type in both directions (xen-xl->xml, xml->xen-xl).
And add a test for it.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Does domain_conf.c (virDomainDefFormatInternal) still need to silently
convert VIR_DOMAIN_OSTYPE_XEN to VIR_DOMAIN_OSTYPE_LINUX? In case of
PVH, VIR_DOMAIN_OSTYPE_LINUX was never reported as a valid option...
---
 src/xenconfig/xen_common.c           | 17 ++++++++++++-----
 src/xenconfig/xen_xl.c               |  5 +++++
 tests/testutilsxen.c                 |  3 ++-
 tests/xlconfigdata/test-pvh-type.cfg | 13 +++++++++++++
 tests/xlconfigdata/test-pvh-type.xml | 25 +++++++++++++++++++++++++
 tests/xlconfigtest.c                 |  1 +
 6 files changed, 58 insertions(+), 6 deletions(-)
 create mode 100644 tests/xlconfigdata/test-pvh-type.cfg
 create mode 100644 tests/xlconfigdata/test-pvh-type.xml

diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index c5d81d1..3c120a0 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -1081,6 +1081,7 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
     virCapsDomainDataPtr capsdata = NULL;
     const char *str;
     int hvm = 0, ret = -1;
+    const char *machine = "xenpv";
 
     if (xenConfigCopyString(conf, "name", &def->name) < 0)
         goto out;
@@ -1089,25 +1090,31 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
         goto out;
 
     if (xenConfigGetString(conf, "type", &str, NULL) == 0 && str) {
-        if (STREQ(str, "pv"))
+        if (STREQ(str, "pv")) {
             hvm = 0;
-        else if (STREQ(str, "hvm"))
+        } else if (STREQ(str, "pvh")) {
+            hvm = 0;
+            machine = "xenpvh";
+        } else if (STREQ(str, "hvm")) {
             hvm = 1;
-        else {
+            machine = "xenfv";
+        } else {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("type %s is not supported"), str);
             return -1;
         }
     } else {
         if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) &&
-            STREQ(str, "hvm"))
+            STREQ(str, "hvm")) {
             hvm = 1;
+            machine = "xenfv";
+        }
     }
 
     def->os.type = (hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN);
 
     if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
-            VIR_ARCH_NONE, def->virtType, NULL, NULL)))
+            VIR_ARCH_NONE, def->virtType, NULL, machine)))
         goto out;
 
     def->os.arch = capsdata->arch;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 19b6604..f1853bd 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -1285,6 +1285,11 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
 
         /* XXX floppy disks */
     } else {
+        if (STREQ(def->os.machine, "xenpvh")) {
+            if (xenConfigSetString(conf, "type", "pvh") < 0)
+                return -1;
+        }
+
         if (def->os.bootloader &&
              xenConfigSetString(conf, "bootloader", def->os.bootloader) < 0)
             return -1;
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index c08ec4a..04f8920 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -18,7 +18,8 @@ testXLInitCaps(void)
         "xenfv"
     };
     static const char *const xen_machines[] = {
-        "xenpv"
+        "xenpv",
+        "xenpvh"
     };
 
     if ((caps = virCapabilitiesNew(virArchFromHost(),
diff --git a/tests/xlconfigdata/test-pvh-type.cfg b/tests/xlconfigdata/test-pvh-type.cfg
new file mode 100644
index 0000000..2493535
--- /dev/null
+++ b/tests/xlconfigdata/test-pvh-type.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+type = "pvh"
+kernel = "/tmp/vmlinuz"
+ramdisk = "/tmp/initrd"
+cmdline = "root=/dev/xvda1 console=hvc0"
diff --git a/tests/xlconfigdata/test-pvh-type.xml b/tests/xlconfigdata/test-pvh-type.xml
new file mode 100644
index 0000000..96b0e7a
--- /dev/null
+++ b/tests/xlconfigdata/test-pvh-type.xml
@@ -0,0 +1,25 @@
+<domain type='xen'>
+  <name>XenGuest2</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='xenpvh'>linux</type>
+    <kernel>/tmp/vmlinuz</kernel>
+    <initrd>/tmp/initrd</initrd>
+    <cmdline>root=/dev/xvda1 console=hvc0</cmdline>
+  </os>
+  <clock offset='utc' adjustment='reset'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <console type='pty'>
+      <target type='xen' port='0'/>
+    </console>
+    <input type='mouse' bus='xen'/>
+    <input type='keyboard' bus='xen'/>
+    <memballoon model='xen'/>
+  </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index ad6a964..b9cf939 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -283,6 +283,7 @@ mymain(void)
     DO_TEST("rbd-multihost-noauth");
     DO_TEST_FORMAT("paravirt-type", false);
     DO_TEST_FORMAT("fullvirt-type", false);
+    DO_TEST("pvh-type");
 
 #ifdef LIBXL_HAVE_DEVICE_CHANNEL
     DO_TEST("channel-pty");
-- 
git-series 0.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] xenconfig: add support for type="pvh"
Posted by Jim Fehlig 6 years, 9 months ago
On 8/5/18 3:48 PM, Marek Marczykowski-Górecki wrote:
> Handle PVH domain type in both directions (xen-xl->xml, xml->xen-xl).
> And add a test for it.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Does domain_conf.c (virDomainDefFormatInternal) still need to silently
> convert VIR_DOMAIN_OSTYPE_XEN to VIR_DOMAIN_OSTYPE_LINUX? In case of
> PVH, VIR_DOMAIN_OSTYPE_LINUX was never reported as a valid option...

I'd prefer that we switch to formatting type as VIR_DOMAIN_OSTYPE_XEN and 
indicating PV vs PVH with the machine attribute. For back compat we'd need to 
continue accepting <type>linux</type> when parsing XML. Other than a lot of 
changes to test suite data files, am I overlooking compatibility problems with 
such a change?

> ---
>   src/xenconfig/xen_common.c           | 17 ++++++++++++-----
>   src/xenconfig/xen_xl.c               |  5 +++++
>   tests/testutilsxen.c                 |  3 ++-
>   tests/xlconfigdata/test-pvh-type.cfg | 13 +++++++++++++
>   tests/xlconfigdata/test-pvh-type.xml | 25 +++++++++++++++++++++++++
>   tests/xlconfigtest.c                 |  1 +
>   6 files changed, 58 insertions(+), 6 deletions(-)
>   create mode 100644 tests/xlconfigdata/test-pvh-type.cfg
>   create mode 100644 tests/xlconfigdata/test-pvh-type.xml
> 
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index c5d81d1..3c120a0 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -1081,6 +1081,7 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>       virCapsDomainDataPtr capsdata = NULL;
>       const char *str;
>       int hvm = 0, ret = -1;
> +    const char *machine = "xenpv";
>   
>       if (xenConfigCopyString(conf, "name", &def->name) < 0)
>           goto out;
> @@ -1089,25 +1090,31 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>           goto out;
>   
>       if (xenConfigGetString(conf, "type", &str, NULL) == 0 && str) {
> -        if (STREQ(str, "pv"))
> +        if (STREQ(str, "pv")) {
>               hvm = 0;
> -        else if (STREQ(str, "hvm"))
> +        } else if (STREQ(str, "pvh")) {
> +            hvm = 0;
> +            machine = "xenpvh";
> +        } else if (STREQ(str, "hvm")) {
>               hvm = 1;
> -        else {
> +            machine = "xenfv";
> +        } else {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                              _("type %s is not supported"), str);
>               return -1;
>           }

Ah, I see you fixed up the braces in this patch. Regardless, 'make syntax-check' 
should pass with each patch.

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Regards,
Jim

>       } else {
>           if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) &&
> -            STREQ(str, "hvm"))
> +            STREQ(str, "hvm")) {
>               hvm = 1;
> +            machine = "xenfv";
> +        }
>       }
>   
>       def->os.type = (hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN);
>   
>       if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
> -            VIR_ARCH_NONE, def->virtType, NULL, NULL)))
> +            VIR_ARCH_NONE, def->virtType, NULL, machine)))
>           goto out;
>   
>       def->os.arch = capsdata->arch;
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 19b6604..f1853bd 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -1285,6 +1285,11 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
>   
>           /* XXX floppy disks */
>       } else {
> +        if (STREQ(def->os.machine, "xenpvh")) {
> +            if (xenConfigSetString(conf, "type", "pvh") < 0)
> +                return -1;
> +        }
> +
>           if (def->os.bootloader &&
>                xenConfigSetString(conf, "bootloader", def->os.bootloader) < 0)
>               return -1;
> diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
> index c08ec4a..04f8920 100644
> --- a/tests/testutilsxen.c
> +++ b/tests/testutilsxen.c
> @@ -18,7 +18,8 @@ testXLInitCaps(void)
>           "xenfv"
>       };
>       static const char *const xen_machines[] = {
> -        "xenpv"
> +        "xenpv",
> +        "xenpvh"
>       };
>   
>       if ((caps = virCapabilitiesNew(virArchFromHost(),
> diff --git a/tests/xlconfigdata/test-pvh-type.cfg b/tests/xlconfigdata/test-pvh-type.cfg
> new file mode 100644
> index 0000000..2493535
> --- /dev/null
> +++ b/tests/xlconfigdata/test-pvh-type.cfg
> @@ -0,0 +1,13 @@
> +name = "XenGuest2"
> +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
> +maxmem = 579
> +memory = 394
> +vcpus = 1
> +localtime = 0
> +on_poweroff = "destroy"
> +on_reboot = "restart"
> +on_crash = "restart"
> +type = "pvh"
> +kernel = "/tmp/vmlinuz"
> +ramdisk = "/tmp/initrd"
> +cmdline = "root=/dev/xvda1 console=hvc0"
> diff --git a/tests/xlconfigdata/test-pvh-type.xml b/tests/xlconfigdata/test-pvh-type.xml
> new file mode 100644
> index 0000000..96b0e7a
> --- /dev/null
> +++ b/tests/xlconfigdata/test-pvh-type.xml
> @@ -0,0 +1,25 @@
> +<domain type='xen'>
> +  <name>XenGuest2</name>
> +  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>592896</memory>
> +  <currentMemory unit='KiB'>403456</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='xenpvh'>linux</type>
> +    <kernel>/tmp/vmlinuz</kernel>
> +    <initrd>/tmp/initrd</initrd>
> +    <cmdline>root=/dev/xvda1 console=hvc0</cmdline>
> +  </os>
> +  <clock offset='utc' adjustment='reset'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <console type='pty'>
> +      <target type='xen' port='0'/>
> +    </console>
> +    <input type='mouse' bus='xen'/>
> +    <input type='keyboard' bus='xen'/>
> +    <memballoon model='xen'/>
> +  </devices>
> +</domain>
> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
> index ad6a964..b9cf939 100644
> --- a/tests/xlconfigtest.c
> +++ b/tests/xlconfigtest.c
> @@ -283,6 +283,7 @@ mymain(void)
>       DO_TEST("rbd-multihost-noauth");
>       DO_TEST_FORMAT("paravirt-type", false);
>       DO_TEST_FORMAT("fullvirt-type", false);
> +    DO_TEST("pvh-type");
>   
>   #ifdef LIBXL_HAVE_DEVICE_CHANNEL
>       DO_TEST("channel-pty");
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] xenconfig: add support for type="pvh"
Posted by Marek Marczykowski-Górecki 6 years, 9 months ago
On Fri, Sep 14, 2018 at 05:21:17PM -0600, Jim Fehlig wrote:
> On 8/5/18 3:48 PM, Marek Marczykowski-Górecki wrote:
> > Handle PVH domain type in both directions (xen-xl->xml, xml->xen-xl).
> > And add a test for it.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> > Does domain_conf.c (virDomainDefFormatInternal) still need to silently
> > convert VIR_DOMAIN_OSTYPE_XEN to VIR_DOMAIN_OSTYPE_LINUX? In case of
> > PVH, VIR_DOMAIN_OSTYPE_LINUX was never reported as a valid option...
> 
> I'd prefer that we switch to formatting type as VIR_DOMAIN_OSTYPE_XEN

See discussion on patch 1/10...

> and
> indicating PV vs PVH with the machine attribute.

It's already here. 

> For back compat we'd need
> to continue accepting <type>linux</type> when parsing XML. Other than a lot
> of changes to test suite data files, am I overlooking compatibility problems
> with such a change?

Still accepting <type>linux</type> obviously must stay. But if we change
what is reported when formatting XML, it may cause:
 - unexpected configuration change, confusing to the user (no manual
   change, but XML is different)
 - that XML may not load on very old libvirt versions (I can't find
   exactly which one, but older than 0.7.2, which was almost 10 years
   ago)

Personally I don't think either of this is a problem.

> 
> > ---
> >   src/xenconfig/xen_common.c           | 17 ++++++++++++-----
> >   src/xenconfig/xen_xl.c               |  5 +++++
> >   tests/testutilsxen.c                 |  3 ++-
> >   tests/xlconfigdata/test-pvh-type.cfg | 13 +++++++++++++
> >   tests/xlconfigdata/test-pvh-type.xml | 25 +++++++++++++++++++++++++
> >   tests/xlconfigtest.c                 |  1 +
> >   6 files changed, 58 insertions(+), 6 deletions(-)
> >   create mode 100644 tests/xlconfigdata/test-pvh-type.cfg
> >   create mode 100644 tests/xlconfigdata/test-pvh-type.xml
> > 
> > diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> > index c5d81d1..3c120a0 100644
> > --- a/src/xenconfig/xen_common.c
> > +++ b/src/xenconfig/xen_common.c
> > @@ -1081,6 +1081,7 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> >       virCapsDomainDataPtr capsdata = NULL;
> >       const char *str;
> >       int hvm = 0, ret = -1;
> > +    const char *machine = "xenpv";
> >       if (xenConfigCopyString(conf, "name", &def->name) < 0)
> >           goto out;
> > @@ -1089,25 +1090,31 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
> >           goto out;
> >       if (xenConfigGetString(conf, "type", &str, NULL) == 0 && str) {
> > -        if (STREQ(str, "pv"))
> > +        if (STREQ(str, "pv")) {
> >               hvm = 0;
> > -        else if (STREQ(str, "hvm"))
> > +        } else if (STREQ(str, "pvh")) {
> > +            hvm = 0;
> > +            machine = "xenpvh";
> > +        } else if (STREQ(str, "hvm")) {
> >               hvm = 1;
> > -        else {
> > +            machine = "xenfv";
> > +        } else {
> >               virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >                              _("type %s is not supported"), str);
> >               return -1;
> >           }
> 
> Ah, I see you fixed up the braces in this patch. Regardless, 'make
> syntax-check' should pass with each patch.
> 
> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
> 
> Regards,
> Jim
> 
> >       } else {
> >           if ((xenConfigGetString(conf, "builder", &str, "linux") == 0) &&
> > -            STREQ(str, "hvm"))
> > +            STREQ(str, "hvm")) {
> >               hvm = 1;
> > +            machine = "xenfv";
> > +        }
> >       }
> >       def->os.type = (hvm ? VIR_DOMAIN_OSTYPE_HVM : VIR_DOMAIN_OSTYPE_XEN);
> >       if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
> > -            VIR_ARCH_NONE, def->virtType, NULL, NULL)))
> > +            VIR_ARCH_NONE, def->virtType, NULL, machine)))
> >           goto out;
> >       def->os.arch = capsdata->arch;
> > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> > index 19b6604..f1853bd 100644
> > --- a/src/xenconfig/xen_xl.c
> > +++ b/src/xenconfig/xen_xl.c
> > @@ -1285,6 +1285,11 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
> >           /* XXX floppy disks */
> >       } else {
> > +        if (STREQ(def->os.machine, "xenpvh")) {
> > +            if (xenConfigSetString(conf, "type", "pvh") < 0)
> > +                return -1;
> > +        }
> > +
> >           if (def->os.bootloader &&
> >                xenConfigSetString(conf, "bootloader", def->os.bootloader) < 0)
> >               return -1;
> > diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
> > index c08ec4a..04f8920 100644
> > --- a/tests/testutilsxen.c
> > +++ b/tests/testutilsxen.c
> > @@ -18,7 +18,8 @@ testXLInitCaps(void)
> >           "xenfv"
> >       };
> >       static const char *const xen_machines[] = {
> > -        "xenpv"
> > +        "xenpv",
> > +        "xenpvh"
> >       };
> >       if ((caps = virCapabilitiesNew(virArchFromHost(),
> > diff --git a/tests/xlconfigdata/test-pvh-type.cfg b/tests/xlconfigdata/test-pvh-type.cfg
> > new file mode 100644
> > index 0000000..2493535
> > --- /dev/null
> > +++ b/tests/xlconfigdata/test-pvh-type.cfg
> > @@ -0,0 +1,13 @@
> > +name = "XenGuest2"
> > +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
> > +maxmem = 579
> > +memory = 394
> > +vcpus = 1
> > +localtime = 0
> > +on_poweroff = "destroy"
> > +on_reboot = "restart"
> > +on_crash = "restart"
> > +type = "pvh"
> > +kernel = "/tmp/vmlinuz"
> > +ramdisk = "/tmp/initrd"
> > +cmdline = "root=/dev/xvda1 console=hvc0"
> > diff --git a/tests/xlconfigdata/test-pvh-type.xml b/tests/xlconfigdata/test-pvh-type.xml
> > new file mode 100644
> > index 0000000..96b0e7a
> > --- /dev/null
> > +++ b/tests/xlconfigdata/test-pvh-type.xml
> > @@ -0,0 +1,25 @@
> > +<domain type='xen'>
> > +  <name>XenGuest2</name>
> > +  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
> > +  <memory unit='KiB'>592896</memory>
> > +  <currentMemory unit='KiB'>403456</currentMemory>
> > +  <vcpu placement='static'>1</vcpu>
> > +  <os>
> > +    <type arch='x86_64' machine='xenpvh'>linux</type>
> > +    <kernel>/tmp/vmlinuz</kernel>
> > +    <initrd>/tmp/initrd</initrd>
> > +    <cmdline>root=/dev/xvda1 console=hvc0</cmdline>
> > +  </os>
> > +  <clock offset='utc' adjustment='reset'/>
> > +  <on_poweroff>destroy</on_poweroff>
> > +  <on_reboot>restart</on_reboot>
> > +  <on_crash>restart</on_crash>
> > +  <devices>
> > +    <console type='pty'>
> > +      <target type='xen' port='0'/>
> > +    </console>
> > +    <input type='mouse' bus='xen'/>
> > +    <input type='keyboard' bus='xen'/>
> > +    <memballoon model='xen'/>
> > +  </devices>
> > +</domain>
> > diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
> > index ad6a964..b9cf939 100644
> > --- a/tests/xlconfigtest.c
> > +++ b/tests/xlconfigtest.c
> > @@ -283,6 +283,7 @@ mymain(void)
> >       DO_TEST("rbd-multihost-noauth");
> >       DO_TEST_FORMAT("paravirt-type", false);
> >       DO_TEST_FORMAT("fullvirt-type", false);
> > +    DO_TEST("pvh-type");
> >   #ifdef LIBXL_HAVE_DEVICE_CHANNEL
> >       DO_TEST("channel-pty");
> > 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] xenconfig: add support for type="pvh"
Posted by Jim Fehlig 6 years, 9 months ago
On 9/14/18 5:41 PM, Marek Marczykowski-Górecki wrote:
> On Fri, Sep 14, 2018 at 05:21:17PM -0600, Jim Fehlig wrote:
>> On 8/5/18 3:48 PM, Marek Marczykowski-Górecki wrote:
>>> Handle PVH domain type in both directions (xen-xl->xml, xml->xen-xl).
>>> And add a test for it.
>>>
>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>> ---
>>> Does domain_conf.c (virDomainDefFormatInternal) still need to silently
>>> convert VIR_DOMAIN_OSTYPE_XEN to VIR_DOMAIN_OSTYPE_LINUX? In case of
>>> PVH, VIR_DOMAIN_OSTYPE_LINUX was never reported as a valid option...
>>
>> I'd prefer that we switch to formatting type as VIR_DOMAIN_OSTYPE_XEN
> 
> See discussion on patch 1/10...

Opps, I thought I had already replied to this but apparently not. Let me try 
again...

I guess it is possible to extend the hack in virDomainDefFormatInternal to also 
check for def->os.machine == "xenpv" before converting to VIR_DOMAIN_OSTYPE_LINUX.

>> and
>> indicating PV vs PVH with the machine attribute.
> 
> It's already here.

Yes, I know :-).

> 
>> For back compat we'd need
>> to continue accepting <type>linux</type> when parsing XML. Other than a lot
>> of changes to test suite data files, am I overlooking compatibility problems
>> with such a change?
> 
> Still accepting <type>linux</type> obviously must stay. But if we change
> what is reported when formatting XML, it may cause:
>   - unexpected configuration change, confusing to the user (no manual
>     change, but XML is different)

And I think this is the reason for danpb's objection.

>   - that XML may not load on very old libvirt versions (I can't find
>     exactly which one, but older than 0.7.2, which was almost 10 years
>     ago)
> 
> Personally I don't think either of this is a problem.

I agree that libvirt producing different guest XML after an update is not 
appealing. But I do think we can extend the existing hack in 
virDomainDefFormatInternal.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list