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@xxxxxxxxx>
> @@ -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)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
