Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
在 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
Re: [libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
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
在 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 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
在 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
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
On Thu, 2018-10-11 at 14:44 +0200, Andrea Bolognani wrote: [...] > With the above addressed, > > Reviewed-by: 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. -- 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
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
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v6 08/13] qemu: Add zPCI address definition check
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
---
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(>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
