> That sounds like a good idea to me, but others may have different opinions.
My original idea was simply to allow DCD to track socket state, because this is what I need. The DTR idea came from the referenced bug. The feature defaults to disabled like many of the other edge cases (like telnet and tn3270), and it's a reasonably small non-intrusive change. > Can we make this generic over all chardevs by moving it to the base class? I'll take a closer look at the base class. I admit I did not spend much time studying it. This seemed like a socket specific feature to me. It seems like it might conflict with other chardevs like pty and serial as they are already using ioctl for interaction with real tty devices. > This feature will need some new tests in tests/test-char.c. I would be happy to add tests but would need some guidance. - Darrin On Thu, Dec 17, 2020 at 9:16 AM Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > Hi > > On Thu, Dec 17, 2020 at 2:54 AM Darrin M. Gorski <dar...@gorski.net> > wrote: > >> >> This patch adds a 'modemctl' option to "-chardev socket" to enable >> control of the socket via the guest serial port. >> The default state of the option is disabled. >> >> 1. disconnect a connected socket when DTR transitions to low, also reject >> new connections while DTR is low. >> 2. provide socket connection status through the carrier detect line (CD >> or DCD) on the guest serial port >> > > That sounds like a good idea to me, but others may have different opinions. > > >> Buglink: https://bugs.launchpad.net/qemu/+bug/1213196 >> >> Signed-off-by: Darrin M. Gorski <dar...@gorski.net> >> >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index 213a4c8dd0..94dd28e0cd 100644 >> --- a/chardev/char-socket.c >> +++ b/chardev/char-socket.c >> @@ -36,6 +36,7 @@ >> #include "qapi/qapi-visit-sockets.h" >> >> #include "chardev/char-io.h" >> +#include "chardev/char-serial.h" >> #include "qom/object.h" >> >> /***********************************************************/ >> @@ -85,6 +86,9 @@ struct SocketChardev { >> bool connect_err_reported; >> >> QIOTask *connect_task; >> + >> + bool is_modemctl; >> + int modem_state; >> > > Can we make this generic over all chardevs by moving it to the base class? > > }; >> typedef struct SocketChardev SocketChardev; >> >> @@ -98,12 +102,18 @@ static void tcp_chr_change_state(SocketChardev *s, >> TCPChardevState state) >> { >> switch (state) { >> case TCP_CHARDEV_STATE_DISCONNECTED: >> + if(s->is_modemctl) { >> + s->modem_state &= ~(CHR_TIOCM_CAR); >> + } >> break; >> case TCP_CHARDEV_STATE_CONNECTING: >> assert(s->state == TCP_CHARDEV_STATE_DISCONNECTED); >> break; >> case TCP_CHARDEV_STATE_CONNECTED: >> assert(s->state == TCP_CHARDEV_STATE_CONNECTING); >> + if(s->is_modemctl) { >> + s->modem_state |= CHR_TIOCM_CAR; >> + } >> break; >> } >> s->state = state; >> @@ -947,6 +957,12 @@ static void tcp_chr_accept(QIONetListener *listener, >> tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING); >> tcp_chr_set_client_ioc_name(chr, cioc); >> tcp_chr_new_client(chr, cioc); >> + >> + if(s->is_modemctl) { >> + if(!(s->modem_state & CHR_TIOCM_DTR)) { >> + tcp_chr_disconnect(chr); /* disconnect if DTR is low */ >> + } >> + } >> } >> >> >> @@ -1322,12 +1338,17 @@ static void qmp_chardev_open_socket(Chardev *chr, >> bool is_tn3270 = sock->has_tn3270 ? sock->tn3270 : false; >> bool is_waitconnect = sock->has_wait ? sock->wait : false; >> bool is_websock = sock->has_websocket ? sock->websocket : false; >> + bool is_modemctl = sock->has_modemctl ? sock->modemctl : false; >> int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; >> SocketAddress *addr; >> >> s->is_listen = is_listen; >> s->is_telnet = is_telnet; >> s->is_tn3270 = is_tn3270; >> + s->is_modemctl = is_modemctl; >> + if(is_modemctl) { >> + s->modem_state = CHR_TIOCM_CTS | CHR_TIOCM_DSR; >> + } >> s->is_websock = is_websock; >> s->do_nodelay = do_nodelay; >> if (sock->tls_creds) { >> @@ -1448,6 +1469,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, >> ChardevBackend *backend, >> sock->tls_creds = g_strdup(qemu_opt_get(opts, "tls-creds")); >> sock->has_tls_authz = qemu_opt_get(opts, "tls-authz"); >> sock->tls_authz = g_strdup(qemu_opt_get(opts, "tls-authz")); >> + sock->has_modemctl = qemu_opt_get(opts, "modemctl"); >> + sock->modemctl = qemu_opt_get_bool(opts, "modemctl", false); >> >> addr = g_new0(SocketAddressLegacy, 1); >> if (path) { >> @@ -1501,6 +1524,51 @@ char_socket_get_connected(Object *obj, Error >> **errp) >> return s->state == TCP_CHARDEV_STATE_CONNECTED; >> } >> >> +/* ioctl support: allow guest to control/track socket state >> + * via modem control lines (DCD/DTR) >> + */ >> +static int >> +char_socket_ioctl(Chardev *chr, int cmd, void *arg) >> +{ >> + SocketChardev *s = SOCKET_CHARDEV(chr); >> + >> + if(!(s->is_modemctl)) { >> + return -ENOTSUP; >> + } >> + >> + switch (cmd) { >> + case CHR_IOCTL_SERIAL_GET_TIOCM: >> + { >> + int *targ = (int *)arg; >> + *targ = s->modem_state; >> + } >> + break; >> + case CHR_IOCTL_SERIAL_SET_TIOCM: >> + { >> + int sarg = *(int *)arg; >> + if (sarg & CHR_TIOCM_RTS) { >> + s->modem_state |= CHR_TIOCM_RTS; >> + } else { >> + s->modem_state &= ~(CHR_TIOCM_RTS); >> + } >> + if (sarg & CHR_TIOCM_DTR) { >> + s->modem_state |= CHR_TIOCM_DTR; >> + } else { >> + s->modem_state &= ~(CHR_TIOCM_DTR); >> + /* disconnect if DTR goes low */ >> + if(s->state == TCP_CHARDEV_STATE_CONNECTED) { >> + tcp_chr_disconnect(chr); >> + } >> + } >> + } >> + break; >> + default: >> + return -ENOTSUP; >> + } >> + >> + return 0; >> +} >> + >> static void char_socket_class_init(ObjectClass *oc, void *data) >> { >> ChardevClass *cc = CHARDEV_CLASS(oc); >> @@ -1516,6 +1584,7 @@ static void char_socket_class_init(ObjectClass *oc, >> void *data) >> cc->chr_add_client = tcp_chr_add_client; >> cc->chr_add_watch = tcp_chr_add_watch; >> cc->chr_update_read_handler = tcp_chr_update_read_handler; >> + cc->chr_ioctl = char_socket_ioctl; >> >> object_class_property_add(oc, "addr", "SocketAddress", >> char_socket_get_addr, NULL, >> diff --git a/chardev/char.c b/chardev/char.c >> index a9b8c5a9aa..601d818f81 100644 >> --- a/chardev/char.c >> +++ b/chardev/char.c >> @@ -928,6 +928,9 @@ QemuOptsList qemu_chardev_opts = { >> },{ >> .name = "logappend", >> .type = QEMU_OPT_BOOL, >> + },{ >> + .name = "modemctl", >> + .type = QEMU_OPT_BOOL, >> #ifdef CONFIG_LINUX >> },{ >> .name = "tight", >> diff --git a/qapi/char.json b/qapi/char.json >> index 58338ed62d..f352bd6350 100644 >> --- a/qapi/char.json >> +++ b/qapi/char.json >> @@ -271,6 +271,9 @@ >> # then attempt a reconnect after the given number of seconds. >> # Setting this to zero disables this function. (default: 0) >> # (Since: 2.2) >> +# @modemctl: allow guest to use modem control signals to control/monitor >> +# the socket state (CD follows is_connected, DTR influences >> +# connect/accept) (default: false) (Since: 5.2) >> # >> # Since: 1.4 >> ## >> @@ -284,6 +287,7 @@ >> '*telnet': 'bool', >> '*tn3270': 'bool', >> '*websocket': 'bool', >> + '*modemctl': 'bool', >> '*reconnect': 'int' }, >> 'base': 'ChardevCommon' } >> >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 459c916d3d..f09072afbf 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -2991,11 +2991,13 @@ DEFHEADING(Character device options:) >> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, >> "-chardev help\n" >> "-chardev >> null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" >> - "-chardev >> socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n" >> - " >> [,server][,nowait][,telnet][,websocket][,reconnect=seconds][,mux=on|off]\n" >> - " >> [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n" >> - "-chardev >> socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,reconnect=seconds]\n" >> - " >> [,mux=on|off][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] >> (unix)\n" >> + "-chardev >> socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay]\n" >> + " >> [,reconnect=seconds][,server][,nowait][,telnet][,websocket]\n" >> + " >> [,modemctl][,mux=on|off][,logfile=PATH][,logappend=on|off]\n" >> + " [,tls-creds=ID][,tls-authz=ID] (tcp)\n" >> + "-chardev >> socket,id=id,path=path[,server][,nowait][,telnet][,websocket]\n" >> + " >> [,reconnect=seconds][,modemctl][,mux=on|off][,logfile=PATH]\n" >> + " [,logappend=on|off][,abstract=on|off][,tight=on|off] >> (unix)\n" >> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n" >> " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n" >> " [,logfile=PATH][,logappend=on|off]\n" >> > > This feature will need some new tests in tests/test-char.c. > > -- > Marc-André Lureau >
