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

Yi Min Zhao posted 13 patches 7 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH v5 08/13] qemu: Add zPCI address definition check
Posted by Yi Min Zhao 7 years, 5 months ago
We should ensure that the Qemu should support zPCI when zPCI address is
defined in XML. Otherwise the error should be reported. So this patch
introduces the validation of zPCI address definition for
qemuDomainDeviceDefValidate().

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

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index aebd58c49c..7d7cd3cfdc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5728,6 +5728,27 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics,
 }
 
 
+static int
+qemuDomainZPCIAddressDefValidate(virDomainDeviceDef *dev,
+                                     virQEMUCapsPtr qemuCaps)
+{
+    virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
+
+    if (!info || (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI))
+        return 0;
+
+    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
 qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                             const virDomainDef *def,
@@ -5741,6 +5762,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
                                             def->emulator)))
         return -1;
 
+    ret = qemuDomainZPCIAddressDefValidate((virDomainDeviceDef *)dev, qemuCaps);
+    if (ret < 0)
+        goto out;
+
     switch ((virDomainDeviceType)dev->type) {
     case VIR_DOMAIN_DEVICE_NET:
         ret = qemuDomainDeviceDefValidateNetwork(dev->data.net);
@@ -5813,6 +5838,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 v5 08/13] qemu: Add zPCI address definition check
Posted by Andrea Bolognani 7 years, 4 months ago
On Tue, 2018-09-04 at 16:39 +0800, Yi Min Zhao wrote:
> We should ensure that the Qemu should support zPCI when zPCI address is

s/Qemu/QEMU/

[...]
> +static int
> +qemuDomainZPCIAddressDefValidate(virDomainDeviceDef *dev,
> +                                     virQEMUCapsPtr qemuCaps)

The second argument is not aligned properly.

I'd also change the name: qemuDomainDeviceDefValidateZPCIAddress()
perhaps? It's quite a mouthful, but also more accurate.

[...]
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       "%s",
> +                       _("This QEMU binary doesn't support zPCI."));

No full stop at the end of the error message, please.

[...]
> @@ -5741,6 +5762,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>                                              def->emulator)))
>          return -1;
>  
> +    ret = qemuDomainZPCIAddressDefValidate((virDomainDeviceDef *)dev, qemuCaps);

This cast is kinds gross. I realize virDomainDeviceGetInfo()
requires the pointer to be non-const, but from a semantics point
of view qemuDomainZPCIAddressDefValidate() should be okay with
being passed a const pointer - it's only validating the device,
not changing it, after all.

Please move the cast into the function instead of requiring the
caller to perform it.

We should also have a generic qemuDomainDeviceDefValidateAddress()
wrapper that calls the zPCI-specific one only for PCI devices, and
call that one from here.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

在 2018/9/11 下午8:44, Andrea Bolognani 写道:
> On Tue, 2018-09-04 at 16:39 +0800, Yi Min Zhao wrote:
>> We should ensure that the Qemu should support zPCI when zPCI address is
> s/Qemu/QEMU/
Sure.
>
> [...]
>> +static int
>> +qemuDomainZPCIAddressDefValidate(virDomainDeviceDef *dev,
>> +                                     virQEMUCapsPtr qemuCaps)
> The second argument is not aligned properly.
>
> I'd also change the name: qemuDomainDeviceDefValidateZPCIAddress()
> perhaps? It's quite a mouthful, but also more accurate.
ok.
>
> [...]
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       "%s",
>> +                       _("This QEMU binary doesn't support zPCI."));
> No full stop at the end of the error message, please.
Could you please explain more?
>
> [...]
>> @@ -5741,6 +5762,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
>>                                               def->emulator)))
>>           return -1;
>>   
>> +    ret = qemuDomainZPCIAddressDefValidate((virDomainDeviceDef *)dev, qemuCaps);
> This cast is kinds gross. I realize virDomainDeviceGetInfo()
> requires the pointer to be non-const, but from a semantics point
> of view qemuDomainZPCIAddressDefValidate() should be okay with
> being passed a const pointer - it's only validating the device,
> not changing it, after all.
>
> Please move the cast into the function instead of requiring the
> caller to perform it.
>
> We should also have a generic qemuDomainDeviceDefValidateAddress()
> wrapper that calls the zPCI-specific one only for PCI devices, and
> call that one from here.
>
Got it.

-- 
Yi Min

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 08/13] qemu: Add zPCI address definition check
Posted by Andrea Bolognani 7 years, 4 months ago
On Thu, 2018-09-13 at 18:08 +0800, Yi Min Zhao wrote:
> > [...]
> > > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > +                       "%s",
> > > +                       _("This QEMU binary doesn't support zPCI."));
> > 
> > No full stop at the end of the error message, please.
> 
> Could you please explain more?

Just remove the full stop, like

  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                 "%s",
                 _("This QEMU binary doesn't support zPCI"));

-- 
Andrea Bolognani / Red Hat / Virtualization

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

在 2018/9/13 下午7:47, Andrea Bolognani 写道:
> Just remove the full stop, like
>
>    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                   "%s",
>                   _("This QEMU binary doesn't support zPCI"));
Ah....Got it.

-- 
Yi Min

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