[libvirt] [PATCH 8/9] nodedev: Abstract nodeDeviceVportCreateDelete as util function

[Michal Privoznik]

On 14.01.2013 15:34, Osier Yang wrote:
> This abstracts nodeDeviceVportCreateDelete as an util function
> virManageVport, which can be further used by later storage patches
> (to support persistent vHBA, I don't want to create the vHBA
> using the public API, which is not good).
> ---
>  src/libvirt_private.syms             |    1 +
>  src/node_device/node_device_driver.c |  101 +++-------------------------------
>  src/node_device/node_device_driver.h |    5 --
>  src/util/virutil.c                   |   84 ++++++++++++++++++++++++++++
>  src/util/virutil.h                   |   11 ++++
>  5 files changed, 104 insertions(+), 98 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2945917..146e57e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1298,6 +1298,7 @@ virIndexToDiskName;
>  virIsCapableFCHost;
>  virIsCapableVport;
>  virIsDevMapperDevice;
> +virManageVport;
>  virParseNumber;
>  virParseVersionString;
>  virPipeReadUntilEOF;
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 050ea62..0d0afe0 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -408,91 +408,6 @@ cleanup:
>      return ret;
>  }
>  
> -
> -static int
> -nodeDeviceVportCreateDelete(const int parent_host,
> -                            const char *wwpn,
> -                            const char *wwnn,
> -                            int operation)
> -{
> -    int retval = 0;
> -    char *operation_path = NULL, *vport_name = NULL;
> -    const char *operation_file = NULL;
> -
> -    switch (operation) {
> -    case VPORT_CREATE:
> -        operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX;
> -        break;
> -    case VPORT_DELETE:
> -        operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX;
> -        break;
> -    default:
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Invalid vport operation (%d)"), operation);
> -        retval = -1;
> -        goto cleanup;
> -        break;
> -    }
> -
> -    if (virAsprintf(&operation_path,
> -                    "%shost%d%s",
> -                    LINUX_SYSFS_FC_HOST_PREFIX,
> -                    parent_host,
> -                    operation_file) < 0) {
> -
> -        virReportOOMError();
> -        retval = -1;
> -        goto cleanup;
> -    }
> -
> -    if (!virFileExists(operation_path)) {
> -        VIR_FREE(operation_path);
> -        if (virAsprintf(&operation_path,
> -                        "%shost%d%s",
> -                        LINUX_SYSFS_SCSI_HOST_PREFIX,
> -                        parent_host,
> -                        operation_file) < 0) {
> -            virReportOOMError();
> -            retval = -1;
> -            goto cleanup;
> -        }
> -
> -        if (!virFileExists(operation_path)) {
> -            VIR_ERROR(_("No vport operation path found for host%d"),
> -                      parent_host);
> -            retval = -1;
> -            goto cleanup;
> -        }
> -    }
> -
> -    VIR_DEBUG("Vport operation path is '%s'", operation_path);
> -
> -    if (virAsprintf(&vport_name,
> -                    "%s:%s",
> -                    wwpn,
> -                    wwnn) < 0) {
> -
> -        virReportOOMError();
> -        retval = -1;
> -        goto cleanup;
> -    }
> -
> -    if (virFileWriteStr(operation_path, vport_name, 0) == -1) {
> -        virReportSystemError(errno,
> -                             _("Write of '%s' to '%s' during "
> -                               "vport create/delete failed"),
> -                             vport_name, operation_path);
> -        retval = -1;
> -    }
> -
> -cleanup:
> -    VIR_FREE(vport_name);
> -    VIR_FREE(operation_path);
> -    VIR_DEBUG("%s", _("Vport operation complete"));
> -    return retval;
> -}
> -
> -
>  static int
>  get_time(time_t *t)
>  {
> @@ -594,10 +509,10 @@ nodeDeviceCreateXML(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> -    if (nodeDeviceVportCreateDelete(parent_host,
> -                                    wwpn,
> -                                    wwnn,
> -                                    VPORT_CREATE) == -1) {
> +    if (virManageVport(parent_host,
> +                       wwpn,
> +                       wwnn,
> +                       VPORT_CREATE) == -1) {
>          goto cleanup;
>      }
>  
> @@ -661,10 +576,10 @@ nodeDeviceDestroy(virNodeDevicePtr dev)
>          goto out;
>      }
>  
> -    if (nodeDeviceVportCreateDelete(parent_host,
> -                                    wwpn,
> -                                    wwnn,
> -                                    VPORT_DELETE) == -1) {
> +    if (virManageVport(parent_host,
> +                       wwpn,
> +                       wwnn,
> +                       VPORT_DELETE) == -1) {
>          goto out;
>      }
>  
> diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h
> index 7775a59..093cdab 100644
> --- a/src/node_device/node_device_driver.h
> +++ b/src/node_device/node_device_driver.h
> @@ -32,11 +32,6 @@
>  # define LINUX_SYSFS_SCSI_HOST_POSTFIX "device"
>  # define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/"
>  
> -# define VPORT_CREATE 0
> -# define VPORT_DELETE 1
> -# define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create"
> -# define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete"
> -
>  # define LINUX_NEW_DEVICE_WAIT_TIME 60
>  
>  # ifdef HAVE_HAL
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 8b74afb..be80397 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -3374,6 +3374,79 @@ cleanup:
>      VIR_FREE(scsi_host_path);
>      return ret;
>  }
> +
> +int
> +virManageVport(const int parent_host,
> +               const char *wwpn,
> +               const char *wwnn,
> +               int operation)
> +{
> +    int ret = -1;
> +    char *operation_path = NULL, *vport_name = NULL;
> +    const char *operation_file = NULL;
> +
> +    switch (operation) {
> +    case VPORT_CREATE:
> +        operation_file = "vport_create";
> +        break;
> +    case VPORT_DELETE:
> +        operation_file = "vport_delete";
> +        break;
> +    default:
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("Invalid vport operation (%d)"), operation);
> +        goto cleanup;
> +    }
> +
> +    if (virAsprintf(&operation_path,
> +                    "%shost%d/%s",
> +                    SYSFS_FC_HOST_PATH,
> +                    parent_host,
> +                    operation_file) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    if (!virFileExists(operation_path)) {
> +        VIR_FREE(operation_path);
> +        if (virAsprintf(&operation_path,
> +                        "%shost%d/%s",
> +                        SYSFS_SCSI_HOST_PATH,
> +                        parent_host,
> +                        operation_file) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        if (!virFileExists(operation_path)) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("vport operation '%s' is not supported for host%d"),
> +                           operation_file, parent_host);

Nice change from VIR_ERROR() to virReportError().

ACK

Michal