We're now able to boot a simple PowerNV8 domain in Libvirt.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 tests/qemuxml2argvdata/powernv8-basic.args  | 31 +++++++++++++++++++++
 tests/qemuxml2argvdata/powernv8-basic.xml   | 16 +++++++++++
 tests/qemuxml2argvtest.c                    |  3 ++
 tests/qemuxml2xmloutdata/powernv8-basic.xml | 28 +++++++++++++++++++
 tests/qemuxml2xmltest.c                     |  3 ++
 tests/testutilsqemu.c                       |  2 +-
 6 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/powernv8-basic.args
 create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml
 create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.xml

diff --git a/tests/qemuxml2argvdata/powernv8-basic.args b/tests/qemuxml2argvdata/powernv8-basic.args
new file mode 100644
index 0000000000..822cf62bdf
--- /dev/null
+++ b/tests/qemuxml2argvdata/powernv8-basic.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine powernv8,usb=off,dump-guest-core=off \
+-accel tcg \
+-m 2048 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid b20fcfe3-4a0a-4039-8735-9e024256e0f7 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-usb \
+-chardev pty,id=charserial0 \
+-device isa-serial,chardev=charserial0,id=serial0,index=0 \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml
new file mode 100644
index 0000000000..a92fc1282c
--- /dev/null
+++ b/tests/qemuxml2argvdata/powernv8-basic.xml
@@ -0,0 +1,16 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='ppc64' machine='powernv8'>hvm</type>
+  </os>
+  <devices>
+    <emulator>/usr/bin/qemu-system-ppc64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <console type='pty'>
+      <target type='serial' port='0'/>
+    </console>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9d2de2a569..7fcfcc83f1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2217,6 +2217,9 @@ mymain(void)
     DO_TEST_PARSE_ERROR_NOCAPS("seclabel-multiple");
     DO_TEST_PARSE_ERROR_NOCAPS("seclabel-device-duplicates");
 
+    DO_TEST("powernv8-basic",
+            QEMU_CAPS_DEVICE_ISA_SERIAL);
+
     DO_TEST("pseries-basic",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_SPAPR_VTY);
diff --git a/tests/qemuxml2xmloutdata/powernv8-basic.xml b/tests/qemuxml2xmloutdata/powernv8-basic.xml
new file mode 100644
index 0000000000..7b798b979d
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/powernv8-basic.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='ppc64' machine='powernv8'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-ppc64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <serial type='pty'>
+      <target type='isa-serial' port='0'>
+        <model name='isa-serial'/>
+      </target>
+    </serial>
+    <console type='pty'>
+      <target type='serial' port='0'/>
+    </console>
+    <audio id='1' type='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ce1cac31c8..5a19fdbda5 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -637,6 +637,9 @@ mymain(void)
             QEMU_CAPS_OBJECT_RNG_EGD);
     DO_TEST_CAPS_LATEST("virtio-rng-builtin");
 
+    DO_TEST("powernv8-basic",
+            QEMU_CAPS_DEVICE_ISA_SERIAL);
+
     DO_TEST("pseries-nvram",
             QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
             QEMU_CAPS_DEVICE_NVRAM);
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index cb665e501b..5d29125ffa 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -54,7 +54,7 @@ static const char *const arm_machines[] = {
     "vexpress-a9", "virt", NULL
 };
 static const char *const ppc64_machines[] = {
-    "pseries", NULL
+    "pseries", "powernv8", NULL
 };
 static const char *const ppc_machines[] = {
     "ppce500", NULL
-- 
2.34.1

On Thu, Jan 20, 2022 at 10:52:11 -0300, Daniel Henrique Barboza wrote:
> We're now able to boot a simple PowerNV8 domain in Libvirt.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  tests/qemuxml2argvdata/powernv8-basic.args  | 31 +++++++++++++++++++++
>  tests/qemuxml2argvdata/powernv8-basic.xml   | 16 +++++++++++
>  tests/qemuxml2argvtest.c                    |  3 ++
>  tests/qemuxml2xmloutdata/powernv8-basic.xml | 28 +++++++++++++++++++
>  tests/qemuxml2xmltest.c                     |  3 ++
>  tests/testutilsqemu.c                       |  2 +-
>  6 files changed, 82 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/powernv8-basic.args
>  create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml
>  create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.xml

[...]

> diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml
> new file mode 100644
> index 0000000000..a92fc1282c
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/powernv8-basic.xml
> @@ -0,0 +1,16 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='ppc64' machine='powernv8'>hvm</type>
> +  </os>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-ppc64</emulator>
> +    <controller type='pci' index='0' model='pcie-root'/>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +  </devices>
> +</domain>

Trying to start this config after this series is applied fails with:

error: internal error: process exited while connecting to monitor: 2022-01-21T14:04:54.119602Z qemu-system-ppc64: -device pnv-phb3,index=0,chip-id=0,id=pcie.0: Parameter 'driver' expects a pluggable device type

qemu-system-ppc-core-6.2.0-2.fc35.x86_64

> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 9d2de2a569..7fcfcc83f1 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2217,6 +2217,9 @@ mymain(void)
>      DO_TEST_PARSE_ERROR_NOCAPS("seclabel-multiple");
>      DO_TEST_PARSE_ERROR_NOCAPS("seclabel-device-duplicates");
>  
> +    DO_TEST("powernv8-basic",
> +            QEMU_CAPS_DEVICE_ISA_SERIAL);

All new test cases shoud use DO_TEST_CAPS_ARCH_LATEST instead of fake
capabilities.

> +
>      DO_TEST("pseries-basic",
>              QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>              QEMU_CAPS_DEVICE_SPAPR_VTY);


[...]


> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index cb665e501b..5d29125ffa 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -54,7 +54,7 @@ static const char *const arm_machines[] = {
>      "vexpress-a9", "virt", NULL
>  };
>  static const char *const ppc64_machines[] = {
> -    "pseries", NULL
> +    "pseries", "powernv8", NULL

NACK to this hunk. Use real capabilities instead.

>  };
>  static const char *const ppc_machines[] = {
>      "ppce500", NULL
> -- 
> 2.34.1
> 

On 1/21/22 11:08, Peter Krempa wrote:
> On Thu, Jan 20, 2022 at 10:52:11 -0300, Daniel Henrique Barboza wrote:
>> We're now able to boot a simple PowerNV8 domain in Libvirt.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   tests/qemuxml2argvdata/powernv8-basic.args  | 31 +++++++++++++++++++++
>>   tests/qemuxml2argvdata/powernv8-basic.xml   | 16 +++++++++++
>>   tests/qemuxml2argvtest.c                    |  3 ++
>>   tests/qemuxml2xmloutdata/powernv8-basic.xml | 28 +++++++++++++++++++
>>   tests/qemuxml2xmltest.c                     |  3 ++
>>   tests/testutilsqemu.c                       |  2 +-
>>   6 files changed, 82 insertions(+), 1 deletion(-)
>>   create mode 100644 tests/qemuxml2argvdata/powernv8-basic.args
>>   create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.xml
> 
> [...]
> 
>> diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml
>> new file mode 100644
>> index 0000000000..a92fc1282c
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/powernv8-basic.xml
>> @@ -0,0 +1,16 @@
>> +<domain type='qemu'>
>> +  <name>QEMUGuest1</name>
>> +  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
>> +  <memory unit='KiB'>2097152</memory>
>> +  <vcpu placement='static'>1</vcpu>
>> +  <os>
>> +    <type arch='ppc64' machine='powernv8'>hvm</type>
>> +  </os>
>> +  <devices>
>> +    <emulator>/usr/bin/qemu-system-ppc64</emulator>
>> +    <controller type='pci' index='0' model='pcie-root'/>
>> +    <console type='pty'>
>> +      <target type='serial' port='0'/>
>> +    </console>
>> +  </devices>
>> +</domain>
> 
> Trying to start this config after this series is applied fails with:
> 
> error: internal error: process exited while connecting to monitor: 2022-01-21T14:04:54.119602Z qemu-system-ppc64: -device pnv-phb3,index=0,chip-id=0,id=pcie.0: Parameter 'driver' expects a pluggable device type
> 
> qemu-system-ppc-core-6.2.0-2.fc35.x86_64


You'll need QEMU upstream to test it. QEMU 6.2.0 doesn't allow the PHBs to be
user created. I think I mentioned in the cover letter that I didn't find a good
way to block Libvirt users from doing exactly as you attempted to do because the
pnv-phb3 is presented, but it's unusable.

The places where we bother checking QEMU version is on qemu_process.c when creating
a log in qemuLogOperation() and in qemu_driver.c in qemuConnectGetVersion. I didn't
find any other case where Libvirt checks for QEMU version to check whether a feature
is valid or not, because well, Libvirt uses capabilities to avoid doing that in the
first place.

> 
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 9d2de2a569..7fcfcc83f1 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -2217,6 +2217,9 @@ mymain(void)
>>       DO_TEST_PARSE_ERROR_NOCAPS("seclabel-multiple");
>>       DO_TEST_PARSE_ERROR_NOCAPS("seclabel-device-duplicates");
>>   
>> +    DO_TEST("powernv8-basic",
>> +            QEMU_CAPS_DEVICE_ISA_SERIAL);
> 
> All new test cases shoud use DO_TEST_CAPS_ARCH_LATEST instead of fake
> capabilities.


I'll fix it here and in all other tests I ended up adding in this series.



Thanks,


Daniel

> 
>> +
>>       DO_TEST("pseries-basic",
>>               QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>>               QEMU_CAPS_DEVICE_SPAPR_VTY);
> 
> 
> [...]
> 
> 
>> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
>> index cb665e501b..5d29125ffa 100644
>> --- a/tests/testutilsqemu.c
>> +++ b/tests/testutilsqemu.c
>> @@ -54,7 +54,7 @@ static const char *const arm_machines[] = {
>>       "vexpress-a9", "virt", NULL
>>   };
>>   static const char *const ppc64_machines[] = {
>> -    "pseries", NULL
>> +    "pseries", "powernv8", NULL
> 
> NACK to this hunk. Use real capabilities instead.
> 
>>   };
>>   static const char *const ppc_machines[] = {
>>       "ppce500", NULL
>> -- 
>> 2.34.1
>>
> 

On Mon, Jan 24, 2022 at 08:57:02 -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 1/21/22 11:08, Peter Krempa wrote:
> > On Thu, Jan 20, 2022 at 10:52:11 -0300, Daniel Henrique Barboza wrote:
> > > We're now able to boot a simple PowerNV8 domain in Libvirt.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > > ---
> > >   tests/qemuxml2argvdata/powernv8-basic.args  | 31 +++++++++++++++++++++
> > >   tests/qemuxml2argvdata/powernv8-basic.xml   | 16 +++++++++++
> > >   tests/qemuxml2argvtest.c                    |  3 ++
> > >   tests/qemuxml2xmloutdata/powernv8-basic.xml | 28 +++++++++++++++++++
> > >   tests/qemuxml2xmltest.c                     |  3 ++
> > >   tests/testutilsqemu.c                       |  2 +-
> > >   6 files changed, 82 insertions(+), 1 deletion(-)
> > >   create mode 100644 tests/qemuxml2argvdata/powernv8-basic.args
> > >   create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml
> > >   create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.xml
> > 
> > [...]
> > 
> > > diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml
> > > new file mode 100644
> > > index 0000000000..a92fc1282c
> > > --- /dev/null
> > > +++ b/tests/qemuxml2argvdata/powernv8-basic.xml
> > > @@ -0,0 +1,16 @@
> > > +<domain type='qemu'>
> > > +  <name>QEMUGuest1</name>
> > > +  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
> > > +  <memory unit='KiB'>2097152</memory>
> > > +  <vcpu placement='static'>1</vcpu>
> > > +  <os>
> > > +    <type arch='ppc64' machine='powernv8'>hvm</type>
> > > +  </os>
> > > +  <devices>
> > > +    <emulator>/usr/bin/qemu-system-ppc64</emulator>
> > > +    <controller type='pci' index='0' model='pcie-root'/>
> > > +    <console type='pty'>
> > > +      <target type='serial' port='0'/>
> > > +    </console>
> > > +  </devices>
> > > +</domain>
> > 
> > Trying to start this config after this series is applied fails with:
> > 
> > error: internal error: process exited while connecting to monitor: 2022-01-21T14:04:54.119602Z qemu-system-ppc64: -device pnv-phb3,index=0,chip-id=0,id=pcie.0: Parameter 'driver' expects a pluggable device type
> > 
> > qemu-system-ppc-core-6.2.0-2.fc35.x86_64
> 
> 
> You'll need QEMU upstream to test it. QEMU 6.2.0 doesn't allow the PHBs to be
> user created. I think I mentioned in the cover letter that I didn't find a good
> way to block Libvirt users from doing exactly as you attempted to do because the
> pnv-phb3 is presented, but it's unusable.
> 
> The places where we bother checking QEMU version is on qemu_process.c when creating
> a log in qemuLogOperation() and in qemu_driver.c in qemuConnectGetVersion. I didn't
> find any other case where Libvirt checks for QEMU version to check whether a feature
> is valid or not, because well, Libvirt uses capabilities to avoid doing that in the
> first place.

In very specific cases when there's absolutely no other possibility to
do so we assert certain virQEMUCaps based on version number. Currently
there's just one. See: virQEMUCapsInitQMPVersionCaps

On 1/24/22 09:04, Peter Krempa wrote:
> On Mon, Jan 24, 2022 at 08:57:02 -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 1/21/22 11:08, Peter Krempa wrote:
>>> On Thu, Jan 20, 2022 at 10:52:11 -0300, Daniel Henrique Barboza wrote:
>>>> We're now able to boot a simple PowerNV8 domain in Libvirt.
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>> ---
>>>>    tests/qemuxml2argvdata/powernv8-basic.args  | 31 +++++++++++++++++++++
>>>>    tests/qemuxml2argvdata/powernv8-basic.xml   | 16 +++++++++++
>>>>    tests/qemuxml2argvtest.c                    |  3 ++
>>>>    tests/qemuxml2xmloutdata/powernv8-basic.xml | 28 +++++++++++++++++++
>>>>    tests/qemuxml2xmltest.c                     |  3 ++
>>>>    tests/testutilsqemu.c                       |  2 +-
>>>>    6 files changed, 82 insertions(+), 1 deletion(-)
>>>>    create mode 100644 tests/qemuxml2argvdata/powernv8-basic.args
>>>>    create mode 100644 tests/qemuxml2argvdata/powernv8-basic.xml
>>>>    create mode 100644 tests/qemuxml2xmloutdata/powernv8-basic.xml
>>>
>>> [...]
>>>
>>>> diff --git a/tests/qemuxml2argvdata/powernv8-basic.xml b/tests/qemuxml2argvdata/powernv8-basic.xml
>>>> new file mode 100644
>>>> index 0000000000..a92fc1282c
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/powernv8-basic.xml
>>>> @@ -0,0 +1,16 @@
>>>> +<domain type='qemu'>
>>>> +  <name>QEMUGuest1</name>
>>>> +  <uuid>b20fcfe3-4a0a-4039-8735-9e024256e0f7</uuid>
>>>> +  <memory unit='KiB'>2097152</memory>
>>>> +  <vcpu placement='static'>1</vcpu>
>>>> +  <os>
>>>> +    <type arch='ppc64' machine='powernv8'>hvm</type>
>>>> +  </os>
>>>> +  <devices>
>>>> +    <emulator>/usr/bin/qemu-system-ppc64</emulator>
>>>> +    <controller type='pci' index='0' model='pcie-root'/>
>>>> +    <console type='pty'>
>>>> +      <target type='serial' port='0'/>
>>>> +    </console>
>>>> +  </devices>
>>>> +</domain>
>>>
>>> Trying to start this config after this series is applied fails with:
>>>
>>> error: internal error: process exited while connecting to monitor: 2022-01-21T14:04:54.119602Z qemu-system-ppc64: -device pnv-phb3,index=0,chip-id=0,id=pcie.0: Parameter 'driver' expects a pluggable device type
>>>
>>> qemu-system-ppc-core-6.2.0-2.fc35.x86_64
>>
>>
>> You'll need QEMU upstream to test it. QEMU 6.2.0 doesn't allow the PHBs to be
>> user created. I think I mentioned in the cover letter that I didn't find a good
>> way to block Libvirt users from doing exactly as you attempted to do because the
>> pnv-phb3 is presented, but it's unusable.
>>
>> The places where we bother checking QEMU version is on qemu_process.c when creating
>> a log in qemuLogOperation() and in qemu_driver.c in qemuConnectGetVersion. I didn't
>> find any other case where Libvirt checks for QEMU version to check whether a feature
>> is valid or not, because well, Libvirt uses capabilities to avoid doing that in the
>> first place.
> 
> In very specific cases when there's absolutely no other possibility to
> do so we assert certain virQEMUCaps based on version number. Currently
> there's just one. See: virQEMUCapsInitQMPVersionCaps


Thanks for the pointer. I'll look it up and see if this is possible with these
PowerNV capabilities I'm adding in this series.


Daniel

>