[PATCH 4/4] virSCSIHostFindByPCI: Fix link detection

Michal Privoznik posted 4 patches 1 year, 1 month ago
[PATCH 4/4] virSCSIHostFindByPCI: Fix link detection
Posted by Michal Privoznik 1 year, 1 month ago
Inside of virSCSIHostFindByPCI() there's a loop which iterates of
entries of "/sys/class/scsi_host" directory trying to identify
all symlinks (which then point to a SCSI device, but that's not
important right now). But the way virFileIsLink() is called can
never return a truthful reply - because it's called over
dent->d_name instead of full path. Fix this by moving the
virFileIsLink() call and passing constructed path into it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virscsihost.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c
index 014b96452c..32d7f2312f 100644
--- a/src/util/virscsihost.c
+++ b/src/util/virscsihost.c
@@ -107,11 +107,11 @@ virSCSIHostFindByPCI(const char *sysfs_prefix,
         char *p = NULL;
         unsigned int read_unique_id;
 
-        if (!virFileIsLink(entry->d_name))
-            continue;
-
         host_link = g_strdup_printf("%s/%s", prefix, entry->d_name);
 
+        if (!virFileIsLink(host_link))
+            continue;
+
         if (virFileResolveLink(host_link, &host_path) < 0)
             return NULL;
 
-- 
2.39.2
Re: [PATCH 4/4] virSCSIHostFindByPCI: Fix link detection
Posted by Ján Tomko 1 year, 1 month ago
On a Wednesday in 2023, Michal Privoznik wrote:
>Inside of virSCSIHostFindByPCI() there's a loop which iterates of
>entries of "/sys/class/scsi_host" directory trying to identify
>all symlinks (which then point to a SCSI device, but that's not
>important right now). But the way virFileIsLink() is called can
>never return a truthful reply - because it's called over
>dent->d_name instead of full path. Fix this by moving the
>virFileIsLink() call and passing constructed path into it.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/util/virscsihost.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c
>index 014b96452c..32d7f2312f 100644
>--- a/src/util/virscsihost.c
>+++ b/src/util/virscsihost.c
>@@ -107,11 +107,11 @@ virSCSIHostFindByPCI(const char *sysfs_prefix,
>         char *p = NULL;
>         unsigned int read_unique_id;
>
>-        if (!virFileIsLink(entry->d_name))
>-            continue;
>-
>         host_link = g_strdup_printf("%s/%s", prefix, entry->d_name);
>
>+        if (!virFileIsLink(host_link))
>+            continue;
>+

Functionally speaking, this is just a micro-optimization. If you left it out
completely, the ResloveLink would copy the path and we would move on to
the next entry when strstr fails to find the parent address. In theory.
In practice, /sys/class/scsi_host/ probably contains nothing else but
symlinks.

Feel free to drop the virFileIsLink call completely.

Jano

>         if (virFileResolveLink(host_link, &host_path) < 0)
>             return NULL;
>
>-- 
>2.39.2
>