This function return value is invariant since 18f3771, so change
its type and remove all dependent checks.

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/hypervisor/virhostdev.c | 6 ++----
 src/util/virpci.c           | 4 +---
 src/util/virpci.h           | 2 +-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index db94a2e056..f8b5ab86e1 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -1131,8 +1131,7 @@ virHostdevUpdateActivePCIDevices(virHostdevManager *mgr,
         if (!actual)
             continue;
 
-        if (virPCIDeviceSetUsedBy(actual, drv_name, dom_name) < 0)
-            goto cleanup;
+        virPCIDeviceSetUsedBy(actual, drv_name, dom_name);
 
         /* Setup the original states for the PCI device */
         virPCIDeviceSetUnbindFromStub(actual, virBitmapIsBitSet(orig, VIR_DOMAIN_HOSTDEV_PCI_ORIGSTATE_UNBIND));
@@ -2480,8 +2479,7 @@ virHostdevUpdateActiveNVMeDevices(virHostdevManager *hostdev_mgr,
 
         /* We must restore some attributes that were lost on daemon restart. */
         virPCIDeviceSetUnbindFromStub(actual, true);
-        if (virPCIDeviceSetUsedBy(actual, drv_name, dom_name) < 0)
-            goto rollback;
+        virPCIDeviceSetUsedBy(actual, drv_name, dom_name);
 
         if (virPCIDeviceListAddCopy(hostdev_mgr->activePCIHostdevs, actual) < 0)
             goto rollback;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 289c0b330b..90617e69c6 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2049,7 +2049,7 @@ virPCIDeviceSetReprobe(virPCIDevice *dev, bool reprobe)
     dev->reprobe = reprobe;
 }
 
-int
+void
 virPCIDeviceSetUsedBy(virPCIDevice *dev,
                       const char *drv_name,
                       const char *dom_name)
@@ -2058,8 +2058,6 @@ virPCIDeviceSetUsedBy(virPCIDevice *dev,
     VIR_FREE(dev->used_by_domname);
     dev->used_by_drvname = g_strdup(drv_name);
     dev->used_by_domname = g_strdup(dom_name);
-
-    return 0;
 }
 
 void
diff --git a/src/util/virpci.h b/src/util/virpci.h
index ba5e0ae6f1..c5dcf9d37f 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -136,7 +136,7 @@ void virPCIDeviceSetStubDriverName(virPCIDevice *dev,
                                    const char *driverName);
 const char *virPCIDeviceGetStubDriverName(virPCIDevice *dev);
 virPCIDeviceAddress *virPCIDeviceGetAddress(virPCIDevice *dev);
-int virPCIDeviceSetUsedBy(virPCIDevice *dev,
+void virPCIDeviceSetUsedBy(virPCIDevice *dev,
                           const char *drv_name,
                           const char *dom_name);
 void virPCIDeviceGetUsedBy(virPCIDevice *dev,
-- 
2.42.2

On Thu, Nov 28, 2024 at 16:28:07 +0300, Alexander Kuznetsov wrote:
> This function return value is invariant since 18f3771, so change
> its type and remove all dependent checks.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/hypervisor/virhostdev.c | 6 ++----
>  src/util/virpci.c           | 4 +---
>  src/util/virpci.h           | 2 +-
>  3 files changed, 4 insertions(+), 8 deletions(-)
> 
...
> diff --git a/src/util/virpci.h b/src/util/virpci.h
> index ba5e0ae6f1..c5dcf9d37f 100644
> --- a/src/util/virpci.h
> +++ b/src/util/virpci.h
> @@ -136,7 +136,7 @@ void virPCIDeviceSetStubDriverName(virPCIDevice *dev,
>                                     const char *driverName);
>  const char *virPCIDeviceGetStubDriverName(virPCIDevice *dev);
>  virPCIDeviceAddress *virPCIDeviceGetAddress(virPCIDevice *dev);
> -int virPCIDeviceSetUsedBy(virPCIDevice *dev,
> +void virPCIDeviceSetUsedBy(virPCIDevice *dev,
>                            const char *drv_name,
>                            const char *dom_name);

Indentation error in the two lines above. "void" is one character longes
then "int".

Jirka

13.12.2024 18:02, Jiri Denemark пишет:
> Indentation error in the two lines above. "void" is one character longes
> then "int".
>
> Jirka
Thanks for your patience!

This is my first experience of sending a set of patches, so could you tell me
what I should do with Reviewed-by tag when sending v3, since 2 patches have it,
and 2 patches don't. Should I add it to my commit message to these 2 patches?
"Submit patches" wasn't clear at this point, so I wanted to clarify

Thanks again for your time and patience!

-- 
Best regards,
Alexander Kuznetsov

On Fri, Dec 13, 2024 at 18:28:38 +0300, Alexander Kuznetsov wrote:
> 
> 13.12.2024 18:02, Jiri Denemark пишет:
> > Indentation error in the two lines above. "void" is one character longes
> > then "int".
> >
> > Jirka
> Thanks for your patience!
> 
> This is my first experience of sending a set of patches, so could you tell me
> what I should do with Reviewed-by tag when sending v3, since 2 patches have it,
> and 2 patches don't. Should I add it to my commit message to these 2 patches?
> "Submit patches" wasn't clear at this point, so I wanted to clarify

Yes, you add them to your patches when sending a new version of the
series. Unless you realize you have to make significant changes and need
the previously acked patches reviewed again, in which case you would not
add the reviewed-by line to the patch even if it was included in the
review.

Jirka

On Thu, Nov 28, 2024 at 16:28:07 +0300, Alexander Kuznetsov wrote:
> This function return value is invariant since 18f3771, so change
> its type and remove all dependent checks.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/hypervisor/virhostdev.c | 6 ++----
>  src/util/virpci.c           | 4 +---
>  src/util/virpci.h           | 2 +-
>  3 files changed, 4 insertions(+), 8 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>