5:53 a.m.
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
5:53 a.m.
New subject: [libvirt] [PATCH 01/21] admin: Make virAdmServerFree() handle NULL gracefully
We don't want to end up like with virDomainFree() and other, right?
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:54 a.m.
New subject: [libvirt] [PATCH 01/21] admin: Make virAdmServerFree() handle NULL gracefully
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 02/21] admin: Check for flags properly
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(a)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
9:53 a.m.
New subject: [libvirt] [PATCH 02/21] admin: Check for flags properly
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(a)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
11:27 a.m.
New subject: [libvirt] [PATCH 02/21] admin: Check for flags properly
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 03/21] server: Store server name in server object
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(a)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
2:38 p.m.
New subject: [libvirt] [PATCH 03/21] server: Store server name in server object
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 04/21] daemon: Get server name from the server itself
Since servers know their name, there is no need to supply such
information twice. Also defeats inconsistencies.
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:38 p.m.
New subject: [libvirt] [PATCH 04/21] daemon: Get server name from the server itself
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 05/21] virerror: Introduce new error type NO_SERVER
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(a)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
9:58 a.m.
New subject: [libvirt] [PATCH 05/21] virerror: Introduce new error type NO_SERVER
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 06/21] daemon: Set error for unknown server name
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:39 p.m.
New subject: [libvirt] [PATCH 06/21] daemon: Set error for unknown server name
On Thu, Mar 10, 2016 at 05:53:55AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/rpc/virnetdaemon.c | 5 +++++
1 file changed, 5 insertions(+)
ACK
Jan
5:53 a.m.
New subject: [libvirt] [PATCH 07/21] daemon: Properly check for clients
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(a)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
9:22 a.m.
New subject: [libvirt] [PATCH 07/21] daemon: Properly check for clients
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(a)redhat.com>
---
src/rpc/virnetdaemon.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
ACK
Jan
9:43 a.m.
New subject: [libvirt] [PATCH 07/21] daemon: Properly check for clients
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(a)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(a)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
8 a.m.
New subject: [libvirt] [PATCH 07/21] daemon: Properly check for clients
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(a)redhat.com> - 1.3.2-2
- Add fix for RHBZ#1315606.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
FWIW: Tested-by: Kashyap Chamarthy <kchamart(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
/kashyap
5:53 a.m.
New subject: [libvirt] [PATCH 08/21] Expose virNetServerGetName
Signed-off-by: Martin Kletzander <mkletzan(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 09/21] admin: Do not work with virAdm on the server side
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(a)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
2:45 p.m.
New subject: [libvirt] [PATCH 09/21] admin: Do not work with virAdm on the server side
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(a)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
5:53 a.m.
New subject: [libvirt] [PATCH 10/21] Change virNetDaemonGetServerNames to virNetDaemonGetServers
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(a)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
2:48 p.m.
New subject: [libvirt] [PATCH 10/21] Change virNetDaemonGetServerNames to virNetDaemonGetServers
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 11/21] admin: Don't use priority for admin APIs
There are no priority workers as they dn't make sense for now.
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:43 a.m.
New subject: [libvirt] [PATCH 11/21] admin: Don't use priority for admin APIs
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 12/21] virt-admin: Don't leak uri all over the place
virAdmConnectGetURI() returns string that needs to be free()'d but we
haven't done that very much.
Signed-off-by: Martin Kletzander <mkletzan(a)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
9:11 a.m.
New subject: [libvirt] [PATCH 12/21] virt-admin: Don't leak uri all over the place
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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:40 a.m.
New subject: [libvirt] [PATCH 12/21] virt-admin: Don't leak uri all over the place
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(a)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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
5:54 a.m.
New subject: [libvirt] [PATCH 13/21] admin: Be consistent when resetting errors
Resetting an error should be the first thing public API does.
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:48 p.m.
New subject: [libvirt] [PATCH 13/21] admin: Be consistent when resetting errors
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(a)redhat.com>
---
src/libvirt-admin.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
ACK
Jan
5:54 a.m.
New subject: [libvirt] [PATCH 14/21] gendispatch: Cluster, don't capture if not needed
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(a)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
2:50 p.m.
New subject: [libvirt] [PATCH 14/21] gendispatch: Cluster, don't capture if not needed
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 15/21] gendispatch: Be able to generate multi-return values
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(a)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
2:55 p.m.
New subject: [libvirt] [PATCH 15/21] gendispatch: Be able to generate multi-return values
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 16/21] admin: Generate ConnectListServers dispatch helpers
Since we have the opportunity now, let's save some precious code lines.
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:57 p.m.
New subject: [libvirt] [PATCH 16/21] admin: Generate ConnectListServers dispatch helpers
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(a)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
3:15 p.m.
New subject: [libvirt] [PATCH 16/21] admin: Generate ConnectListServers dispatch helpers
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 17/21] gendispatch: Accept server as an argument
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:58 p.m.
New subject: [libvirt] [PATCH 17/21] gendispatch: Accept server as an argument
On Thu, Mar 10, 2016 at 05:54:06AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/rpc/gendispatch.pl | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
ACK
Jan
5:54 a.m.
New subject: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer
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(a)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
2:58 p.m.
New subject: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer
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(a)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
1:29 p.m.
New subject: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer
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(a)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.
12:47 p.m.
New subject: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer
On 10/03/16 05:54, Martin Kletzander wrote:
ACK with that commentary mentioned above changed.
Erik
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(a)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.
1:47 p.m.
New subject: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 19/21] gendispatch: Remember the name of snapshot variable name
Until now, the script assumed that snapshot name is 'snap', but that's
going to change.
Signed-off-by: Martin Kletzander <mkletzan(a)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
2:59 p.m.
New subject: [libvirt] [PATCH 19/21] gendispatch: Remember the name of snapshot variable name
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(a)redhat.com>
---
src/rpc/gendispatch.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
ACK
Jan
5:54 a.m.
New subject: [libvirt] [PATCH 20/21] gendispatch: Support modern listing of more types
Signed-off-by: Martin Kletzander <mkletzan(a)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
3:26 p.m.
New subject: [libvirt] [PATCH 20/21] gendispatch: Support modern listing of more types
On Thu, Mar 10, 2016 at 05:54:09AM +0100, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan(a)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
3:32 p.m.
New subject: [libvirt] [PATCH 20/21] gendispatch: Support modern listing of more types
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH 21/21] remote: Generate what's possible
Since gendisplatch can now generate "modern" *ListAll* functions, let
them all be generated.
Signed-off-by: Martin Kletzander <mkletzan(a)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
3:27 p.m.
New subject: [libvirt] [PATCH 21/21] remote: Generate what's possible
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(a)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
6:36 a.m.
New subject: [libvirt] [PATCH 22/21] virt-admin: Don't tell everyone needlessly we're connected
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(a)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
3:43 p.m.
New subject: [libvirt] [PATCH 22/21] virt-admin: Don't tell everyone needlessly we're connected
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(a)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
3:36 p.m.
New subject: [libvirt] [PATCH 23/21 (last one, hopefully)] gendispatch: Use proper error for limit checking
All other places use VIR_ERR_RPC except this one, let's be consistent,
shall we?
Signed-off-by: Martin Kletzander <mkletzan(a)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
3:53 p.m.
New subject: [libvirt] [PATCH 23/21 (last one, hopefully)] gendispatch: Use proper error for limit checking
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(a)redhat.com>
---
src/rpc/gendispatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
Jan
3109
days inactive
3115
days old
55 comments
5 participants
participants (5)
-
Erik Skultety -
Ján Tomko -
Kashyap Chamarthy -
Martin Kletzander -
Richard W.M. Jones