[libvirt] [PATCH 07/26] qemu: Allow qemuBuildControllerDevStr() to return NULL

[Laine Stump]

On 06/02/2017 12:07 PM, Andrea Bolognani wrote:
> We will soon need to be able to return a NULL pointer
> without the caller considering that an error: to make
> it possible, change the return type to int and use
> an out parameter for the string instead.
> 
> Add some documentation for the function as well.
> ---
>  src/qemu/qemu_command.c | 53 ++++++++++++++++++++++++++++++++++++-------------
>  src/qemu/qemu_command.h |  9 +++++----
>  src/qemu/qemu_hotplug.c |  5 ++++-
>  3 files changed, 48 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 015af10..a0403bf 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2664,10 +2664,31 @@ qemuCheckSCSIControllerIOThreads(const virDomainDef *domainDef,
>  }
>  
>  
> -char *
> +/**
> + * qemuBuildControllerDevStr:
> + * @domainDef: domain definition
> + * @def: controller definition
> + * @qemuCaps: QEMU binary capabilities
> + * @devstr: device string
> + * @nusbcontroller: number of USB controllers
> + *
> + * Turn @def into a description of the controller that QEMU will understand,
> + * to be used either on the command line or through the monitor.
> + *
> + * The description will be returned in @devstr and can be NULL, eg. when
> + * passing in one of the built-in controllers. The returned string must be
> + * freed by the caller.
> + *
> + * The number pointed to by @nusbcontroller will be increased by one every
> + * time the description for a USB controller has been generated successfully.
> + *
> + * Returns: 0 on success, <0 on failure
> + */
> +int
>  qemuBuildControllerDevStr(const virDomainDef *domainDef,
>                            virDomainControllerDefPtr def,
>                            virQEMUCapsPtr qemuCaps,
> +                          char **devstr,
>                            int *nusbcontroller)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;

Maybe initialize *devstr to NULL in case the caller hasn't?

Reviewed-by: Laine Stump <laine at laine.org>


> @@ -2676,11 +2697,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>  
>      if (!qemuCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps,
>                                          "controller"))
> -        return NULL;
> +        return -1;
>  
>      if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
>          if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
> -            return NULL;
> +            return -1;
>      }
>  
>      if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
> @@ -2688,22 +2709,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>          if (def->queues) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("'queues' is only supported by virtio-scsi controller"));
> -            return NULL;
> +            return -1;
>          }
>          if (def->cmd_per_lun) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("'cmd_per_lun' is only supported by virtio-scsi controller"));
> -            return NULL;
> +            return -1;
>          }
>          if (def->max_sectors) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("'max_sectors' is only supported by virtio-scsi controller"));
> -            return NULL;
> +            return -1;
>          }
>          if (def->ioeventfd) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("'ioeventfd' is only supported by virtio-scsi controller"));
> -            return NULL;
> +            return -1;
>          }
>      }
>  
> @@ -3114,11 +3135,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
>      if (virBufferCheckError(&buf) < 0)
>          goto error;
>  
> -    return virBufferContentAndReset(&buf);
> +    *devstr = virBufferContentAndReset(&buf);
> +    return 0;
>  
>   error:
>      virBufferFreeAndReset(&buf);
> -    return NULL;
> +    return -1;
>  }
>  
>  
> @@ -3213,12 +3235,15 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>                  continue;
>              }
>  
> -            virCommandAddArg(cmd, "-device");
> -            if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps,
> -                                                     &usbcontroller)))
> +            if (qemuBuildControllerDevStr(def, cont, qemuCaps,
> +                                          &devstr, &usbcontroller) < 0)
>                  return -1;
> -            virCommandAddArg(cmd, devstr);
> -            VIR_FREE(devstr);
> +
> +            if (devstr) {
> +                virCommandAddArg(cmd, "-device");
> +                virCommandAddArg(cmd, devstr);
> +                VIR_FREE(devstr);
> +            }
>          }
>      }
>  
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 09cb00e..9a2ab29 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -118,10 +118,11 @@ char *qemuBuildDriveDevStr(const virDomainDef *def,
>                             virQEMUCapsPtr qemuCaps);
>  
>  /* Current, best practice */
> -char *qemuBuildControllerDevStr(const virDomainDef *domainDef,
> -                                virDomainControllerDefPtr def,
> -                                virQEMUCapsPtr qemuCaps,
> -                                int *nusbcontroller);
> +int qemuBuildControllerDevStr(const virDomainDef *domainDef,
> +                              virDomainControllerDefPtr def,
> +                              virQEMUCapsPtr qemuCaps,
> +                              char **devstr,
> +                              int *nusbcontroller);
>  
>  int qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>                                const char **backendType,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4a7d997..f910012 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -523,7 +523,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
>      if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller) < 0)
>          goto cleanup;
>  
> -    if (!(devstr = qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, NULL)))
> +    if (qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, &devstr, NULL) < 0)
> +        goto cleanup;
> +
> +    if (!devstr)
>          goto cleanup;
>  
>      if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0)
>