Re: [PATCH v2 07/10] qemu_driver.c: validate 'driverName' earlier in qemuNodeDeviceDetachFlags()
On 2/2/21 8:06 PM, Daniel Henrique Barboza wrote:
The validation of 'driverName' does not depend on any other
state and can be
done right on the start of the function. We can fail earlier while avoiding
a cleanup jump.
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/qemu/qemu_driver.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 64ae8fafc0..c6ba33e4ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11982,6 +11982,25 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
virCheckFlags(0, -1);
+ if (!driverName)
+ driverName = "vfio";
+
+ if (STREQ(driverName, "vfio") && !vfio) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("VFIO device assignment is currently not "
+ "supported on this system"));
+ return -1;
+ } else if (STREQ(driverName, "kvm")) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("KVM device assignment is no longer "
+ "supported on this system"));
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown driver name '%s'"), driverName);
+ return -1;
+ }
+
Coverity points out the rest of this is unreachable now (even with the
subsequent patch).
if (!(nodeconn = virGetConnectNodeDev()))
goto cleanup;
@@ -12013,27 +12032,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
if (!pci)
goto cleanup;
- if (!driverName)
- driverName = "vfio";
-
- if (STREQ(driverName, "vfio")) {
- if (!vfio) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("VFIO device assignment is currently not "
- "supported on this system"));
- goto cleanup;
- }
- virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
This section seems to be more than just code motion... it used to pass
thru here when vfio = true, but with the movement it would fall into the
catch all else.
John
- } else if (STREQ(driverName, "kvm")) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("KVM device assignment is no longer "
- "supported on this system"));
- goto cleanup;
- } else {
- virReportError(VIR_ERR_INVALID_ARG,
- _("unknown driver name '%s'"), driverName);
- goto cleanup;
- }
+ virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
ret = virHostdevPCINodeDeviceDetach(hostdev_mgr, pci);
cleanup: