[v2] bhyve: TCP console support

[PATCH v2 5/5] bhyve: sync error messages

Posted by Roman Bogorodskiy 1 month, 3 weeks ago

Use the same error messages for serial devices validation which are
already used in bhyve_domain.c

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 src/bhyve/bhyve_command.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 18f65cf757..5757a41e7e 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -171,7 +171,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
         /* bhyve supports 4 ports: com1, com2, com3, com4 */
         if (chr->target.port > 3) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("only two serial ports are supported"));
+                           _("Only four serial ports are supported"));
             return -1;
         }
 
@@ -190,7 +190,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
             break;
         default:
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("only nmdm and tcp console types are supported"));
+                           _("Only 'nmdm' and 'tcp' console types are supported"));
             return -1;
         }
     }
-- 
2.49.0

Re: [PATCH v2 5/5] bhyve: sync error messages

Posted by Daniel P. Berrangé via Devel 1 month, 3 weeks ago

On Wed, Jul 16, 2025 at 06:28:47PM +0200, Roman Bogorodskiy wrote:
> Use the same error messages for serial devices validation which are
> already used in bhyve_domain.c
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
>  src/bhyve/bhyve_command.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 18f65cf757..5757a41e7e 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -171,7 +171,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
>          /* bhyve supports 4 ports: com1, com2, com3, com4 */
>          if (chr->target.port > 3) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("only two serial ports are supported"));
> +                           _("Only four serial ports are supported"));
>              return -1;
>          }
>  
> @@ -190,7 +190,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
>              break;
>          default:
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("only nmdm and tcp console types are supported"));
> +                           _("Only 'nmdm' and 'tcp' console types are supported"));
>              return -1;
>          }
>      }

FWIW, in the QEMU driver, when building the CLI, we make the assumption that
the domani validation has been performed, so we wouldn't repeat things like
the "chr->target.port > 3" check again.

With 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 v2 5/5] bhyve: sync error messages

Posted by Roman Bogorodskiy 1 month, 3 weeks ago

  Daniel P. Berrangé wrote:

> On Wed, Jul 16, 2025 at 06:28:47PM +0200, Roman Bogorodskiy wrote:
> > Use the same error messages for serial devices validation which are
> > already used in bhyve_domain.c
> > 
> > Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> > ---
> >  src/bhyve/bhyve_command.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> > 
> > diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> > index 18f65cf757..5757a41e7e 100644
> > --- a/src/bhyve/bhyve_command.c
> > +++ b/src/bhyve/bhyve_command.c
> > @@ -171,7 +171,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
> >          /* bhyve supports 4 ports: com1, com2, com3, com4 */
> >          if (chr->target.port > 3) {
> >              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -                           _("only two serial ports are supported"));
> > +                           _("Only four serial ports are supported"));
> >              return -1;
> >          }
> >  
> > @@ -190,7 +190,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommand *cmd)
> >              break;
> >          default:
> >              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > -                           _("only nmdm and tcp console types are supported"));
> > +                           _("Only 'nmdm' and 'tcp' console types are supported"));
> >              return -1;
> >          }
> >      }
> 
> FWIW, in the QEMU driver, when building the CLI, we make the assumption that
> the domani validation has been performed, so we wouldn't repeat things like
> the "chr->target.port > 3" check again.

Yes, I think I need to do the same. Not sure if it's better to do in one
shot or gradually move validation to bhyve_domain.c when touching
related areas. Probably doing in one shot is better for consistency of
the codebase.

> With 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 :|
>