>> Yes, but they can override the default behaviour, or just reuse some code. >> If it doesn't fit, don't worry about it, we can just start with socket.
Whew. I spent several hours yesterday trying to figure out how to alter the base class to support this - DCD could be reasonably simple, but DTR is another matter. I will continue to look at that, but I do think moving forward with sockets (since this patch already works that way) is a good compromise. > You can look at char_socket_server_test. Will do. - Darrin On Fri, Dec 18, 2020 at 6:57 AM Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > Hi > > On Thu, Dec 17, 2020 at 9:54 PM Darrin M. Gorski <dar...@gorski.net> > wrote: > >> >> > 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. >> > > Yes, but they can override the default behaviour, or just reuse some code. > > If it doesn't fit, don't worry about it, we can just start with socket. > > >> > This feature will need some new tests in tests/test-char.c. >> >> I would be happy to add tests but would need some guidance. >> > > You can look at char_socket_server_test. You can extend > CharSocketServerTestConfig to check for modemctl behaviour. It will need to > send the ioctl with qemu_chr_fe_ioctl (I admit it wasn't tested so far, > because only serial and parallel chardev did implement it, and it's not > easy to test those) > > Let me know if you need more help > > >> - 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 >>> >> > > -- > Marc-André Lureau >
