On 6/30/21 1:05 AM, William Douglas wrote:
> Add function to build the the json structure to configure a PTY in
> cloud-hypervisor. The configuration only supports a single serial or
> console device.
>
> Signed-off-by: William Douglas <william.douglas@xxxxxxxxx>
> ---
> src/ch/ch_monitor.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 5e7d6e3189..d4289b75ce 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -89,6 +89,65 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
> return -1;
> }
>
> +static int
> +virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef)
> +{
> + virJSONValue *ptyc = virJSONValueNewObject();
> + virJSONValue *ptys = virJSONValueNewObject();
> +
> + if (vmdef->nconsoles || vmdef->nserials) {
This ^^^ looks redundant since you explicitly check for problematic
configuration below.
> + if ((vmdef->nconsoles &&
> + vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)
> + && (vmdef->nserials &&
> + vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single console or serial can be configured for this domain"));
> + goto cleanup;
> + } else if (vmdef->nconsoles > 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single console can be configured for this domain"));
> + goto cleanup;
> + } else if (vmdef->nserials > 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single serial can be configured for this domain"));
> + goto cleanup;
> + }
> + }
> +
> + if (vmdef->nconsoles) {
> + if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
> + if (virJSONValueObjectAppendString(ptyc, "mode", "Pty") < 0)
> + goto cleanup;
> + if (virJSONValueObjectAppend(content, "console", &ptyc) < 0)
> + goto cleanup;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Console can only be enabled for a PTY"));
> + goto cleanup;
> + }
> + }
> +
> + if (vmdef->nserials) {
> + if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
> + if (virJSONValueObjectAppendString(ptys, "mode", "Pty") < 0)
> + goto cleanup;
> + if (virJSONValueObjectAppend(content, "serial", &ptys) < 0)
> + goto cleanup;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Serial can only be enabled for a PTY"));
> + goto cleanup;
> + }
> + }
> +
> + return 0;
> +
> + cleanup:
> + virJSONValueFree(ptyc);
> + virJSONValueFree(ptys);
You can avoid these explicit calls to virJSONValueFree() if you declare
those variables as g_autoptr() like this:
g_autoptr(virJSONValue) ptyc = NULL;
g_autoptr(virJSONValue) pyts = NULL;
and you can even do that in the inner most blocks where the vars are used.
> + return -1;
Then, this return -1 can be at all those places where 'goto cleanup' is.
> +}
> +
> static int
> virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
> {
>
Michal
