| Message ID | 20181218100002.11219-2-xieyongji@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-user-blk: Add support for backend reconnecting | expand |
Hi On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > From: Xie Yongji <xieyongji@baidu.com> > > New option "disconnected" is added to init the chardev socket > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > to connect when necessary. Now it would be used for unix domain > socket of vhost-user-blk device to support reconnect. What difference does that make if you wait for connection in qemu_chr_fe_wait_connected(), or during chardev setup? "disconnected" is misleading, would it be possible to reuse "wait/nowait" instead? Tests would be welcome. > > Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru> > Signed-off-by: Xie Yongji <xieyongji@baidu.com> > Signed-off-by: Zhang Yu <zhangyu31@baidu.com> > --- > chardev/char-socket.c | 10 ++++++++++ > chardev/char.c | 3 +++ > qapi/char.json | 3 +++ > qemu-options.hx | 28 ++++++++++++++++------------ > 4 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index eaa8e8b68f..2464d7abad 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -70,6 +70,7 @@ typedef struct { > TCPChardevTelnetInit *telnet_init; > > bool is_websock; > + bool is_disconnected; > > GSource *reconnect_timer; > int64_t reconnect_time; > @@ -1000,6 +1001,7 @@ 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_disconnected = sock->has_disconnected ? sock->disconnected : false; > int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; > QIOChannelSocket *sioc = NULL; > SocketAddress *addr; > @@ -1072,6 +1074,10 @@ static void qmp_chardev_open_socket(Chardev *chr, > s->reconnect_time = reconnect; > } > > + if (!s->is_listen && is_disconnected) { > + return; > + } > + > if (s->reconnect_time) { > tcp_chr_connect_async(chr); > } else { > @@ -1125,6 +1131,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > bool is_tn3270 = qemu_opt_get_bool(opts, "tn3270", false); > bool is_websock = qemu_opt_get_bool(opts, "websocket", false); > bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true); > + bool is_disconnected = !is_listen && > + qemu_opt_get_bool(opts, "disconnected", false); > int64_t reconnect = qemu_opt_get_number(opts, "reconnect", 0); > const char *path = qemu_opt_get(opts, "path"); > const char *host = qemu_opt_get(opts, "host"); > @@ -1176,6 +1184,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > sock->websocket = is_websock; > sock->has_wait = true; > sock->wait = is_waitconnect; > + sock->has_disconnected = true; > + sock->disconnected = is_disconnected; > sock->has_reconnect = qemu_opt_find(opts, "reconnect"); > sock->reconnect = reconnect; > sock->tls_creds = g_strdup(tls_creds); > diff --git a/chardev/char.c b/chardev/char.c > index ccba36bafb..fb916eb176 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -862,6 +862,9 @@ QemuOptsList qemu_chardev_opts = { > },{ > .name = "delay", > .type = QEMU_OPT_BOOL, > + },{ > + .name = "disconnected", > + .type = QEMU_OPT_BOOL, > },{ > .name = "reconnect", > .type = QEMU_OPT_NUMBER, > diff --git a/qapi/char.json b/qapi/char.json > index 77ed847972..b634442229 100644 > --- a/qapi/char.json > +++ b/qapi/char.json > @@ -258,6 +258,8 @@ > # sockets (default: false) (Since: 2.10) > # @websocket: enable websocket protocol on server > # sockets (default: false) (Since: 3.1) > +# @disconnected: init a client socket in disconnected > +# state (default: false) (Since: 4.0) > # @reconnect: For a client socket, if a socket is disconnected, > # then attempt a reconnect after the given number of seconds. > # Setting this to zero disables this function. (default: 0) > @@ -274,6 +276,7 @@ > '*telnet': 'bool', > '*tn3270': 'bool', > '*websocket': 'bool', > + '*disconnected': 'bool', > '*reconnect': 'int' }, > 'base': 'ChardevCommon' } > > diff --git a/qemu-options.hx b/qemu-options.hx > index df42116ecc..f9d3495dc8 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2418,10 +2418,10 @@ 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] (tcp)\n" > - "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,reconnect=seconds]\n" > - " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n" > + " [,server][,nowait][,telnet][,websocket][,disconnected][,reconnect=seconds]\n" > + " [,mux=on|off][,logfile=PATH][,logappend=on|off][,tls-creds=ID] (tcp)\n" > + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,disconnected]\n" > + " [,reconnect=seconds][,mux=on|off][,logfile=PATH][,logappend=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" > @@ -2548,7 +2548,7 @@ The available backends are: > A void device. This device will not emit any data, and will drop any data it > receives. The null backend does not take any options. > > -@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}] > +@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,disconnected][,reconnect=@var{seconds}][,tls-creds=@var{id}] > > Create a two-way stream socket, which can be either a TCP or a unix socket. A > unix socket will be created if @option{path} is specified. Behaviour is > @@ -2565,6 +2565,9 @@ escape sequences. > @option{websocket} specifies that the socket uses WebSocket protocol for > communication. > > +@option{disconnected} specifies that the non-server socket should not try > +to connect the remote during initialization. > + > @option{reconnect} sets the timeout for reconnecting on non-server sockets when > the remote end goes away. qemu will delay this many seconds and then attempt > to reconnect. Zero disables reconnecting, and is the default. > @@ -3087,18 +3090,19 @@ telnet on port 5555 to access the QEMU port. > localhost 5555 > @end table > > -@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,reconnect=@var{seconds}] > +@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,disconnected] > +[,reconnect=@var{seconds}] > The TCP Net Console has two modes of operation. It can send the serial > I/O to a location or wait for a connection from a location. By default > the TCP Net Console is sent to @var{host} at the @var{port}. If you use > the @var{server} option QEMU will wait for a client socket application > to connect to the port before continuing, unless the @code{nowait} > option was specified. The @code{nodelay} option disables the Nagle buffering > -algorithm. The @code{reconnect} option only applies if @var{noserver} is > -set, if the connection goes down it will attempt to reconnect at the > -given interval. If @var{host} is omitted, 0.0.0.0 is assumed. Only > -one TCP connection at a time is accepted. You can use @code{telnet} to > -connect to the corresponding character device. > +algorithm. If @var{noserver} is specified, the @code{disconnected} will disallow > +QEMU to connect during initialization, and the @code{reconnect} will ask QEMU > +to reconnect at the given interval when the connection goes down. If @var{host} > +is omitted, 0.0.0.0 is assumed. Only one TCP connection at a time is accepted. > +You can use @code{telnet} to connect to the corresponding character device. > @table @code > @item Example to send tcp console to 192.168.0.2 port 4444 > -serial tcp:192.168.0.2:4444 > @@ -3121,7 +3125,7 @@ type "send break" followed by pressing the enter key. > The WebSocket protocol is used instead of raw tcp socket. The port acts as > a WebSocket server. Client mode is not supported. > > -@item unix:@var{path}[,server][,nowait][,reconnect=@var{seconds}] > +@item unix:@var{path}[,server][,nowait][,disconnected][,reconnect=@var{seconds}] > A unix domain socket is used instead of a tcp socket. The option works the > same as if you had specified @code{-serial tcp} except the unix domain socket > @var{path} is used for connections. > -- > 2.17.1 >
On Tue, 18 Dec 2018 at 20:24, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > Hi > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > New option "disconnected" is added to init the chardev socket > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > to connect when necessary. Now it would be used for unix domain > > socket of vhost-user-blk device to support reconnect. > > What difference does that make if you wait for connection in > qemu_chr_fe_wait_connected(), or during chardev setup? > Wait for connection in qemu_chr_fe_wait_connected() could make it possible to start qemu before backend. Maybe we need to support this case? Thanks, Yongji
On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > Hi > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > New option "disconnected" is added to init the chardev socket > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > to connect when necessary. Now it would be used for unix domain > > socket of vhost-user-blk device to support reconnect. > > What difference does that make if you wait for connection in > qemu_chr_fe_wait_connected(), or during chardev setup? > > "disconnected" is misleading, would it be possible to reuse > "wait/nowait" instead? Currently we default to doing a blocking connect in foreground, except if reconnect is non-zero, in which case we do a connect async in the background. This "disconnected" proposal effectively does a blocking connect, but delayed to later in startup. IOW, this could already be achieved if "reconnect" were set to non-zero. If the usage doesn't want reconnect though, I tend to agree that we should use the exisiting wait/nowait options to let it be controlled. I don't see that this "disconnected" option gives a compelling benefit over using wait/nowait. Regards, Daniel
On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > Hi > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > New option "disconnected" is added to init the chardev socket > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > to connect when necessary. Now it would be used for unix domain > > > socket of vhost-user-blk device to support reconnect. > > > > What difference does that make if you wait for connection in > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > "disconnected" is misleading, would it be possible to reuse > > "wait/nowait" instead? > > Currently we default to doing a blocking connect in foreground, > except if reconnect is non-zero, in which case we do a connect > async in the background. This "disconnected" proposal effectively > does a blocking connect, but delayed to later in startup. > > IOW, this could already be achieved if "reconnect" were set to > non-zero. If the usage doesn't want reconnect though, I tend > to agree that we should use the exisiting wait/nowait options > to let it be controlled. I don't see that this "disconnected" > option gives a compelling benefit over using wait/nowait. > > > Regards, > Daniel So the tricky thing is that we can not expose the device to guest until we get a connection and can query it for the first time. After that is completed, we can reasonably support disconnected operation at least for net. We could do hotplug at time of connect I guess ... > -- > |: 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 :|
On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > Hi > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > to connect when necessary. Now it would be used for unix domain > > > > socket of vhost-user-blk device to support reconnect. > > > > > > What difference does that make if you wait for connection in > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > "disconnected" is misleading, would it be possible to reuse > > > "wait/nowait" instead? > > > > Currently we default to doing a blocking connect in foreground, > > except if reconnect is non-zero, in which case we do a connect > > async in the background. This "disconnected" proposal effectively > > does a blocking connect, but delayed to later in startup. > > > > IOW, this could already be achieved if "reconnect" were set to > > non-zero. If the usage doesn't want reconnect though, I tend > > to agree that we should use the exisiting wait/nowait options > > to let it be controlled. I don't see that this "disconnected" > > option gives a compelling benefit over using wait/nowait. > > So the tricky thing is that we can not expose the > device to guest until we get a connection and can query > it for the first time. After that is completed, > we can reasonably support disconnected operation at least for net. The device code would still use qemu_chr_fe_wait_connected() in my proposal, so that its no different to the situation with the proposed "disconnected" flag. Regards, Daniel
On Wed, 19 Dec 2018 at 00:09, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > Hi > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > to connect when necessary. Now it would be used for unix domain > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > What difference does that make if you wait for connection in > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > "wait/nowait" instead? > > > > > > Currently we default to doing a blocking connect in foreground, > > > except if reconnect is non-zero, in which case we do a connect > > > async in the background. This "disconnected" proposal effectively > > > does a blocking connect, but delayed to later in startup. > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > non-zero. If the usage doesn't want reconnect though, I tend > > > to agree that we should use the exisiting wait/nowait options > > > to let it be controlled. I don't see that this "disconnected" > > > option gives a compelling benefit over using wait/nowait. > > > > So the tricky thing is that we can not expose the > > device to guest until we get a connection and can query > > it for the first time. After that is completed, > > we can reasonably support disconnected operation at least for net. > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > so that its no different to the situation with the proposed "disconnected" > flag. > I considered resuing wait/nowait before. One problem is that we will have a thread to do connect async if backend is not started during chardev setup. Then if we use qemu_chr_fe_wait_connected() manually to connect in device code, we may have some conflict in main thread like: qemu_chr_fe_wait_connected() tcp_chr_wait_connected() tcp_chr_connect() s->connected = 1; qemu_chr_socket_connected() check_report_connect_error() qemu_chr_socket_restart_timer() assert(s->connected == 0) /* qemu crash */ Thanks, Yongji
On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > Hi > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > to connect when necessary. Now it would be used for unix domain > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > What difference does that make if you wait for connection in > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > "wait/nowait" instead? > > > > > > Currently we default to doing a blocking connect in foreground, > > > except if reconnect is non-zero, in which case we do a connect > > > async in the background. This "disconnected" proposal effectively > > > does a blocking connect, but delayed to later in startup. > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > non-zero. If the usage doesn't want reconnect though, I tend > > > to agree that we should use the exisiting wait/nowait options > > > to let it be controlled. I don't see that this "disconnected" > > > option gives a compelling benefit over using wait/nowait. > > > > So the tricky thing is that we can not expose the > > device to guest until we get a connection and can query > > it for the first time. After that is completed, > > we can reasonably support disconnected operation at least for net. > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > so that its no different to the situation with the proposed "disconnected" > flag. > > Regards, > Daniel I guess so, but wouldn't it be confusing to users that one says "nowait" and qemu still waits for a connection and does not run the VM? That's different from how nowait behaves normally. > -- > |: 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 :|
On Wed, Dec 19, 2018 at 10:55:09AM -0500, Michael S. Tsirkin wrote: > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > > Hi > > > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > > to connect when necessary. Now it would be used for unix domain > > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > > > What difference does that make if you wait for connection in > > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > > "wait/nowait" instead? > > > > > > > > Currently we default to doing a blocking connect in foreground, > > > > except if reconnect is non-zero, in which case we do a connect > > > > async in the background. This "disconnected" proposal effectively > > > > does a blocking connect, but delayed to later in startup. > > > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > > non-zero. If the usage doesn't want reconnect though, I tend > > > > to agree that we should use the exisiting wait/nowait options > > > > to let it be controlled. I don't see that this "disconnected" > > > > option gives a compelling benefit over using wait/nowait. > > > > > > So the tricky thing is that we can not expose the > > > device to guest until we get a connection and can query > > > it for the first time. After that is completed, > > > we can reasonably support disconnected operation at least for net. > > > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > > so that its no different to the situation with the proposed "disconnected" > > flag. > > > > Regards, > > Daniel > > I guess so, but wouldn't it be confusing to users that one says > "nowait" and qemu still waits for a connection and does not > run the VM? That's different from how nowait behaves normally. Well "nowait" is only referring to the chardev behaviour which is still an accurate description. The fact that the network then waits during startup is a seperate matter. We don't define chardev semantics in terms of possible users as there are many distinct users of chardevs in QEMU. Regards, Daniel
On Wed, Dec 19, 2018 at 04:01:02PM +0000, Daniel P. Berrangé wrote: > On Wed, Dec 19, 2018 at 10:55:09AM -0500, Michael S. Tsirkin wrote: > > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > > > Hi > > > > > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > > > to connect when necessary. Now it would be used for unix domain > > > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > > > > > What difference does that make if you wait for connection in > > > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > > > "wait/nowait" instead? > > > > > > > > > > Currently we default to doing a blocking connect in foreground, > > > > > except if reconnect is non-zero, in which case we do a connect > > > > > async in the background. This "disconnected" proposal effectively > > > > > does a blocking connect, but delayed to later in startup. > > > > > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > > > non-zero. If the usage doesn't want reconnect though, I tend > > > > > to agree that we should use the exisiting wait/nowait options > > > > > to let it be controlled. I don't see that this "disconnected" > > > > > option gives a compelling benefit over using wait/nowait. > > > > > > > > So the tricky thing is that we can not expose the > > > > device to guest until we get a connection and can query > > > > it for the first time. After that is completed, > > > > we can reasonably support disconnected operation at least for net. > > > > > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > > > so that its no different to the situation with the proposed "disconnected" > > > flag. > > > > > > Regards, > > > Daniel > > > > I guess so, but wouldn't it be confusing to users that one says > > "nowait" and qemu still waits for a connection and does not > > run the VM? That's different from how nowait behaves normally. > > Well "nowait" is only referring to the chardev behaviour which is > still an accurate description. man page seems to say nowait specifies that QEMU should not block waiting for a client to connect to a listening socket. if we do wait for a client to connect then how is it accurate? > The fact that the network then waits during startup is a seperate > matter. We don't define chardev semantics in terms of possible > users as there are many distinct users of chardevs in QEMU. > > 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 :|
On Wed, Dec 19, 2018 at 11:50:38AM -0500, Michael S. Tsirkin wrote: > On Wed, Dec 19, 2018 at 04:01:02PM +0000, Daniel P. Berrangé wrote: > > On Wed, Dec 19, 2018 at 10:55:09AM -0500, Michael S. Tsirkin wrote: > > > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > > > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > > > > Hi > > > > > > > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > > > > to connect when necessary. Now it would be used for unix domain > > > > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > > > > > > > What difference does that make if you wait for connection in > > > > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > > > > "wait/nowait" instead? > > > > > > > > > > > > Currently we default to doing a blocking connect in foreground, > > > > > > except if reconnect is non-zero, in which case we do a connect > > > > > > async in the background. This "disconnected" proposal effectively > > > > > > does a blocking connect, but delayed to later in startup. > > > > > > > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > > > > non-zero. If the usage doesn't want reconnect though, I tend > > > > > > to agree that we should use the exisiting wait/nowait options > > > > > > to let it be controlled. I don't see that this "disconnected" > > > > > > option gives a compelling benefit over using wait/nowait. > > > > > > > > > > So the tricky thing is that we can not expose the > > > > > device to guest until we get a connection and can query > > > > > it for the first time. After that is completed, > > > > > we can reasonably support disconnected operation at least for net. > > > > > > > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > > > > so that its no different to the situation with the proposed "disconnected" > > > > flag. > > > > > > > > Regards, > > > > Daniel > > > > > > I guess so, but wouldn't it be confusing to users that one says > > > "nowait" and qemu still waits for a connection and does not > > > run the VM? That's different from how nowait behaves normally. > > > > Well "nowait" is only referring to the chardev behaviour which is > > still an accurate description. > > man page seems to say > > nowait specifies that QEMU should not block waiting > for a client to connect to a listening socket. > > if we do wait for a client to connect then how is it accurate? Well the manpage needs to be clarified that this applies to the initialization of the chardev. What a downstream objet/device does with the chardev is outside the scope of chardev config options. Regards, Daniel
On Wed, Dec 19, 2018 at 05:09:09PM +0000, Daniel P. Berrangé wrote: > On Wed, Dec 19, 2018 at 11:50:38AM -0500, Michael S. Tsirkin wrote: > > On Wed, Dec 19, 2018 at 04:01:02PM +0000, Daniel P. Berrangé wrote: > > > On Wed, Dec 19, 2018 at 10:55:09AM -0500, Michael S. Tsirkin wrote: > > > > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > > > > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > > > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > > > > > Hi > > > > > > > > > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > > > > > to connect when necessary. Now it would be used for unix domain > > > > > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > > > > > > > > > What difference does that make if you wait for connection in > > > > > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > > > > > "wait/nowait" instead? > > > > > > > > > > > > > > Currently we default to doing a blocking connect in foreground, > > > > > > > except if reconnect is non-zero, in which case we do a connect > > > > > > > async in the background. This "disconnected" proposal effectively > > > > > > > does a blocking connect, but delayed to later in startup. > > > > > > > > > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > > > > > non-zero. If the usage doesn't want reconnect though, I tend > > > > > > > to agree that we should use the exisiting wait/nowait options > > > > > > > to let it be controlled. I don't see that this "disconnected" > > > > > > > option gives a compelling benefit over using wait/nowait. > > > > > > > > > > > > So the tricky thing is that we can not expose the > > > > > > device to guest until we get a connection and can query > > > > > > it for the first time. After that is completed, > > > > > > we can reasonably support disconnected operation at least for net. > > > > > > > > > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > > > > > so that its no different to the situation with the proposed "disconnected" > > > > > flag. > > > > > > > > > > Regards, > > > > > Daniel > > > > > > > > I guess so, but wouldn't it be confusing to users that one says > > > > "nowait" and qemu still waits for a connection and does not > > > > run the VM? That's different from how nowait behaves normally. > > > > > > Well "nowait" is only referring to the chardev behaviour which is > > > still an accurate description. > > > > man page seems to say > > > > nowait specifies that QEMU should not block waiting > > for a client to connect to a listening socket. > > > > if we do wait for a client to connect then how is it accurate? > > Well the manpage needs to be clarified that this applies to the > initialization of the chardev. What a downstream objet/device > does with the chardev is outside the scope of chardev config > options. > > Regards, > Daniel Practically qemu blocks. I suspect the distinction which object within qemu does it will be lost on users. And it's not exactly about waiting anyway. It's about whether we should stop the VM on disconnect which I feel is different. So I think we need a new flag along the lines of disconnect=ignore/exit/stop with proper documentation. > -- > |: 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 :|
On Wed, 19 Dec 2018 at 23:55, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote: > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote: > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote: > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote: > > > > > Hi > > > > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <elohimes@gmail.com> wrote: > > > > > > > > > > > > From: Xie Yongji <xieyongji@baidu.com> > > > > > > > > > > > > New option "disconnected" is added to init the chardev socket > > > > > > in disconnected state. Then we can use qemu_chr_fe_wait_connected() > > > > > > to connect when necessary. Now it would be used for unix domain > > > > > > socket of vhost-user-blk device to support reconnect. > > > > > > > > > > What difference does that make if you wait for connection in > > > > > qemu_chr_fe_wait_connected(), or during chardev setup? > > > > > > > > > > "disconnected" is misleading, would it be possible to reuse > > > > > "wait/nowait" instead? > > > > > > > > Currently we default to doing a blocking connect in foreground, > > > > except if reconnect is non-zero, in which case we do a connect > > > > async in the background. This "disconnected" proposal effectively > > > > does a blocking connect, but delayed to later in startup. > > > > > > > > IOW, this could already be achieved if "reconnect" were set to > > > > non-zero. If the usage doesn't want reconnect though, I tend > > > > to agree that we should use the exisiting wait/nowait options > > > > to let it be controlled. I don't see that this "disconnected" > > > > option gives a compelling benefit over using wait/nowait. > > > > > > So the tricky thing is that we can not expose the > > > device to guest until we get a connection and can query > > > it for the first time. After that is completed, > > > we can reasonably support disconnected operation at least for net. > > > > The device code would still use qemu_chr_fe_wait_connected() in my proposal, > > so that its no different to the situation with the proposed "disconnected" > > flag. > > > > Regards, > > Daniel > > I guess so, but wouldn't it be confusing to users that one says > "nowait" and qemu still waits for a connection and does not > run the VM? That's different from how nowait behaves normally. > With this patch: diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 2464d7a..ffe3a60 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -97,7 +97,10 @@ static void qemu_chr_socket_restart_timer(Chardev *chr) SocketChardev *s = SOCKET_CHARDEV(chr); char *name; - assert(s->connected == 0); + if (s->connected) { + return; + } + name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label); s->reconnect_timer = qemu_chr_timeout_add_ms(chr, s->reconnect_time * 1000, We can use qemu_chr_fe_wait_connected() without adding redundant "wait" or "disconnected" option. The disadavantage is that backend may have two connection from qemu during initialization. But I think backend should support this case. Thanks, Yongji
diff --git a/chardev/char-socket.c b/chardev/char-socket.c index eaa8e8b68f..2464d7abad 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -70,6 +70,7 @@ typedef struct { TCPChardevTelnetInit *telnet_init; bool is_websock; + bool is_disconnected; GSource *reconnect_timer; int64_t reconnect_time; @@ -1000,6 +1001,7 @@ 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_disconnected = sock->has_disconnected ? sock->disconnected : false; int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; QIOChannelSocket *sioc = NULL; SocketAddress *addr; @@ -1072,6 +1074,10 @@ static void qmp_chardev_open_socket(Chardev *chr, s->reconnect_time = reconnect; } + if (!s->is_listen && is_disconnected) { + return; + } + if (s->reconnect_time) { tcp_chr_connect_async(chr); } else { @@ -1125,6 +1131,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, bool is_tn3270 = qemu_opt_get_bool(opts, "tn3270", false); bool is_websock = qemu_opt_get_bool(opts, "websocket", false); bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true); + bool is_disconnected = !is_listen && + qemu_opt_get_bool(opts, "disconnected", false); int64_t reconnect = qemu_opt_get_number(opts, "reconnect", 0); const char *path = qemu_opt_get(opts, "path"); const char *host = qemu_opt_get(opts, "host"); @@ -1176,6 +1184,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, sock->websocket = is_websock; sock->has_wait = true; sock->wait = is_waitconnect; + sock->has_disconnected = true; + sock->disconnected = is_disconnected; sock->has_reconnect = qemu_opt_find(opts, "reconnect"); sock->reconnect = reconnect; sock->tls_creds = g_strdup(tls_creds); diff --git a/chardev/char.c b/chardev/char.c index ccba36bafb..fb916eb176 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -862,6 +862,9 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "delay", .type = QEMU_OPT_BOOL, + },{ + .name = "disconnected", + .type = QEMU_OPT_BOOL, },{ .name = "reconnect", .type = QEMU_OPT_NUMBER, diff --git a/qapi/char.json b/qapi/char.json index 77ed847972..b634442229 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -258,6 +258,8 @@ # sockets (default: false) (Since: 2.10) # @websocket: enable websocket protocol on server # sockets (default: false) (Since: 3.1) +# @disconnected: init a client socket in disconnected +# state (default: false) (Since: 4.0) # @reconnect: For a client socket, if a socket is disconnected, # then attempt a reconnect after the given number of seconds. # Setting this to zero disables this function. (default: 0) @@ -274,6 +276,7 @@ '*telnet': 'bool', '*tn3270': 'bool', '*websocket': 'bool', + '*disconnected': 'bool', '*reconnect': 'int' }, 'base': 'ChardevCommon' } diff --git a/qemu-options.hx b/qemu-options.hx index df42116ecc..f9d3495dc8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2418,10 +2418,10 @@ 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] (tcp)\n" - "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,reconnect=seconds]\n" - " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n" + " [,server][,nowait][,telnet][,websocket][,disconnected][,reconnect=seconds]\n" + " [,mux=on|off][,logfile=PATH][,logappend=on|off][,tls-creds=ID] (tcp)\n" + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,disconnected]\n" + " [,reconnect=seconds][,mux=on|off][,logfile=PATH][,logappend=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" @@ -2548,7 +2548,7 @@ The available backends are: A void device. This device will not emit any data, and will drop any data it receives. The null backend does not take any options. -@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}] +@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,disconnected][,reconnect=@var{seconds}][,tls-creds=@var{id}] Create a two-way stream socket, which can be either a TCP or a unix socket. A unix socket will be created if @option{path} is specified. Behaviour is @@ -2565,6 +2565,9 @@ escape sequences. @option{websocket} specifies that the socket uses WebSocket protocol for communication. +@option{disconnected} specifies that the non-server socket should not try +to connect the remote during initialization. + @option{reconnect} sets the timeout for reconnecting on non-server sockets when the remote end goes away. qemu will delay this many seconds and then attempt to reconnect. Zero disables reconnecting, and is the default. @@ -3087,18 +3090,19 @@ telnet on port 5555 to access the QEMU port. localhost 5555 @end table -@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,reconnect=@var{seconds}] +@item tcp:[@var{host}]:@var{port}[,@var{server}][,nowait][,nodelay][,disconnected] +[,reconnect=@var{seconds}] The TCP Net Console has two modes of operation. It can send the serial I/O to a location or wait for a connection from a location. By default the TCP Net Console is sent to @var{host} at the @var{port}. If you use the @var{server} option QEMU will wait for a client socket application to connect to the port before continuing, unless the @code{nowait} option was specified. The @code{nodelay} option disables the Nagle buffering -algorithm. The @code{reconnect} option only applies if @var{noserver} is -set, if the connection goes down it will attempt to reconnect at the -given interval. If @var{host} is omitted, 0.0.0.0 is assumed. Only -one TCP connection at a time is accepted. You can use @code{telnet} to -connect to the corresponding character device. +algorithm. If @var{noserver} is specified, the @code{disconnected} will disallow +QEMU to connect during initialization, and the @code{reconnect} will ask QEMU +to reconnect at the given interval when the connection goes down. If @var{host} +is omitted, 0.0.0.0 is assumed. Only one TCP connection at a time is accepted. +You can use @code{telnet} to connect to the corresponding character device. @table @code @item Example to send tcp console to 192.168.0.2 port 4444 -serial tcp:192.168.0.2:4444 @@ -3121,7 +3125,7 @@ type "send break" followed by pressing the enter key. The WebSocket protocol is used instead of raw tcp socket. The port acts as a WebSocket server. Client mode is not supported. -@item unix:@var{path}[,server][,nowait][,reconnect=@var{seconds}] +@item unix:@var{path}[,server][,nowait][,disconnected][,reconnect=@var{seconds}] A unix domain socket is used instead of a tcp socket. The option works the same as if you had specified @code{-serial tcp} except the unix domain socket @var{path} is used for connections.