Re: [PATCH 04/10] ch_monitor: Add pty json builder function

2021-07-12 Thread Michal Prívozník
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 
> ---
>  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", ) < 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", ) < 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



[PATCH 04/10] ch_monitor: Add pty json builder function

2021-06-29 Thread William Douglas
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 
---
 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) {
+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", ) < 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", ) < 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);
+return -1;
+}
+
 static int
 virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
 {
-- 
2.31.1