1. Patchew
  2. Libvirt
  3. [PATCH v3 0/8] ch: Add Console support
  4. View patch

[PATCH 5/8] ch_monitor: Add pty json builder function

William Douglas posted 8 patches 4 years, 5 months ago
There is a newer version of this series
[PATCH 5/8] ch_monitor: Add pty json builder function
Posted by William Douglas 4 years, 5 months ago 
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;
+    }
+
+    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;
+        }
+    }
+
+    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
Re: [PATCH 5/8] ch_monitor: Add pty json builder function
Posted by Daniel P. Berrangé 4 years, 5 months ago 
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.

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 5/8] ch_monitor: Add pty json builder function
Posted by Douglas, William 4 years, 5 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.