Re: [PATCH] qga: set FreeBSD virtio-serial channel to raw mode

[Roman Bogorodskiy <bogorodskiy@gmail.com>]

  Oleksij Samorukov wrote:

> On FreeBSD the virtio-console port is a tty, unlike Linux's
> /dev/virtio-ports which is a character device.  It defaults to canonical
> mode with echo and software flow control enabled, so the QGA stream
> protocol does not work: the initial guest-sync handshake is held in the
> line discipline and the host eventually times out.
> 
> The existing FreeBSD-specific code only clears ECHO.  Configure the port
> the same way as the ISA serial path: disable canonical mode, echo and
> software flow control, and set VMIN/VTIME.  Use TCSANOW instead of
> TCSAFLUSH so a handshake that arrives between open() and tcsetattr() is
> not discarded.
> 
> Tested on FreeBSD 15.1 aarch64 under UTM/QEMU.  Before the change
> utmctl ip-address timed out and the guest received zero bytes; after the
> change QGA commands and guest-set-time work.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: c6cd588bb3a29a831c862780631a7d2145ade5de ("qga: Add initial FreeBSD support")
> Signed-off-by: Oleksij Samorukov <samm@net-art.cz>
> ---
>  qga/channel-posix.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index 9ccc8b7..c6dfb2d 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -156,9 +156,10 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>  #endif
>  #ifdef __FreeBSD__
>          /*
> -         * In the default state channel sends echo of every command to a
> -         * client. The client program doesn't expect this and raises an
> -         * error. Suppress echo by resetting ECHO terminal flag.
> +         * FreeBSD's virtio-console port is a tty (unlike Linux's
> +         * /dev/virtio-ports, which is a character device) and defaults to
> +         * canonical mode with echo and software flow control enabled.
> +         * QGA uses a raw stream protocol, so put the port in raw mode.
>           */
>          struct termios tio;
>          if (tcgetattr(fd, &tio) < 0) {
> @@ -166,8 +167,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>              close(fd);
>              return false;
>          }
> -        tio.c_lflag &= ~ECHO;
> -        if (tcsetattr(fd, TCSAFLUSH, &tio) < 0) {
> +        tio.c_iflag &= ~(IGNBRK | BRKINT | IGNPAR | PARMRK | INPCK | ISTRIP |
> +                         INLCR | IGNCR | ICRNL | IXON | IXOFF | IXANY |
> +                         IMAXBEL);
> +        tio.c_oflag = 0;
> +        tio.c_lflag = 0;
> +        tio.c_cflag |= CLOCAL | CREAD;
> +        tio.c_cc[VMIN] = 1;
> +        tio.c_cc[VTIME] = 0;
> +        /*
> +         * Use TCSANOW rather than TCSAFLUSH: the host may already have sent
> +         * the initial handshake between open() and tcsetattr().
> +         */
> +        if (tcsetattr(fd, TCSANOW, &tio) < 0) {
>              error_setg_errno(errp, errno, "error setting channel termios attrs");
>              close(fd);
>              return false;
> -- 
> 2.54.0 (Apple Git-157)
> 

Interestingly, the issue does not apply to libvirt (bhyve and qemu
drivers) talking to the guest agent, likely because libvirt terminates
guest agent commands with '\n'.

Either way, I have tested this change on:

 - FreeBSD 16-CURRENT amd64 guest under libvirt/qemu
 - FreeBSD 14.4 amd64 guest under libvirt/bhyve
 - FreeBSD 15.0 aarch64 guest under libvirt/bhyve

and did not notice any regressions.

Tested-by: Roman Bogorodskiy <bogorodskiy@gmail.com>