[libvirt] [PATCH 00/21] Various (tiny) clean-ups and changes for daemon/admin
I wanted to make virAdmConnectListAllServers()'s dispatch function generated by our mighty gendispatch script. And when doing that I stumbled upon various other things, changed what seemed better and at the end I ended up with one huge commit. Because lot of those changes should not be done together in one commit, I split each tiny part so it's nicer to review. However, some might point out that some commits do not belong together. To that I say it was a pain splitting the commit, don't make me split the series as well, please =) This "conflicts" with Erik's series that deals with worker threads tuning. We about know that and we will merge those together, or rather rebase on top of each other based on which version of virAdmConnectLookupServer{,ByName}() you and we like better. Martin Kletzander (21): admin: Make virAdmServerFree() handle NULL gracefully admin: Check for flags properly server: Store server name in server object daemon: Get server name from the server itself virerror: Introduce new error type NO_SERVER daemon: Set error for unknown server name daemon: Properly check for clients Expose virNetServerGetName admin: Do not work with virAdm on the server side Change virNetDaemonGetServerNames to virNetDaemonGetServers admin: Don't use priority for admin APIs virt-admin: Don't leak uri all over the place admin: Be consistent when resetting errors gendispatch: Cluster, don't capture if not needed gendispatch: Be able to generate multi-return values admin: Generate ConnectListServers dispatch helpers gendispatch: Accept server as an argument admin: Add virAdmConnectLookupServer gendispatch: Remember the name of snapshot variable name gendispatch: Support modern listing of more types remote: Generate what's possible daemon/admin.c | 49 +-- daemon/admin_server.c | 36 +-- daemon/admin_server.h | 12 +- daemon/libvirtd.c | 10 +- daemon/remote.c | 599 ------------------------------------ include/libvirt/libvirt-admin.h | 4 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 - src/admin/admin_protocol.x | 21 +- src/admin/admin_remote.c | 63 ---- src/admin_protocol-structs | 8 + src/libvirt-admin.c | 48 ++- src/libvirt_admin_private.syms | 2 + src/libvirt_admin_public.syms | 1 + src/libvirt_remote.syms | 2 + src/locking/lock_daemon.c | 5 +- src/logging/log_daemon.c | 5 +- src/lxc/lxc_controller.c | 5 +- src/remote/remote_driver.c | 654 ---------------------------------------- src/remote/remote_protocol.x | 40 +-- src/rpc/gendispatch.pl | 374 +++++++++++++++++------ src/rpc/virnetdaemon.c | 65 ++-- src/rpc/virnetdaemon.h | 3 +- src/rpc/virnetserver.c | 20 +- src/rpc/virnetserver.h | 6 +- src/util/virerror.c | 6 + tests/virnetdaemontest.c | 10 +- tools/virt-admin.c | 8 +- 28 files changed, 509 insertions(+), 1549 deletions(-) -- 2.7.2
We don't want to end up like with virDomainFree() and other, right? Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 36674441b18b..54ae5ad3135e 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -588,6 +588,10 @@ int virAdmServerFree(virAdmServerPtr srv) VIR_DEBUG("server=%p", srv); virResetLastError(); + + if (!srv) + return 0; + virCheckAdmServerReturn(srv, -1); virObjectUnref(srv); -- 2.7.2
On 10/03/16 05:53, Martin Kletzander wrote:
We don't want to end up like with virDomainFree() and other, right?
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 36674441b18b..54ae5ad3135e 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -588,6 +588,10 @@ int virAdmServerFree(virAdmServerPtr srv) VIR_DEBUG("server=%p", srv);
virResetLastError(); + + if (!srv) + return 0; + virCheckAdmServerReturn(srv, -1);
virObjectUnref(srv);
ACK Erik
Function virAdmConnectListServers() forgot to check for flags at all, virAdmConnectOpen() on the other hand checked them but did no dispatch the error. virCheckFlags() should be used only when there should be no other thing done after erroring out and since they are used on different places then just public API, they cannot dispatch errors. So let' suse virCheckFlagsGoto instead. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 54ae5ad3135e..44b4d4090e59 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -204,7 +204,7 @@ virAdmConnectOpen(const char *name, unsigned int flags) VIR_DEBUG("flags=%x", flags); virResetLastError(); - virCheckFlags(VIR_CONNECT_NO_ALIASES, NULL); + virCheckFlagsGoto(VIR_CONNECT_NO_ALIASES, error); if (!(conn = virAdmConnectNew())) goto error; @@ -603,7 +603,7 @@ int virAdmServerFree(virAdmServerPtr srv) * @conn: daemon connection reference * @servers: Pointer to a list to store an array containing objects or NULL * if the list is not required (number of servers only) - * @flags: bitwise-OR of virAdmConnectListServersFlags + * @flags: unused, must be 0 * * Collect list of all servers provided by daemon the client is connected to. * @@ -624,6 +624,7 @@ virAdmConnectListServers(virAdmConnectPtr conn, VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags); virResetLastError(); + virCheckFlagsGoto(0, error); if (servers) *servers = NULL; -- 2.7.2
On 10/03/16 05:53, Martin Kletzander wrote:
Function virAdmConnectListServers() forgot to check for flags at all, virAdmConnectOpen() on the other hand checked them but did no dispatch the error. virCheckFlags() should be used only when there should be no other thing done after erroring out and since they are used on different places then just public API, they cannot dispatch errors. So let' suse virCheckFlagsGoto instead.
s/let' s/let's /
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 54ae5ad3135e..44b4d4090e59 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -204,7 +204,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
VIR_DEBUG("flags=%x", flags); virResetLastError(); - virCheckFlags(VIR_CONNECT_NO_ALIASES, NULL); + virCheckFlagsGoto(VIR_CONNECT_NO_ALIASES, error);
if (!(conn = virAdmConnectNew())) goto error; @@ -603,7 +603,7 @@ int virAdmServerFree(virAdmServerPtr srv) * @conn: daemon connection reference * @servers: Pointer to a list to store an array containing objects or NULL * if the list is not required (number of servers only) - * @flags: bitwise-OR of virAdmConnectListServersFlags + * @flags: unused, must be 0 *
you could as well make use of the description we use across libvirt: "extra flags; not used yet, so callers should always pass 0"
* Collect list of all servers provided by daemon the client is connected to. * @@ -624,6 +624,7 @@ virAdmConnectListServers(virAdmConnectPtr conn, VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags);
virResetLastError(); + virCheckFlagsGoto(0, error);
if (servers) *servers = NULL;
ACK Erik
On Thu, Mar 10, 2016 at 09:53:59AM +0100, Erik Skultety wrote:
On 10/03/16 05:53, Martin Kletzander wrote:
Function virAdmConnectListServers() forgot to check for flags at all, virAdmConnectOpen() on the other hand checked them but did no dispatch the error. virCheckFlags() should be used only when there should be no other thing done after erroring out and since they are used on different places then just public API, they cannot dispatch errors. So let' suse virCheckFlagsGoto instead.
s/let' s/let's /
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 54ae5ad3135e..44b4d4090e59 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -204,7 +204,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
VIR_DEBUG("flags=%x", flags); virResetLastError(); - virCheckFlags(VIR_CONNECT_NO_ALIASES, NULL); + virCheckFlagsGoto(VIR_CONNECT_NO_ALIASES, error);
if (!(conn = virAdmConnectNew())) goto error; @@ -603,7 +603,7 @@ int virAdmServerFree(virAdmServerPtr srv) * @conn: daemon connection reference * @servers: Pointer to a list to store an array containing objects or NULL * if the list is not required (number of servers only) - * @flags: bitwise-OR of virAdmConnectListServersFlags + * @flags: unused, must be 0 *
you could as well make use of the description we use across libvirt: "extra flags; not used yet, so callers should always pass 0"
This was taken from other API so I kinda thought it'll be consistent. I will change libvirt-admin to use what other use (and you mentioned). Thanks for catching that. I changed both things in my local tree for now.
* Collect list of all servers provided by daemon the client is connected to. * @@ -624,6 +624,7 @@ virAdmConnectListServers(virAdmConnectPtr conn, VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags);
virResetLastError(); + virCheckFlagsGoto(0, error);
if (servers) *servers = NULL;
ACK
Erik
At first I did not want to do this, but after trying to implement some newer feaures in the admin API I realized we need that to make our lives easier. On the other hand they are not saved redundantly and the virNetServer objects are still kept in a hash table. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 6 ++++-- src/locking/lock_daemon.c | 3 ++- src/logging/log_daemon.c | 3 ++- src/lxc/lxc_controller.c | 3 ++- src/rpc/virnetdaemon.c | 1 + src/rpc/virnetserver.c | 20 ++++++++++++++++++-- src/rpc/virnetserver.h | 6 +++++- tests/virnetdaemontest.c | 8 +++++--- 8 files changed, 39 insertions(+), 11 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index d1b98c9c3d02..beddec1bb852 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1382,7 +1382,8 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(srv = virNetServerNew(config->min_workers, + if (!(srv = virNetServerNew("libvirtd", + config->min_workers, config->max_workers, config->prio_workers, config->max_clients, @@ -1456,7 +1457,8 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(srvAdm = virNetServerNew(config->admin_min_workers, + if (!(srvAdm = virNetServerNew("admin", + config->admin_min_workers, config->admin_max_workers, 0, config->admin_max_clients, diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index cd70aa94d7a2..c89b842f0faf 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -160,7 +160,8 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) return NULL; } - if (!(srv = virNetServerNew(1, 1, 0, config->max_clients, + if (!(srv = virNetServerNew("virtlockd", + 1, 1, 0, config->max_clients, config->max_clients, -1, 0, NULL, virLockDaemonClientNew, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 07a03c24199a..866e8a85f4fa 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -150,7 +150,8 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) return NULL; } - if (!(logd->srv = virNetServerNew(1, 1, 0, config->max_clients, + if (!(logd->srv = virNetServerNew("virtlogd", + 1, 1, 0, config->max_clients, config->max_clients, -1, 0, NULL, virLogDaemonClientNew, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 76bef82e709c..21cf71fa08a5 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -928,7 +928,8 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl->name) < 0) return -1; - if (!(srv = virNetServerNew(0, 0, 0, 1, + if (!(srv = virNetServerNew("LXC", + 0, 0, 0, 1, 0, -1, 0, NULL, virLXCControllerClientPrivateNew, diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 298fbf4ab086..7ae06dd38007 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -276,6 +276,7 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, } srv = virNetServerNewPostExecRestart(object, + serverName, clientPrivNew, clientPrivNewPostExecRestart, clientPrivPreExecRestart, diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 547e52e409c0..cf48e50603e4 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -49,6 +49,8 @@ struct _virNetServerJob { struct _virNetServer { virObjectLockable parent; + char *name; + virThreadPoolPtr workers; char *mdnsGroupName; @@ -304,7 +306,8 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, } -virNetServerPtr virNetServerNew(size_t min_workers, +virNetServerPtr virNetServerNew(const char *name, + size_t min_workers, size_t max_workers, size_t priority_workers, size_t max_clients, @@ -332,6 +335,9 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv))) goto error; + if (VIR_STRDUP(srv->name, name) < 0) + goto error; + srv->nclients_max = max_clients; srv->nclients_unauth_max = max_anonymous_clients; srv->keepaliveInterval = keepaliveInterval; @@ -359,6 +365,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, + const char *name, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -427,7 +434,8 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - if (!(srv = virNetServerNew(min_workers, max_workers, + if (!(srv = virNetServerNew(name, + min_workers, max_workers, priority_workers, max_clients, max_anonymous_clients, keepaliveInterval, keepaliveCount, @@ -734,6 +742,8 @@ void virNetServerDispose(void *obj) virNetServerPtr srv = obj; size_t i; + VIR_FREE(srv->name); + for (i = 0; i < srv->nservices; i++) virNetServerServiceToggle(srv->services[i], false); @@ -861,3 +871,9 @@ virNetServerStart(virNetServerPtr srv) return virNetServerMDNSStart(srv->mdns); } + +const char * +virNetServerGetName(virNetServerPtr srv) +{ + return srv->name; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 89d8db9b9ee4..aa244401a27f 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -34,7 +34,8 @@ # include "virjson.h" -virNetServerPtr virNetServerNew(size_t min_workers, +virNetServerPtr virNetServerNew(const char *name, + size_t min_workers, size_t max_workers, size_t priority_workers, size_t max_clients, @@ -48,6 +49,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, void *clientPrivOpaque); virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, + const char *name, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -85,4 +87,6 @@ void virNetServerUpdateServices(virNetServerPtr srv, bool enabled); int virNetServerStart(virNetServerPtr srv); +const char *virNetServerGetName(virNetServerPtr srv); + #endif /* __VIR_NET_SERVER_H__ */ diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 2f855fdde2f2..443a406b77f2 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -28,7 +28,7 @@ #if defined(HAVE_SOCKETPAIR) && defined(WITH_YAJL) static virNetServerPtr -testCreateServer(const char *host, int family) +testCreateServer(const char *server_name, const char *host, int family) { virNetServerPtr srv = NULL; virNetServerServicePtr svc1 = NULL, svc2 = NULL; @@ -49,7 +49,8 @@ testCreateServer(const char *host, int family) goto cleanup; } - if (!(srv = virNetServerNew(10, 50, 5, 100, 10, + if (!(srv = virNetServerNew(server_name, + 10, 50, 5, 100, 10, 120, 5, mdns_group, NULL, @@ -155,7 +156,8 @@ static char *testGenerateJSON(const char *server_name) if (!has_ipv4 && !has_ipv6) return NULL; - if (!(srv = testCreateServer(has_ipv4 ? "127.0.0.1" : "::1", + if (!(srv = testCreateServer(server_name, + has_ipv4 ? "127.0.0.1" : "::1", has_ipv4 ? AF_INET : AF_INET6))) goto cleanup; -- 2.7.2
On Thu, Mar 10, 2016 at 05:53:52AM +0100, Martin Kletzander wrote:
At first I did not want to do this, but after trying to implement some newer feaures in the admin API I realized we need that to make our lives easier. On the other hand they are not saved redundantly and the virNetServer objects are still kept in a hash table.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 6 ++++-- src/locking/lock_daemon.c | 3 ++- src/logging/log_daemon.c | 3 ++- src/lxc/lxc_controller.c | 3 ++- src/rpc/virnetdaemon.c | 1 + src/rpc/virnetserver.c | 20 ++++++++++++++++++-- src/rpc/virnetserver.h | 6 +++++- tests/virnetdaemontest.c | 8 +++++--- 8 files changed, 39 insertions(+), 11 deletions(-)
ACK Jan
Since servers know their name, there is no need to supply such information twice. Also defeats inconsistencies. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 4 ++-- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/rpc/virnetdaemon.h | 1 - tests/virnetdaemontest.c | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index beddec1bb852..3d38a46ee7d2 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1400,7 +1400,7 @@ int main(int argc, char **argv) { } if (!(dmn = virNetDaemonNew()) || - virNetDaemonAddServer(dmn, "libvirtd", srv) < 0) { + virNetDaemonAddServer(dmn, srv) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1474,7 +1474,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (virNetDaemonAddServer(dmn, "admin", srvAdm) < 0) { + if (virNetDaemonAddServer(dmn, srvAdm) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c89b842f0faf..973e6918c9fe 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -171,7 +171,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) goto error; if (!(lockd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(lockd->dmn, "virtlockd", srv) < 0) + virNetDaemonAddServer(lockd->dmn, srv) < 0) goto error; if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 866e8a85f4fa..68f0647638b3 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -161,7 +161,7 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) goto error; if (!(logd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(logd->dmn, "virtlogd", logd->srv) < 0) + virNetDaemonAddServer(logd->dmn, logd->srv) < 0) goto error; if (!(logd->handler = virLogHandlerNew(privileged, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 21cf71fa08a5..b1b55f0e02e1 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -968,7 +968,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) goto error; if (!(ctrl->daemon = virNetDaemonNew()) || - virNetDaemonAddServer(ctrl->daemon, "LXC", srv) < 0) + virNetDaemonAddServer(ctrl->daemon, srv) < 0) goto error; virNetDaemonUpdateServices(ctrl->daemon, true); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 7ae06dd38007..3dc47792a8ba 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -158,10 +158,10 @@ virNetDaemonNew(void) int virNetDaemonAddServer(virNetDaemonPtr dmn, - const char *serverName, virNetServerPtr srv) { int ret = -1; + const char *serverName = virNetServerGetName(srv); virObjectLock(dmn); diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 9a3404f544fd..0084dfd6527a 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -38,7 +38,6 @@ virNetDaemonPtr virNetDaemonNew(void); int virNetDaemonAddServer(virNetDaemonPtr dmn, - const char *serverName, virNetServerPtr srv); virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 443a406b77f2..1608923d09d7 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -164,7 +164,7 @@ static char *testGenerateJSON(const char *server_name) if (!(dmn = virNetDaemonNew())) goto cleanup; - if (virNetDaemonAddServer(dmn, server_name, srv) < 0) + if (virNetDaemonAddServer(dmn, srv) < 0) goto cleanup; if (!(json = virNetDaemonPreExecRestart(dmn))) -- 2.7.2
On Thu, Mar 10, 2016 at 05:53:53AM +0100, Martin Kletzander wrote:
Since servers know their name, there is no need to supply such information twice. Also defeats inconsistencies.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 4 ++-- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 2 +- src/rpc/virnetdaemon.h | 1 - tests/virnetdaemontest.c | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-)
ACK Jan
This serves the same purpose as VIR_ERR_NO_xxx where xxx is any object that API can be called upon. Only this partucular one is used for daemon's servers. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- include/libvirt/virterror.h | 1 + src/util/virerror.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 83f76d84cbbc..1f0702b5890a 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -311,6 +311,7 @@ typedef enum { VIR_ERR_XML_INVALID_SCHEMA = 92, /* XML document doesn't validate against schema */ VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */ + VIR_ERR_NO_SERVER = 95, /* Server was not found */ } virErrorNumber; /** diff --git a/src/util/virerror.c b/src/util/virerror.c index 377c2b11a2c2..61b9ea016032 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1380,6 +1380,12 @@ virErrorMsg(virErrorNumber error, const char *info) case VIR_ERR_MIGRATE_FINISH_OK: errmsg = _("migration successfully aborted"); break; + case VIR_ERR_NO_SERVER: + if (info == NULL) + errmsg = _("Server not found"); + else + errmsg = _("Server not found: %s"); + break; } return errmsg; } -- 2.7.2
On 10/03/16 05:53, Martin Kletzander wrote:
This serves the same purpose as VIR_ERR_NO_xxx where xxx is any object that API can be called upon. Only this partucular one is used for daemon's servers.
s/partucular/particular/
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- include/libvirt/virterror.h | 1 + src/util/virerror.c | 6 ++++++ 2 files changed, 7 insertions(+)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 83f76d84cbbc..1f0702b5890a 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -311,6 +311,7 @@ typedef enum { VIR_ERR_XML_INVALID_SCHEMA = 92, /* XML document doesn't validate against schema */ VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */ + VIR_ERR_NO_SERVER = 95, /* Server was not found */ } virErrorNumber;
/** diff --git a/src/util/virerror.c b/src/util/virerror.c index 377c2b11a2c2..61b9ea016032 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1380,6 +1380,12 @@ virErrorMsg(virErrorNumber error, const char *info) case VIR_ERR_MIGRATE_FINISH_OK: errmsg = _("migration successfully aborted"); break; + case VIR_ERR_NO_SERVER: + if (info == NULL) + errmsg = _("Server not found"); + else + errmsg = _("Server not found: %s"); + break; } return errmsg; }
ACK Erik
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 3dc47792a8ba..8ddcd9baddc9 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -187,6 +187,11 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, srv = virObjectRef(virHashLookup(dmn->servers, serverName)); virObjectUnlock(dmn); + if (!srv) { + virReportError(VIR_ERR_NO_SERVER, + _("No server named '%s'"), serverName); + } + return srv; } -- 2.7.2
virHashForEach() returns 0 if everything went ince, so our session daemon was timing out even when there was a client connected. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1315606 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 8ddcd9baddc9..9181ad7c474b 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -849,15 +849,23 @@ virNetDaemonClose(virNetDaemonPtr dmn) static int daemonServerHasClients(void *payload, const void *key ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + bool *clients = opaque; virNetServerPtr srv = payload; - return virNetServerHasClients(srv); + if (virNetServerHasClients(srv)) + *clients = true; + + return 0; } bool virNetDaemonHasClients(virNetDaemonPtr dmn) { - return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; + bool ret = false; + + virHashForEach(dmn->servers, daemonServerHasClients, &ret); + + return ret; } -- 2.7.2
On Thu, Mar 10, 2016 at 05:53:56AM +0100, Martin Kletzander wrote:
virHashForEach() returns 0 if everything went ince, so our session
*nice
daemon was timing out even when there was a client connected.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1315606
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
ACK Jan
On Thu, Mar 10, 2016 at 05:53:56AM +0100, Martin Kletzander wrote:
virHashForEach() returns 0 if everything went ince, so our session daemon was timing out even when there was a client connected.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1315606
I have tested this patch extensively. It fixes a very obvious bug in libvirtd (see comments on the bug above). I have also added it to the Fedora 24 and Rawhide libvirt packages because the bug is so serious it needed fixing urgently. Therefore: ACK. Rich.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 8ddcd9baddc9..9181ad7c474b 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -849,15 +849,23 @@ virNetDaemonClose(virNetDaemonPtr dmn) static int daemonServerHasClients(void *payload, const void *key ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + bool *clients = opaque; virNetServerPtr srv = payload;
- return virNetServerHasClients(srv); + if (virNetServerHasClients(srv)) + *clients = true; + + return 0; }
bool virNetDaemonHasClients(virNetDaemonPtr dmn) { - return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; + bool ret = false; + + virHashForEach(dmn->servers, daemonServerHasClients, &ret); + + return ret; } -- 2.7.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On Thu, Mar 10, 2016 at 05:53:56AM +0100, Martin Kletzander wrote:
virHashForEach() returns 0 if everything went ince, so our session daemon was timing out even when there was a client connected.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1315606
Can confirm your patch fixes this bug -- tested on F23 with the libvirt packages from Rawhide with this fix. $ rpm -q libvirt libvirt-1.3.2-2.fc25.x86_64 $ rpm -q libvirt --changelog | head -2 * Wed Mar 09 2016 Richard W.M. Jones <rjones@redhat.com> - 1.3.2-2 - Add fix for RHBZ#1315606.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
FWIW: Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
--- src/rpc/virnetdaemon.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 8ddcd9baddc9..9181ad7c474b 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -849,15 +849,23 @@ virNetDaemonClose(virNetDaemonPtr dmn) static int daemonServerHasClients(void *payload, const void *key ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + bool *clients = opaque; virNetServerPtr srv = payload;
- return virNetServerHasClients(srv); + if (virNetServerHasClients(srv)) + *clients = true; + + return 0; }
bool virNetDaemonHasClients(virNetDaemonPtr dmn) { - return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; + bool ret = false; + + virHashForEach(dmn->servers, daemonServerHasClients, &ret); + + return ret; } -- 2.7.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- /kashyap
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_remote.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 90a453c8be9c..4a3c5dfc8835 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -100,6 +100,7 @@ virNetServerAddClient; virNetServerAddProgram; virNetServerAddService; virNetServerClose; +virNetServerGetName; virNetServerHasClients; virNetServerNew; virNetServerNewPostExecRestart; -- 2.7.2
virAdm is prefix only used on the client side. Or at least for now. On server, though, this corresponds to virNet structures (virAdmConnect is virNetDaemon, virAdmServer should be virNetServer, in the future virAdmClient will be resolved to virNetServerClient, and so on). This will also make future work clearer and easier. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin.c | 6 +++--- daemon/admin_server.c | 6 +++--- daemon/admin_server.h | 7 +++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/daemon/admin.c b/daemon/admin.c index 0c1ddc07095b..cae2a84d84b8 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -82,9 +82,9 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, static void make_nonnull_server(admin_nonnull_server *srv_dst, - virAdmServerPtr srv_src) + virNetServerPtr srv_src) { - ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, virNetServerGetName(srv_src))); } /* Functions */ @@ -141,7 +141,7 @@ adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED, admin_connect_list_servers_args *args, admin_connect_list_servers_ret *ret) { - virAdmServerPtr *servers = NULL; + virNetServerPtr *servers = NULL; int nservers = 0; int rv = -1; size_t i; diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 0196bfe8d47d..0ccb39e91c77 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -38,12 +38,12 @@ VIR_LOG_INIT("daemon.admin_server"); int adminDaemonListServers(virNetDaemonPtr dmn, - virAdmServerPtr **servers, + virNetServerPtr **servers, unsigned int flags) { int ret = -1; const char **srv_names = NULL; - virAdmServerPtr *srvs = NULL; + virNetServerPtr *srvs = NULL; size_t i; ssize_t nsrvs = 0; @@ -57,7 +57,7 @@ adminDaemonListServers(virNetDaemonPtr dmn, goto cleanup; for (i = 0; i < nsrvs; i++) { - if (!(srvs[i] = virAdmGetServer(NULL, srv_names[i]))) + if (!(srvs[i] = virNetDaemonGetServer(dmn, srv_names[i]))) goto cleanup; } diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 2a5aa163352c..606442c19f5f 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -26,9 +26,8 @@ # include "rpc/virnetdaemon.h" -int -adminDaemonListServers(virNetDaemonPtr dmn, - virAdmServerPtr **servers, - unsigned int flags); +int adminDaemonListServers(virNetDaemonPtr dmn, + virNetServerPtr **servers, + unsigned int flags); #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ -- 2.7.2
On Thu, Mar 10, 2016 at 05:53:58AM +0100, Martin Kletzander wrote:
virAdm is prefix only used on the client side. Or at least for now. On server, though, this corresponds to virNet structures (virAdmConnect is virNetDaemon, virAdmServer should be virNetServer, in the future virAdmClient will be resolved to virNetServerClient, and so on).
This will also make future work clearer and easier.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin.c | 6 +++--- daemon/admin_server.c | 6 +++--- daemon/admin_server.h | 7 +++---- 3 files changed, 9 insertions(+), 10 deletions(-)
ACK Jan
For now it does not matter which ones we return as the code is similarly complex, however it will fit in with other constructs in the future, mainly when we will be able to generate dispatch helpers. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 18 +----------------- src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 43 +++++++++++++++++++++++++++++-------------- src/rpc/virnetdaemon.h | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 0ccb39e91c77..a35f33130607 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -42,33 +42,17 @@ adminDaemonListServers(virNetDaemonPtr dmn, unsigned int flags) { int ret = -1; - const char **srv_names = NULL; virNetServerPtr *srvs = NULL; - size_t i; - ssize_t nsrvs = 0; virCheckFlags(0, -1); - if ((nsrvs = virNetDaemonGetServerNames(dmn, &srv_names)) < 0) + if ((ret = virNetDaemonGetServers(dmn, &srvs)) < 0) goto cleanup; if (servers) { - if (VIR_ALLOC_N(srvs, nsrvs) < 0) - goto cleanup; - - for (i = 0; i < nsrvs; i++) { - if (!(srvs[i] = virNetDaemonGetServer(dmn, srv_names[i]))) - goto cleanup; - } - *servers = srvs; srvs = NULL; } - - ret = nsrvs; - cleanup: - VIR_FREE(srv_names); - virObjectListFree(srvs); return ret; } diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 4a3c5dfc8835..66f93833afda 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -65,6 +65,7 @@ virNetDaemonAddSignalHandler; virNetDaemonAutoShutdown; virNetDaemonClose; virNetDaemonGetServer; +virNetDaemonGetServers; virNetDaemonHasClients; virNetDaemonIsPrivileged; virNetDaemonNew; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 9181ad7c474b..aeeeff23cb91 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -196,39 +196,54 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, } +struct collectData { + virNetServerPtr **servers; + size_t nservers; +}; + + +static int +collectServers(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNetServerPtr srv = virObjectRef(payload); + struct collectData *data = opaque; + + if (!srv) + return -1; + + return VIR_APPEND_ELEMENT(*data->servers, data->nservers, srv); +} + + /* * Returns number of names allocated in *servers, on error sets * *servers to NULL and returns -1. List of *servers must be free()d, * but not the items in it (similarly to virHashGetItems). */ ssize_t -virNetDaemonGetServerNames(virNetDaemonPtr dmn, - const char ***servers) +virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers) { - virHashKeyValuePairPtr items = NULL; - size_t nservers = 0; + struct collectData data = { servers, 0 }; ssize_t ret = -1; - size_t i; *servers = NULL; virObjectLock(dmn); - items = virHashGetItems(dmn->servers, NULL); - if (!items) + if (virHashForEach(dmn->servers, collectServers, &data) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get all servers from daemon")); goto cleanup; - - for (i = 0; items[i].key; i++) { - if (VIR_APPEND_ELEMENT(*servers, nservers, items[i].key) < 0) - goto cleanup; } - ret = nservers; + ret = data.nservers; cleanup: if (ret < 0) - VIR_FREE(*servers); - VIR_FREE(items); + virObjectListFree(*servers); virObjectUnlock(dmn); return ret; } diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 0084dfd6527a..211009c5030a 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -82,6 +82,6 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, const char *serverName); -ssize_t virNetDaemonGetServerNames(virNetDaemonPtr dmn, const char ***servers); +ssize_t virNetDaemonGetServers(virNetDaemonPtr dmn, virNetServerPtr **servers); #endif /* __VIR_NET_DAEMON_H__ */ -- 2.7.2
On Thu, Mar 10, 2016 at 05:53:59AM +0100, Martin Kletzander wrote:
For now it does not matter which ones we return as the code is similarly complex, however it will fit in with other constructs in the future, mainly when we will be able to generate dispatch helpers.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 18 +----------------- src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 43 +++++++++++++++++++++++++++++-------------- src/rpc/virnetdaemon.h | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-)
ssize_t -virNetDaemonGetServerNames(virNetDaemonPtr dmn, - const char ***servers) +virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers) { - virHashKeyValuePairPtr items = NULL; - size_t nservers = 0; + struct collectData data = { servers, 0 }; ssize_t ret = -1; - size_t i;
*servers = NULL;
virObjectLock(dmn);
- items = virHashGetItems(dmn->servers, NULL); - if (!items) + if (virHashForEach(dmn->servers, collectServers, &data) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get all servers from daemon")); goto cleanup; - - for (i = 0; items[i].key; i++) { - if (VIR_APPEND_ELEMENT(*servers, nservers, items[i].key) < 0) - goto cleanup; }
- ret = nservers; + ret = data.nservers;
cleanup: if (ret < 0) - VIR_FREE(*servers); - VIR_FREE(items); + virObjectListFree(*servers);
This should be virObjectListFreeCount, since the array is no longer allocated upfront. ACK with that fixed. Jan
There are no priority workers as they dn't make sense for now. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/admin/admin_protocol.x | 1 - 1 file changed, 1 deletion(-) diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 089ce57c748e..742ed2a89cd1 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -104,7 +104,6 @@ enum admin_procedure { /** * @generate: none - * @priority: high */ ADMIN_PROC_CONNECT_LIST_SERVERS = 4 }; -- 2.7.2
On 10/03/16 05:54, Martin Kletzander wrote:
There are no priority workers as they dn't make sense for now.
s/dn't/don't
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/admin/admin_protocol.x | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 089ce57c748e..742ed2a89cd1 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -104,7 +104,6 @@ enum admin_procedure {
/** * @generate: none - * @priority: high */ ADMIN_PROC_CONNECT_LIST_SERVERS = 4 };
ACK Erik
virAdmConnectGetURI() returns string that needs to be free()'d but we haven't done that very much. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virt-admin.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c47053639dd0..bc9ae9366280 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -69,9 +69,6 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virErrorPtr error; char *uri = NULL; - if (reason == VIR_CONNECT_CLOSE_REASON_CLIENT) - return; - error = virSaveLastError(); uri = virAdmConnectGetURI(conn); @@ -98,6 +95,8 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virSetError(error); virFreeError(error); } + + VIR_FREE(uri); } static int @@ -323,7 +322,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) int nsrvs = 0; size_t i; bool ret = false; - const char *uri = NULL; + char *uri = NULL; virAdmServerPtr *srvs = NULL; vshAdmControlPtr priv = ctl->privData; @@ -347,6 +346,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virAdmServerFree(srvs[i]); VIR_FREE(srvs); } + VIR_FREE(uri); return ret; } -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:01AM +0100, Martin Kletzander wrote:
virAdmConnectGetURI() returns string that needs to be free()'d but we haven't done that very much.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virt-admin.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c47053639dd0..bc9ae9366280 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -69,9 +69,6 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virErrorPtr error; char *uri = NULL;
- if (reason == VIR_CONNECT_CLOSE_REASON_CLIENT) - return; -
What is the reason for this change?
error = virSaveLastError(); uri = virAdmConnectGetURI(conn);
@@ -98,6 +95,8 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virSetError(error); virFreeError(error); } + + VIR_FREE(uri); }
static int
This is already freed as of commit 34111a60
@@ -323,7 +322,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) int nsrvs = 0; size_t i; bool ret = false; - const char *uri = NULL; + char *uri = NULL; virAdmServerPtr *srvs = NULL; vshAdmControlPtr priv = ctl->privData;
@@ -347,6 +346,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virAdmServerFree(srvs[i]); VIR_FREE(srvs); } + VIR_FREE(uri);
return ret; }
ACK to these two hunks. Maybe s/all over the place/in cmdSrvList/ in the commit message. Jan
-- 2.7.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 10, 2016 at 09:11:07AM +0100, Ján Tomko wrote:
On Thu, Mar 10, 2016 at 05:54:01AM +0100, Martin Kletzander wrote:
virAdmConnectGetURI() returns string that needs to be free()'d but we haven't done that very much.
I also did: s/ very much/ here since that didn't make sense after cleaning up this commit.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virt-admin.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c47053639dd0..bc9ae9366280 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -69,9 +69,6 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virErrorPtr error; char *uri = NULL;
- if (reason == VIR_CONNECT_CLOSE_REASON_CLIENT) - return; -
What is the reason for this change?
Oh, I added this here probably because of another refactoring I started, where the two lines below were moved up, but that vanished and this probably stayed. Thanks for finding that out.
error = virSaveLastError(); uri = virAdmConnectGetURI(conn);
@@ -98,6 +95,8 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, virSetError(error); virFreeError(error); } + + VIR_FREE(uri); }
static int
This is already freed as of commit 34111a60
Yes, this series started some time ago ;)
@@ -323,7 +322,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) int nsrvs = 0; size_t i; bool ret = false; - const char *uri = NULL; + char *uri = NULL; virAdmServerPtr *srvs = NULL; vshAdmControlPtr priv = ctl->privData;
@@ -347,6 +346,7 @@ cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) virAdmServerFree(srvs[i]); VIR_FREE(srvs); } + VIR_FREE(uri);
return ret; }
ACK to these two hunks. Maybe s/all over the place/in cmdSrvList/ in the commit message.
Done ✓ I changed it locally to reflect that changes.
Jan
-- 2.7.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Resetting an error should be the first thing public API does. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 44b4d4090e59..add4fcc3576a 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -371,11 +371,12 @@ virAdmConnectIsAlive(virAdmConnectPtr conn) VIR_DEBUG("conn=%p", conn); + virResetLastError(); + if (!conn) return 0; virCheckAdmConnectReturn(conn, -1); - virResetLastError(); priv = conn->privateData; virObjectLock(priv); -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:02AM +0100, Martin Kletzander wrote:
Resetting an error should be the first thing public API does.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt-admin.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK Jan
We were using parenteses for grouping admin|remote even though we didn't need to capture what's in it. That caused some changes to be grater than needed and, to be honest, some confusion as well. Let's use it as it should be used. It'll also make future changes more consistent. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 118 ++++++++++++++++++++++++------------------------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5c99409edb26..29f2c6ac033b 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -523,16 +523,16 @@ elsif ($mode eq "server") { push(@free_list, " virObjectUnref(snapshot);\n" . " virObjectUnref(dom);"); - } elsif ($args_member =~ m/^(?:(admin|remote)_string|remote_uuid) (\S+)<\S+>;/) { + } elsif ($args_member =~ m/^(?:(?:admin|remote)_string|remote_uuid) (\S+)<\S+>;/) { push_privconn(\@args_list); - push(@args_list, "args->$2.$2_val"); - push(@args_list, "args->$2.$2_len"); - } elsif ($args_member =~ m/^(?:opaque|(admin|remote)_nonnull_string) (\S+)<\S+>;(.*)$/) { + push(@args_list, "args->$1.$1_val"); + push(@args_list, "args->$1.$1_len"); + } elsif ($args_member =~ m/^(?:opaque|(?:admin|remote)_nonnull_string) (\S+)<\S+>;(.*)$/) { push_privconn(\@args_list); my $cast = ""; - my $arg_name = $2; - my $annotation = $3; + my $arg_name = $1; + my $annotation = $2; if ($annotation ne "") { if ($annotation =~ m/\s*\/\*\s*(.*)\s*\*\//) { @@ -571,16 +571,16 @@ elsif ($mode eq "server") { push_privconn(\@args_list); push(@args_list, "(unsigned char *) args->$1"); - } elsif ($args_member =~ m/^(admin|remote)_string (\S+);/) { + } elsif ($args_member =~ m/^(?:admin|remote)_string (\S+);/) { push_privconn(\@args_list); - push(@vars_list, "char *$2"); - push(@optionals_list, "$2"); - push(@args_list, "$2"); - } elsif ($args_member =~ m/^(admin|remote)_nonnull_string (\S+);/) { + push(@vars_list, "char *$1"); + push(@optionals_list, "$1"); + push(@args_list, "$1"); + } elsif ($args_member =~ m/^(?:admin|remote)_nonnull_string (\S+);/) { push_privconn(\@args_list); - push(@args_list, "args->$2"); + push(@args_list, "args->$1"); } elsif ($args_member =~ m/^(unsigned )?int (\S+);/) { push_privconn(\@args_list); @@ -637,52 +637,52 @@ elsif ($mode eq "server") { } else { die "unhandled type for multi-return-value: $ret_member"; } - } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { + } elsif ($ret_member =~ m/^(?:admin|remote)_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { push(@vars_list, "int len"); - splice(@args_list, int($4), 0, ("ret->$2.$2_val")); - push(@ret_list, "ret->$2.$2_len = len;"); - push(@free_list_on_error, "VIR_FREE(ret->$2.$2_val);"); + splice(@args_list, int($3), 0, ("ret->$1.$1_val")); + push(@ret_list, "ret->$1.$1_len = len;"); + push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);"); $single_ret_var = "len"; $single_ret_by_ref = 0; $single_ret_check = " < 0"; $single_ret_as_list = 1; - $single_ret_list_name = $2; - $single_ret_list_max_var = "max$2"; - $single_ret_list_max_define = $3; + $single_ret_list_name = $1; + $single_ret_list_max_var = "max$1"; + $single_ret_list_max_define = $2; } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+)<\S+>;/) { # error out on unannotated arrays die "$1_nonnull_string array without insert@<offset> annotation: $ret_member"; - } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+);/) { + } elsif ($ret_member =~ m/^(?:admin|remote)_nonnull_string (\S+);/) { if ($call->{ProcName} eq "ConnectGetType") { # SPECIAL: virConnectGetType returns a constant string that must # not be freed. Therefore, duplicate the string here. - push(@vars_list, "const char *$2"); + push(@vars_list, "const char *$1"); push(@ret_list, "/* We have to VIR_STRDUP because remoteDispatchClientRequest will"); push(@ret_list, " * free this string after it's been serialised. */"); push(@ret_list, "if (VIR_STRDUP(ret->type, type) < 0)"); push(@ret_list, " goto cleanup;"); } else { - push(@vars_list, "char *$2"); - push(@ret_list, "ret->$2 = $2;"); + push(@vars_list, "char *$1"); + push(@ret_list, "ret->$1 = $1;"); } - $single_ret_var = $2; + $single_ret_var = $1; $single_ret_by_ref = 0; $single_ret_check = " == NULL"; - } elsif ($ret_member =~ m/^(admin|remote)_string (\S+);/) { - push(@vars_list, "char *$2 = NULL"); - push(@vars_list, "char **$2_p = NULL"); - push(@ret_list, "ret->$2 = $2_p;"); - push(@free_list, " VIR_FREE($2);"); - push(@free_list_on_error, "VIR_FREE($2_p);"); + } elsif ($ret_member =~ m/^(?:admin|remote)_string (\S+);/) { + push(@vars_list, "char *$1 = NULL"); + push(@vars_list, "char **$1_p = NULL"); + push(@ret_list, "ret->$1 = $1_p;"); + push(@free_list, " VIR_FREE($1);"); + push(@free_list_on_error, "VIR_FREE($1_p);"); push(@prepare_ret_list, - "if (VIR_ALLOC($2_p) < 0)\n" . + "if (VIR_ALLOC($1_p) < 0)\n" . " goto cleanup;\n" . "\n" . - " if (VIR_STRDUP(*$2_p, $2) < 0)\n". + " if (VIR_STRDUP(*$1_p, $1) < 0)\n". " goto cleanup;\n"); - $single_ret_var = $2; + $single_ret_var = $1; $single_ret_by_ref = 0; $single_ret_check = " == NULL"; } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|domain_snapshot) (\S+);/) { @@ -899,7 +899,7 @@ elsif ($mode eq "server") { if ($single_ret_as_list) { print " if (args->$single_ret_list_max_var > $single_ret_list_max_define) {\n"; - print " virReportError(VIR_ERR_INTERNAL_ERROR,\n"; + print " virReportError(VIR_ERR_RPC,\n"; print " \"%s\", _(\"max$single_ret_list_name > $single_ret_list_max_define\"));\n"; print " goto cleanup;\n"; print " }\n"; @@ -1140,14 +1140,14 @@ elsif ($mode eq "client") { } elsif ($args_member =~ m/^remote_uuid (\S+);/) { push(@args_list, "const unsigned char *$1"); push(@setters_list, "memcpy(args.$1, $1, VIR_UUID_BUFLEN);"); - } elsif ($args_member =~ m/^(admin|remote)_string (\S+);/) { - push(@args_list, "const char *$2"); - push(@setters_list, "args.$2 = $2 ? (char **)&$2 : NULL;"); - } elsif ($args_member =~ m/^(admin|remote)_nonnull_string (\S+)<(\S+)>;(.*)$/) { + } elsif ($args_member =~ m/^(?:admin|remote)_string (\S+);/) { + push(@args_list, "const char *$1"); + push(@setters_list, "args.$1 = $1 ? (char **)&$1 : NULL;"); + } elsif ($args_member =~ m/^(?:admin|remote)_nonnull_string (\S+)<(\S+)>;(.*)$/) { my $type_name = "const char **"; - my $arg_name = $2; - my $limit = $3; - my $annotation = $4; + my $arg_name = $1; + my $limit = $2; + my $annotation = $3; if ($annotation ne "") { if ($annotation =~ m/\s*\/\*\s*\((.*)\)\s*\*\//) { @@ -1161,10 +1161,10 @@ elsif ($mode eq "client") { push(@args_list, "unsigned int ${arg_name}len"); push(@setters_list, "args.$arg_name.${arg_name}_val = (char **)$arg_name;"); push(@setters_list, "args.$arg_name.${arg_name}_len = ${arg_name}len;"); - push(@args_check_list, { name => "\"$arg_name\"", arg => "${arg_name}len", limit => $3 }); - } elsif ($args_member =~ m/^(admin|remote)_nonnull_string (\S+);/) { - push(@args_list, "const char *$2"); - push(@setters_list, "args.$2 = (char *)$2;"); + push(@args_check_list, { name => "\"$arg_name\"", arg => "${arg_name}len", limit => $2 }); + } elsif ($args_member =~ m/^(?:admin|remote)_nonnull_string (\S+);/) { + push(@args_list, "const char *$1"); + push(@setters_list, "args.$1 = (char *)$1;"); } elsif ($args_member =~ m/^opaque (\S+)<(\S+)>;(.*)$/) { my $type_name = "const char *"; my $arg_name = $1; @@ -1191,9 +1191,9 @@ elsif ($mode eq "client") { push(@setters_list, "args.$arg_name.${arg_name}_val = (char *)$arg_name;"); push(@setters_list, "args.$arg_name.${arg_name}_len = ${arg_name}len;"); push(@args_check_list, { name => "\"$arg_name\"", arg => "${arg_name}len", limit => $limit }); - } elsif ($args_member =~ m/^(admin|remote)_string (\S+)<(\S+)>;/) { - my $arg_name = $2; - my $limit = $3; + } elsif ($args_member =~ m/^(?:admin|remote)_string (\S+)<(\S+)>;/) { + my $arg_name = $1; + my $limit = $2; push(@args_list, "const char *$arg_name"); push(@args_list, "int ${arg_name}len"); @@ -1312,25 +1312,25 @@ elsif ($mode eq "client") { die "unhandled type for multi-return-value for " . "procedure $call->{name}: $ret_member"; } - } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { - splice(@args_list, int($4), 0, ("char **const $2")); - push(@ret_list, "rv = ret.$2.$2_len;"); + } elsif ($ret_member =~ m/^(?:admin|remote)_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { + splice(@args_list, int($3), 0, ("char **const $1")); + push(@ret_list, "rv = ret.$1.$1_len;"); $single_ret_var = "int rv = -1"; $single_ret_type = "int"; $single_ret_as_list = 1; - $single_ret_list_name = $2; - $single_ret_list_max_var = "max$2"; - $single_ret_list_max_define = $3; + $single_ret_list_name = $1; + $single_ret_list_max_var = "max$1"; + $single_ret_list_max_define = $2; } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+)<\S+>;/) { # error out on unannotated arrays die "$1_nonnull_string array without insert@<offset> annotation: $ret_member"; - } elsif ($ret_member =~ m/^(admin|remote)_nonnull_string (\S+);/) { - push(@ret_list, "rv = ret.$2;"); + } elsif ($ret_member =~ m/^(?:admin|remote)_nonnull_string (\S+);/) { + push(@ret_list, "rv = ret.$1;"); $single_ret_var = "char *rv = NULL"; $single_ret_type = "char *"; - } elsif ($ret_member =~ m/^(admin|remote)_string (\S+);/) { - push(@ret_list, "rv = ret.$2 ? *ret.$2 : NULL;"); - push(@ret_list, "VIR_FREE(ret.$2);"); + } elsif ($ret_member =~ m/^(?:admin|remote)_string (\S+);/) { + push(@ret_list, "rv = ret.$1 ? *ret.$1 : NULL;"); + push(@ret_list, "VIR_FREE(ret.$1);"); $single_ret_var = "char *rv = NULL"; $single_ret_type = "char *"; } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|node_device|interface|secret|nwfilter|domain_snapshot) (\S+);/) { -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:03AM +0100, Martin Kletzander wrote:
We were using parenteses for grouping admin|remote even though we didn't
parentheses
need to capture what's in it. That caused some changes to be grater
greater
than needed and, to be honest, some confusion as well. Let's use it as it should be used. It'll also make future changes more consistent.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 118 ++++++++++++++++++++++++------------------------- 1 file changed, 59 insertions(+), 59 deletions(-)
@@ -899,7 +899,7 @@ elsif ($mode eq "server") {
if ($single_ret_as_list) { print " if (args->$single_ret_list_max_var > $single_ret_list_max_define) {\n"; - print " virReportError(VIR_ERR_INTERNAL_ERROR,\n"; + print " virReportError(VIR_ERR_RPC,\n"; print " \"%s\", _(\"max$single_ret_list_name > $single_ret_list_max_define\"));\n"; print " goto cleanup;\n"; print " }\n";
ACK minus this unrelated hunk. Jan
Let's call it modern_ret_as_list as opposed to single_ret_as_list. The latter was able to return list of things. However the new, more modern, version came and it is used since listAllDomains till nowadays in ListServers. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 200 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 172 insertions(+), 28 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 29f2c6ac033b..d6ce9b85e158 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -617,6 +617,9 @@ elsif ($mode eq "server") { my $single_ret_list_max_var = "undefined"; my $single_ret_list_max_define = "undefined"; my $multi_ret = 0; + my $modern_ret_as_list = 0; + my $modern_ret_struct_name = "undefined"; + my $single_ret_list_error_msg_type = "undefined"; if ($rettype ne "void" and scalar(@{$call->{ret_members}}) > 1) { @@ -633,7 +636,16 @@ elsif ($mode eq "server") { push(@ret_list, "memcpy(ret->$3, tmp.$3, sizeof(ret->$3));"); } elsif ($ret_member =~ m/^(unsigned )?(char|short|int|hyper) (\S+);/) { - push(@ret_list, "ret->$3 = tmp.$3;"); + if (!$modern_ret_as_list) { + push(@ret_list, "ret->$3 = tmp.$3;"); + } + } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + $modern_ret_struct_name = $1; + $single_ret_list_error_msg_type = $1; + $single_ret_list_name = $2; + $single_ret_list_max_define = $3; + + $modern_ret_as_list = 1; } else { die "unhandled type for multi-return-value: $ret_member"; } @@ -817,27 +829,43 @@ elsif ($mode eq "server") { # select struct type for multi-return-value functions if ($multi_ret) { - if (!(defined $call->{ret_offset})) { - die "multi-return-value without insert@<offset> annotation: $call->{ret}"; - } + if (defined $call->{ret_offset}) { + push_privconn(\@args_list); - push_privconn(\@args_list); + if ($modern_ret_as_list) { + my $struct_name = name_to_TypeName($modern_ret_struct_name); - my $struct_name = $call->{ProcName}; - $struct_name =~ s/Get//; + if ($structprefix eq "admin") { + $struct_name = "Net${struct_name}"; + } - splice(@args_list, $call->{ret_offset}, 0, ("&tmp")); + push(@vars_list, "vir${struct_name}Ptr *result = NULL"); + push(@vars_list, "int nresults = 0"); - if ($call->{ProcName} eq "DomainBlockStats" || - $call->{ProcName} eq "DomainInterfaceStats") { - # SPECIAL: virDomainBlockStats and virDomainInterfaceStats - # have a 'Struct' suffix on the actual struct name - # and take the struct size as additional argument - $struct_name .= "Struct"; - splice(@args_list, $call->{ret_offset} + 1, 0, ("sizeof(tmp)")); - } + @args_list = grep {!/\bneed_results\b/} @args_list; - push(@vars_list, "vir$struct_name tmp"); + splice(@args_list, $call->{ret_offset}, 0, + ("args->need_results ? &result : NULL")); + } else { + my $struct_name = $call->{ProcName}; + $struct_name =~ s/Get//; + + splice(@args_list, $call->{ret_offset}, 0, ("&tmp")); + + if ($call->{ProcName} eq "DomainBlockStats" || + $call->{ProcName} eq "DomainInterfaceStats") { + # SPECIAL: virDomainBlockStats and virDomainInterfaceStats + # have a 'Struct' suffix on the actual struct name + # and take the struct size as additional argument + $struct_name .= "Struct"; + splice(@args_list, $call->{ret_offset} + 1, 0, ("sizeof(tmp)")); + } + + push(@vars_list, "vir$struct_name tmp"); + } + } else { + die "multi-return-value without insert@<offset> annotation: $call->{ret}"; + } } if ($call->{streamflag} ne "none") { @@ -868,6 +896,10 @@ elsif ($mode eq "server") { print "{\n"; print " int rv = -1;\n"; + if ($modern_ret_as_list) { + print " size_t i;\n"; + } + foreach my $var (@vars_list) { print " $var;\n"; } @@ -974,9 +1006,17 @@ elsif ($mode eq "server") { print " goto cleanup;\n"; print "\n"; } else { - print " if (vir$call->{ProcName}("; - print join(', ', @args_list); - print ") < 0)\n"; + if ($modern_ret_as_list) { + print " if ((nresults = \n"; + my $indln = " $prefix$call->{ProcName}("; + print $indln; + print join(",\n" . ' ' x (length $indln), @args_list); + print ")) < 0)\n"; + } else { + print " if ($prefix$call->{ProcName}("; + print join(', ', @args_list); + print ") < 0)\n"; + } print " goto cleanup;\n"; print "\n"; } @@ -995,6 +1035,29 @@ elsif ($mode eq "server") { print "\n"; } + if ($modern_ret_as_list) { + print " if (nresults > $single_ret_list_max_define) {\n"; + print " virReportError(VIR_ERR_INTERNAL_ERROR,\n"; + print " _(\"Too many ${single_ret_list_error_msg_type}s '%d' for limit '%d'\"),\n"; + print " nresults, $single_ret_list_max_define);\n"; + print " goto cleanup;\n"; + print " }\n"; + print "\n"; + print " if (result && nresults) {\n"; + print " if (VIR_ALLOC_N(ret->$single_ret_list_name.${single_ret_list_name}_val, nresults) < 0)\n"; + print " goto cleanup;\n"; + print "\n"; + print " ret->$single_ret_list_name.${single_ret_list_name}_len = nresults;\n"; + print " for (i = 0; i < nresults; i++)\n"; + print " make_nonnull_$modern_ret_struct_name(ret->$single_ret_list_name.${single_ret_list_name}_val + i, result[i]);\n"; + print " } else {\n"; + print " ret->$single_ret_list_name.${single_ret_list_name}_len = 0;\n"; + print " ret->$single_ret_list_name.${single_ret_list_name}_val = NULL;\n"; + print " }\n"; + print "\n"; + print " ret->ret = nresults;\n"; + } + if (@prepare_ret_list) { print " "; print join("\n ", @prepare_ret_list); @@ -1030,6 +1093,14 @@ elsif ($mode eq "server") { print "\n"; } + if ($modern_ret_as_list) { + print " if (result) {\n"; + print " for (i = 0; i < nresults; i++)\n"; + print " virObjectUnref(result[i]);\n"; + print " }\n"; + print " VIR_FREE(result);\n"; + } + print " return rv;\n"; print "}\n\n\n\n"; } @@ -1277,6 +1348,9 @@ elsif ($mode eq "client") { my $single_ret_list_max_define = "undefined"; my $single_ret_cleanup = 0; my $multi_ret = 0; + my $modern_ret_as_list = 0; + my $modern_ret_struct_name = 0; + my $modern_ret_var_type = "undefined"; if ($rettype ne "void" and scalar(@{$call->{ret_members}}) > 1) { @@ -1296,6 +1370,20 @@ elsif ($mode eq "client") { } push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); + } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + my $proc_name = name_to_TypeName($1); + + if ($structprefix eq "admin") { + $modern_ret_var_type = "virAdm${proc_name}Ptr"; + } else { + $modern_ret_var_type = "vir${proc_name}Ptr"; + } + + $modern_ret_struct_name = $1; + $single_ret_list_name = $2; + $single_ret_list_max_var = $3; + + $modern_ret_as_list = 1; } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) { # just make all other array types fail die "unhandled type for multi-return-value for " . @@ -1305,7 +1393,7 @@ elsif ($mode eq "client") { my $sign = ""; $sign = "U" if ($1); push(@ret_list, "HYPER_TO_${sign}LONG(result->$3, ret.$3);"); - } else { + } elsif (!$modern_ret_as_list) { push(@ret_list, "result->$3 = ret.$3;"); } } else { @@ -1434,16 +1522,40 @@ elsif ($mode eq "client") { } } + if ($modern_ret_as_list) { + # clear arguments and setters we don't want in this code + @args_list = grep {!/\bneed_results\b/} @args_list; + @setters_list = grep {!/\bneed_results\b/} @setters_list; + + push(@vars_list, "${modern_ret_var_type} *tmp_results = NULL"); + push(@setters_list, "args.need_results = !!result;"); + + $single_ret_var = "int rv = -1"; + $single_ret_type = "int"; + } + # select struct type for multi-return-value functions if ($multi_ret) { + my $struct_name = "undefined"; + if (!(defined $call->{ret_offset})) { die "multi-return-value without insert@<offset> annotation: $call->{ret}"; } - my $struct_name = $call->{ProcName}; - $struct_name =~ s/Get//; + if ($modern_ret_as_list) { + $struct_name = name_to_TypeName($modern_ret_struct_name); + + $struct_name .= "Ptr **"; + if ($structprefix eq "admin") { + $struct_name = "Adm${struct_name}"; + } + } else { + $struct_name = $call->{ProcName}; - splice(@args_list, $call->{ret_offset}, 0, ("vir${struct_name}Ptr result")); + $struct_name =~ s/Get//; + $struct_name = "${struct_name}Ptr " + } + splice(@args_list, $call->{ret_offset}, 0, ("vir$struct_name result")); } if ($call->{streamflag} ne "none") { @@ -1482,7 +1594,8 @@ elsif ($mode eq "client") { print " $var;\n"; } - if ($single_ret_as_list) { + if ($single_ret_as_list or + $modern_ret_as_list) { print " size_t i;\n"; } @@ -1595,7 +1708,8 @@ elsif ($mode eq "client") { print " }\n"; print "\n"; - if ($single_ret_as_list) { + if ($single_ret_as_list or + $modern_ret_as_list) { print " if (ret.$single_ret_list_name.${single_ret_list_name}_len > $single_ret_list_max_var) {\n"; print " virReportError(VIR_ERR_RPC,\n"; print " _(\"too many remote ${single_ret_list_error_msg_type}s: %d > %d\"),\n"; @@ -1603,6 +1717,9 @@ elsif ($mode eq "client") { print " goto cleanup;\n"; print " }\n"; print "\n"; + } + + if ($single_ret_as_list) { print " /* This call is caller-frees (although that isn't clear from\n"; print " * the documentation). However xdr_free will free up both the\n"; print " * names and the list of pointers, so we have to VIR_STRDUP the\n"; @@ -1618,8 +1735,23 @@ elsif ($mode eq "client") { print " }\n"; print " }\n"; print "\n"; + } elsif ($modern_ret_as_list) { + print " if (result) {\n"; + print " if (VIR_ALLOC_N(tmp_results, ret.$single_ret_list_name.${single_ret_list_name}_len + 1) < 0)\n"; + print " goto cleanup;\n"; + print "\n"; + print " for (i = 0; i < ret.$single_ret_list_name.${single_ret_list_name}_len; i++) {\n"; + print " tmp_results[i] = get_nonnull_$modern_ret_struct_name($priv_src, ret.$single_ret_list_name.${single_ret_list_name}_val[i]);\n"; + print " if (!tmp_results[i])\n"; + print " goto cleanup;\n"; + print " }\n"; + print " *result = tmp_results;\n"; + print " tmp_results = NULL;\n"; + print " }\n"; + print "\n"; } + if (@ret_list2) { print " "; print join("\n ", @ret_list2); @@ -1641,12 +1773,24 @@ elsif ($mode eq "client") { } if ($multi_ret or !@ret_list) { - print " rv = 0;\n"; + if ($modern_ret_as_list) { + print " rv = ret.ret;\n"; + } else { + print " rv = 0;\n"; + } } - if ($single_ret_as_list or $single_ret_cleanup) { + if ($single_ret_as_list or $single_ret_cleanup or $modern_ret_as_list) { print "\n"; print "cleanup:\n"; + if ($modern_ret_as_list) { + print " if (tmp_results) {\n"; + print " for (i = 0; i < ret.$single_ret_list_name.${single_ret_list_name}_len; i++)\n"; + print " virObjectUnref(tmp_results[i]);\n"; + print " VIR_FREE(tmp_results);\n"; + print " }\n"; + print "\n"; + } print " xdr_free((xdrproc_t)xdr_$call->{ret}, (char *)&ret);\n"; } -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:04AM +0100, Martin Kletzander wrote:
Let's call it modern_ret_as_list as opposed to single_ret_as_list. The latter was able to return list of things. However the new, more modern, version came and it is used since listAllDomains till nowadays in ListServers.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 200 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 172 insertions(+), 28 deletions(-)
@@ -1277,6 +1348,9 @@ elsif ($mode eq "client") { my $single_ret_list_max_define = "undefined"; my $single_ret_cleanup = 0; my $multi_ret = 0; + my $modern_ret_as_list = 0;
+ my $modern_ret_struct_name = 0;
The other strings use "undefined" as the initializer.
+ my $modern_ret_var_type = "undefined";
if ($rettype ne "void" and scalar(@{$call->{ret_members}}) > 1) { @@ -1296,6 +1370,20 @@ elsif ($mode eq "client") { }
push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); + } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + my $proc_name = name_to_TypeName($1); + + if ($structprefix eq "admin") { + $modern_ret_var_type = "virAdm${proc_name}Ptr"; + } else { + $modern_ret_var_type = "vir${proc_name}Ptr"; + } + + $modern_ret_struct_name = $1;
This should also set: $single_ret_list_error_msg_type = $1; Otherwise the commit after this generates a funny error in remoteAdminConnectListServers: + if (ret.servers.servers_len > ADMIN_SERVER_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("too many remote undefineds: %d > %d"), + ret.servers.servers_len, ADMIN_SERVER_LIST_MAX); + goto cleanup; + }
+ $single_ret_list_name = $2; + $single_ret_list_max_var = $3; + + $modern_ret_as_list = 1; } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) { # just make all other array types fail die "unhandled type for multi-return-value for " .
- my $struct_name = $call->{ProcName}; - $struct_name =~ s/Get//; + if ($modern_ret_as_list) { + $struct_name = name_to_TypeName($modern_ret_struct_name); + + $struct_name .= "Ptr **"; + if ($structprefix eq "admin") { + $struct_name = "Adm${struct_name}"; + } + } else { + $struct_name = $call->{ProcName};
- splice(@args_list, $call->{ret_offset}, 0, ("vir${struct_name}Ptr result")); + $struct_name =~ s/Get//; + $struct_name = "${struct_name}Ptr " + } + splice(@args_list, $call->{ret_offset}, 0, ("vir$struct_name result"));
There's an extra space between ${struct_name} and result.
}
if ($call->{streamflag} ne "none") { @@ -1482,7 +1594,8 @@ elsif ($mode eq "client") { print " $var;\n"; }
- if ($single_ret_as_list) { + if ($single_ret_as_list or + $modern_ret_as_list) { print " size_t i;\n"; }
ACK with the nits fixed. Jan
Since we have the opportunity now, let's save some precious code lines. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin.c | 45 --------------------------------- daemon/admin_server.c | 6 ++--- daemon/admin_server.h | 6 ++--- po/POTFILES.in | 1 - src/admin/admin_protocol.x | 4 +-- src/admin/admin_remote.c | 63 ---------------------------------------------- 6 files changed, 8 insertions(+), 117 deletions(-) diff --git a/daemon/admin.c b/daemon/admin.c index cae2a84d84b8..3169cddb5807 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -133,49 +133,4 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; } -static int -adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, - admin_connect_list_servers_args *args, - admin_connect_list_servers_ret *ret) -{ - virNetServerPtr *servers = NULL; - int nservers = 0; - int rv = -1; - size_t i; - struct daemonAdmClientPrivate *priv = - virNetServerClientGetPrivateData(client); - - if ((nservers = - adminDaemonListServers(priv->dmn, - args->need_results ? &servers : NULL, - args->flags)) < 0) - goto cleanup; - - if (servers && nservers) { - if (VIR_ALLOC_N(ret->servers.servers_val, nservers) < 0) - goto cleanup; - - ret->servers.servers_len = nservers; - for (i = 0; i < nservers; i++) - make_nonnull_server(ret->servers.servers_val + i, servers[i]); - } else { - ret->servers.servers_len = 0; - ret->servers.servers_val = NULL; - } - - ret->ret = nservers; - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (servers && nservers > 0) - for (i = 0; i < nservers; i++) - virObjectUnref(servers[i]); - VIR_FREE(servers); - return rv; -} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c index a35f33130607..6eabbe4ae6d5 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -37,9 +37,9 @@ VIR_LOG_INIT("daemon.admin_server"); int -adminDaemonListServers(virNetDaemonPtr dmn, - virNetServerPtr **servers, - unsigned int flags) +adminConnectListServers(virNetDaemonPtr dmn, + virNetServerPtr **servers, + unsigned int flags) { int ret = -1; virNetServerPtr *srvs = NULL; diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 606442c19f5f..b77653f6c384 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -26,8 +26,8 @@ # include "rpc/virnetdaemon.h" -int adminDaemonListServers(virNetDaemonPtr dmn, - virNetServerPtr **servers, - unsigned int flags); +int adminConnectListServers(virNetDaemonPtr dmn, + virNetServerPtr **servers, + unsigned int flags); #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/po/POTFILES.in b/po/POTFILES.in index ff207cbb03e7..171d2b1e1534 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -10,7 +10,6 @@ gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c -src/admin/admin_remote.c src/bhyve/bhyve_command.c src/bhyve/bhyve_device.c src/bhyve/bhyve_driver.c diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 742ed2a89cd1..205bfe8f7a52 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -60,7 +60,7 @@ struct admin_connect_list_servers_args { unsigned int flags; }; -struct admin_connect_list_servers_ret { +struct admin_connect_list_servers_ret { /* insert@1 */ admin_nonnull_server servers<ADMIN_SERVER_LIST_MAX>; unsigned int ret; }; @@ -103,7 +103,7 @@ enum admin_procedure { ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, /** - * @generate: none + * @generate: both */ ADMIN_PROC_CONNECT_LIST_SERVERS = 4 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 3745b9e7017a..21e0dd30c4bf 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -224,66 +224,3 @@ remoteAdminPrivNew(const char *sock_path) virObjectUnref(priv); return NULL; } - -static int -remoteAdminConnectListServers(virAdmConnectPtr conn, - virAdmServerPtr **servers, - unsigned int flags) -{ - int rv = -1; - size_t i; - virAdmServerPtr *tmp_srvs = NULL; - remoteAdminPrivPtr priv = conn->privateData; - admin_connect_list_servers_args args; - admin_connect_list_servers_ret ret; - - args.need_results = !!servers; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - virObjectLock(priv); - - if (call(conn, - 0, - ADMIN_PROC_CONNECT_LIST_SERVERS, - (xdrproc_t) xdr_admin_connect_list_servers_args, - (char *) &args, - (xdrproc_t) xdr_admin_connect_list_servers_ret, - (char *) &ret) == -1) - goto done; - - if (ret.servers.servers_len > ADMIN_SERVER_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many servers '%d' for limit '%d'"), - ret.servers.servers_len, ADMIN_SERVER_LIST_MAX); - goto cleanup; - } - - if (servers) { - if (VIR_ALLOC_N(tmp_srvs, ret.servers.servers_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.servers.servers_len; i++) { - tmp_srvs[i] = get_nonnull_server(conn, ret.servers.servers_val[i]); - if (!tmp_srvs[i]) - goto cleanup; - } - *servers = tmp_srvs; - tmp_srvs = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_srvs) { - for (i = 0; i < ret.servers.servers_len; i++) - virObjectUnref(tmp_srvs[i]); - VIR_FREE(tmp_srvs); - } - - xdr_free((xdrproc_t) xdr_admin_connect_list_servers_ret, (char *) &ret); - - done: - virObjectUnlock(priv); - return rv; -} -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:05AM +0100, Martin Kletzander wrote:
Since we have the opportunity now, let's save some precious code lines.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin.c | 45 --------------------------------- daemon/admin_server.c | 6 ++--- daemon/admin_server.h | 6 ++--- po/POTFILES.in | 1 - src/admin/admin_protocol.x | 4 +-- src/admin/admin_remote.c | 63 ---------------------------------------------- 6 files changed, 8 insertions(+), 117 deletions(-)
ACK, beautiful diffstat. The new generated version of adminDispatchConnectListServers has the following check: + if (!priv->dmn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } Is it dead code or has it been missing in the manual implementation? Jan
On Thu, Mar 10, 2016 at 02:57:39PM +0100, Ján Tomko wrote:
On Thu, Mar 10, 2016 at 05:54:05AM +0100, Martin Kletzander wrote:
Since we have the opportunity now, let's save some precious code lines.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin.c | 45 --------------------------------- daemon/admin_server.c | 6 ++--- daemon/admin_server.h | 6 ++--- po/POTFILES.in | 1 - src/admin/admin_protocol.x | 4 +-- src/admin/admin_remote.c | 63 ---------------------------------------------- 6 files changed, 8 insertions(+), 117 deletions(-)
ACK, beautiful diffstat.
Check the stat for the whole series thinking about the fact that this series adds new things :)
The new generated version of adminDispatchConnectListServers has the following check:
+ if (!priv->dmn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + }
Is it dead code or has it been missing in the manual implementation?
This is one of the things that this patch "fixed". That should've been there before as well.
Jan
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index d6ce9b85e158..bd422842bc48 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -600,6 +600,16 @@ elsif ($mode eq "server") { } else { push(@args_list, "args->$arg_name"); } + } elsif ($args_member =~ m/^admin_nonnull_(server) (\S+);/) { + my $type_name = name_to_TypeName($1); + + push(@vars_list, "virNet${type_name}Ptr $2 = NULL"); + push(@getters_list, + " if (!($2 = get_nonnull_$1(priv->dmn, args->$2)))\n" . + " goto cleanup;\n"); + push(@args_list, "$2"); + push(@free_list, + " virObjectUnref($2);"); } elsif ($args_member =~ m/^(\/)?\*/) { # ignore comments } else { @@ -819,6 +829,16 @@ elsif ($mode eq "server") { } elsif ($ret_member =~ m/^opaque (\S+)<\S+>;/) { # error out on unannotated arrays die "opaque array without insert@<offset> annotation: $ret_member"; + } elsif ($ret_member =~ m/^admin_nonnull_(server) (\S+);/) { + my $type_name = name_to_TypeName($1); + + push(@vars_list, "virNet${type_name}Ptr $2 = NULL"); + push(@ret_list, "make_nonnull_$1(&ret->$2, $2);"); + push(@free_list, + " virObjectUnref($2);"); + $single_ret_var = $2; + $single_ret_by_ref = 0; + $single_ret_check = " == NULL"; } elsif ($ret_member =~ m/^(\/)?\*/) { # ignore comments } else { @@ -1317,6 +1337,17 @@ elsif ($mode eq "client") { push(@args_list, "$type_name $arg_name"); push(@setters_list, "args.$arg_name = $arg_name;"); + } elsif ($args_member =~ m/^admin_nonnull_(server) (\S+);/) { + my $name = $1; + my $arg_name = $2; + my $type_name = name_to_TypeName($name); + + if ($is_first_arg) { + $priv_src = "$arg_name->conn"; + } + + push(@args_list, "virAdm${type_name}Ptr $arg_name"); + push(@setters_list, "make_nonnull_$1(&args.$arg_name, $arg_name);"); } elsif ($args_member =~ m/^(\/)?\*/) { # ignore comments } else { @@ -1513,6 +1544,16 @@ elsif ($mode eq "client") { $single_ret_var = "unsigned long long rv = 0"; $single_ret_type = "unsigned long long"; } + } elsif ($ret_member =~ m/^admin_nonnull_(server) (\S+);/) { + my $name = $1; + my $arg_name = $2; + my $type_name = name_to_TypeName($name); + + push(@ret_list, "rv = get_nonnull_$name($priv_src, ret.$arg_name);"); + push(@ret_list, "xdr_free((xdrproc_t)xdr_$rettype, (char *)&ret);"); + + $single_ret_var = "virAdm${type_name}Ptr rv = NULL"; + $single_ret_type = "virAdm${type_name}Ptr"; } elsif ($ret_member =~ m/^(\/)?\*/) { # ignore comments } else { -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:06AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
ACK Jan
It does not have a suffix ByName because there are no other means of looking up the server and since the name is known, this should be the preferred one. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 5 +++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 81 insertions(+), 1 deletion(-) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 6eabbe4ae6d5..1d16bc9e9379 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -56,3 +56,13 @@ adminConnectListServers(virNetDaemonPtr dmn, cleanup: return ret; } + +virNetServerPtr +adminConnectLookupServer(virNetDaemonPtr dmn, + const char *name, + unsigned int flags) +{ + virCheckFlags(flags, NULL); + + return virNetDaemonGetServer(dmn, name); +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index b77653f6c384..9d0adf02c7ad 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -25,9 +25,14 @@ # define __LIBVIRTD_ADMIN_SERVER_H__ # include "rpc/virnetdaemon.h" +# include "rpc/virnetserver.h" int adminConnectListServers(virNetDaemonPtr dmn, virNetServerPtr **servers, unsigned int flags); +virNetServerPtr adminConnectLookupServer(virNetDaemonPtr dmn, + const char *name, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index e9ec3941023a..25bcbf47ddeb 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -106,6 +106,10 @@ int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn, const char *virAdmServerGetName(virAdmServerPtr srv); +virAdmServerPtr virAdmConnectLookupServer(virAdmConnectPtr conn, + const char *name, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 205bfe8f7a52..6590980ce1ca 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -65,6 +65,15 @@ struct admin_connect_list_servers_ret { /* insert@1 */ unsigned int ret; }; +struct admin_connect_lookup_server_args { + admin_nonnull_string name; + unsigned int flags; +}; + +struct admin_connect_lookup_server_ret { + admin_nonnull_server srv; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -105,5 +114,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_LIST_SERVERS = 4 + ADMIN_PROC_CONNECT_LIST_SERVERS = 4, + + /** + * @generate: both + */ + ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 8f2633ae7ce2..d8aca0617147 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -19,9 +19,17 @@ struct admin_connect_list_servers_ret { } servers; u_int ret; }; +struct admin_connect_lookup_server_args { + admin_nonnull_string name; + u_int flags; +}; +struct admin_connect_lookup_server_ret { + admin_nonnull_server srv; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, ADMIN_PROC_CONNECT_LIST_SERVERS = 4, + ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index add4fcc3576a..7a7bebf7d6c9 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -639,3 +639,39 @@ virAdmConnectListServers(virAdmConnectPtr conn, virDispatchError(NULL); return -1; } + +/** + * virAdmConnectLookupServer: + * @conn: daemon connection reference + * @name: name of the server too lookup + * @flags: unused, must be 0 + * + * Collect list of all servers provided by daemon the client is connected to. + * + * Returns the number of servers available on daemon side or -1 in case of a + * failure, setting @servers to NULL. There is a guaranteed extra element set + * to NULL in the @servers list returned to make the iteration easier, excluding + * this extra element from the final count. + * Caller is responsible to call virAdmServerFree() on each list element, + * followed by freeing @servers. + */ +virAdmServerPtr +virAdmConnectLookupServer(virAdmConnectPtr conn, + const char *name, + unsigned int flags) +{ + virAdmServerPtr ret = NULL; + + VIR_DEBUG("conn=%p, name=%s, flags=%x", conn, NULLSTR(name), flags); + virResetLastError(); + + virCheckAdmConnectGoto(conn, cleanup); + virCheckNonNullArgGoto(name, cleanup); + virCheckFlagsGoto(0, cleanup); + + ret = remoteAdminConnectLookupServer(conn, name, flags); + cleanup: + if (!ret) + virDispatchError(NULL); + return ret; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index f22137bcc6af..268f1e6f2fdd 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,8 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; +xdr_admin_connect_lookup_server_args; +xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 52ff2dc1b411..58d085e1f118 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -24,4 +24,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectListServers; virAdmServerGetName; virAdmServerFree; + virAdmConnectLookupServer; }; -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:07AM +0100, Martin Kletzander wrote:
It does not have a suffix ByName because there are no other means of looking up the server and since the name is known, this should be the preferred one.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 5 +++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 81 insertions(+), 1 deletion(-)
ACK from me, maybe Erik wants to see this too. Jan
On Thu, Mar 10, 2016 at 02:58:52PM +0100, Ján Tomko wrote:
On Thu, Mar 10, 2016 at 05:54:07AM +0100, Martin Kletzander wrote:
It does not have a suffix ByName because there are no other means of looking up the server and since the name is known, this should be the preferred one.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 5 +++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 81 insertions(+), 1 deletion(-)
ACK from me, maybe Erik wants to see this too.
I pushed the series except this patch, thanks for the reviews, I'll let Erik express himself about this one.
On 10/03/16 05:54, Martin Kletzander wrote:
It does not have a suffix ByName because there are no other means of looking up the server and since the name is known, this should be the preferred one.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 5 +++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 81 insertions(+), 1 deletion(-)
... + +/** + * virAdmConnectLookupServer: + * @conn: daemon connection reference + * @name: name of the server too lookup + * @flags: unused, must be 0 + * + * Collect list of all servers provided by daemon the client is connected to. + * + * Returns the number of servers available on daemon side or -1 in case of a + * failure, setting @servers to NULL. There is a guaranteed extra element set + * to NULL in the @servers list returned to make the iteration easier, excluding + * this extra element from the final count. + * Caller is responsible to call virAdmServerFree() on each list element, + * followed by freeing @servers. + */
^^ This isn't quite describing what the function does, is it? Power of copy-paste-edit I guess :).
+virAdmServerPtr +virAdmConnectLookupServer(virAdmConnectPtr conn, + const char *name, + unsigned int flags) +{ + virAdmServerPtr ret = NULL; + + VIR_DEBUG("conn=%p, name=%s, flags=%x", conn, NULLSTR(name), flags); + virResetLastError(); + + virCheckAdmConnectGoto(conn, cleanup); + virCheckNonNullArgGoto(name, cleanup); + virCheckFlagsGoto(0, cleanup); + + ret = remoteAdminConnectLookupServer(conn, name, flags); + cleanup: + if (!ret) + virDispatchError(NULL); + return ret; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index f22137bcc6af..268f1e6f2fdd 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,8 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; +xdr_admin_connect_lookup_server_args; +xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args;
# datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 52ff2dc1b411..58d085e1f118 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -24,4 +24,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectListServers; virAdmServerGetName; virAdmServerFree; + virAdmConnectLookupServer; };
Just a reminder, not that we should do something about it right now, but let's not forget about the fact, that all the symbols are introduced one after another within different releases. It's been already 2 releases, since we pushed first admin-related code. So, before we enable admin, the library version will have to be changed from 1.3.0 to the appropriate version of the exact release in which we finally enable it.
ACK with that commentary mentioned above changed. Erik
On Wed, Mar 16, 2016 at 12:47:53PM +0100, Erik Skultety wrote:
On 10/03/16 05:54, Martin Kletzander wrote:
It does not have a suffix ByName because there are no other means of looking up the server and since the name is known, this should be the preferred one.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 5 +++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 16 +++++++++++++++- src/admin_protocol-structs | 8 ++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 8 files changed, 81 insertions(+), 1 deletion(-)
... + +/** + * virAdmConnectLookupServer: + * @conn: daemon connection reference + * @name: name of the server too lookup + * @flags: unused, must be 0 + * + * Collect list of all servers provided by daemon the client is connected to. + * + * Returns the number of servers available on daemon side or -1 in case of a + * failure, setting @servers to NULL. There is a guaranteed extra element set + * to NULL in the @servers list returned to make the iteration easier, excluding + * this extra element from the final count. + * Caller is responsible to call virAdmServerFree() on each list element, + * followed by freeing @servers. + */
^^ This isn't quite describing what the function does, is it? Power of copy-paste-edit I guess :).
It stayed rotting in one of my branches for a while and when I needed to send it, I forgot that it's not fixed completely. Not even Jan found that out, so at least now I know somebody reads that ;) Thanks for spotting it.
+virAdmServerPtr +virAdmConnectLookupServer(virAdmConnectPtr conn, + const char *name, + unsigned int flags) +{ + virAdmServerPtr ret = NULL; + + VIR_DEBUG("conn=%p, name=%s, flags=%x", conn, NULLSTR(name), flags); + virResetLastError(); + + virCheckAdmConnectGoto(conn, cleanup); + virCheckNonNullArgGoto(name, cleanup); + virCheckFlagsGoto(0, cleanup); + + ret = remoteAdminConnectLookupServer(conn, name, flags); + cleanup: + if (!ret) + virDispatchError(NULL); + return ret; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index f22137bcc6af..268f1e6f2fdd 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,8 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; +xdr_admin_connect_lookup_server_args; +xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args;
# datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 52ff2dc1b411..58d085e1f118 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -24,4 +24,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectListServers; virAdmServerGetName; virAdmServerFree; + virAdmConnectLookupServer; };
Just a reminder, not that we should do something about it right now, but let's not forget about the fact, that all the symbols are introduced one after another within different releases. It's been already 2 releases, since we pushed first admin-related code. So, before we enable admin, the library version will have to be changed from 1.3.0 to the appropriate version of the exact release in which we finally enable it.
ACK with that commentary mentioned above changed.
Yes, that's another thing I'm keeping in my mind which I hope I don't forget. However, nobody knows if the same things as with the description is not going to happen. Thanks for the review.
Erik
Until now, the script assumed that snapshot name is 'snap', but that's going to change. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index bd422842bc48..21f16d19bbed 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -510,14 +510,14 @@ elsif ($mode eq "server") { push(@args_list, "$2"); push(@free_list, " virObjectUnref($2);"); - } elsif ($args_member =~ m/^remote_nonnull_domain_snapshot /) { + } elsif ($args_member =~ m/^remote_nonnull_domain_snapshot (\S+);$/) { push(@vars_list, "virDomainPtr dom = NULL"); push(@vars_list, "virDomainSnapshotPtr snapshot = NULL"); push(@getters_list, - " if (!(dom = get_nonnull_domain(priv->conn, args->snap.dom)))\n" . + " if (!(dom = get_nonnull_domain(priv->conn, args->${1}.dom)))\n" . " goto cleanup;\n" . "\n" . - " if (!(snapshot = get_nonnull_domain_snapshot(dom, args->snap)))\n" . + " if (!(snapshot = get_nonnull_domain_snapshot(dom, args->${1})))\n" . " goto cleanup;\n"); push(@args_list, "snapshot"); push(@free_list, -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:08AM +0100, Martin Kletzander wrote:
Until now, the script assumed that snapshot name is 'snap', but that's going to change.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
ACK Jan
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 21f16d19bbed..5e800ab05e41 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -649,7 +649,7 @@ elsif ($mode eq "server") { if (!$modern_ret_as_list) { push(@ret_list, "ret->$3 = tmp.$3;"); } - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { $modern_ret_struct_name = $1; $single_ret_list_error_msg_type = $1; $single_ret_list_name = $2; @@ -1401,7 +1401,7 @@ elsif ($mode eq "client") { } push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { my $proc_name = name_to_TypeName($1); if ($structprefix eq "admin") { @@ -1413,6 +1413,7 @@ elsif ($mode eq "client") { $modern_ret_struct_name = $1; $single_ret_list_name = $2; $single_ret_list_max_var = $3; + $single_ret_list_error_msg_type = $1; $modern_ret_as_list = 1; } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) { @@ -1729,12 +1730,13 @@ elsif ($mode eq "client") { $callflags = "REMOTE_CALL_LXC"; } + my $call_priv = $priv_src; if ($structprefix ne "admin") { - $priv_src = "$priv_src, priv"; + $call_priv = "$call_priv, priv"; } print "\n"; - print " if (call($priv_src, $callflags, $call->{constname},\n"; + print " if (call($call_priv, $callflags, $call->{constname},\n"; print " (xdrproc_t)xdr_$argtype, (char *)$call_args,\n"; print " (xdrproc_t)xdr_$rettype, (char *)$call_ret) == -1) {\n"; @@ -1777,6 +1779,9 @@ elsif ($mode eq "client") { print " }\n"; print "\n"; } elsif ($modern_ret_as_list) { + if ($modern_ret_struct_name eq "domain_snapshot") { + $priv_src =~ s/->conn//; + } print " if (result) {\n"; print " if (VIR_ALLOC_N(tmp_results, ret.$single_ret_list_name.${single_ret_list_name}_len + 1) < 0)\n"; print " goto cleanup;\n"; -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:09AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 21f16d19bbed..5e800ab05e41 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -649,7 +649,7 @@ elsif ($mode eq "server") { if (!$modern_ret_as_list) { push(@ret_list, "ret->$3 = tmp.$3;"); } - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { $modern_ret_struct_name = $1; $single_ret_list_error_msg_type = $1; $single_ret_list_name = $2; @@ -1401,7 +1401,7 @@ elsif ($mode eq "client") { }
push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { my $proc_name = name_to_TypeName($1);
if ($structprefix eq "admin") {
These lines are way too long. I don't know whether splitting the regex over multiple lines with /x or putting parts of it in a separate variable is the preferred way.
@@ -1413,6 +1413,7 @@ elsif ($mode eq "client") { $modern_ret_struct_name = $1; $single_ret_list_name = $2; $single_ret_list_max_var = $3; + $single_ret_list_error_msg_type = $1;
$modern_ret_as_list = 1; } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) {
This hunk should be in the commit adding the other assignments. ACK minus the hunk error_msg hunk. Jan
On Thu, Mar 10, 2016 at 03:26:04PM +0100, Ján Tomko wrote:
On Thu, Mar 10, 2016 at 05:54:09AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 21f16d19bbed..5e800ab05e41 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -649,7 +649,7 @@ elsif ($mode eq "server") { if (!$modern_ret_as_list) { push(@ret_list, "ret->$3 = tmp.$3;"); } - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { $modern_ret_struct_name = $1; $single_ret_list_error_msg_type = $1; $single_ret_list_name = $2; @@ -1401,7 +1401,7 @@ elsif ($mode eq "client") { }
push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); - } elsif ($ret_member =~ m/admin_nonnull_(server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { my $proc_name = name_to_TypeName($1);
if ($structprefix eq "admin") {
These lines are way too long. I don't know whether splitting the regex over multiple lines with /x or putting parts of it in a separate variable is the preferred way.
Looking at other similar lines we just don't care about the line lengths. Having said that, I'm OK with splitting them, but in the whole file at once, if anyone wants to tackle that.
@@ -1413,6 +1413,7 @@ elsif ($mode eq "client") { $modern_ret_struct_name = $1; $single_ret_list_name = $2; $single_ret_list_max_var = $3; + $single_ret_list_error_msg_type = $1;
$modern_ret_as_list = 1; } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) {
This hunk should be in the commit adding the other assignments.
Yes, fixed, thanks.
ACK minus the hunk error_msg hunk.
Jan
Since gendisplatch can now generate "modern" *ListAll* functions, let them all be generated. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/remote.c | 599 --------------------------------------- src/remote/remote_driver.c | 654 ------------------------------------------- src/remote/remote_protocol.x | 40 +-- 3 files changed, 20 insertions(+), 1273 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index f5ca2acc98e2..2bf9e8392252 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1512,64 +1512,6 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS } static int -remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_domains_args *args, - remote_connect_list_all_domains_ret *ret) -{ - virDomainPtr *doms = NULL; - int ndomains = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((ndomains = virConnectListAllDomains(priv->conn, - args->need_results ? &doms : NULL, - args->flags)) < 0) - goto cleanup; - - if (ndomains > REMOTE_DOMAIN_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domains '%d' for limit '%d'"), - ndomains, REMOTE_DOMAIN_LIST_MAX); - goto cleanup; - } - - if (doms && ndomains) { - if (VIR_ALLOC_N(ret->domains.domains_val, ndomains) < 0) - goto cleanup; - - ret->domains.domains_len = ndomains; - - for (i = 0; i < ndomains; i++) - make_nonnull_domain(ret->domains.domains_val + i, doms[i]); - } else { - ret->domains.domains_len = 0; - ret->domains.domains_val = NULL; - } - - ret->ret = ndomains; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (doms && ndomains > 0) - for (i = 0; i < ndomains; i++) - virObjectUnref(doms[i]); - VIR_FREE(doms); - return rv; -} - -static int remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, @@ -4564,547 +4506,6 @@ remoteDispatchDomainGetDiskErrors(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } -static int -remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_domain_list_all_snapshots_args *args, - remote_domain_list_all_snapshots_ret *ret) -{ - virDomainSnapshotPtr *snaps = NULL; - int nsnaps = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - virDomainPtr dom = NULL; - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if (!(dom = get_nonnull_domain(priv->conn, args->dom))) - goto cleanup; - - if ((nsnaps = virDomainListAllSnapshots(dom, - args->need_results ? &snaps : NULL, - args->flags)) < 0) - goto cleanup; - - if (nsnaps > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domain snapshots '%d' for limit '%d'"), - nsnaps, REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); - goto cleanup; - } - - if (snaps && nsnaps) { - if (VIR_ALLOC_N(ret->snapshots.snapshots_val, nsnaps) < 0) - goto cleanup; - - ret->snapshots.snapshots_len = nsnaps; - - for (i = 0; i < nsnaps; i++) - make_nonnull_domain_snapshot(ret->snapshots.snapshots_val + i, - snaps[i]); - } else { - ret->snapshots.snapshots_len = 0; - ret->snapshots.snapshots_val = NULL; - } - - ret->ret = nsnaps; - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - virObjectUnref(dom); - if (snaps && nsnaps > 0) - for (i = 0; i < nsnaps; i++) - virObjectUnref(snaps[i]); - VIR_FREE(snaps); - return rv; -} - -static int -remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_domain_snapshot_list_all_children_args *args, - remote_domain_snapshot_list_all_children_ret *ret) -{ - virDomainSnapshotPtr *snaps = NULL; - int nsnaps = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - virDomainPtr dom = NULL; - virDomainSnapshotPtr snapshot = NULL; - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if (!(dom = get_nonnull_domain(priv->conn, args->snapshot.dom))) - goto cleanup; - - if (!(snapshot = get_nonnull_domain_snapshot(dom, args->snapshot))) - goto cleanup; - - if ((nsnaps = virDomainSnapshotListAllChildren(snapshot, - args->need_results ? &snaps : NULL, - args->flags)) < 0) - goto cleanup; - - if (nsnaps > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domain snapshots '%d' for limit '%d'"), - nsnaps, REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); - goto cleanup; - } - - if (snaps && nsnaps) { - if (VIR_ALLOC_N(ret->snapshots.snapshots_val, nsnaps) < 0) - goto cleanup; - - ret->snapshots.snapshots_len = nsnaps; - - for (i = 0; i < nsnaps; i++) - make_nonnull_domain_snapshot(ret->snapshots.snapshots_val + i, - snaps[i]); - } else { - ret->snapshots.snapshots_len = 0; - ret->snapshots.snapshots_val = NULL; - } - - ret->ret = nsnaps; - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - virObjectUnref(snapshot); - virObjectUnref(dom); - if (snaps && nsnaps > 0) - for (i = 0; i < nsnaps; i++) - virObjectUnref(snaps[i]); - VIR_FREE(snaps); - return rv; -} - -static int -remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_storage_pools_args *args, - remote_connect_list_all_storage_pools_ret *ret) -{ - virStoragePoolPtr *pools = NULL; - int npools = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((npools = virConnectListAllStoragePools(priv->conn, - args->need_results ? &pools : NULL, - args->flags)) < 0) - goto cleanup; - - if (npools > REMOTE_STORAGE_POOL_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many storage pools '%d' for limit '%d'"), - npools, REMOTE_STORAGE_POOL_LIST_MAX); - goto cleanup; - } - - if (pools && npools) { - if (VIR_ALLOC_N(ret->pools.pools_val, npools) < 0) - goto cleanup; - - ret->pools.pools_len = npools; - - for (i = 0; i < npools; i++) - make_nonnull_storage_pool(ret->pools.pools_val + i, pools[i]); - } else { - ret->pools.pools_len = 0; - ret->pools.pools_val = NULL; - } - - ret->ret = npools; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (pools && npools > 0) - for (i = 0; i < npools; i++) - virObjectUnref(pools[i]); - VIR_FREE(pools); - return rv; -} - -static int -remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_storage_pool_list_all_volumes_args *args, - remote_storage_pool_list_all_volumes_ret *ret) -{ - virStorageVolPtr *vols = NULL; - virStoragePoolPtr pool = NULL; - int nvols = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if (!(pool = get_nonnull_storage_pool(priv->conn, args->pool))) - goto cleanup; - - if ((nvols = virStoragePoolListAllVolumes(pool, - args->need_results ? &vols : NULL, - args->flags)) < 0) - goto cleanup; - - if (nvols > REMOTE_STORAGE_VOL_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many storage volumes '%d' for limit '%d'"), - nvols, REMOTE_STORAGE_VOL_LIST_MAX); - goto cleanup; - } - - if (vols && nvols) { - if (VIR_ALLOC_N(ret->vols.vols_val, nvols) < 0) - goto cleanup; - - ret->vols.vols_len = nvols; - - for (i = 0; i < nvols; i++) - make_nonnull_storage_vol(ret->vols.vols_val + i, vols[i]); - } else { - ret->vols.vols_len = 0; - ret->vols.vols_val = NULL; - } - - ret->ret = nvols; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (vols && nvols > 0) - for (i = 0; i < nvols; i++) - virObjectUnref(vols[i]); - VIR_FREE(vols); - virObjectUnref(pool); - return rv; -} - -static int -remoteDispatchConnectListAllNetworks(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_networks_args *args, - remote_connect_list_all_networks_ret *ret) -{ - virNetworkPtr *nets = NULL; - int nnets = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((nnets = virConnectListAllNetworks(priv->conn, - args->need_results ? &nets : NULL, - args->flags)) < 0) - goto cleanup; - - if (nnets > REMOTE_NETWORK_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many networks '%d' for limit '%d'"), - nnets, REMOTE_NETWORK_LIST_MAX); - goto cleanup; - } - - if (nets && nnets) { - if (VIR_ALLOC_N(ret->nets.nets_val, nnets) < 0) - goto cleanup; - - ret->nets.nets_len = nnets; - - for (i = 0; i < nnets; i++) - make_nonnull_network(ret->nets.nets_val + i, nets[i]); - } else { - ret->nets.nets_len = 0; - ret->nets.nets_val = NULL; - } - - ret->ret = nnets; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (nets && nnets > 0) - for (i = 0; i < nnets; i++) - virObjectUnref(nets[i]); - VIR_FREE(nets); - return rv; -} - -static int -remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_interfaces_args *args, - remote_connect_list_all_interfaces_ret *ret) -{ - virInterfacePtr *ifaces = NULL; - int nifaces = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((nifaces = virConnectListAllInterfaces(priv->conn, - args->need_results ? &ifaces : NULL, - args->flags)) < 0) - goto cleanup; - - if (nifaces > REMOTE_INTERFACE_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many interfaces '%d' for limit '%d'"), - nifaces, REMOTE_INTERFACE_LIST_MAX); - goto cleanup; - } - - if (ifaces && nifaces) { - if (VIR_ALLOC_N(ret->ifaces.ifaces_val, nifaces) < 0) - goto cleanup; - - ret->ifaces.ifaces_len = nifaces; - - for (i = 0; i < nifaces; i++) - make_nonnull_interface(ret->ifaces.ifaces_val + i, ifaces[i]); - } else { - ret->ifaces.ifaces_len = 0; - ret->ifaces.ifaces_val = NULL; - } - - ret->ret = nifaces; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (ifaces && nifaces > 0) - for (i = 0; i < nifaces; i++) - virObjectUnref(ifaces[i]); - VIR_FREE(ifaces); - return rv; -} - -static int -remoteDispatchConnectListAllNodeDevices(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_node_devices_args *args, - remote_connect_list_all_node_devices_ret *ret) -{ - virNodeDevicePtr *devices = NULL; - int ndevices = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((ndevices = virConnectListAllNodeDevices(priv->conn, - args->need_results ? &devices : NULL, - args->flags)) < 0) - goto cleanup; - - if (ndevices > REMOTE_NODE_DEVICE_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many node devices '%d' for limit '%d'"), - ndevices, REMOTE_NODE_DEVICE_LIST_MAX); - goto cleanup; - } - - if (devices && ndevices) { - if (VIR_ALLOC_N(ret->devices.devices_val, ndevices) < 0) - goto cleanup; - - ret->devices.devices_len = ndevices; - - for (i = 0; i < ndevices; i++) - make_nonnull_node_device(ret->devices.devices_val + i, devices[i]); - } else { - ret->devices.devices_len = 0; - ret->devices.devices_val = NULL; - } - - ret->ret = ndevices; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (devices && ndevices > 0) - for (i = 0; i < ndevices; i++) - virObjectUnref(devices[i]); - VIR_FREE(devices); - return rv; -} - -static int -remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_nwfilters_args *args, - remote_connect_list_all_nwfilters_ret *ret) -{ - virNWFilterPtr *filters = NULL; - int nfilters = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((nfilters = virConnectListAllNWFilters(priv->conn, - args->need_results ? &filters : NULL, - args->flags)) < 0) - goto cleanup; - - if (nfilters > REMOTE_NWFILTER_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many network filters '%d' for limit '%d'"), - nfilters, REMOTE_NWFILTER_LIST_MAX); - goto cleanup; - } - - if (filters && nfilters) { - if (VIR_ALLOC_N(ret->filters.filters_val, nfilters) < 0) - goto cleanup; - - ret->filters.filters_len = nfilters; - - for (i = 0; i < nfilters; i++) - make_nonnull_nwfilter(ret->filters.filters_val + i, filters[i]); - } else { - ret->filters.filters_len = 0; - ret->filters.filters_val = NULL; - } - - ret->ret = nfilters; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (filters && nfilters > 0) - for (i = 0; i < nfilters; i++) - virObjectUnref(filters[i]); - VIR_FREE(filters); - return rv; -} - -static int -remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - remote_connect_list_all_secrets_args *args, - remote_connect_list_all_secrets_ret *ret) -{ - virSecretPtr *secrets = NULL; - int nsecrets = 0; - size_t i; - int rv = -1; - struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - - if (!priv->conn) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); - goto cleanup; - } - - if ((nsecrets = virConnectListAllSecrets(priv->conn, - args->need_results ? &secrets : NULL, - args->flags)) < 0) - goto cleanup; - - if (nsecrets > REMOTE_SECRET_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many secrets '%d' for limit '%d'"), - nsecrets, REMOTE_SECRET_LIST_MAX); - goto cleanup; - } - - if (secrets && nsecrets) { - if (VIR_ALLOC_N(ret->secrets.secrets_val, nsecrets) < 0) - goto cleanup; - - ret->secrets.secrets_len = nsecrets; - - for (i = 0; i < nsecrets; i++) - make_nonnull_secret(ret->secrets.secrets_val + i, secrets[i]); - } else { - ret->secrets.secrets_len = 0; - ret->secrets.secrets_val = NULL; - } - - ret->ret = nsecrets; - - rv = 0; - - cleanup: - if (rv < 0) - virNetMessageSaveError(rerr); - if (secrets && nsecrets > 0) - for (i = 0; i < nsecrets; i++) - virObjectUnref(secrets[i]); - VIR_FREE(secrets); - return rv; -} static int remoteDispatchNodeGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 2daa50786d22..d342227d4675 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1627,71 +1627,6 @@ remoteConnectListDomains(virConnectPtr conn, int *ids, int maxids) } static int -remoteConnectListAllDomains(virConnectPtr conn, - virDomainPtr **domains, - unsigned int flags) -{ - int rv = -1; - size_t i; - virDomainPtr *doms = NULL; - remote_connect_list_all_domains_args args; - remote_connect_list_all_domains_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!domains; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS, - (xdrproc_t) xdr_remote_connect_list_all_domains_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_domains_ret, - (char *) &ret) == -1) - goto done; - - if (ret.domains.domains_len > REMOTE_DOMAIN_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domains '%d' for limit '%d'"), - ret.domains.domains_len, REMOTE_DOMAIN_LIST_MAX); - goto cleanup; - } - - if (domains) { - if (VIR_ALLOC_N(doms, ret.domains.domains_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.domains.domains_len; i++) { - doms[i] = get_nonnull_domain(conn, ret.domains.domains_val[i]); - if (!doms[i]) - goto cleanup; - } - *domains = doms; - doms = NULL; - } - - rv = ret.ret; - - cleanup: - if (doms) { - for (i = 0; i < ret.domains.domains_len; i++) - virObjectUnref(doms[i]); - VIR_FREE(doms); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_domains_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, u_int ret_errors_len, int limit, @@ -2945,71 +2880,6 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, return rv; } -static int -remoteConnectListAllNetworks(virConnectPtr conn, - virNetworkPtr **nets, - unsigned int flags) -{ - int rv = -1; - size_t i; - virNetworkPtr *tmp_nets = NULL; - remote_connect_list_all_networks_args args; - remote_connect_list_all_networks_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!nets; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS, - (xdrproc_t) xdr_remote_connect_list_all_networks_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_networks_ret, - (char *) &ret) == -1) - goto done; - - if (ret.nets.nets_len > REMOTE_NETWORK_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many networks '%d' for limit '%d'"), - ret.nets.nets_len, REMOTE_NETWORK_LIST_MAX); - goto cleanup; - } - - if (nets) { - if (VIR_ALLOC_N(tmp_nets, ret.nets.nets_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.nets.nets_len; i++) { - tmp_nets[i] = get_nonnull_network(conn, ret.nets.nets_val[i]); - if (!tmp_nets[i]) - goto cleanup; - } - *nets = tmp_nets; - tmp_nets = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_nets) { - for (i = 0; i < ret.nets.nets_len; i++) - virObjectUnref(tmp_nets[i]); - VIR_FREE(tmp_nets); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_networks_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - static int remoteConnectNetworkEventRegisterAny(virConnectPtr conn, @@ -3202,267 +3072,6 @@ remoteConnectDomainQemuMonitorEventDeregister(virConnectPtr conn, return rv; } - -static int -remoteConnectListAllInterfaces(virConnectPtr conn, - virInterfacePtr **ifaces, - unsigned int flags) -{ - int rv = -1; - size_t i; - virInterfacePtr *tmp_ifaces = NULL; - remote_connect_list_all_interfaces_args args; - remote_connect_list_all_interfaces_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!ifaces; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES, - (xdrproc_t) xdr_remote_connect_list_all_interfaces_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_interfaces_ret, - (char *) &ret) == -1) - goto done; - - if (ret.ifaces.ifaces_len > REMOTE_INTERFACE_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many interfaces '%d' for limit '%d'"), - ret.ifaces.ifaces_len, REMOTE_INTERFACE_LIST_MAX); - goto cleanup; - } - - if (ifaces) { - if (VIR_ALLOC_N(tmp_ifaces, ret.ifaces.ifaces_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.ifaces.ifaces_len; i++) { - tmp_ifaces[i] = get_nonnull_interface(conn, ret.ifaces.ifaces_val[i]); - if (!tmp_ifaces[i]) - goto cleanup; - } - *ifaces = tmp_ifaces; - tmp_ifaces = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_ifaces) { - for (i = 0; i < ret.ifaces.ifaces_len; i++) - virObjectUnref(tmp_ifaces[i]); - } - VIR_FREE(tmp_ifaces); - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_interfaces_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int -remoteConnectListAllNodeDevices(virConnectPtr conn, - virNodeDevicePtr **devices, - unsigned int flags) -{ - int rv = -1; - size_t i; - virNodeDevicePtr *tmp_devices = NULL; - remote_connect_list_all_node_devices_args args; - remote_connect_list_all_node_devices_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!devices; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES, - (xdrproc_t) xdr_remote_connect_list_all_node_devices_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_node_devices_ret, - (char *) &ret) == -1) - goto done; - - if (ret.devices.devices_len > REMOTE_NODE_DEVICE_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many node devices '%d' for limit '%d'"), - ret.devices.devices_len, REMOTE_NODE_DEVICE_LIST_MAX); - goto cleanup; - } - - if (devices) { - if (VIR_ALLOC_N(tmp_devices, ret.devices.devices_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.devices.devices_len; i++) { - tmp_devices[i] = get_nonnull_node_device(conn, ret.devices.devices_val[i]); - if (!tmp_devices[i]) - goto cleanup; - } - *devices = tmp_devices; - tmp_devices = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_devices) { - for (i = 0; i < ret.devices.devices_len; i++) - virObjectUnref(tmp_devices[i]); - VIR_FREE(tmp_devices); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_node_devices_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int -remoteConnectListAllNWFilters(virConnectPtr conn, - virNWFilterPtr **filters, - unsigned int flags) -{ - int rv = -1; - size_t i; - virNWFilterPtr *tmp_filters = NULL; - remote_connect_list_all_nwfilters_args args; - remote_connect_list_all_nwfilters_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!filters; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS, - (xdrproc_t) xdr_remote_connect_list_all_nwfilters_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, - (char *) &ret) == -1) - goto done; - - if (ret.filters.filters_len > REMOTE_NWFILTER_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many network filters '%d' for limit '%d'"), - ret.filters.filters_len, REMOTE_NWFILTER_LIST_MAX); - goto cleanup; - } - - if (filters) { - if (VIR_ALLOC_N(tmp_filters, ret.filters.filters_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.filters.filters_len; i++) { - tmp_filters[i] = get_nonnull_nwfilter(conn, ret.filters.filters_val[i]); - if (!tmp_filters[i]) - goto cleanup; - } - *filters = tmp_filters; - tmp_filters = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_filters) { - for (i = 0; i < ret.filters.filters_len; i++) - virObjectUnref(tmp_filters[i]); - VIR_FREE(tmp_filters); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int -remoteConnectListAllSecrets(virConnectPtr conn, - virSecretPtr **secrets, - unsigned int flags) -{ - int rv = -1; - size_t i; - virSecretPtr *tmp_secrets = NULL; - remote_connect_list_all_secrets_args args; - remote_connect_list_all_secrets_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!secrets; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_SECRETS, - (xdrproc_t) xdr_remote_connect_list_all_secrets_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_secrets_ret, - (char *) &ret) == -1) - goto done; - - if (ret.secrets.secrets_len > REMOTE_SECRET_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many secrets '%d' for limit '%d'"), - ret.secrets.secrets_len, REMOTE_SECRET_LIST_MAX); - goto cleanup; - } - - if (secrets) { - if (VIR_ALLOC_N(tmp_secrets, ret.secrets.secrets_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.secrets.secrets_len; i++) { - tmp_secrets[i] = get_nonnull_secret(conn, ret.secrets.secrets_val[i]); - if (!tmp_secrets[i]) - goto cleanup; - } - *secrets = tmp_secrets; - tmp_secrets = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_secrets) { - for (i = 0; i < ret.secrets.secrets_len; i++) - virObjectUnref(tmp_secrets[i]); - VIR_FREE(tmp_secrets); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_secrets_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - /*----------------------------------------------------------------------*/ static char * @@ -3498,138 +3107,6 @@ remoteConnectFindStoragePoolSources(virConnectPtr conn, return rv; } -static int -remoteConnectListAllStoragePools(virConnectPtr conn, - virStoragePoolPtr **pools, - unsigned int flags) -{ - int rv = -1; - size_t i; - virStoragePoolPtr *tmp_pools = NULL; - remote_connect_list_all_storage_pools_args args; - remote_connect_list_all_storage_pools_ret ret; - - struct private_data *priv = conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!pools; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(conn, - priv, - 0, - REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS, - (xdrproc_t) xdr_remote_connect_list_all_storage_pools_args, - (char *) &args, - (xdrproc_t) xdr_remote_connect_list_all_storage_pools_ret, - (char *) &ret) == -1) - goto done; - - if (ret.pools.pools_len > REMOTE_STORAGE_POOL_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many storage pools '%d' for limit '%d'"), - ret.pools.pools_len, REMOTE_STORAGE_POOL_LIST_MAX); - goto cleanup; - } - - if (pools) { - if (VIR_ALLOC_N(tmp_pools, ret.pools.pools_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.pools.pools_len; i++) { - tmp_pools[i] = get_nonnull_storage_pool(conn, ret.pools.pools_val[i]); - if (!tmp_pools[i]) - goto cleanup; - } - *pools = tmp_pools; - tmp_pools = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_pools) { - for (i = 0; i < ret.pools.pools_len; i++) - virObjectUnref(tmp_pools[i]); - VIR_FREE(tmp_pools); - } - - xdr_free((xdrproc_t) xdr_remote_connect_list_all_storage_pools_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int -remoteStoragePoolListAllVolumes(virStoragePoolPtr pool, - virStorageVolPtr **vols, - unsigned int flags) -{ - int rv = -1; - size_t i; - virStorageVolPtr *tmp_vols = NULL; - remote_storage_pool_list_all_volumes_args args; - remote_storage_pool_list_all_volumes_ret ret; - - struct private_data *priv = pool->conn->privateData; - - remoteDriverLock(priv); - - make_nonnull_storage_pool(&args.pool, pool); - args.need_results = !!vols; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(pool->conn, - priv, - 0, - REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES, - (xdrproc_t) xdr_remote_storage_pool_list_all_volumes_args, - (char *) &args, - (xdrproc_t) xdr_remote_storage_pool_list_all_volumes_ret, - (char *) &ret) == -1) - goto done; - - if (ret.vols.vols_len > REMOTE_STORAGE_VOL_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many storage volumes '%d' for limit '%d'"), - ret.vols.vols_len, REMOTE_STORAGE_VOL_LIST_MAX); - goto cleanup; - } - - if (vols) { - if (VIR_ALLOC_N(tmp_vols, ret.vols.vols_len + 1) < 0) - goto cleanup; - - for (i = 0; i < ret.vols.vols_len; i++) { - tmp_vols[i] = get_nonnull_storage_vol(pool->conn, ret.vols.vols_val[i]); - if (!tmp_vols[i]) - goto cleanup; - } - *vols = tmp_vols; - tmp_vols = NULL; - } - - rv = ret.ret; - - cleanup: - if (tmp_vols) { - for (i = 0; i < ret.vols.vols_len; i++) - virObjectUnref(tmp_vols[i]); - VIR_FREE(tmp_vols); - } - - xdr_free((xdrproc_t) xdr_remote_storage_pool_list_all_volumes_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - - /*----------------------------------------------------------------------*/ static int @@ -6616,137 +6093,6 @@ remoteDomainGetInterfaceParameters(virDomainPtr domain, return rv; } -static int -remoteDomainListAllSnapshots(virDomainPtr dom, - virDomainSnapshotPtr **snapshots, - unsigned int flags) -{ - int rv = -1; - size_t i; - virDomainSnapshotPtr *snaps = NULL; - remote_domain_list_all_snapshots_args args; - remote_domain_list_all_snapshots_ret ret; - - struct private_data *priv = dom->conn->privateData; - - remoteDriverLock(priv); - - make_nonnull_domain(&args.dom, dom); - args.need_results = !!snapshots; - args.flags = flags; - - memset(&ret, 0, sizeof(ret)); - if (call(dom->conn, - priv, - 0, - REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS, - (xdrproc_t) xdr_remote_domain_list_all_snapshots_args, - (char *) &args, - (xdrproc_t) xdr_remote_domain_list_all_snapshots_ret, - (char *) &ret) == -1) - goto done; - - if (ret.snapshots.snapshots_len > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domain snapshots '%d' for limit '%d'"), - ret.snapshots.snapshots_len, - REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); - goto cleanup; - } - - if (snapshots) { - if (VIR_ALLOC_N(snaps, ret.snapshots.snapshots_len + 1) < 0) - goto cleanup; - for (i = 0; i < ret.snapshots.snapshots_len; i++) { - snaps[i] = get_nonnull_domain_snapshot(dom, ret.snapshots.snapshots_val[i]); - if (!snaps[i]) - goto cleanup; - } - *snapshots = snaps; - snaps = NULL; - } - - rv = ret.ret; - - cleanup: - if (snaps) { - for (i = 0; i < ret.snapshots.snapshots_len; i++) - virObjectUnref(snaps[i]); - VIR_FREE(snaps); - } - - xdr_free((xdrproc_t) xdr_remote_domain_list_all_snapshots_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} - -static int -remoteDomainSnapshotListAllChildren(virDomainSnapshotPtr parent, - virDomainSnapshotPtr **snapshots, - unsigned int flags) -{ - int rv = -1; - size_t i; - virDomainSnapshotPtr *snaps = NULL; - remote_domain_snapshot_list_all_children_args args; - remote_domain_snapshot_list_all_children_ret ret; - - struct private_data *priv = parent->domain->conn->privateData; - - remoteDriverLock(priv); - - args.need_results = !!snapshots; - args.flags = flags; - make_nonnull_domain_snapshot(&args.snapshot, parent); - - memset(&ret, 0, sizeof(ret)); - if (call(parent->domain->conn, - priv, - 0, - REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN, - (xdrproc_t) xdr_remote_domain_snapshot_list_all_children_args, - (char *) &args, - (xdrproc_t) xdr_remote_domain_snapshot_list_all_children_ret, - (char *) &ret) == -1) - goto done; - - if (ret.snapshots.snapshots_len > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { - virReportError(VIR_ERR_RPC, - _("Too many domain snapshots '%d' for limit '%d'"), - ret.snapshots.snapshots_len, - REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); - goto cleanup; - } - - if (snapshots) { - if (VIR_ALLOC_N(snaps, ret.snapshots.snapshots_len + 1) < 0) - goto cleanup; - for (i = 0; i < ret.snapshots.snapshots_len; i++) { - snaps[i] = get_nonnull_domain_snapshot(parent->domain, ret.snapshots.snapshots_val[i]); - if (!snaps[i]) - goto cleanup; - } - *snapshots = snaps; - snaps = NULL; - } - - rv = ret.ret; - - cleanup: - if (snaps) { - for (i = 0; i < ret.snapshots.snapshots_len; i++) - virObjectUnref(snaps[i]); - VIR_FREE(snaps); - } - - xdr_free((xdrproc_t) xdr_remote_domain_snapshot_list_all_children_ret, (char *) &ret); - - done: - remoteDriverUnlock(priv); - return rv; -} static int remoteNodeGetMemoryParameters(virConnectPtr conn, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 952686c5a61b..b4cb8f110967 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2553,7 +2553,7 @@ struct remote_domain_list_all_snapshots_args { unsigned int flags; }; -struct remote_domain_list_all_snapshots_ret { +struct remote_domain_list_all_snapshots_ret { /* insert@1 */ remote_nonnull_domain_snapshot snapshots<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; int ret; }; @@ -2583,7 +2583,7 @@ struct remote_domain_snapshot_list_all_children_args { unsigned int flags; }; -struct remote_domain_snapshot_list_all_children_ret { +struct remote_domain_snapshot_list_all_children_ret { /* insert@1 */ remote_nonnull_domain_snapshot snapshots<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; int ret; }; @@ -2822,7 +2822,7 @@ struct remote_connect_list_all_domains_args { unsigned int flags; }; -struct remote_connect_list_all_domains_ret { +struct remote_connect_list_all_domains_ret { /* insert@1 */ remote_nonnull_domain domains<REMOTE_DOMAIN_LIST_MAX>; unsigned int ret; }; @@ -2832,7 +2832,7 @@ struct remote_connect_list_all_storage_pools_args { unsigned int flags; }; -struct remote_connect_list_all_storage_pools_ret { +struct remote_connect_list_all_storage_pools_ret { /* insert@1 */ remote_nonnull_storage_pool pools<REMOTE_STORAGE_POOL_LIST_MAX>; unsigned int ret; }; @@ -2843,7 +2843,7 @@ struct remote_storage_pool_list_all_volumes_args { unsigned int flags; }; -struct remote_storage_pool_list_all_volumes_ret { +struct remote_storage_pool_list_all_volumes_ret { /* insert@1 */ remote_nonnull_storage_vol vols<REMOTE_STORAGE_VOL_LIST_MAX>; unsigned int ret; }; @@ -2853,7 +2853,7 @@ struct remote_connect_list_all_networks_args { unsigned int flags; }; -struct remote_connect_list_all_networks_ret { +struct remote_connect_list_all_networks_ret { /* insert@1 */ remote_nonnull_network nets<REMOTE_NETWORK_LIST_MAX>; unsigned int ret; }; @@ -2863,7 +2863,7 @@ struct remote_connect_list_all_interfaces_args { unsigned int flags; }; -struct remote_connect_list_all_interfaces_ret { +struct remote_connect_list_all_interfaces_ret { /* insert@1 */ remote_nonnull_interface ifaces<REMOTE_INTERFACE_LIST_MAX>; unsigned int ret; }; @@ -2873,7 +2873,7 @@ struct remote_connect_list_all_node_devices_args { unsigned int flags; }; -struct remote_connect_list_all_node_devices_ret { +struct remote_connect_list_all_node_devices_ret { /* insert@1 */ remote_nonnull_node_device devices<REMOTE_NODE_DEVICE_LIST_MAX>; unsigned int ret; }; @@ -2883,7 +2883,7 @@ struct remote_connect_list_all_nwfilters_args { unsigned int flags; }; -struct remote_connect_list_all_nwfilters_ret { +struct remote_connect_list_all_nwfilters_ret { /* insert@1 */ remote_nonnull_nwfilter filters<REMOTE_NWFILTER_LIST_MAX>; unsigned int ret; }; @@ -2893,7 +2893,7 @@ struct remote_connect_list_all_secrets_args { unsigned int flags; }; -struct remote_connect_list_all_secrets_ret { +struct remote_connect_list_all_secrets_ret { /* insert@1 */ remote_nonnull_secret secrets<REMOTE_SECRET_LIST_MAX>; unsigned int ret; }; @@ -5147,7 +5147,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_domains * @aclfilter: domain:getattr @@ -5155,14 +5155,14 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /** - * @generate: none + * @generate: both * @priority: high * @acl: domain:read */ REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /** - * @generate: none + * @generate: both * @priority: high * @acl: domain:read */ @@ -5202,7 +5202,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_storage_pools * @aclfilter: storage_pool:getattr @@ -5210,7 +5210,7 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, /** - * @generate: none + * @generate: both * @priority: high * @acl: storage_pool:search_storage_vols * @aclfilter: storage_vol:getattr @@ -5218,7 +5218,7 @@ enum remote_procedure { REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_networks * @aclfilter: network:getattr @@ -5226,7 +5226,7 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS = 283, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_interfaces * @aclfilter: interface:getattr @@ -5234,7 +5234,7 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_node_devices * @aclfilter: node_device:getattr @@ -5242,7 +5242,7 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_nwfilters * @aclfilter: nwfilter:getattr @@ -5250,7 +5250,7 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, /** - * @generate: none + * @generate: both * @priority: high * @acl: connect:search_secrets * @aclfilter: secret:getattr -- 2.7.2
On Thu, Mar 10, 2016 at 05:54:10AM +0100, Martin Kletzander wrote:
Since gendisplatch can now generate "modern" *ListAll* functions, let them all be generated.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/remote.c | 599 --------------------------------------- src/remote/remote_driver.c | 654 ------------------------------------------- src/remote/remote_protocol.x | 40 +-- 3 files changed, 20 insertions(+), 1273 deletions(-)
Many deletions, such ACK. Jan
There are cases when we don't want to tell the user we are connected. That's for example when we first connect to the server without the command 'connect' itself. That helps to clear out output of first command, mainly when running non-interactively. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virt-admin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index bc9ae9366280..5ce35f8dd5d3 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -119,8 +119,6 @@ vshAdmConnect(vshControl *ctl, unsigned int flags) if (priv->wantReconnect) vshPrint(ctl, "%s\n", _("Reconnected to the admin server")); - else - vshPrint(ctl, "%s\n", _("Connected to the admin server")); } return 0; @@ -288,6 +286,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; vshAdmControlPtr priv = ctl->privData; + bool connected = priv->conn; if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) return false; @@ -296,6 +295,8 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) ctl->connname = vshStrdup(ctl, name); vshAdmReconnect(ctl); + if (!connected) + vshPrint(ctl, "%s\n", _("Connected to the admin server")); return !!priv->conn; } -- 2.7.2
On 10/03/16 06:36, Martin Kletzander wrote:
There are cases when we don't want to tell the user we are connected. That's for example when we first connect to the server without the command 'connect' itself. That helps to clear out output of first command, mainly when running non-interactively.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virt-admin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index bc9ae9366280..5ce35f8dd5d3 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -119,8 +119,6 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
if (priv->wantReconnect) vshPrint(ctl, "%s\n", _("Reconnected to the admin server")); - else - vshPrint(ctl, "%s\n", _("Connected to the admin server")); }
return 0; @@ -288,6 +286,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; vshAdmControlPtr priv = ctl->privData; + bool connected = priv->conn;
if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) return false; @@ -296,6 +295,8 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) ctl->connname = vshStrdup(ctl, name);
vshAdmReconnect(ctl); + if (!connected) + vshPrint(ctl, "%s\n", _("Connected to the admin server"));
return !!priv->conn; }
ACK Erik
All other places use VIR_ERR_RPC except this one, let's be consistent, shall we? Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 6c3ded4a1e9c..fc8423a6b621 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -951,7 +951,7 @@ elsif ($mode eq "server") { if ($single_ret_as_list) { print " if (args->$single_ret_list_max_var > $single_ret_list_max_define) {\n"; - print " virReportError(VIR_ERR_INTERNAL_ERROR,\n"; + print " virReportError(VIR_ERR_RPC,\n"; print " \"%s\", _(\"max$single_ret_list_name > $single_ret_list_max_define\"));\n"; print " goto cleanup;\n"; print " }\n"; -- 2.7.2
On Thu, Mar 10, 2016 at 03:36:22PM +0100, Martin Kletzander wrote:
All other places use VIR_ERR_RPC except this one, let's be consistent, shall we?
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/gendispatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK Jan
participants (5)
-
Erik Skultety -
Ján Tomko -
Kashyap Chamarthy -
Martin Kletzander -
Richard W.M. Jones