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@xxxxxxxxxx>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> ---
> 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:
>
