[libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check

Yi Min Zhao posted 13 patches 6 years, 9 months ago
There is a newer version of this series
[libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Yi Min Zhao 6 years, 9 months ago
We should ensure that the QEMU should support zPCI when zPCI address is
defined in XML. Otherwise the error should be reported. This patch
introduces a generic validation function
qemuDomainDeviceDefValidateAddress() which calls
qemuDomainDeviceDefValidateZPCIAddress() if address type is PCI
address.

Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
---
 src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fa9113e542..94a14c582b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5798,6 +5798,39 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
 }
 
 
+static int
+qemuDomainDeviceDefValidateZPCIAddress(virDomainDeviceInfoPtr info,
+                                       virQEMUCapsPtr qemuCaps)
+{
+    if (!virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci) &&
+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       "%s",
+                       _("This QEMU binary doesn't support zPCI"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
+                                   virQEMUCapsPtr qemuCaps)
+{
+    virDomainDeviceInfoPtr info =
+        virDomainDeviceGetInfo((virDomainDeviceDef *)dev);
+
+    if (!info)
+        return 0;
+
+    if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+        return qemuDomainDeviceDefValidateZPCIAddress(info, qemuCaps);
+
+    return 0;
+}
+
+
 static int
 qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                             const virDomainDef *def,
@@ -5811,6 +5844,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                                             def->emulator)))
         return -1;
 
+    ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps);
+    if (ret < 0)
+        goto out;
+
     switch ((virDomainDeviceType)dev->type) {
     case VIR_DOMAIN_DEVICE_NET:
         ret = qemuDomainDeviceDefValidateNetwork(dev->data.net);
@@ -5886,6 +5923,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
         break;
     }
 
+ out:
     virObjectUnref(qemuCaps);
     return ret;
 }
-- 
Yi Min

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Andrea Bolognani 6 years, 9 months ago
On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote:
[...]
> +static int
> +qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
> +                                   virQEMUCapsPtr qemuCaps)
> +{
> +    virDomainDeviceInfoPtr info =
> +        virDomainDeviceGetInfo((virDomainDeviceDef *)dev);
> +
> +    if (!info)
> +        return 0;

Using

  virDomainDeviceInfoPtr info;

  if (!(info = virDomainDeviceGetInfo((virDomainDeviceDef *)dev)))
    return 0;

here would look much better.

[...]
> @@ -5811,6 +5844,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>                                              def->emulator)))
>          return -1;
>  
> +    ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps);
> +    if (ret < 0)
> +        goto out;

This could be

  if ((ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps)) < 0)
      ...

[...]
> @@ -5886,6 +5923,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>          break;
>      }
>  
> + out:
>      virObjectUnref(qemuCaps);
>      return ret;

'cleanup' would be a more appropriate name for the label since
you're releasing resources when you reach it.


With the above addressed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Yi Min Zhao 6 years, 9 months ago

在 2018/10/11 下午8:44, Andrea Bolognani 写道:
> On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote:
> [...]
>> +static int
>> +qemuDomainDeviceDefValidateAddress(const virDomainDeviceDef *dev,
>> +                                   virQEMUCapsPtr qemuCaps)
>> +{
>> +    virDomainDeviceInfoPtr info =
>> +        virDomainDeviceGetInfo((virDomainDeviceDef *)dev);
>> +
>> +    if (!info)
>> +        return 0;
> Using
>
>    virDomainDeviceInfoPtr info;
>
>    if (!(info = virDomainDeviceGetInfo((virDomainDeviceDef *)dev)))
>      return 0;
>
> here would look much better.
>
> [...]
>> @@ -5811,6 +5844,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>>                                               def->emulator)))
>>           return -1;
>>   
>> +    ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps);
>> +    if (ret < 0)
>> +        goto out;
> This could be
>
>    if ((ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps)) < 0)
>        ...
>
> [...]
>> @@ -5886,6 +5923,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>>           break;
>>       }
>>   
>> + out:
>>       virObjectUnref(qemuCaps);
>>       return ret;
> 'cleanup' would be a more appropriate name for the label since
> you're releasing resources when you reach it.
>
>
> With the above addressed,
>
>    Reviewed-by: Andrea Bolognani <abologna@redhat.com>
>
Thanks! Will update the code as your comments.

-- 
Yi Min

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Andrea Bolognani 6 years, 9 months ago
On Thu, 2018-10-11 at 14:44 +0200, Andrea Bolognani wrote:
[...]
> With the above addressed,
> 
>   Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Forgot to mention: it would be really nice if you added a negative
test case showing that using zPCI addresses on eg. x86 will result
in a validation error.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Yi Min Zhao 6 years, 9 months ago

在 2018/10/11 下午9:08, Andrea Bolognani 写道:
> On Thu, 2018-10-11 at 14:44 +0200, Andrea Bolognani wrote:
> [...]
>> With the above addressed,
>>
>>    Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> Forgot to mention: it would be really nice if you added a negative
> test case showing that using zPCI addresses on eg. x86 will result
> in a validation error.
>
OK. Let me have a try. Haven't added this kind of test before.

-- 
Yi Min

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Andrea Bolognani 6 years, 9 months ago
On Mon, 2018-10-15 at 09:31 +0800, Yi Min Zhao wrote:
> 在 2018/10/11 下午9:08, Andrea Bolognani 写道:
> > Forgot to mention: it would be really nice if you added a negative
> > test case showing that using zPCI addresses on eg. x86 will result
> > in a validation error.
> 
> OK. Let me have a try. Haven't added this kind of test before.

It's very easy: just use DO_TEST_PARSE_ERROR() or DO_TEST_FAILURE()
instead of DO_TEST() :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
Posted by Yi Min Zhao 6 years, 9 months ago

在 2018/10/15 下午2:59, Andrea Bolognani 写道:
> On Mon, 2018-10-15 at 09:31 +0800, Yi Min Zhao wrote:
>> 在 2018/10/11 下午9:08, Andrea Bolognani 写道:
>>> Forgot to mention: it would be really nice if you added a negative
>>> test case showing that using zPCI addresses on eg. x86 will result
>>> in a validation error.
>> OK. Let me have a try. Haven't added this kind of test before.
> It's very easy: just use DO_TEST_PARSE_ERROR() or DO_TEST_FAILURE()
> instead of DO_TEST() :)
>
Yes, I have found sample code. Thanks!

-- 
Yi Min

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