On Fri, 2021-08-27 at 17:38 +0100, Daniel P. Berrangé wrote:
On Thu, Aug 26, 2021 at 02:59:19PM -0700, 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.
The devices themselves still aren't allowed in configurations yet though.
Signed-off-by: William Douglas <william.douglas@intel.com> --- src/ch/ch_monitor.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 28d1c213cc..1ff956b61e 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -89,6 +89,59 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef) return -1; } +static int +virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef) +{ + virJSONValue *ptys = virJSONValueNewObject(); + + 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")); + return -1; + } else if (vmdef->nconsoles > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only a single console can be configured for this domain")); + return -1; + } else if (vmdef->nserials > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only a single serial can be configured for this domain")); + return -1; + }
IIRC, we'd recommend that these checks go into the 'deviceValidateCallback' impl that you have in ch_domain.c, so that they get run at time the user loads the XML into libvirt, rather than when the Vm is started.
Looking a little at the qemu code I was curious if this should be in the domainValidateCallback instead?
The code below can still safely assume the checks have been done by the validate function.
+ + if (vmdef->nconsoles) { + g_autoptr(virJSONValue) pty = virJSONValueNewObject(); + if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) { + if (virJSONValueObjectAppendString(pty, "mode", "Pty") < 0) + return -1; + if (virJSONValueObjectAppend(content, "console", &pty) < 0) + return -1; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Console can only be enabled for a PTY")); + return -1; + }
The check for type != PTY ought to be in the above validation block too, so this code then just assumes PTY.
+ } + + if (vmdef->nserials) { + g_autoptr(virJSONValue) pty = virJSONValueNewObject(); + if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) { + if (virJSONValueObjectAppendString(ptys, "mode", "Pty") < 0) + return -1; + if (virJSONValueObjectAppend(content, "serial", &pty) < 0) + return -1; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Serial can only be enabled for a PTY")); + return -1; + } + } + + return 0; +} + static int virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef) { @@ -370,6 +423,9 @@ virCHMonitorBuildVMJson(virDomainDef *vmdef, char **jsonstr) goto cleanup; } + if (virCHMonitorBuildPTYJson(content, vmdef) < 0) + goto cleanup; + if (virCHMonitorBuildCPUJson(content, vmdef) < 0) goto cleanup; -- 2.31.1
Regards, Daniel