[libvirt] [PATCH REPOST 0/2] Admin API: Add support for client disconnect
Rebased version of https://www.redhat.com/archives/libvir-list/2016-April/msg01987.html onto current HEAD to resolve some rebase conflicts. The original patch 1/3 was dropped, since it has already been merged with client-info series: https://www.redhat.com/archives/libvir-list/2016-April/msg01979.html, so Erik Skultety (2): admin: Introduce virAdmClientClose API virt-admin: Introduce client-disconnect command daemon/admin.c | 6 ++++ daemon/admin_server.c | 9 +++++ daemon/admin_server.h | 3 ++ include/libvirt/libvirt-admin.h | 2 ++ src/admin/admin_protocol.x | 12 ++++++- src/admin_protocol-structs | 5 +++ src/libvirt-admin.c | 30 +++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + tools/virt-admin.c | 73 +++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 5 +++ 11 files changed, 146 insertions(+), 1 deletion(-) -- 2.4.11
Once we're able to list and identify all clients connected to a specific server, we can then support force-closing a connection. This patch introduces a simple API calling virNetServerClientClose on a specific client, which can be later extended easily, e.g. by sending an event once the client is disconnected successfully. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 6 ++++++ daemon/admin_server.c | 9 +++++++++ daemon/admin_server.h | 3 +++ include/libvirt/libvirt-admin.h | 2 ++ src/admin/admin_protocol.x | 12 +++++++++++- src/admin_protocol-structs | 5 +++++ src/libvirt-admin.c | 30 ++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + 9 files changed, 68 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 3de09ca..5fa906f 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -94,6 +94,12 @@ make_nonnull_server(admin_nonnull_server *srv_dst, ignore_value(VIR_STRDUP_QUIET(srv_dst->name, virNetServerGetName(srv_src))); } +static virNetServerClientPtr +get_nonnull_client(virNetServerPtr srv, admin_nonnull_client clnt) +{ + return virNetServerGetClient(srv, clnt.id); +} + static void make_nonnull_client(admin_nonnull_client *clt_dst, virNetServerClientPtr clt_src) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 2fc4675..93a463a 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -303,3 +303,12 @@ adminClientGetInfo(virNetServerClientPtr client, virObjectUnref(identity); return ret; } + +int adminClientClose(virNetServerClientPtr client, + unsigned int flags) +{ + virCheckFlags(0, -1); + + virNetServerClientClose(client); + return 0; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 95c76b9..2953e10 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -59,4 +59,7 @@ int adminClientGetInfo(virNetServerClientPtr client, int *nparams, unsigned int flags); +int adminClientClose(virNetServerClientPtr client, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 0a1ea61..4e6074e 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -349,6 +349,8 @@ int virAdmClientGetInfo(virAdmClientPtr client, int *nparams, unsigned int flags); +int virAdmClientClose(virAdmClientPtr client, unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 67bdbf3..1da7f90 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -160,6 +160,11 @@ struct admin_client_get_info_ret { /* insert@1 */ admin_typed_param params<ADMIN_CLIENT_INFO_PARAMETERS_MAX>; }; +struct admin_client_close_args { + admin_nonnull_client clnt; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -230,5 +235,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CLIENT_GET_INFO = 10 + ADMIN_PROC_CLIENT_GET_INFO = 10, + + /** + * @generate: both + */ + ADMIN_PROC_CLIENT_CLOSE = 11 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index ea9adf6..b4db415 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -105,6 +105,10 @@ struct admin_client_get_info_ret { admin_typed_param * params_val; } params; }; +struct admin_client_close_args { + admin_nonnull_client clnt; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -116,4 +120,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_LIST_CLIENTS = 8, ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, ADMIN_PROC_CLIENT_GET_INFO = 10, + ADMIN_PROC_CLIENT_CLOSE = 11, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 4ad816b..4c86dd7 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -976,3 +976,33 @@ virAdmClientGetInfo(virAdmClientPtr client, virDispatchError(NULL); return -1; } + +/** + * virAdmClientClose: + * @client: a valid client object reference + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Close @client's connection to daemon forcefully. + * + * Returns 0 if the daemon's connection with @client was closed successfully + * or -1 in case of an error. + */ +int virAdmClientClose(virAdmClientPtr client, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, flags=%x", client, flags); + virResetLastError(); + + virCheckAdmClientGoto(client, error); + virCheckFlagsGoto(0, error); + + if ((ret = remoteAdminClientClose(client, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index affe8c1..e55b91e 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -6,6 +6,7 @@ # # admin/admin_protocol.x +xdr_admin_client_close_args; xdr_admin_client_get_info_args; xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_ret; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 27e4a1d..57df1f4 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -34,4 +34,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerSetThreadPoolParameters; virAdmServerListClients; virAdmClientGetInfo; + virAdmClientClose; }; -- 2.4.11
On 04.05.2016 16:46, Erik Skultety wrote:
Once we're able to list and identify all clients connected to a specific server, we can then support force-closing a connection. This patch introduces a simple API calling virNetServerClientClose on a specific client, which can be later extended easily, e.g. by sending an event once the client is disconnected successfully.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 6 ++++++ daemon/admin_server.c | 9 +++++++++ daemon/admin_server.h | 3 +++ include/libvirt/libvirt-admin.h | 2 ++ src/admin/admin_protocol.x | 12 +++++++++++- src/admin_protocol-structs | 5 +++++ src/libvirt-admin.c | 30 ++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + 9 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/daemon/admin.c b/daemon/admin.c index 3de09ca..5fa906f 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -94,6 +94,12 @@ make_nonnull_server(admin_nonnull_server *srv_dst, ignore_value(VIR_STRDUP_QUIET(srv_dst->name, virNetServerGetName(srv_src))); }
+static virNetServerClientPtr +get_nonnull_client(virNetServerPtr srv, admin_nonnull_client clnt) +{ + return virNetServerGetClient(srv, clnt.id); +} + static void make_nonnull_client(admin_nonnull_client *clt_dst, virNetServerClientPtr clt_src) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 2fc4675..93a463a 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -303,3 +303,12 @@ adminClientGetInfo(virNetServerClientPtr client, virObjectUnref(identity); return ret; } + +int adminClientClose(virNetServerClientPtr client, + unsigned int flags) +{ + virCheckFlags(0, -1); + + virNetServerClientClose(client); + return 0; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 95c76b9..2953e10 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -59,4 +59,7 @@ int adminClientGetInfo(virNetServerClientPtr client, int *nparams, unsigned int flags);
+int adminClientClose(virNetServerClientPtr client, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 0a1ea61..4e6074e 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -349,6 +349,8 @@ int virAdmClientGetInfo(virAdmClientPtr client, int *nparams, unsigned int flags);
+int virAdmClientClose(virAdmClientPtr client, unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 67bdbf3..1da7f90 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -160,6 +160,11 @@ struct admin_client_get_info_ret { /* insert@1 */ admin_typed_param params<ADMIN_CLIENT_INFO_PARAMETERS_MAX>; };
+struct admin_client_close_args { + admin_nonnull_client clnt; + unsigned int flags; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -230,5 +235,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_CLIENT_GET_INFO = 10 + ADMIN_PROC_CLIENT_GET_INFO = 10, + + /** + * @generate: both + */ + ADMIN_PROC_CLIENT_CLOSE = 11 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index ea9adf6..b4db415 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -105,6 +105,10 @@ struct admin_client_get_info_ret { admin_typed_param * params_val; } params; }; +struct admin_client_close_args { + admin_nonnull_client clnt; + u_int flags; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -116,4 +120,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_LIST_CLIENTS = 8, ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, ADMIN_PROC_CLIENT_GET_INFO = 10, + ADMIN_PROC_CLIENT_CLOSE = 11, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 4ad816b..4c86dd7 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -976,3 +976,33 @@ virAdmClientGetInfo(virAdmClientPtr client, virDispatchError(NULL); return -1; } + +/** + * virAdmClientClose: + * @client: a valid client object reference + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Close @client's connection to daemon forcefully. + * + * Returns 0 if the daemon's connection with @client was closed successfully + * or -1 in case of an error. + */ +int virAdmClientClose(virAdmClientPtr client, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, flags=%x", client, flags); + virResetLastError(); + + virCheckAdmClientGoto(client, error); + virCheckFlagsGoto(0, error);
Ewww. No. This should be checked on the server side. Not client.
+ + if ((ret = remoteAdminClientClose(client, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index affe8c1..e55b91e 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -6,6 +6,7 @@ #
# admin/admin_protocol.x +xdr_admin_client_close_args; xdr_admin_client_get_info_args; xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_ret; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 27e4a1d..57df1f4 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -34,4 +34,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerSetThreadPoolParameters; virAdmServerListClients; virAdmClientGetInfo; + virAdmClientClose; };
Michal
+ +/** + * virAdmClientClose: + * @client: a valid client object reference + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Close @client's connection to daemon forcefully. + * + * Returns 0 if the daemon's connection with @client was closed successfully + * or -1 in case of an error. + */ +int virAdmClientClose(virAdmClientPtr client, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, flags=%x", client, flags); + virResetLastError(); + + virCheckAdmClientGoto(client, error); + virCheckFlagsGoto(0, error);
Ewww. No. This should be checked on the server side. Not client.
Well, this approach was used with all the other APIs as well. In my opinion, we don't necessarily need to contact the remote daemon to check for the flags at this moment, since we explicitly document, that we don't support any flags yet. Once we change that and start supporting any flags, it will have to be removed of course, with all the remaining occurrences in libvirt-admin.c and the doc string will need to be modified anyway. Another thing is, if callers would like to use some flags, they'll most probably use our enumerated types or constants for that, for which they will have to update the admin library and once they update, the check will be gone. Erik
On 05.05.2016 10:48, Erik Skultety wrote:
+ +/** + * virAdmClientClose: + * @client: a valid client object reference + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Close @client's connection to daemon forcefully. + * + * Returns 0 if the daemon's connection with @client was closed successfully + * or -1 in case of an error. + */ +int virAdmClientClose(virAdmClientPtr client, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, flags=%x", client, flags); + virResetLastError(); + + virCheckAdmClientGoto(client, error); + virCheckFlagsGoto(0, error);
Ewww. No. This should be checked on the server side. Not client.
Well, this approach was used with all the other APIs as well. In my opinion, we don't necessarily need to contact the remote daemon to check for the flags at this moment, since we explicitly document, that we don't support any flags yet. Once we change that and start supporting any flags, it will have to be removed of course, with all the remaining occurrences in libvirt-admin.c and the doc string will need to be modified anyway. Another thing is, if callers would like to use some flags, they'll most probably use our enumerated types or constants for that, for which they will have to update the admin library and once they update, the check will be gone.
That's not the approach we have in the rest of our calls. The reasoning behind is, that if you connect to a newer daemon that already supports new flag, you can just pass numeric value (magic constant) to the API from your app and everything works. But if we leave this check in, API will fail. Then, what if you introduce new flag (among with removing this check), and then another one (in a different commit). But somebody doing backports wants to backport just the latter. Well, they can't because the check would still be there. Moreover, as we add new flags to our APIs, in general flags known to client and to server can be different, because you could have newer client connected to older server or vice versa. Therefore I don't see much value in this client side check. Michal
On Thu, May 05, 2016 at 12:22:02PM +0200, Michal Privoznik wrote:
On 05.05.2016 10:48, Erik Skultety wrote:
+ +/** + * virAdmClientClose: + * @client: a valid client object reference + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Close @client's connection to daemon forcefully. + * + * Returns 0 if the daemon's connection with @client was closed successfully + * or -1 in case of an error. + */ +int virAdmClientClose(virAdmClientPtr client, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, flags=%x", client, flags); + virResetLastError(); + + virCheckAdmClientGoto(client, error); + virCheckFlagsGoto(0, error);
Ewww. No. This should be checked on the server side. Not client.
Well, this approach was used with all the other APIs as well. In my opinion, we don't necessarily need to contact the remote daemon to check for the flags at this moment, since we explicitly document, that we don't support any flags yet. Once we change that and start supporting any flags, it will have to be removed of course, with all the remaining occurrences in libvirt-admin.c and the doc string will need to be modified anyway. Another thing is, if callers would like to use some flags, they'll most probably use our enumerated types or constants for that, for which they will have to update the admin library and once they update, the check will be gone.
That's not the approach we have in the rest of our calls. The reasoning behind is, that if you connect to a newer daemon that already supports new flag, you can just pass numeric value (magic constant) to the API from your app and everything works. But if we leave this check in, API will fail.
Then, what if you introduce new flag (among with removing this check), and then another one (in a different commit). But somebody doing backports wants to backport just the latter. Well, they can't because the check would still be there.
Moreover, as we add new flags to our APIs, in general flags known to client and to server can be different, because you could have newer client connected to older server or vice versa.
Therefore I don't see much value in this client side check.
Agreed, we never want to check flags in the public API entry points. It is actively harmful as you describe. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Enable the client disconnect within virt-admin. Also, update the man page accordingly. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 5 ++++ 2 files changed, 78 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index d6f7084..530409c 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -742,6 +742,73 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) VIR_FREE(timestr); return ret; } + +/* ------------------------- + * Command client-disconnect + * ------------------------- + */ + +static const vshCmdInfo info_client_disconnect[] = { + {.name = "help", + .data = N_("force disconnect a client from the given server") + }, + {.name = "desc", + .data = N_("Force close a specific client's connection to the given " + "server.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_client_disconnect[] = { + {.name = "client", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("client which to disconnect, specified by ID"), + }, + {.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("server which the client is currently connected to"), + }, + {.name = NULL} +}; + +static bool +cmdClientDisconnect(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + const char *srvname = NULL; + unsigned long long id = 0; + virAdmServerPtr srv = NULL; + virAdmClientPtr client = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + return false; + + if (vshCommandOptULongLongWrap(ctl, cmd, "client", &id) < 0) + return false; + + if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) + goto cleanup; + + if (!(client = virAdmServerLookupClient(srv, id, 0))) + goto cleanup; + + if (virAdmClientClose(client, 0) < 0) { + vshError(ctl, _("Failed to disconnect client '%llu' from server %s"), + id, virAdmServerGetName(srv)); + goto cleanup; + } + + vshPrint(ctl, _("Client '%llu' disconnected"), id); + ret = true; + cleanup: + virAdmClientFree(client); + virAdmServerFree(srv); + return ret; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -1069,6 +1136,12 @@ static const vshCmdDef managementCmds[] = { .info = info_srv_threadpool_set, .flags = 0 }, + {.name = "client-disconnect", + .handler = cmdClientDisconnect, + .opts = opts_client_disconnect, + .info = info_client_disconnect, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index bf64e1e..b90f037 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -270,6 +270,11 @@ B<Examples> readonly : no sock_addr : 127.0.0.1:57060 +=item B<client-disconnect> I<server> I<client> + +Close a connection originating from I<client>. The I<server> argument +specifies the name of the server I<client> is currently connected to. + =back =head1 ENVIRONMENT -- 2.4.11
On 04.05.2016 16:46, Erik Skultety wrote:
Enable the client disconnect within virt-admin. Also, update the man page accordingly.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 5 ++++ 2 files changed, 78 insertions(+)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index d6f7084..530409c 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -742,6 +742,73 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) VIR_FREE(timestr); return ret; } + +/* ------------------------- + * Command client-disconnect + * ------------------------- + */ + +static const vshCmdInfo info_client_disconnect[] = { + {.name = "help", + .data = N_("force disconnect a client from the given server") + }, + {.name = "desc", + .data = N_("Force close a specific client's connection to the given " + "server.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_client_disconnect[] = { + {.name = "client", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("client which to disconnect, specified by ID"), + }, + {.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("server which the client is currently connected to"), + }, + {.name = NULL} +};
Just like in client-info, I think these two arguments should be swapped.
+ +static bool +cmdClientDisconnect(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + const char *srvname = NULL; + unsigned long long id = 0; + virAdmServerPtr srv = NULL; + virAdmClientPtr client = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + return false; + + if (vshCommandOptULongLongWrap(ctl, cmd, "client", &id) < 0) + return false; + + if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) + goto cleanup; + + if (!(client = virAdmServerLookupClient(srv, id, 0))) + goto cleanup; + + if (virAdmClientClose(client, 0) < 0) { + vshError(ctl, _("Failed to disconnect client '%llu' from server %s"), + id, virAdmServerGetName(srv)); + goto cleanup; + } + + vshPrint(ctl, _("Client '%llu' disconnected"), id); + ret = true; + cleanup: + virAdmClientFree(client); + virAdmServerFree(srv); + return ret; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -1069,6 +1136,12 @@ static const vshCmdDef managementCmds[] = { .info = info_srv_threadpool_set, .flags = 0 }, + {.name = "client-disconnect", + .handler = cmdClientDisconnect, + .opts = opts_client_disconnect, + .info = info_client_disconnect, + .flags = 0 + }, {.name = NULL} };
diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index bf64e1e..b90f037 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -270,6 +270,11 @@ B<Examples> readonly : no sock_addr : 127.0.0.1:57060
+=item B<client-disconnect> I<server> I<client> + +Close a connection originating from I<client>. The I<server> argument +specifies the name of the server I<client> is currently connected to. + =back
=head1 ENVIRONMENT
Michal
On 04.05.2016 16:46, Erik Skultety wrote:
Rebased version of https://www.redhat.com/archives/libvir-list/2016-April/msg01987.html onto current HEAD to resolve some rebase conflicts. The original patch 1/3 was dropped, since it has already been merged with client-info series: https://www.redhat.com/archives/libvir-list/2016-April/msg01979.html, so
Erik Skultety (2): admin: Introduce virAdmClientClose API virt-admin: Introduce client-disconnect command
daemon/admin.c | 6 ++++ daemon/admin_server.c | 9 +++++ daemon/admin_server.h | 3 ++ include/libvirt/libvirt-admin.h | 2 ++ src/admin/admin_protocol.x | 12 ++++++- src/admin_protocol-structs | 5 +++ src/libvirt-admin.c | 30 +++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + tools/virt-admin.c | 73 +++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 5 +++ 11 files changed, 146 insertions(+), 1 deletion(-)
ACK series, but please see my comments before pushing. Michal
On 05/05/16 07:43, Michal Privoznik wrote:
On 04.05.2016 16:46, Erik Skultety wrote:
Rebased version of https://www.redhat.com/archives/libvir-list/2016-April/msg01987.html onto current HEAD to resolve some rebase conflicts. The original patch 1/3 was dropped, since it has already been merged with client-info series: https://www.redhat.com/archives/libvir-list/2016-April/msg01979.html, so
Erik Skultety (2): admin: Introduce virAdmClientClose API virt-admin: Introduce client-disconnect command
daemon/admin.c | 6 ++++ daemon/admin_server.c | 9 +++++ daemon/admin_server.h | 3 ++ include/libvirt/libvirt-admin.h | 2 ++ src/admin/admin_protocol.x | 12 ++++++- src/admin_protocol-structs | 5 +++ src/libvirt-admin.c | 30 +++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 1 + tools/virt-admin.c | 73 +++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 5 +++ 11 files changed, 146 insertions(+), 1 deletion(-)
ACK series, but please see my comments before pushing.
Michal
Adjusted both patches and pushed, thanks for review. Erik
participants (3)
-
Daniel P. Berrange -
Erik Skultety -
Michal Privoznik