[libvirt] [PATCH 07/12] virpci: Remove unused virPCIDeviceWaitForCleanup

Michal Privoznik posted 12 patches 6 years, 5 months ago
[libvirt] [PATCH 07/12] virpci: Remove unused virPCIDeviceWaitForCleanup
Posted by Michal Privoznik 6 years, 5 months ago
This function is no longer used after previous commit.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |   1 -
 src/util/virpci.c        | 108 ---------------------------------------
 src/util/virpci.h        |   1 -
 3 files changed, 110 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9db4ac7933..6c57351866 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2696,7 +2696,6 @@ virPCIDeviceSetStubDriver;
 virPCIDeviceSetUnbindFromStub;
 virPCIDeviceSetUsedBy;
 virPCIDeviceUnbind;
-virPCIDeviceWaitForCleanup;
 virPCIEDeviceInfoFree;
 virPCIELinkSpeedTypeFromString;
 virPCIELinkSpeedTypeToString;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 9c9ffa55c2..ea5be62033 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1552,114 +1552,6 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
     return 0;
 }
 
-/* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on
- * the host when doing device passthrough.  This can lead to a race
- * condition where the hypervisor is still cleaning up the device while
- * libvirt is trying to re-attach it to the host device driver.  To avoid
- * this situation, we look through /proc/iomem, and if the hypervisor is
- * still holding on to the bar (denoted by the string in the matcher
- * variable), then we can wait around a bit for that to clear up.
- *
- * A typical /proc/iomem looks like this (snipped for brevity):
- * 00010000-0008efff : System RAM
- * 0008f000-0008ffff : reserved
- * ...
- * 00100000-cc9fcfff : System RAM
- *   00200000-00483d3b : Kernel code
- *   00483d3c-005c88df : Kernel data
- * cc9fd000-ccc71fff : ACPI Non-volatile Storage
- * ...
- * d0200000-d02fffff : PCI Bus #05
- *   d0200000-d021ffff : 0000:05:00.0
- *     d0200000-d021ffff : e1000e
- *   d0220000-d023ffff : 0000:05:00.0
- *     d0220000-d023ffff : e1000e
- * ...
- * f0000000-f0003fff : 0000:00:1b.0
- *   f0000000-f0003fff : kvm_assigned_device
- *
- * Returns 0 if we are clear to continue, and 1 if the hypervisor is still
- * holding on to the resource.
- */
-int
-virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher)
-{
-    FILE *fp;
-    char line[160];
-    char *tmp;
-    unsigned long long start, end;
-    unsigned int domain, bus, slot, function;
-    bool in_matching_device;
-    int ret;
-    size_t match_depth;
-
-    fp = fopen("/proc/iomem", "r");
-    if (!fp) {
-        /* If we failed to open iomem, we just basically ignore the error.  The
-         * unbind might succeed anyway, and besides, it's very likely we have
-         * no way to report the error
-         */
-        VIR_DEBUG("Failed to open /proc/iomem, trying to continue anyway");
-        return 0;
-    }
-
-    ret = 0;
-    in_matching_device = false;
-    match_depth = 0;
-    while (fgets(line, sizeof(line), fp) != 0) {
-        /* the logic here is a bit confusing.  For each line, we look to
-         * see if it matches the domain:bus:slot.function we were given.
-         * If this line matches the DBSF, then any subsequent lines indented
-         * by 2 spaces are the PCI regions for this device.  It's also
-         * possible that none of the PCI regions are currently mapped, in
-         * which case we have no indented regions.  This code handles all
-         * of these situations
-         */
-        if (in_matching_device && (strspn(line, " ") == (match_depth + 2))) {
-            /* expected format: <start>-<end> : <suffix> */
-            if (/* start */
-                virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' ||
-                /* end */
-                virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 ||
-                (tmp = STRSKIP(tmp, " : ")) == NULL)
-                continue;
-
-            if (STRPREFIX(tmp, matcher)) {
-                ret = 1;
-                break;
-            }
-        } else {
-            in_matching_device = false;
-
-            /* expected format: <start>-<end> : <domain>:<bus>:<slot>.<function> */
-            if (/* start */
-                virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' ||
-                /* end */
-                virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 ||
-                (tmp = STRSKIP(tmp, " : ")) == NULL ||
-                /* domain */
-                virStrToLong_ui(tmp, &tmp, 16, &domain) < 0 || *tmp != ':' ||
-                /* bus */
-                virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || *tmp != ':' ||
-                /* slot */
-                virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || *tmp != '.' ||
-                /* function */
-                virStrToLong_ui(tmp + 1, &tmp, 16, &function) < 0 || *tmp != '\n')
-                continue;
-
-            if (domain != dev->address.domain || bus != dev->address.bus ||
-                slot != dev->address.slot || function != dev->address.function)
-                continue;
-            in_matching_device = true;
-            match_depth = strspn(line, " ");
-        }
-    }
-
-    VIR_FORCE_FCLOSE(fp);
-
-    return ret;
-}
-
 static char *
 virPCIDeviceReadID(virPCIDevicePtr dev, const char *id_name)
 {
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 4ac0d230a4..dc20f91710 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -197,7 +197,6 @@ char *virPCIDeviceGetIOMMUGroupDev(virPCIDevicePtr dev);
 
 int virPCIDeviceIsAssignable(virPCIDevicePtr dev,
                              int strict_acs_check);
-int virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher);
 
 virPCIDeviceAddressPtr
 virPCIGetDeviceAddressFromSysfsLink(const char *device_link);
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] virpci: Remove unused virPCIDeviceWaitForCleanup
Posted by Ján Tomko 6 years, 5 months ago
On Tue, Aug 20, 2019 at 04:30:27PM +0200, Michal Privoznik wrote:
>This function is no longer used after previous commit.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/libvirt_private.syms |   1 -
> src/util/virpci.c        | 108 ---------------------------------------
> src/util/virpci.h        |   1 -
> 3 files changed, 110 deletions(-)
>

:,-)

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] virpci: Remove unused virPCIDeviceWaitForCleanup
Posted by Daniel Henrique Barboza 6 years, 5 months ago

On 8/20/19 11:30 AM, Michal Privoznik wrote:
> This function is no longer used after previous commit.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>   src/libvirt_private.syms |   1 -
>   src/util/virpci.c        | 108 ---------------------------------------
>   src/util/virpci.h        |   1 -
>   3 files changed, 110 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9db4ac7933..6c57351866 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2696,7 +2696,6 @@ virPCIDeviceSetStubDriver;
>   virPCIDeviceSetUnbindFromStub;
>   virPCIDeviceSetUsedBy;
>   virPCIDeviceUnbind;
> -virPCIDeviceWaitForCleanup;
>   virPCIEDeviceInfoFree;
>   virPCIELinkSpeedTypeFromString;
>   virPCIELinkSpeedTypeToString;
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 9c9ffa55c2..ea5be62033 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -1552,114 +1552,6 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
>       return 0;
>   }
>   
> -/* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on
> - * the host when doing device passthrough.  This can lead to a race
> - * condition where the hypervisor is still cleaning up the device while
> - * libvirt is trying to re-attach it to the host device driver.  To avoid
> - * this situation, we look through /proc/iomem, and if the hypervisor is
> - * still holding on to the bar (denoted by the string in the matcher
> - * variable), then we can wait around a bit for that to clear up.
> - *
> - * A typical /proc/iomem looks like this (snipped for brevity):
> - * 00010000-0008efff : System RAM
> - * 0008f000-0008ffff : reserved
> - * ...
> - * 00100000-cc9fcfff : System RAM
> - *   00200000-00483d3b : Kernel code
> - *   00483d3c-005c88df : Kernel data
> - * cc9fd000-ccc71fff : ACPI Non-volatile Storage
> - * ...
> - * d0200000-d02fffff : PCI Bus #05
> - *   d0200000-d021ffff : 0000:05:00.0
> - *     d0200000-d021ffff : e1000e
> - *   d0220000-d023ffff : 0000:05:00.0
> - *     d0220000-d023ffff : e1000e
> - * ...
> - * f0000000-f0003fff : 0000:00:1b.0
> - *   f0000000-f0003fff : kvm_assigned_device
> - *
> - * Returns 0 if we are clear to continue, and 1 if the hypervisor is still
> - * holding on to the resource.
> - */
> -int
> -virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher)
> -{
> -    FILE *fp;
> -    char line[160];
> -    char *tmp;
> -    unsigned long long start, end;
> -    unsigned int domain, bus, slot, function;
> -    bool in_matching_device;
> -    int ret;
> -    size_t match_depth;
> -
> -    fp = fopen("/proc/iomem", "r");
> -    if (!fp) {
> -        /* If we failed to open iomem, we just basically ignore the error.  The
> -         * unbind might succeed anyway, and besides, it's very likely we have
> -         * no way to report the error
> -         */
> -        VIR_DEBUG("Failed to open /proc/iomem, trying to continue anyway");
> -        return 0;
> -    }
> -
> -    ret = 0;
> -    in_matching_device = false;
> -    match_depth = 0;
> -    while (fgets(line, sizeof(line), fp) != 0) {
> -        /* the logic here is a bit confusing.  For each line, we look to
> -         * see if it matches the domain:bus:slot.function we were given.
> -         * If this line matches the DBSF, then any subsequent lines indented
> -         * by 2 spaces are the PCI regions for this device.  It's also
> -         * possible that none of the PCI regions are currently mapped, in
> -         * which case we have no indented regions.  This code handles all
> -         * of these situations
> -         */
> -        if (in_matching_device && (strspn(line, " ") == (match_depth + 2))) {
> -            /* expected format: <start>-<end> : <suffix> */
> -            if (/* start */
> -                virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' ||
> -                /* end */
> -                virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 ||
> -                (tmp = STRSKIP(tmp, " : ")) == NULL)
> -                continue;
> -
> -            if (STRPREFIX(tmp, matcher)) {
> -                ret = 1;
> -                break;
> -            }
> -        } else {
> -            in_matching_device = false;
> -
> -            /* expected format: <start>-<end> : <domain>:<bus>:<slot>.<function> */
> -            if (/* start */
> -                virStrToLong_ull(line, &tmp, 16, &start) < 0 || *tmp != '-' ||
> -                /* end */
> -                virStrToLong_ull(tmp + 1, &tmp, 16, &end) < 0 ||
> -                (tmp = STRSKIP(tmp, " : ")) == NULL ||
> -                /* domain */
> -                virStrToLong_ui(tmp, &tmp, 16, &domain) < 0 || *tmp != ':' ||
> -                /* bus */
> -                virStrToLong_ui(tmp + 1, &tmp, 16, &bus) < 0 || *tmp != ':' ||
> -                /* slot */
> -                virStrToLong_ui(tmp + 1, &tmp, 16, &slot) < 0 || *tmp != '.' ||
> -                /* function */
> -                virStrToLong_ui(tmp + 1, &tmp, 16, &function) < 0 || *tmp != '\n')
> -                continue;
> -
> -            if (domain != dev->address.domain || bus != dev->address.bus ||
> -                slot != dev->address.slot || function != dev->address.function)
> -                continue;
> -            in_matching_device = true;
> -            match_depth = strspn(line, " ");
> -        }
> -    }
> -
> -    VIR_FORCE_FCLOSE(fp);
> -
> -    return ret;
> -}
> -
>   static char *
>   virPCIDeviceReadID(virPCIDevicePtr dev, const char *id_name)
>   {
> diff --git a/src/util/virpci.h b/src/util/virpci.h
> index 4ac0d230a4..dc20f91710 100644
> --- a/src/util/virpci.h
> +++ b/src/util/virpci.h
> @@ -197,7 +197,6 @@ char *virPCIDeviceGetIOMMUGroupDev(virPCIDevicePtr dev);
>   
>   int virPCIDeviceIsAssignable(virPCIDevicePtr dev,
>                                int strict_acs_check);
> -int virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher);
>   
>   virPCIDeviceAddressPtr
>   virPCIGetDeviceAddressFromSysfsLink(const char *device_link);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list