Message ID | db00ff13f62fe97634132e43105b9bf63bbbf582.1589193717.git.lukasstraub2@web.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Introduce 'yank' oob qmp command to recover from hanging qemu | expand |
On Mon, May 11, 2020 at 01:14:41PM +0200, Lukas Straub wrote: > Add qio_channel_set_yank function to channel and to channel-socket, > which will register a yank function. The yank function calls > shutdown() on the socket. > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > Makefile.objs | 1 + > include/io/channel-socket.h | 1 + > include/io/channel.h | 12 ++++++++++++ > io/channel-socket.c | 29 +++++++++++++++++++++++++++++ > io/channel.c | 9 +++++++++ > 5 files changed, 52 insertions(+) Assuming we want the yank feature (which I'm not entirely convinced of), then I don't think any of this addition should exist. The QIOChannel class already provides a "qio_channel_shutdown" method which can be invoked. The layer above which is using the QIOChannel should be calling this existing qio_channel_shutdown method in response to any yank request. The I/O layer shouldn't have any direct dependancy on the yank feature. Regards, Daniel
On Mon, 11 May 2020 12:51:46 +0100 Daniel P. Berrangé <berrange@redhat.com> wrote: > On Mon, May 11, 2020 at 01:14:41PM +0200, Lukas Straub wrote: > > Add qio_channel_set_yank function to channel and to channel-socket, > > which will register a yank function. The yank function calls > > shutdown() on the socket. > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > --- > > Makefile.objs | 1 + > > include/io/channel-socket.h | 1 + > > include/io/channel.h | 12 ++++++++++++ > > io/channel-socket.c | 29 +++++++++++++++++++++++++++++ > > io/channel.c | 9 +++++++++ > > 5 files changed, 52 insertions(+) > > Assuming we want the yank feature (which I'm not entirely convinced > of), then I don't think any of this addition should exist. The > QIOChannel class already provides a "qio_channel_shutdown" method > which can be invoked. The layer above which is using the QIOChannel > should be calling this existing qio_channel_shutdown method in > response to any yank request. The I/O layer shouldn't have any > direct dependancy on the yank feature. Having the code here simplifys the code in the other places. Regards, Lukas Straub > > Regards, > Daniel
On Mon, May 11, 2020 at 10:00:42PM +0200, Lukas Straub wrote: > On Mon, 11 May 2020 12:51:46 +0100 > Daniel P. Berrangé <berrange@redhat.com> wrote: > > > On Mon, May 11, 2020 at 01:14:41PM +0200, Lukas Straub wrote: > > > Add qio_channel_set_yank function to channel and to channel-socket, > > > which will register a yank function. The yank function calls > > > shutdown() on the socket. > > > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > > --- > > > Makefile.objs | 1 + > > > include/io/channel-socket.h | 1 + > > > include/io/channel.h | 12 ++++++++++++ > > > io/channel-socket.c | 29 +++++++++++++++++++++++++++++ > > > io/channel.c | 9 +++++++++ > > > 5 files changed, 52 insertions(+) > > > > Assuming we want the yank feature (which I'm not entirely convinced > > of), then I don't think any of this addition should exist. The > > QIOChannel class already provides a "qio_channel_shutdown" method > > which can be invoked. The layer above which is using the QIOChannel > > should be calling this existing qio_channel_shutdown method in > > response to any yank request. The I/O layer shouldn't have any > > direct dependancy on the yank feature. > > Having the code here simplifys the code in the other places. This introduces a depedancy from the IO channel code into the system emulator infra which is not desired. The goal is to keep the io/ module isolated and as self-contained as possible. I don't think it makes a significant difference to the yank code to keep it out of the io layer, and simply call the QIOChannel objects via their existing public API. Regards, Daniel
diff --git a/Makefile.objs b/Makefile.objs index a7c967633a..889115775c 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -24,6 +24,7 @@ block-obj-m = block/ crypto-obj-y = crypto/ io-obj-y = io/ +io-obj-y += yank.o endif # CONFIG_SOFTMMU or CONFIG_TOOLS diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h index 777ff5954e..0fa7a364f3 100644 --- a/include/io/channel-socket.h +++ b/include/io/channel-socket.h @@ -48,6 +48,7 @@ struct QIOChannelSocket { socklen_t localAddrLen; struct sockaddr_storage remoteAddr; socklen_t remoteAddrLen; + bool yank; }; diff --git a/include/io/channel.h b/include/io/channel.h index d4557f0930..782b618694 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -132,6 +132,8 @@ struct QIOChannelClass { bool enabled); void (*io_set_delay)(QIOChannel *ioc, bool enabled); + void (*io_set_yank)(QIOChannel *ioc, + bool enabled); off_t (*io_seek)(QIOChannel *ioc, off_t offset, int whence, @@ -550,6 +552,16 @@ int qio_channel_shutdown(QIOChannel *ioc, void qio_channel_set_delay(QIOChannel *ioc, bool enabled); +/** + * qio_channel_set_yank: + * @ioc: the channel object + * @enabled: the new flag state + * + * Controls wether this channel participates in yanking. + */ +void qio_channel_set_yank(QIOChannel *ioc, + bool enabled); + /** * qio_channel_set_cork: * @ioc: the channel object diff --git a/io/channel-socket.c b/io/channel-socket.c index b74f5b92a0..be03946d29 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -26,6 +26,7 @@ #include "io/channel-watch.h" #include "trace.h" #include "qapi/clone-visitor.h" +#include "yank.h" #define SOCKET_MAX_FDS 16 @@ -55,6 +56,7 @@ qio_channel_socket_new(void) sioc = QIO_CHANNEL_SOCKET(object_new(TYPE_QIO_CHANNEL_SOCKET)); sioc->fd = -1; + sioc->yank = 0; ioc = QIO_CHANNEL(sioc); qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN); @@ -395,10 +397,19 @@ qio_channel_socket_accept(QIOChannelSocket *ioc, return NULL; } +static void qio_channel_socket_yank(void *opaque) +{ + QIOChannel *ioc = opaque; + QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); + + shutdown(sioc->fd, SHUT_RDWR); +} + static void qio_channel_socket_init(Object *obj) { QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj); ioc->fd = -1; + ioc->yank = 0; } static void qio_channel_socket_finalize(Object *obj) @@ -422,6 +433,9 @@ static void qio_channel_socket_finalize(Object *obj) closesocket(ioc->fd); ioc->fd = -1; } + if (ioc->yank) { + yank_unregister_function(qio_channel_socket_yank, ioc); + } } @@ -686,6 +700,20 @@ qio_channel_socket_set_delay(QIOChannel *ioc, &v, sizeof(v)); } +static void +qio_channel_socket_set_yank(QIOChannel *ioc, + bool enabled) +{ + QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc); + + if (sioc->yank) { + yank_unregister_function(qio_channel_socket_yank, ioc); + } + sioc->yank = enabled; + if (sioc->yank) { + yank_register_function(qio_channel_socket_yank, ioc); + } +} static void qio_channel_socket_set_cork(QIOChannel *ioc, @@ -784,6 +812,7 @@ static void qio_channel_socket_class_init(ObjectClass *klass, ioc_klass->io_shutdown = qio_channel_socket_shutdown; ioc_klass->io_set_cork = qio_channel_socket_set_cork; ioc_klass->io_set_delay = qio_channel_socket_set_delay; + ioc_klass->io_set_yank = qio_channel_socket_set_yank; ioc_klass->io_create_watch = qio_channel_socket_create_watch; ioc_klass->io_set_aio_fd_handler = qio_channel_socket_set_aio_fd_handler; } diff --git a/io/channel.c b/io/channel.c index e4376eb0bc..0c4095e0e0 100644 --- a/io/channel.c +++ b/io/channel.c @@ -373,6 +373,15 @@ void qio_channel_set_delay(QIOChannel *ioc, } } +void qio_channel_set_yank(QIOChannel *ioc, + bool enabled) +{ + QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc); + + if (klass->io_set_yank) { + klass->io_set_yank(ioc, enabled); + } +} void qio_channel_set_cork(QIOChannel *ioc, bool enabled)
Add qio_channel_set_yank function to channel and to channel-socket, which will register a yank function. The yank function calls shutdown() on the socket. Signed-off-by: Lukas Straub <lukasstraub2@web.de> --- Makefile.objs | 1 + include/io/channel-socket.h | 1 + include/io/channel.h | 12 ++++++++++++ io/channel-socket.c | 29 +++++++++++++++++++++++++++++ io/channel.c | 9 +++++++++ 5 files changed, 52 insertions(+)