1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] conf: Resolve corner case on fc_host deletion

John Ferlan posted 1 patch 6 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170429153610.23944-1-jferlan@redhat.com
src/conf/node_device_conf.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
[libvirt] [PATCH] conf: Resolve corner case on fc_host deletion
Posted by John Ferlan 6 years, 12 months ago 
https://bugzilla.redhat.com/show_bug.cgi?id=1420740

Testing found an inventive way to cause an error at shutdown by providing the
parent name for the fc host creation using the "same name" as the HBA. Since
the code thus assumed the parent host name provided was the parent HBA and
just extracted out the host number and sent that along to the vport_destroy
this avoided checks made for equality.

So just add the equality check to that path to resolve.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/node_device_conf.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 85cfd83..3f995da 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2127,17 +2127,25 @@ virNodeDeviceDeleteVport(virConnectPtr conn,
         goto cleanup;
     }
 
+    if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
+        goto cleanup;
+
     /* If at startup time we provided a parent, then use that to
      * get the parent_host value; otherwise, we have to determine
      * the parent scsi_host which we did not save at startup time
      */
     if (fchost->parent) {
+        /* Someone provided a parent string at startup time that
+         * was the same as the scsi_host - meaning we have a pool
+         * backed to an HBA, so there won't be a vHBA to delete */
+        if (STREQ(scsi_host_name, fchost->parent)) {
+            ret = 0;
+            goto cleanup;
+        }
+
         if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0)
             goto cleanup;
     } else {
-        if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
-            goto cleanup;
-
         if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name)))
             goto cleanup;
 
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Resolve corner case on fc_host deletion
Posted by John Ferlan 6 years, 11 months ago 
ping?

Tks -

John

On 04/29/2017 11:36 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1420740
> 
> Testing found an inventive way to cause an error at shutdown by providing the
> parent name for the fc host creation using the "same name" as the HBA. Since
> the code thus assumed the parent host name provided was the parent HBA and
> just extracted out the host number and sent that along to the vport_destroy
> this avoided checks made for equality.
> 
> So just add the equality check to that path to resolve.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/node_device_conf.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 85cfd83..3f995da 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2127,17 +2127,25 @@ virNodeDeviceDeleteVport(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> +        goto cleanup;
> +
>      /* If at startup time we provided a parent, then use that to
>       * get the parent_host value; otherwise, we have to determine
>       * the parent scsi_host which we did not save at startup time
>       */
>      if (fchost->parent) {
> +        /* Someone provided a parent string at startup time that
> +         * was the same as the scsi_host - meaning we have a pool
> +         * backed to an HBA, so there won't be a vHBA to delete */
> +        if (STREQ(scsi_host_name, fchost->parent)) {
> +            ret = 0;
> +            goto cleanup;
> +        }
> +
>          if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0)
>              goto cleanup;
>      } else {
> -        if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> -            goto cleanup;
> -
>          if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name)))
>              goto cleanup;
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Resolve corner case on fc_host deletion
Posted by John Ferlan 6 years, 11 months ago 
ping^2

Tks,

John

On 04/29/2017 11:36 AM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1420740
> 
> Testing found an inventive way to cause an error at shutdown by providing the
> parent name for the fc host creation using the "same name" as the HBA. Since
> the code thus assumed the parent host name provided was the parent HBA and
> just extracted out the host number and sent that along to the vport_destroy
> this avoided checks made for equality.
> 
> So just add the equality check to that path to resolve.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/node_device_conf.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 85cfd83..3f995da 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2127,17 +2127,25 @@ virNodeDeviceDeleteVport(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> +        goto cleanup;
> +
>      /* If at startup time we provided a parent, then use that to
>       * get the parent_host value; otherwise, we have to determine
>       * the parent scsi_host which we did not save at startup time
>       */
>      if (fchost->parent) {
> +        /* Someone provided a parent string at startup time that
> +         * was the same as the scsi_host - meaning we have a pool
> +         * backed to an HBA, so there won't be a vHBA to delete */
> +        if (STREQ(scsi_host_name, fchost->parent)) {
> +            ret = 0;
> +            goto cleanup;
> +        }
> +
>          if (virSCSIHostGetNumber(fchost->parent, &parent_host) < 0)
>              goto cleanup;
>      } else {
> -        if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> -            goto cleanup;
> -
>          if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name)))
>              goto cleanup;
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Resolve corner case on fc_host deletion
Posted by Michal Privoznik 6 years, 11 months ago 
On 04/29/2017 05:36 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1420740
> 
> Testing found an inventive way to cause an error at shutdown by providing the
> parent name for the fc host creation using the "same name" as the HBA. Since
> the code thus assumed the parent host name provided was the parent HBA and
> just extracted out the host number and sent that along to the vport_destroy
> this avoided checks made for equality.
> 
> So just add the equality check to that path to resolve.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/node_device_conf.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)

ACK.

Sorry for the delay.

Michal

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.