12:36 p.m.
This series removes special case code from the generator and moves
it to annotations in the .x files. There are also cleanups of sign
mismatches between the different API layers and additional moves of
functions to generated code.
This series superseds patch 4/5 of the last series [1] and implements
Eric's suggestions.
[1] https://www.redhat.com/archives/libvir-list/2011-May/msg00991.html
configure.ac | 1 +
daemon/remote.c | 289 ++-----------------------
daemon/remote_generator.pl | 473 ++++++++++++++++++++++++++++------------
src/driver.h | 2 +-
src/nwfilter/nwfilter_driver.c | 2 +-
src/remote/remote_driver.c | 268 ++--------------------
src/remote/remote_protocol.x | 247 +++++++++++----------
src/remote_protocol-structs | 40 ++--
8 files changed, 532 insertions(+), 790 deletions(-)
Matthias
12:36 p.m.
New subject: [libvirt] [PATCH 1/7] remote generator: Handle virDomainCreateWithFlags
Add special case code for updating a given domain object instead of
returning a new one.
---
daemon/remote.c | 30 -----------------------
daemon/remote_generator.pl | 55 +++++++++++++++++++++++++++++------------
src/remote/remote_driver.c | 32 ------------------------
src/remote/remote_protocol.x | 2 +-
4 files changed, 40 insertions(+), 79 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 80783b3..fcda2c5 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -895,36 +895,6 @@ cleanup:
}
static int
-remoteDispatchDomainCreateWithFlags(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client ATTRIBUTE_UNUSED,
- virConnectPtr conn,
- remote_message_header *hdr ATTRIBUTE_UNUSED,
- remote_error *rerr,
- remote_domain_create_with_flags_args *args,
- remote_domain_create_with_flags_ret *ret)
-{
- int rv = -1;
- virDomainPtr dom = NULL;
-
- if (!(dom = get_nonnull_domain(conn, args->dom)))
- goto cleanup;
-
- if (virDomainCreateWithFlags(dom, args->flags) < 0)
- goto cleanup;
-
- make_nonnull_domain(&ret->dom, dom);
-
- rv = 0;
-
-cleanup:
- if (rv < 0)
- remoteDispatchError(rerr);
- if (dom)
- virDomainFree(dom);
- return rv;
-}
-
-static int
remoteDispatchDomainGetSecurityLabel(struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index d21f959..b23e3b1 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -450,14 +450,22 @@ elsif ($opt_b) {
} elsif ($ret_member =~
m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|domain_snapshot)
(\S+);/) {
my $type_name = name_to_ProcName($1);
- push(@vars_list, "vir${type_name}Ptr $2 = NULL");
- push(@ret_list, "make_nonnull_$1(&ret->$2, $2);");
- push(@free_list,
- " if ($2)\n" .
- " vir${type_name}Free($2);");
- $single_ret_var = $2;
- $single_ret_by_ref = 0;
- $single_ret_check = " == NULL";
+ if ($call->{ProcName} eq "DomainCreateWithFlags") {
+ # SPECIAL: virDomainCreateWithFlags updates the given
+ # domain object instead of returning a new one
+ push(@ret_list, "make_nonnull_$1(&ret->$2,
$2);");
+ $single_ret_var = undef;
+ $single_ret_by_ref = 1;
+ } else {
+ push(@vars_list, "vir${type_name}Ptr $2 = NULL");
+ push(@ret_list, "make_nonnull_$1(&ret->$2,
$2);");
+ push(@free_list,
+ " if ($2)\n" .
+ " vir${type_name}Free($2);");
+ $single_ret_var = $2;
+ $single_ret_by_ref = 0;
+ $single_ret_check = " == NULL";
+ }
} elsif ($ret_member =~ m/^int (\S+)<(\S+)>;/) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
@@ -679,7 +687,12 @@ elsif ($opt_b) {
if ($single_ret_by_ref) {
print " if (vir$prefix$proc_name(";
print join(', ', @args_list);
- print ", &$single_ret_var) < 0)\n";
+
+ if (defined $single_ret_var) {
+ print ", &$single_ret_var";
+ }
+
+ print ") < 0)\n";
} else {
print " if (($single_ret_var = vir$prefix$proc_name(";
print join(', ', @args_list);
@@ -979,15 +992,25 @@ elsif ($opt_k) {
$priv_name = "${name}PrivateData";
}
- if ($name eq "domain_snapshot") {
- push(@ret_list, "rv = get_nonnull_$name(dom,
ret.$arg_name);");
+ if ($call->{ProcName} eq "DomainCreateWithFlags") {
+ # SPECIAL: virDomainCreateWithFlags updates the given
+ # domain object instead of returning a new one
+ push(@ret_list, "dom->id = ret.dom.id;");
+ push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret},
(char *)&ret);");
+ push(@ret_list, "rv = 0;");
+ $single_ret_var = "int rv = -1";
+ $single_ret_type = "int";
} else {
- push(@ret_list, "rv = get_nonnull_$name($priv_src,
ret.$arg_name);");
- }
+ if ($name eq "domain_snapshot") {
+ push(@ret_list, "rv = get_nonnull_$name(dom,
ret.$arg_name);");
+ } else {
+ push(@ret_list, "rv = get_nonnull_$name($priv_src,
ret.$arg_name);");
+ }
- push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret}, (char
*)&ret);");
- $single_ret_var = "vir${type_name}Ptr rv = NULL";
- $single_ret_type = "vir${type_name}Ptr";
+ push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret},
(char *)&ret);");
+ $single_ret_var = "vir${type_name}Ptr rv = NULL";
+ $single_ret_type = "vir${type_name}Ptr";
+ }
} elsif ($ret_member =~ m/^int (\S+);/) {
my $arg_name = $1;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 8c69743..69b888d 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2441,38 +2441,6 @@ done:
return rv;
}
-static int
-remoteDomainCreateWithFlags (virDomainPtr domain, unsigned int flags)
-{
- int rv = -1;
- remote_domain_create_with_flags_args args;
- remote_domain_create_with_flags_ret ret;
- struct private_data *priv = domain->conn->privateData;
-
- remoteDriverLock(priv);
-
- make_nonnull_domain (&args.dom, domain);
- args.flags = flags;
-
- memset (&ret, 0, sizeof ret);
- if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS,
- (xdrproc_t) xdr_remote_domain_create_with_flags_args,
- (char *) &args,
- (xdrproc_t) xdr_remote_domain_create_with_flags_ret,
- (char *) &ret) == -1)
- goto done;
-
- domain->id = ret.dom.id;
- xdr_free ((xdrproc_t) &xdr_remote_domain_create_with_flags_ret,
- (char *) &ret);
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
- return rv;
-}
-
static char *
remoteDomainGetSchedulerType (virDomainPtr domain, int *nparams)
{
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index f0da95d..9df86da 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2266,7 +2266,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194, /* autogen autogen */
REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195, /* autogen autogen */
- REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_MEMORY_PARAMETERS = 197, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_GET_MEMORY_PARAMETERS = 198, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 199, /* autogen autogen */
--
1.7.0.4
11:08 a.m.
New subject: [libvirt] [PATCH 1/7] remote generator: Handle virDomainCreateWithFlags
On Mon, May 23, 2011 at 07:36:04PM +0200, Matthias Bolte wrote:
Add special case code for updating a given domain object instead of
returning a new one.
---
daemon/remote.c | 30 -----------------------
daemon/remote_generator.pl | 55 +++++++++++++++++++++++++++++------------
src/remote/remote_driver.c | 32 ------------------------
src/remote/remote_protocol.x | 2 +-
4 files changed, 40 insertions(+), 79 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 80783b3..fcda2c5 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -895,36 +895,6 @@ cleanup:
}
static int
-remoteDispatchDomainCreateWithFlags(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client ATTRIBUTE_UNUSED,
- virConnectPtr conn,
- remote_message_header *hdr ATTRIBUTE_UNUSED,
- remote_error *rerr,
- remote_domain_create_with_flags_args *args,
- remote_domain_create_with_flags_ret *ret)
-{
- int rv = -1;
- virDomainPtr dom = NULL;
-
- if (!(dom = get_nonnull_domain(conn, args->dom)))
- goto cleanup;
-
- if (virDomainCreateWithFlags(dom, args->flags) < 0)
- goto cleanup;
-
- make_nonnull_domain(&ret->dom, dom);
-
- rv = 0;
-
-cleanup:
- if (rv < 0)
- remoteDispatchError(rerr);
- if (dom)
- virDomainFree(dom);
- return rv;
-}
-
-static int
remoteDispatchDomainGetSecurityLabel(struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index d21f959..b23e3b1 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -450,14 +450,22 @@ elsif ($opt_b) {
} elsif ($ret_member =~
m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|domain_snapshot)
(\S+);/) {
my $type_name = name_to_ProcName($1);
- push(@vars_list, "vir${type_name}Ptr $2 = NULL");
- push(@ret_list, "make_nonnull_$1(&ret->$2, $2);");
- push(@free_list,
- " if ($2)\n" .
- " vir${type_name}Free($2);");
- $single_ret_var = $2;
- $single_ret_by_ref = 0;
- $single_ret_check = " == NULL";
+ if ($call->{ProcName} eq "DomainCreateWithFlags") {
+ # SPECIAL: virDomainCreateWithFlags updates the given
+ # domain object instead of returning a new one
+ push(@ret_list, "make_nonnull_$1(&ret->$2,
$2);");
+ $single_ret_var = undef;
+ $single_ret_by_ref = 1;
+ } else {
+ push(@vars_list, "vir${type_name}Ptr $2 = NULL");
+ push(@ret_list, "make_nonnull_$1(&ret->$2,
$2);");
+ push(@free_list,
+ " if ($2)\n" .
+ " vir${type_name}Free($2);");
+ $single_ret_var = $2;
+ $single_ret_by_ref = 0;
+ $single_ret_check = " == NULL";
+ }
} elsif ($ret_member =~ m/^int (\S+)<(\S+)>;/) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
@@ -679,7 +687,12 @@ elsif ($opt_b) {
if ($single_ret_by_ref) {
print " if (vir$prefix$proc_name(";
print join(', ', @args_list);
- print ", &$single_ret_var) < 0)\n";
+
+ if (defined $single_ret_var) {
+ print ", &$single_ret_var";
+ }
+
+ print ") < 0)\n";
} else {
print " if (($single_ret_var = vir$prefix$proc_name(";
print join(', ', @args_list);
@@ -979,15 +992,25 @@ elsif ($opt_k) {
$priv_name = "${name}PrivateData";
}
- if ($name eq "domain_snapshot") {
- push(@ret_list, "rv = get_nonnull_$name(dom,
ret.$arg_name);");
+ if ($call->{ProcName} eq "DomainCreateWithFlags") {
+ # SPECIAL: virDomainCreateWithFlags updates the given
+ # domain object instead of returning a new one
+ push(@ret_list, "dom->id = ret.dom.id;");
+ push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret},
(char *)&ret);");
+ push(@ret_list, "rv = 0;");
+ $single_ret_var = "int rv = -1";
+ $single_ret_type = "int";
} else {
- push(@ret_list, "rv = get_nonnull_$name($priv_src,
ret.$arg_name);");
- }
+ if ($name eq "domain_snapshot") {
+ push(@ret_list, "rv = get_nonnull_$name(dom,
ret.$arg_name);");
+ } else {
+ push(@ret_list, "rv = get_nonnull_$name($priv_src,
ret.$arg_name);");
+ }
- push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret}, (char
*)&ret);");
- $single_ret_var = "vir${type_name}Ptr rv = NULL";
- $single_ret_type = "vir${type_name}Ptr";
+ push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret},
(char *)&ret);");
+ $single_ret_var = "vir${type_name}Ptr rv = NULL";
+ $single_ret_type = "vir${type_name}Ptr";
+ }
} elsif ($ret_member =~ m/^int (\S+);/) {
my $arg_name = $1;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 8c69743..69b888d 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2441,38 +2441,6 @@ done:
return rv;
}
-static int
-remoteDomainCreateWithFlags (virDomainPtr domain, unsigned int flags)
-{
- int rv = -1;
- remote_domain_create_with_flags_args args;
- remote_domain_create_with_flags_ret ret;
- struct private_data *priv = domain->conn->privateData;
-
- remoteDriverLock(priv);
-
- make_nonnull_domain (&args.dom, domain);
- args.flags = flags;
-
- memset (&ret, 0, sizeof ret);
- if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS,
- (xdrproc_t) xdr_remote_domain_create_with_flags_args,
- (char *) &args,
- (xdrproc_t) xdr_remote_domain_create_with_flags_ret,
- (char *) &ret) == -1)
- goto done;
-
- domain->id = ret.dom.id;
- xdr_free ((xdrproc_t) &xdr_remote_domain_create_with_flags_ret,
- (char *) &ret);
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
- return rv;
-}
-
static char *
remoteDomainGetSchedulerType (virDomainPtr domain, int *nparams)
{
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index f0da95d..9df86da 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2266,7 +2266,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194, /* autogen autogen */
REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195, /* autogen autogen */
- REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_MEMORY_PARAMETERS = 197, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_GET_MEMORY_PARAMETERS = 198, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 199, /* autogen autogen */
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 2/7] remote generator: Rename virNodeGetCellsFreeMemory parameters to common pattern
This allows to remove some special case code from the generator.
---
daemon/remote_generator.pl | 3 +--
src/remote/remote_driver.c | 8 ++++----
src/remote/remote_protocol.x | 4 ++--
src/remote_protocol-structs | 8 ++++----
4 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index b23e3b1..2da0f2e 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -505,17 +505,16 @@ elsif ($opt_b) {
$single_ret_by_ref = 0;
$single_ret_as_list = 1;
$single_ret_list_name = $1;
+ $single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
my $conn = shift(@args_list);
if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
$single_ret_check = " <= 0";
- $single_ret_list_max_var = "maxCells";
unshift(@args_list, "(unsigned long long
*)ret->$1.$1_val");
} else {
$single_ret_check = " < 0";
- $single_ret_list_max_var = "max$1";
unshift(@args_list, "ret->$1.$1_val");
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 69b888d..fb542c5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1715,7 +1715,7 @@ remoteNodeGetCellsFreeMemory(virConnectPtr conn,
}
args.startCell = startCell;
- args.maxCells = maxCells;
+ args.maxcells = maxCells;
memset (&ret, 0, sizeof ret);
if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CELLS_FREE_MEMORY,
@@ -1723,12 +1723,12 @@ remoteNodeGetCellsFreeMemory(virConnectPtr conn,
(xdrproc_t) xdr_remote_node_get_cells_free_memory_ret, (char *)&ret) ==
-1)
goto done;
- for (i = 0 ; i < ret.freeMems.freeMems_len ; i++)
- freeMems[i] = ret.freeMems.freeMems_val[i];
+ for (i = 0 ; i < ret.cells.cells_len ; i++)
+ freeMems[i] = ret.cells.cells_val[i];
xdr_free((xdrproc_t) xdr_remote_node_get_cells_free_memory_ret, (char *) &ret);
- rv = ret.freeMems.freeMems_len;
+ rv = ret.cells.cells_len;
done:
remoteDriverUnlock(priv);
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9df86da..3e9bd5c 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -442,11 +442,11 @@ struct remote_get_capabilities_ret {
struct remote_node_get_cells_free_memory_args {
int startCell;
- int maxCells;
+ int maxcells;
};
struct remote_node_get_cells_free_memory_ret {
- hyper freeMems<REMOTE_NODE_MAX_CELLS>;
+ hyper cells<REMOTE_NODE_MAX_CELLS>;
};
struct remote_node_get_free_memory_ret {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 414b4d5..5491f73 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -153,13 +153,13 @@ struct remote_get_capabilities_ret {
};
struct remote_node_get_cells_free_memory_args {
int startCell;
- int maxCells;
+ int maxcells;
};
struct remote_node_get_cells_free_memory_ret {
struct {
- u_int freeMems_len;
- int64_t * freeMems_val;
- } freeMems;
+ u_int cells_len;
+ int64_t * cells_val;
+ } cells;
};
struct remote_node_get_free_memory_ret {
int64_t freeMem;
--
1.7.0.4
11:09 a.m.
New subject: [libvirt] [PATCH 2/7] remote generator: Rename virNodeGetCellsFreeMemory parameters to common pattern
On Mon, May 23, 2011 at 07:36:05PM +0200, Matthias Bolte wrote:
This allows to remove some special case code from the generator.
---
daemon/remote_generator.pl | 3 +--
src/remote/remote_driver.c | 8 ++++----
src/remote/remote_protocol.x | 4 ++--
src/remote_protocol-structs | 8 ++++----
4 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index b23e3b1..2da0f2e 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -505,17 +505,16 @@ elsif ($opt_b) {
$single_ret_by_ref = 0;
$single_ret_as_list = 1;
$single_ret_list_name = $1;
+ $single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
my $conn = shift(@args_list);
if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
$single_ret_check = " <= 0";
- $single_ret_list_max_var = "maxCells";
unshift(@args_list, "(unsigned long long
*)ret->$1.$1_val");
} else {
$single_ret_check = " < 0";
- $single_ret_list_max_var = "max$1";
unshift(@args_list, "ret->$1.$1_val");
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 69b888d..fb542c5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1715,7 +1715,7 @@ remoteNodeGetCellsFreeMemory(virConnectPtr conn,
}
args.startCell = startCell;
- args.maxCells = maxCells;
+ args.maxcells = maxCells;
memset (&ret, 0, sizeof ret);
if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CELLS_FREE_MEMORY,
@@ -1723,12 +1723,12 @@ remoteNodeGetCellsFreeMemory(virConnectPtr conn,
(xdrproc_t) xdr_remote_node_get_cells_free_memory_ret, (char *)&ret)
== -1)
goto done;
- for (i = 0 ; i < ret.freeMems.freeMems_len ; i++)
- freeMems[i] = ret.freeMems.freeMems_val[i];
+ for (i = 0 ; i < ret.cells.cells_len ; i++)
+ freeMems[i] = ret.cells.cells_val[i];
xdr_free((xdrproc_t) xdr_remote_node_get_cells_free_memory_ret, (char *) &ret);
- rv = ret.freeMems.freeMems_len;
+ rv = ret.cells.cells_len;
done:
remoteDriverUnlock(priv);
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9df86da..3e9bd5c 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -442,11 +442,11 @@ struct remote_get_capabilities_ret {
struct remote_node_get_cells_free_memory_args {
int startCell;
- int maxCells;
+ int maxcells;
};
struct remote_node_get_cells_free_memory_ret {
- hyper freeMems<REMOTE_NODE_MAX_CELLS>;
+ hyper cells<REMOTE_NODE_MAX_CELLS>;
};
struct remote_node_get_free_memory_ret {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 414b4d5..5491f73 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -153,13 +153,13 @@ struct remote_get_capabilities_ret {
};
struct remote_node_get_cells_free_memory_args {
int startCell;
- int maxCells;
+ int maxcells;
};
struct remote_node_get_cells_free_memory_ret {
struct {
- u_int freeMems_len;
- int64_t * freeMems_val;
- } freeMems;
+ u_int cells_len;
+ int64_t * cells_val;
+ } cells;
};
struct remote_node_get_free_memory_ret {
int64_t freeMem;
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 3/7] remote generator: Make call-by-reference handling stricter
Several functions return values by reference parameters. This is realized
by passing the members of remote_CALL_ret by reference to the called
function.
The position of this parameters in the function signature follows some
patterns with some exceptions. This patterns and exceptions are hardcoded
in the generator.
Add an insert@<offset> annotation to the remote_CALL_ret struct members
for functions that return lists to remove some of the hardcoded patterns
and exceptions.
---
daemon/remote_generator.pl | 69 ++++++++++++++---------------------------
src/remote/remote_protocol.x | 37 ++++++++++++----------
2 files changed, 44 insertions(+), 62 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 2da0f2e..8b3794f 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -405,8 +405,9 @@ elsif ($opt_b) {
} else {
die "unhandled type for multi-return-value:
$ret_member";
}
- } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^remote_nonnull_string
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
+ 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";
@@ -416,18 +417,9 @@ elsif ($opt_b) {
$single_ret_list_name = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- if ($call->{ProcName} eq "NodeListDevices") {
- my $conn = shift(@args_list);
- my $cap = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $cap);
- unshift(@args_list, $conn);
- } else {
- my $conn = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $conn);
- }
+ } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "remote_nonnull_string array without insert@<offset>
annotation: $ret_member";
} elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
if ($call->{ProcName} eq "GetType") {
# SPECIAL: virConnectGetType returns a constant string that must
@@ -466,8 +458,9 @@ elsif ($opt_b) {
$single_ret_by_ref = 0;
$single_ret_check = " == NULL";
}
- } elsif ($ret_member =~ m/^int (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^int
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
+ 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";
@@ -477,10 +470,9 @@ elsif ($opt_b) {
$single_ret_list_name = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- my $conn = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $conn);
+ } elsif ($ret_member =~ m/^int (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "int array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^int (\S+);/) {
push(@vars_list, "int $1");
push(@ret_list, "ret->$1 = $1;");
@@ -497,7 +489,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
}
}
- } elsif ($ret_member =~ m/^hyper (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
@@ -508,17 +500,16 @@ elsif ($opt_b) {
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
- my $conn = shift(@args_list);
-
if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
$single_ret_check = " <= 0";
- unshift(@args_list, "(unsigned long long
*)ret->$1.$1_val");
+ splice(@args_list, int($3), 0, ("(unsigned long long
*)ret->$1.$1_val"));
} else {
$single_ret_check = " < 0";
- unshift(@args_list, "ret->$1.$1_val");
+ splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
}
-
- unshift(@args_list, $conn);
+ } elsif ($ret_member =~ m/^hyper (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "hyper array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
my $type_name;
my $ret_name = $2;
@@ -945,30 +936,18 @@ elsif ($opt_k) {
die "unhandled type for multi-return-value for " .
"procedure $call->{name}: $ret_member";
}
- } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^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 = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- my $first_arg = shift(@args_list);
- my $second_arg;
-
- if ($call->{ProcName} eq "NodeListDevices") {
- $second_arg = shift(@args_list);
- }
-
- unshift(@args_list, "char **const $1");
-
- if (defined $second_arg) {
- unshift(@args_list, $second_arg);
- }
-
- unshift(@args_list, $first_arg);
-
- push(@ret_list, "rv = ret.$1.$1_len;");
- $single_ret_var = "int rv = -1";
- $single_ret_type = "int";
+ } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "remote_nonnull_string array without insert@<offset>
annotation: $ret_member";
} elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
push(@ret_list, "rv = ret.$1;");
$single_ret_var = "char *rv = NULL";
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 3e9bd5c..63f7ebb 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -368,7 +368,10 @@ struct remote_memory_param {
*
* Please follow the naming convention carefully - this file is
* parsed by 'remote_generator.pl'.
- */
+ *
+ * 'remote_CALL_ret' members that are filled via call-by-reference must be
+ * annotated with a insert@<offset> comment to indicate the offset in the
+ * parameter list of the function to be called. */
struct remote_open_args {
/* NB. "name" might be NULL although in practice you can't
@@ -446,7 +449,7 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- hyper cells<REMOTE_NODE_MAX_CELLS>;
+ hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
};
struct remote_node_get_free_memory_ret {
@@ -600,7 +603,7 @@ struct remote_list_domains_args {
};
struct remote_list_domains_ret {
- int ids<REMOTE_DOMAIN_ID_LIST_MAX>;
+ int ids<REMOTE_DOMAIN_ID_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_domains_ret {
@@ -801,7 +804,7 @@ struct remote_list_defined_domains_args {
};
struct remote_list_defined_domains_ret {
- remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_domains_ret {
@@ -949,7 +952,7 @@ struct remote_list_networks_args {
};
struct remote_list_networks_ret {
- remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_networks_ret {
@@ -961,7 +964,7 @@ struct remote_list_defined_networks_args {
};
struct remote_list_defined_networks_ret {
- remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_network_lookup_by_uuid_args {
@@ -1049,7 +1052,7 @@ struct remote_list_nwfilters_args {
};
struct remote_list_nwfilters_ret {
- remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_nwfilter_lookup_by_uuid_args {
@@ -1101,7 +1104,7 @@ struct remote_list_interfaces_args {
};
struct remote_list_interfaces_ret {
- remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_interfaces_ret {
@@ -1113,7 +1116,7 @@ struct remote_list_defined_interfaces_args {
};
struct remote_list_defined_interfaces_ret {
- remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>; /*
insert@1 */
};
struct remote_interface_lookup_by_name_args {
@@ -1215,7 +1218,7 @@ struct remote_list_storage_pools_args {
};
struct remote_list_storage_pools_ret {
- remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_storage_pools_ret {
@@ -1227,7 +1230,7 @@ struct remote_list_defined_storage_pools_args {
};
struct remote_list_defined_storage_pools_ret {
- remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_find_storage_pool_sources_args {
@@ -1357,7 +1360,7 @@ struct remote_storage_pool_list_volumes_args {
};
struct remote_storage_pool_list_volumes_ret {
- remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>; /* insert@1 */
};
@@ -1465,7 +1468,7 @@ struct remote_node_list_devices_args {
};
struct remote_node_list_devices_ret {
- remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>; /* insert@2 */
};
struct remote_node_device_lookup_by_name_args {
@@ -1507,7 +1510,7 @@ struct remote_node_device_list_caps_args {
};
struct remote_node_device_list_caps_ret {
- remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>; /* insert@1 */
};
struct remote_node_device_dettach_args {
@@ -1588,7 +1591,7 @@ struct remote_list_secrets_args {
};
struct remote_list_secrets_ret {
- remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>;
+ remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>; /* insert@1 */
};
struct remote_secret_lookup_by_uuid_args {
@@ -1900,7 +1903,7 @@ struct remote_domain_snapshot_list_names_args {
};
struct remote_domain_snapshot_list_names_ret {
- remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>;
+ remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert@1
*/
};
struct remote_domain_snapshot_lookup_by_name_args {
@@ -2006,7 +2009,7 @@ struct remote_domain_migrate_prepare_tunnel3_args {
};
struct remote_domain_migrate_prepare_tunnel3_ret {
- opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>;
+ opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>; /* insert@3 */
};
struct remote_domain_migrate_perform3_args {
--
1.7.0.4
11:11 a.m.
New subject: [libvirt] [PATCH 3/7] remote generator: Make call-by-reference handling stricter
On Mon, May 23, 2011 at 07:36:06PM +0200, Matthias Bolte wrote:
Several functions return values by reference parameters. This is
realized
by passing the members of remote_CALL_ret by reference to the called
function.
The position of this parameters in the function signature follows some
patterns with some exceptions. This patterns and exceptions are hardcoded
in the generator.
Add an insert@<offset> annotation to the remote_CALL_ret struct members
for functions that return lists to remove some of the hardcoded patterns
and exceptions.
---
daemon/remote_generator.pl | 69 ++++++++++++++---------------------------
src/remote/remote_protocol.x | 37 ++++++++++++----------
2 files changed, 44 insertions(+), 62 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 2da0f2e..8b3794f 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -405,8 +405,9 @@ elsif ($opt_b) {
} else {
die "unhandled type for multi-return-value:
$ret_member";
}
- } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/)
{
+ } elsif ($ret_member =~ m/^remote_nonnull_string
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
+ 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";
@@ -416,18 +417,9 @@ elsif ($opt_b) {
$single_ret_list_name = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- if ($call->{ProcName} eq "NodeListDevices") {
- my $conn = shift(@args_list);
- my $cap = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $cap);
- unshift(@args_list, $conn);
- } else {
- my $conn = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $conn);
- }
+ } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "remote_nonnull_string array without insert@<offset>
annotation: $ret_member";
} elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
if ($call->{ProcName} eq "GetType") {
# SPECIAL: virConnectGetType returns a constant string that
must
@@ -466,8 +458,9 @@ elsif ($opt_b) {
$single_ret_by_ref = 0;
$single_ret_check = " == NULL";
}
- } elsif ($ret_member =~ m/^int (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^int
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
+ 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";
@@ -477,10 +470,9 @@ elsif ($opt_b) {
$single_ret_list_name = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- my $conn = shift(@args_list);
- unshift(@args_list, "ret->$1.$1_val");
- unshift(@args_list, $conn);
+ } elsif ($ret_member =~ m/^int (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "int array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^int (\S+);/) {
push(@vars_list, "int $1");
push(@ret_list, "ret->$1 = $1;");
@@ -497,7 +489,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
}
}
- } elsif ($ret_member =~ m/^hyper (\S+)<(\S+)>;/) {
+ } elsif ($ret_member =~ m/^hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
push(@free_list_on_error,
"VIR_FREE(ret->$1.$1_val);");
@@ -508,17 +500,16 @@ elsif ($opt_b) {
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
- my $conn = shift(@args_list);
-
if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
$single_ret_check = " <= 0";
- unshift(@args_list, "(unsigned long long
*)ret->$1.$1_val");
+ splice(@args_list, int($3), 0, ("(unsigned long long
*)ret->$1.$1_val"));
} else {
$single_ret_check = " < 0";
- unshift(@args_list, "ret->$1.$1_val");
+ splice(@args_list, int($3), 0,
("ret->$1.$1_val"));
}
-
- unshift(@args_list, $conn);
+ } elsif ($ret_member =~ m/^hyper (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "hyper array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
my $type_name;
my $ret_name = $2;
@@ -945,30 +936,18 @@ elsif ($opt_k) {
die "unhandled type for multi-return-value for " .
"procedure $call->{name}: $ret_member";
}
- } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/)
{
+ } elsif ($ret_member =~ m/^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 = $1;
$single_ret_list_max_var = "max$1";
$single_ret_list_max_define = $2;
-
- my $first_arg = shift(@args_list);
- my $second_arg;
-
- if ($call->{ProcName} eq "NodeListDevices") {
- $second_arg = shift(@args_list);
- }
-
- unshift(@args_list, "char **const $1");
-
- if (defined $second_arg) {
- unshift(@args_list, $second_arg);
- }
-
- unshift(@args_list, $first_arg);
-
- push(@ret_list, "rv = ret.$1.$1_len;");
- $single_ret_var = "int rv = -1";
- $single_ret_type = "int";
+ } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
+ # error out on unannotated arrays
+ die "remote_nonnull_string array without insert@<offset>
annotation: $ret_member";
} elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
push(@ret_list, "rv = ret.$1;");
$single_ret_var = "char *rv = NULL";
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 3e9bd5c..63f7ebb 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -368,7 +368,10 @@ struct remote_memory_param {
*
* Please follow the naming convention carefully - this file is
* parsed by 'remote_generator.pl'.
- */
+ *
+ * 'remote_CALL_ret' members that are filled via call-by-reference must be
+ * annotated with a insert@<offset> comment to indicate the offset in the
+ * parameter list of the function to be called. */
struct remote_open_args {
/* NB. "name" might be NULL although in practice you can't
@@ -446,7 +449,7 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- hyper cells<REMOTE_NODE_MAX_CELLS>;
+ hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
};
struct remote_node_get_free_memory_ret {
@@ -600,7 +603,7 @@ struct remote_list_domains_args {
};
struct remote_list_domains_ret {
- int ids<REMOTE_DOMAIN_ID_LIST_MAX>;
+ int ids<REMOTE_DOMAIN_ID_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_domains_ret {
@@ -801,7 +804,7 @@ struct remote_list_defined_domains_args {
};
struct remote_list_defined_domains_ret {
- remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_domains_ret {
@@ -949,7 +952,7 @@ struct remote_list_networks_args {
};
struct remote_list_networks_ret {
- remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_networks_ret {
@@ -961,7 +964,7 @@ struct remote_list_defined_networks_args {
};
struct remote_list_defined_networks_ret {
- remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_network_lookup_by_uuid_args {
@@ -1049,7 +1052,7 @@ struct remote_list_nwfilters_args {
};
struct remote_list_nwfilters_ret {
- remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_nwfilter_lookup_by_uuid_args {
@@ -1101,7 +1104,7 @@ struct remote_list_interfaces_args {
};
struct remote_list_interfaces_ret {
- remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>; /* insert@1 */
};
struct remote_num_of_defined_interfaces_ret {
@@ -1113,7 +1116,7 @@ struct remote_list_defined_interfaces_args {
};
struct remote_list_defined_interfaces_ret {
- remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>; /*
insert@1 */
};
struct remote_interface_lookup_by_name_args {
@@ -1215,7 +1218,7 @@ struct remote_list_storage_pools_args {
};
struct remote_list_storage_pools_ret {
- remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert@1
*/
};
struct remote_num_of_defined_storage_pools_ret {
@@ -1227,7 +1230,7 @@ struct remote_list_defined_storage_pools_args {
};
struct remote_list_defined_storage_pools_ret {
- remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert@1
*/
};
struct remote_find_storage_pool_sources_args {
@@ -1357,7 +1360,7 @@ struct remote_storage_pool_list_volumes_args {
};
struct remote_storage_pool_list_volumes_ret {
- remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>; /* insert@1 */
};
@@ -1465,7 +1468,7 @@ struct remote_node_list_devices_args {
};
struct remote_node_list_devices_ret {
- remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>; /* insert@2 */
};
struct remote_node_device_lookup_by_name_args {
@@ -1507,7 +1510,7 @@ struct remote_node_device_list_caps_args {
};
struct remote_node_device_list_caps_ret {
- remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>;
+ remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>; /* insert@1 */
};
struct remote_node_device_dettach_args {
@@ -1588,7 +1591,7 @@ struct remote_list_secrets_args {
};
struct remote_list_secrets_ret {
- remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>;
+ remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>; /* insert@1 */
};
struct remote_secret_lookup_by_uuid_args {
@@ -1900,7 +1903,7 @@ struct remote_domain_snapshot_list_names_args {
};
struct remote_domain_snapshot_list_names_ret {
- remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>;
+ remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /*
insert@1 */
};
struct remote_domain_snapshot_lookup_by_name_args {
@@ -2006,7 +2009,7 @@ struct remote_domain_migrate_prepare_tunnel3_args {
};
struct remote_domain_migrate_prepare_tunnel3_ret {
- opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>;
+ opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>; /* insert@3 */
};
struct remote_domain_migrate_perform3_args {
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 4/7] remote generator: Handle stream-using functions
Extend procedure annotation in the .x file for stream handling.
Adds a missing remoteStreamRelease call to remoteDomainScreenshot
error path.
---
daemon/remote.c | 238 ------------------------------------------
daemon/remote_generator.pl | 114 +++++++++++++++++----
src/remote/remote_driver.c | 207 ------------------------------------
src/remote/remote_protocol.x | 38 +++++---
4 files changed, 120 insertions(+), 477 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index fcda2c5..6ca4daa 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1176,50 +1176,6 @@ cleanup:
}
static int
-remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client,
- virConnectPtr conn,
- remote_message_header *hdr,
- remote_error *rerr,
- remote_domain_migrate_prepare_tunnel_args
*args,
- void *ret ATTRIBUTE_UNUSED)
-{
- char *dname;
- struct qemud_client_stream *stream = NULL;
- int rv = -1;
-
- if (!conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- dname = args->dname == NULL ? NULL : *args->dname;
-
- if (!(stream = remoteCreateClientStream(conn, hdr)))
- goto cleanup;
-
- if (virDomainMigratePrepareTunnel(conn, stream->st,
- args->flags, dname, args->resource,
- args->dom_xml) < 0)
- goto cleanup;
-
- if (remoteAddClientStream(client, stream, 0) < 0)
- goto cleanup;
-
- rv = 0;
-
-cleanup:
- if (rv < 0) {
- remoteDispatchError(rerr);
- if (stream) {
- virStreamAbort(stream->st);
- remoteFreeClientStream(client, stream);
- }
- }
- return rv;
-}
-
-static int
remoteDispatchDomainPinVcpu(struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
@@ -2612,96 +2568,6 @@ cleanup:
return rv;
}
-static int remoteDispatchStorageVolUpload(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client,
- virConnectPtr conn,
- remote_message_header *hdr,
- remote_error *rerr,
- remote_storage_vol_upload_args *args,
- void *ret ATTRIBUTE_UNUSED)
-{
- struct qemud_client_stream *stream = NULL;
- virStorageVolPtr vol = NULL;
- int rv = -1;
-
- if (!conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- if (!(vol = get_nonnull_storage_vol(conn, args->vol)))
- goto cleanup;
-
- if (!(stream = remoteCreateClientStream(conn, hdr)))
- goto cleanup;
-
- if (virStorageVolUpload(vol, stream->st,
- args->offset, args->length,
- args->flags) < 0)
- goto cleanup;
-
- if (remoteAddClientStream(client, stream, 0) < 0)
- goto cleanup;
-
- rv = 0;
-
-cleanup:
- if (rv < 0)
- remoteDispatchError(rerr);
- if (vol)
- virStorageVolFree(vol);
- if (stream && rv != 0) {
- virStreamAbort(stream->st);
- remoteFreeClientStream(client, stream);
- }
- return rv;
-}
-
-static int remoteDispatchStorageVolDownload(struct qemud_server *server
ATTRIBUTE_UNUSED,
- struct qemud_client *client,
- virConnectPtr conn,
- remote_message_header *hdr,
- remote_error *rerr,
- remote_storage_vol_download_args *args,
- void *ret ATTRIBUTE_UNUSED)
-{
- struct qemud_client_stream *stream = NULL;
- virStorageVolPtr vol = NULL;
- int rv = -1;
-
- if (!conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- if (!(vol = get_nonnull_storage_vol(conn, args->vol)))
- goto cleanup;
-
- if (!(stream = remoteCreateClientStream(conn, hdr)))
- goto cleanup;
-
- if (virStorageVolDownload(vol, stream->st,
- args->offset, args->length,
- args->flags) < 0)
- goto cleanup;
-
- if (remoteAddClientStream(client, stream, 1) < 0)
- goto cleanup;
-
- rv = 0;
-
-cleanup:
- if (rv < 0)
- remoteDispatchError(rerr);
- if (vol)
- virStorageVolFree(vol);
- if (stream && rv != 0) {
- virStreamAbort(stream->st);
- remoteFreeClientStream(client, stream);
- }
- return rv;
-}
-
/***************************
* Register / deregister events
@@ -3035,53 +2901,6 @@ cleanup:
}
-static int
-remoteDispatchDomainOpenConsole(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client,
- virConnectPtr conn,
- remote_message_header *hdr,
- remote_error *rerr,
- remote_domain_open_console_args *args,
- void *ret ATTRIBUTE_UNUSED)
-{
- struct qemud_client_stream *stream = NULL;
- virDomainPtr dom = NULL;
- int rv = -1;
-
- if (!conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- if (!(dom = get_nonnull_domain(conn, args->dom)))
- goto cleanup;
-
- if (!(stream = remoteCreateClientStream(conn, hdr)))
- goto cleanup;
-
- if (virDomainOpenConsole(dom,
- args->devname ? *args->devname : NULL,
- stream->st,
- args->flags) < 0)
- goto cleanup;
-
- if (remoteAddClientStream(client, stream, 1) < 0)
- goto cleanup;
-
- rv = 0;
-
-cleanup:
- if (rv < 0)
- remoteDispatchError(rerr);
- if (stream && rv < 0) {
- virStreamAbort(stream->st);
- remoteFreeClientStream(client, stream);
- }
- if (dom)
- virDomainFree(dom);
- return rv;
-}
-
#include "remote_dispatch_bodies.h"
#include "qemu_dispatch_bodies.h"
@@ -3192,63 +3011,6 @@ cleanup:
}
static int
-remoteDispatchDomainMigratePrepareTunnel3(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client,
- virConnectPtr conn,
- remote_message_header *hdr,
- remote_error *rerr,
- remote_domain_migrate_prepare_tunnel3_args
*args,
- remote_domain_migrate_prepare_tunnel3_ret
*ret)
-{
- char *dname;
- char *cookieout = NULL;
- int cookieoutlen = 0;
- struct qemud_client_stream *stream = NULL;
- int rv = -1;
-
- if (!conn) {
- virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
- goto cleanup;
- }
-
- dname = args->dname == NULL ? NULL : *args->dname;
-
- if (!(stream = remoteCreateClientStream(conn, hdr))) {
- virReportOOMError();
- goto cleanup;
- }
-
- if (virDomainMigratePrepareTunnel3(conn, stream->st,
- args->cookie_in.cookie_in_val,
- args->cookie_in.cookie_in_len,
- &cookieout, &cookieoutlen,
- args->flags, dname, args->resource,
- args->dom_xml) < 0)
- goto cleanup;
-
- if (remoteAddClientStream(client, stream, 0) < 0)
- goto cleanup;
-
- /* remoteDispatchClientRequest will free cookie
- */
- ret->cookie_out.cookie_out_len = cookieoutlen;
- ret->cookie_out.cookie_out_val = cookieout;
-
- rv = 0;
-
-cleanup:
- if (rv < 0) {
- remoteDispatchError(rerr);
- VIR_FREE(cookieout);
- }
- if (stream && rv < 0) {
- virStreamAbort(stream->st);
- remoteFreeClientStream(client, stream);
- }
- return rv;
-}
-
-static int
remoteDispatchDomainMigratePerform3(struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 8b3794f..c405dc8 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -43,7 +43,7 @@ sub name_to_ProcName {
# Read the input file (usually remote_protocol.x) and form an
# opinion about the name, args and return type of each RPC.
-my ($name, $ProcName, $id, $flags, $gen, %calls, @calls);
+my ($name, $ProcName, $id, $flags, %calls, @calls);
# only generate a close method if -c was passed
if ($opt_c) {
@@ -135,19 +135,31 @@ while (<PROTOCOL>) {
$ProcName = name_to_ProcName ($name);
if ($opt_b or $opt_k) {
- if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*\*\/\s*$/)) {
+ if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) {
die "invalid generator flags for ${procprefix}_PROC_${name}"
}
- $gen = $opt_b ? $1 : $2;
+ my $genmode = $opt_b ? $1 : $2;
+ my $genflags = $3;
- if ($gen eq "autogen") {
+ if ($genmode eq "autogen") {
push(@autogen, $ProcName);
- } elsif ($gen eq "skipgen") {
+ } elsif ($genmode eq "skipgen") {
# ignore it
} else {
die "invalid generator flags for ${procprefix}_PROC_${name}"
}
+
+ if (defined $genflags and $genflags ne "") {
+ if ($genflags =~ m/^\|\s*(read|write)stream@(\d+)\s*$/) {
+ $calls{$name}->{streamflag} = $1;
+ $calls{$name}->{streamoffset} = int($2);
+ } else {
+ die "invalid generator flags for
${procprefix}_PROC_${name}"
+ }
+ } else {
+ $calls{$name}->{streamflag} = "none";
+ }
}
$calls[$id] = $calls{$name};
@@ -531,6 +543,18 @@ elsif ($opt_b) {
} else {
$single_ret_by_ref = 1;
}
+ } elsif ($ret_member =~ m/^opaque
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
+ push(@vars_list, "char *$1 = NULL");
+ push(@vars_list, "int $1_len = 0");
+ splice(@args_list, int($3), 0, ("&$1",
"&$1_len"));
+ push(@ret_list, "ret->$1.$1_val = $1;");
+ push(@ret_list, "ret->$1.$1_len = $1_len;");
+ push(@free_list_on_error, "VIR_FREE($1);");
+ $single_ret_var = undef;
+ $single_ret_by_ref = 1;
+ } 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/^(\/)?\*/) {
# ignore comments
} else {
@@ -569,6 +593,14 @@ elsif ($opt_b) {
push(@vars_list, "vir$struct_name tmp");
}
+ if ($call->{streamflag} ne "none") {
+ splice(@args_list, $call->{streamoffset}, 0,
("stream->st"));
+ push(@free_list_on_error, "if (stream) {");
+ push(@free_list_on_error, " virStreamAbort(stream->st);");
+ push(@free_list_on_error, " remoteFreeClientStream(client,
stream);");
+ push(@free_list_on_error, "}");
+ }
+
# print functions signature
print "\n";
print "static int\n";
@@ -601,6 +633,10 @@ elsif ($opt_b) {
print " $var;\n";
}
+ if ($call->{streamflag} ne "none") {
+ print " struct qemud_client_stream *stream = NULL;\n";
+ }
+
print "\n";
print " if (!conn) {\n";
print " virNetError(VIR_ERR_INTERNAL_ERROR, \"%s\",
_(\"connection not open\"));\n";
@@ -631,6 +667,12 @@ elsif ($opt_b) {
print "\n";
}
+ if ($call->{streamflag} ne "none") {
+ print " if (!(stream = remoteCreateClientStream(conn,
hdr)))\n";
+ print " goto cleanup;\n";
+ print "\n";
+ }
+
if ($call->{ret} eq "void") {
print " if (vir$call->{ProcName}(";
print join(', ', @args_list);
@@ -691,25 +733,30 @@ elsif ($opt_b) {
print " goto cleanup;\n";
print "\n";
-
- if (@ret_list) {
- print " ";
- }
-
- print join("\n ", @ret_list);
- print "\n";
} else {
print " if (vir$call->{ProcName}(";
print join(', ', @args_list);
print ") < 0)\n";
-
print " goto cleanup;\n";
print "\n";
+ }
+
+ if ($call->{streamflag} ne "none") {
+ print " if (remoteAddClientStream(client, stream, ";
- if (@ret_list) {
- print " ";
+ if ($call->{streamflag} eq "write") {
+ print "0";
+ } else {
+ print "1";
}
+ print ") < 0)\n";
+ print " goto cleanup;\n";
+ print "\n";
+ }
+
+ if (@ret_list) {
+ print " ";
print join("\n ", @ret_list);
print "\n";
}
@@ -865,9 +912,14 @@ elsif ($opt_k) {
}
}
- if ($call->{ProcName} eq "DomainMigrateSetMaxDowntime"
and
- $arg_name eq "downtime") {
- $type_name = "unsigned long long";
+ # SPECIAL: some hyper parameters map to long longs
+ if (($call->{ProcName} eq "DomainMigrateSetMaxDowntime"
and
+ $arg_name eq "downtime") or
+ ($call->{ProcName} eq "StorageVolUpload" and
+ ($arg_name eq "offset" or $arg_name eq
"length")) or
+ ($call->{ProcName} eq "StorageVolDownload" and
+ ($arg_name eq "offset" or $arg_name eq
"length"))) {
+ $type_name .= " long";
}
push(@args_list, "$type_name $arg_name");
@@ -1004,6 +1056,7 @@ elsif ($opt_k) {
$single_ret_type = "int";
} elsif ($ret_member =~ m/^unsigned hyper (\S+);/) {
my $arg_name = $1;
+
if ($call->{ProcName} =~ m/Get(Lib)?Version/) {
push(@args_list, "unsigned long *$arg_name");
push(@ret_list, "if ($arg_name) *$arg_name =
ret.$arg_name;");
@@ -1053,6 +1106,10 @@ elsif ($opt_k) {
}
}
+ if ($call->{streamflag} ne "none") {
+ splice(@args_list, $call->{streamoffset}, 0, ("virStreamPtr
st"));
+ }
+
# print function
print "\n";
print "static $single_ret_type\n";
@@ -1073,9 +1130,22 @@ elsif ($opt_k) {
print " int i;\n";
}
+ if ($call->{streamflag} ne "none") {
+ print " struct private_stream_data *privst = NULL;\n";
+ }
+
print "\n";
print " remoteDriverLock(priv);\n";
+ if ($call->{streamflag} ne "none") {
+ print "\n";
+ print " if (!(privst = remoteStreamOpen(st,
REMOTE_PROC_$call->{UC_NAME}, priv->counter)))\n";
+ print " goto done;\n";
+ print "\n";
+ print " st->driver = &remoteStreamDrv;\n";
+ print " st->privateData = privst;\n";
+ }
+
if ($call->{ProcName} eq "SupportsFeature") {
# SPECIAL: VIR_DRV_FEATURE_REMOTE feature is handled directly
print "\n";
@@ -1124,8 +1194,14 @@ elsif ($opt_k) {
print "\n";
print " if (call($priv_src, priv, 0,
${procprefix}_PROC_$call->{UC_NAME},\n";
print " (xdrproc_t)xdr_$call->{args}, (char
*)$call_args,\n";
- print " (xdrproc_t)xdr_$call->{ret}, (char *)$call_ret) ==
-1)\n";
+ print " (xdrproc_t)xdr_$call->{ret}, (char *)$call_ret) ==
-1) {\n";
+
+ if ($call->{streamflag} ne "none") {
+ print " remoteStreamRelease(st);\n";
+ }
+
print " goto done;\n";
+ print " }\n";
print "\n";
if ($single_ret_as_list) {
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fb542c5..3556b85 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -4607,48 +4607,6 @@ static virStreamDriver remoteStreamDrv = {
.streamRemoveCallback = remoteStreamEventRemoveCallback,
};
-
-static int
-remoteDomainMigratePrepareTunnel(virConnectPtr conn,
- virStreamPtr st,
- unsigned long flags,
- const char *dname,
- unsigned long resource,
- const char *dom_xml)
-{
- struct private_data *priv = conn->privateData;
- struct private_stream_data *privst = NULL;
- int rv = -1;
- remote_domain_migrate_prepare_tunnel_args args;
-
- remoteDriverLock(priv);
-
- if (!(privst = remoteStreamOpen(st, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL,
priv->counter)))
- goto done;
-
- st->driver = &remoteStreamDrv;
- st->privateData = privst;
-
- args.flags = flags;
- args.dname = dname == NULL ? NULL : (char **) &dname;
- args.resource = resource;
- args.dom_xml = (char *) dom_xml;
-
- if (call(conn, priv, 0, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL,
- (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel_args, (char *)
&args,
- (xdrproc_t) xdr_void, NULL) == -1) {
- remoteStreamRelease(st);
- goto done;
- }
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
-
- return rv;
-}
-
static int remoteDomainEventRegisterAny(virConnectPtr conn,
virDomainPtr dom,
int eventID,
@@ -4743,171 +4701,6 @@ done:
return rv;
}
-static char *
-remoteDomainScreenshot (virDomainPtr domain,
- virStreamPtr st,
- unsigned int screen,
- unsigned int flags)
-{
- struct private_data *priv = domain->conn->privateData;
- struct private_stream_data *privst = NULL;
- remote_domain_screenshot_args args;
- remote_domain_screenshot_ret ret;
- char *rv = NULL;
-
- remoteDriverLock(priv);
-
- if (!(privst = remoteStreamOpen(st,
- REMOTE_PROC_DOMAIN_SCREENSHOT,
- priv->counter)))
- goto done;
-
- st->driver = &remoteStreamDrv;
- st->privateData = privst;
-
- make_nonnull_domain(&args.dom, domain);
- args.flags = flags;
- args.screen = screen;
-
- memset(&ret, 0, sizeof(ret));
- if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_SCREENSHOT,
- (xdrproc_t) xdr_remote_domain_screenshot_args, (char *) &args,
- (xdrproc_t) xdr_remote_domain_screenshot_ret, (char *) &ret) == -1)
- goto done;
-
- rv = ret.mime ? *ret.mime : NULL;
- VIR_FREE(ret.mime);
-
-done:
- remoteDriverUnlock(priv);
- return rv;
-}
-
-static int
-remoteStorageVolUpload(virStorageVolPtr vol,
- virStreamPtr st,
- unsigned long long offset,
- unsigned long long length,
- unsigned int flags)
-{
- struct private_data *priv = vol->conn->privateData;
- struct private_stream_data *privst = NULL;
- int rv = -1;
- remote_storage_vol_upload_args args;
-
- remoteDriverLock(priv);
-
- if (!(privst = remoteStreamOpen(st,
- REMOTE_PROC_STORAGE_VOL_UPLOAD,
- priv->counter)))
- goto done;
-
- st->driver = &remoteStreamDrv;
- st->privateData = privst;
-
- make_nonnull_storage_vol(&args.vol, vol);
- args.offset = offset;
- args.length = length;
- args.flags = flags;
-
- if (call (vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_UPLOAD,
- (xdrproc_t) xdr_remote_storage_vol_upload_args, (char *) &args,
- (xdrproc_t) xdr_void, NULL) == -1) {
- remoteStreamRelease(st);
- goto done;
- }
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
-
- return rv;
-}
-
-
-static int
-remoteStorageVolDownload(virStorageVolPtr vol,
- virStreamPtr st,
- unsigned long long offset,
- unsigned long long length,
- unsigned int flags)
-{
- struct private_data *priv = vol->conn->privateData;
- struct private_stream_data *privst = NULL;
- int rv = -1;
- remote_storage_vol_download_args args;
-
- remoteDriverLock(priv);
-
- if (!(privst = remoteStreamOpen(st,
- REMOTE_PROC_STORAGE_VOL_DOWNLOAD,
- priv->counter)))
- goto done;
-
- st->driver = &remoteStreamDrv;
- st->privateData = privst;
-
- make_nonnull_storage_vol(&args.vol, vol);
- args.offset = offset;
- args.length = length;
- args.flags = flags;
-
- if (call (vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_DOWNLOAD,
- (xdrproc_t) xdr_remote_storage_vol_download_args, (char *) &args,
- (xdrproc_t) xdr_void, NULL) == -1) {
- remoteStreamRelease(st);
- goto done;
- }
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
-
- return rv;
-}
-
-
-static int
-remoteDomainOpenConsole(virDomainPtr dom,
- const char *devname,
- virStreamPtr st,
- unsigned int flags)
-{
- struct private_data *priv = dom->conn->privateData;
- struct private_stream_data *privst = NULL;
- int rv = -1;
- remote_domain_open_console_args args;
-
- remoteDriverLock(priv);
-
- if (!(privst = remoteStreamOpen(st, REMOTE_PROC_DOMAIN_OPEN_CONSOLE,
priv->counter)))
- goto done;
-
- st->driver = &remoteStreamDrv;
- st->privateData = privst;
-
- make_nonnull_domain (&args.dom, dom);
- args.devname = devname ? (char **)&devname : NULL;
- args.flags = flags;
-
- if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_OPEN_CONSOLE,
- (xdrproc_t) xdr_remote_domain_open_console_args, (char *) &args,
- (xdrproc_t) xdr_void, NULL) == -1) {
- remoteStreamRelease(st);
- goto done;
- }
-
- rv = 0;
-
-done:
- remoteDriverUnlock(priv);
-
- return rv;
-}
-
-
/*----------------------------------------------------------------------*/
static int
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 63f7ebb..5b9300a 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2054,7 +2054,14 @@ const REMOTE_PROTOCOL_VERSION = 1;
enum remote_procedure {
/* Each function must have a two-word comment. The first word is
* whether remote_generator.pl handles daemon, the second whether
- * it handles src/remote. */
+ * it handles src/remote. Additional flags can be specified after a
+ * pipe.
+ *
+ * The (readstream|writestream)@<offset> flag lets daemon and src/remote
+ * create a stream. The direction is defined from the src/remote point
+ * of view. A readstream transfers data from daemon to src/remote. The
+ * <offset> specifies at which offset the stream parameter is inserted
+ * in the function parameter list. */
REMOTE_PROC_OPEN = 1, /* skipgen skipgen */
REMOTE_PROC_CLOSE = 2, /* skipgen skipgen */
REMOTE_PROC_GET_TYPE = 3, /* autogen skipgen */
@@ -2216,7 +2223,7 @@ enum remote_procedure {
REMOTE_PROC_SECRET_GET_VALUE = 145, /* skipgen skipgen */
REMOTE_PROC_SECRET_UNDEFINE = 146, /* autogen autogen */
REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147, /* autogen autogen */
- REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148, /* autogen autogen | writestream@1
*/
REMOTE_PROC_IS_SECURE = 149, /* autogen skipgen */
REMOTE_PROC_DOMAIN_IS_ACTIVE = 150, /* autogen autogen */
@@ -2275,35 +2282,40 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 199, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS = 200, /* autogen autogen */
- REMOTE_PROC_DOMAIN_OPEN_CONSOLE = 201, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_OPEN_CONSOLE = 201, /* autogen autogen | readstream@2 */
REMOTE_PROC_DOMAIN_IS_UPDATED = 202, /* autogen autogen */
REMOTE_PROC_GET_SYSINFO = 203, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_MEMORY_FLAGS = 204, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS = 205, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, /* autogen autogen */
- REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, /* skipgen skipgen */
- REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, /* skipgen skipgen */
+ REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, /* autogen autogen | writestream@1 */
+ REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, /* autogen autogen | readstream@1 */
REMOTE_PROC_DOMAIN_INJECT_NMI = 210, /* autogen autogen */
- REMOTE_PROC_DOMAIN_SCREENSHOT = 211, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_SCREENSHOT = 211, /* skipgen autogen | readstream@1 */
REMOTE_PROC_DOMAIN_GET_STATE = 212, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_BEGIN3 = 213, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_PREPARE3 = 214, /* skipgen skipgen */
- REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3 = 215, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3 = 215, /* autogen skipgen | writestream@1
*/
REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3 = 216, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_FINISH3 = 217, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219 /* skipgen skipgen */
- /*
- * Notice how the entries are grouped in sets of 10 ?
+ /* Notice how the entries are grouped in sets of 10 ?
* Nice isn't it. Please keep it this way when adding more.
- */
-
- /* Each function must have a two-word comment. The first word is
+ *
+ * Each function must have a two-word comment. The first word is
* whether remote_generator.pl handles daemon, the second whether
- * it handles src/remote. */
+ * it handles src/remote. Additional flags can be specified after a
+ * pipe.
+ *
+ * The (readstream|writestream)@<offset> flag lets daemon and src/remote
+ * create a stream. The direction is defined from the src/remote point
+ * of view. A readstream transfers data from daemon to src/remote. The
+ * <offset> specifies at which offset the stream parameter is inserted
+ * in the function parameter list. */
};
/*
--
1.7.0.4
11:15 a.m.
New subject: [libvirt] [PATCH 4/7] remote generator: Handle stream-using functions
On Mon, May 23, 2011 at 07:36:07PM +0200, Matthias Bolte wrote:
Extend procedure annotation in the .x file for stream handling.
Adds a missing remoteStreamRelease call to remoteDomainScreenshot
error path.
---
daemon/remote.c | 238 ------------------------------------------
daemon/remote_generator.pl | 114 +++++++++++++++++----
src/remote/remote_driver.c | 207 ------------------------------------
src/remote/remote_protocol.x | 38 +++++---
4 files changed, 120 insertions(+), 477 deletions(-)
diff --git a/src/remote/remote_protocol.x
b/src/remote/remote_protocol.x
index 63f7ebb..5b9300a 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2054,7 +2054,14 @@ const REMOTE_PROTOCOL_VERSION = 1;
enum remote_procedure {
/* Each function must have a two-word comment. The first word is
* whether remote_generator.pl handles daemon, the second whether
- * it handles src/remote. */
+ * it handles src/remote. Additional flags can be specified after a
+ * pipe.
+ *
+ * The (readstream|writestream)@<offset> flag lets daemon and src/remote
+ * create a stream. The direction is defined from the src/remote point
+ * of view. A readstream transfers data from daemon to src/remote. The
+ * <offset> specifies at which offset the stream parameter is inserted
+ * in the function parameter list. */
REMOTE_PROC_OPEN = 1, /* skipgen skipgen */
REMOTE_PROC_CLOSE = 2, /* skipgen skipgen */
REMOTE_PROC_GET_TYPE = 3, /* autogen skipgen */
@@ -2216,7 +2223,7 @@ enum remote_procedure {
REMOTE_PROC_SECRET_GET_VALUE = 145, /* skipgen skipgen */
REMOTE_PROC_SECRET_UNDEFINE = 146, /* autogen autogen */
REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147, /* autogen autogen */
- REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148, /* autogen autogen | writestream@1
*/
REMOTE_PROC_IS_SECURE = 149, /* autogen skipgen */
REMOTE_PROC_DOMAIN_IS_ACTIVE = 150, /* autogen autogen */
@@ -2275,35 +2282,40 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 199, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS = 200, /* autogen autogen */
- REMOTE_PROC_DOMAIN_OPEN_CONSOLE = 201, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_OPEN_CONSOLE = 201, /* autogen autogen | readstream@2 */
Technically this is a bi-directional stream, but that doesn't
matter from a code-gen POV, so fine.
REMOTE_PROC_DOMAIN_IS_UPDATED = 202, /* autogen autogen */
REMOTE_PROC_GET_SYSINFO = 203, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_MEMORY_FLAGS = 204, /* autogen autogen */
REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS = 205, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, /* autogen autogen */
- REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, /* skipgen skipgen */
- REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, /* skipgen skipgen */
+ REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, /* autogen autogen | writestream@1 */
+ REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, /* autogen autogen | readstream@1 */
REMOTE_PROC_DOMAIN_INJECT_NMI = 210, /* autogen autogen */
- REMOTE_PROC_DOMAIN_SCREENSHOT = 211, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_SCREENSHOT = 211, /* skipgen autogen | readstream@1 */
REMOTE_PROC_DOMAIN_GET_STATE = 212, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_BEGIN3 = 213, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_PREPARE3 = 214, /* skipgen skipgen */
- REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3 = 215, /* skipgen skipgen */
+ REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3 = 215, /* autogen skipgen | writestream@1
*/
REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3 = 216, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_FINISH3 = 217, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219 /* skipgen skipgen */
- /*
- * Notice how the entries are grouped in sets of 10 ?
+ /* Notice how the entries are grouped in sets of 10 ?
* Nice isn't it. Please keep it this way when adding more.
- */
-
- /* Each function must have a two-word comment. The first word is
+ *
+ * Each function must have a two-word comment. The first word is
* whether remote_generator.pl handles daemon, the second whether
- * it handles src/remote. */
+ * it handles src/remote. Additional flags can be specified after a
+ * pipe.
+ *
+ * The (readstream|writestream)@<offset> flag lets daemon and src/remote
+ * create a stream. The direction is defined from the src/remote point
+ * of view. A readstream transfers data from daemon to src/remote. The
+ * <offset> specifies at which offset the stream parameter is inserted
+ * in the function parameter list. */
};
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 5/7] remote generator: Fix XDR sign mismatch for virNodeGet(Cells)FreeMemory
virNodeGetFreeMemory used unsigned long long in the public API but
signed hyper in the XDR protocol. Convert the XDR protocol to use
unsigned hyper.
As explained by Eric before, this doesn't affect the on-the-wire protocol.
---
daemon/remote_generator.pl | 18 ++++++------------
src/remote/remote_protocol.x | 4 ++--
src/remote_protocol-structs | 4 ++--
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index c405dc8..9143b3a 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -501,7 +501,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
}
}
- } elsif ($ret_member =~ m/^hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
+ } elsif ($ret_member =~ m/^(?:unsigned )?hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
@@ -519,7 +519,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
}
- } elsif ($ret_member =~ m/^hyper (\S+)<\S+>;/) {
+ } elsif ($ret_member =~ m/^(?:unsigned )?hyper (\S+)<\S+>;/) {
# error out on unannotated arrays
die "hyper array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
@@ -1063,20 +1063,14 @@ elsif ($opt_k) {
push(@ret_list, "rv = 0;");
$single_ret_var = "int rv = -1";
$single_ret_type = "int";
- } else {
- push(@ret_list, "rv = ret.$arg_name;");
- $single_ret_var = "unsigned long rv = 0";
- $single_ret_type = "unsigned long";
- }
- } elsif ($ret_member =~ m/^hyper (\S+);/) {
- my $arg_name = $1;
-
- if ($call->{ProcName} eq "NodeGetFreeMemory") {
+ } elsif ($call->{ProcName} eq "NodeGetFreeMemory") {
push(@ret_list, "rv = ret.$arg_name;");
$single_ret_var = "unsigned long long rv = 0";
$single_ret_type = "unsigned long long";
} else {
- die "unhandled type for return value: $ret_member";
+ push(@ret_list, "rv = ret.$arg_name;");
+ $single_ret_var = "unsigned long rv = 0";
+ $single_ret_type = "unsigned long";
}
} elsif ($ret_member =~ m/^(\/)?\*/) {
# ignore comments
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 5b9300a..f5218cd 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -449,11 +449,11 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
+ unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
};
struct remote_node_get_free_memory_ret {
- hyper freeMem;
+ unsigned hyper freeMem;
};
struct remote_domain_get_scheduler_type_args {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 5491f73..fd23fa0 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -158,11 +158,11 @@ struct remote_node_get_cells_free_memory_args {
struct remote_node_get_cells_free_memory_ret {
struct {
u_int cells_len;
- int64_t * cells_val;
+ uint64_t * cells_val;
} cells;
};
struct remote_node_get_free_memory_ret {
- int64_t freeMem;
+ uint64_t freeMem;
};
struct remote_domain_get_scheduler_type_args {
remote_nonnull_domain dom;
--
1.7.0.4
11:16 a.m.
New subject: [libvirt] [PATCH 5/7] remote generator: Fix XDR sign mismatch for virNodeGet(Cells)FreeMemory
On Mon, May 23, 2011 at 07:36:08PM +0200, Matthias Bolte wrote:
virNodeGetFreeMemory used unsigned long long in the public API but
signed hyper in the XDR protocol. Convert the XDR protocol to use
unsigned hyper.
As explained by Eric before, this doesn't affect the on-the-wire protocol.
---
daemon/remote_generator.pl | 18 ++++++------------
src/remote/remote_protocol.x | 4 ++--
src/remote_protocol-structs | 4 ++--
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index c405dc8..9143b3a 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -501,7 +501,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
}
}
- } elsif ($ret_member =~ m/^hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
+ } elsif ($ret_member =~ m/^(?:unsigned )?hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
push(@ret_list, "ret->$1.$1_len = len;");
push(@free_list_on_error,
"VIR_FREE(ret->$1.$1_val);");
@@ -519,7 +519,7 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
splice(@args_list, int($3), 0,
("ret->$1.$1_val"));
}
- } elsif ($ret_member =~ m/^hyper (\S+)<\S+>;/) {
+ } elsif ($ret_member =~ m/^(?:unsigned )?hyper (\S+)<\S+>;/) {
# error out on unannotated arrays
die "hyper array without insert@<offset> annotation:
$ret_member";
} elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
@@ -1063,20 +1063,14 @@ elsif ($opt_k) {
push(@ret_list, "rv = 0;");
$single_ret_var = "int rv = -1";
$single_ret_type = "int";
- } else {
- push(@ret_list, "rv = ret.$arg_name;");
- $single_ret_var = "unsigned long rv = 0";
- $single_ret_type = "unsigned long";
- }
- } elsif ($ret_member =~ m/^hyper (\S+);/) {
- my $arg_name = $1;
-
- if ($call->{ProcName} eq "NodeGetFreeMemory") {
+ } elsif ($call->{ProcName} eq "NodeGetFreeMemory") {
push(@ret_list, "rv = ret.$arg_name;");
$single_ret_var = "unsigned long long rv = 0";
$single_ret_type = "unsigned long long";
} else {
- die "unhandled type for return value: $ret_member";
+ push(@ret_list, "rv = ret.$arg_name;");
+ $single_ret_var = "unsigned long rv = 0";
+ $single_ret_type = "unsigned long";
}
} elsif ($ret_member =~ m/^(\/)?\*/) {
# ignore comments
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 5b9300a..f5218cd 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -449,11 +449,11 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
+ unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
};
struct remote_node_get_free_memory_ret {
- hyper freeMem;
+ unsigned hyper freeMem;
};
struct remote_domain_get_scheduler_type_args {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 5491f73..fd23fa0 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -158,11 +158,11 @@ struct remote_node_get_cells_free_memory_args {
struct remote_node_get_cells_free_memory_ret {
struct {
u_int cells_len;
- int64_t * cells_val;
+ uint64_t * cells_val;
} cells;
};
struct remote_node_get_free_memory_ret {
- int64_t freeMem;
+ uint64_t freeMem;
};
struct remote_domain_get_scheduler_type_args {
remote_nonnull_domain dom;
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 6/7] Fix sign mismatches between public API, driver API and XDR protocol
In most cases this affects flags parameters that are unsigned in the
public and driver API but signed in the XDR protocol. Switch the
XDR protocol to unsigned for those.
A counterexample is virNWFilterGetXMLDesc. Its flags parameter is signed
in the public API and XDR protocol, but unsigned in the driver API.
---
daemon/remote_generator.pl | 16 ----------------
src/driver.h | 2 +-
src/nwfilter/nwfilter_driver.c | 2 +-
src/remote/remote_protocol.x | 30 +++++++++++++++---------------
src/remote_protocol-structs | 30 +++++++++++++++---------------
5 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 9143b3a..ac808fb 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -896,22 +896,6 @@ elsif ($opt_k) {
$type_name .= $2;
$type_name =~ s/hyper/long/;
- if ($type_name eq "int") {
- # fix bad decisions in the xdr protocol
- if ($arg_name eq "flags" and
- $call->{ProcName} ne "DomainCoreDump" and
- $call->{ProcName} ne "DomainGetXMLDesc" and
- $call->{ProcName} ne "NetworkGetXMLDesc") {
- $type_name = "unsigned int";
- } elsif ($arg_name eq "nvcpus" and
- $call->{ProcName} eq "DomainSetVcpus") {
- $type_name = "unsigned int";
- } elsif ($arg_name eq "vcpu" and
- $call->{ProcName} eq "DomainPinVcpu") {
- $type_name = "unsigned int";
- }
- }
-
# SPECIAL: some hyper parameters map to long longs
if (($call->{ProcName} eq "DomainMigrateSetMaxDowntime"
and
$arg_name eq "downtime") or
diff --git a/src/driver.h b/src/driver.h
index 450dd53..58e8f02 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1282,7 +1282,7 @@ typedef int
typedef char *
(*virDrvNWFilterGetXMLDesc) (virNWFilterPtr nwfilter,
- unsigned int flags);
+ int flags);
typedef struct _virNWFilterDriver virNWFilterDriver;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index db3d789..d9ac17e 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -407,7 +407,7 @@ cleanup:
static char *
nwfilterGetXMLDesc(virNWFilterPtr obj,
- unsigned int flags) {
+ int flags) {
virNWFilterDriverStatePtr driver = obj->conn->nwfilterPrivateData;
virNWFilterObjPtr nwfilter;
char *ret = NULL;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index f5218cd..2b9f9de 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -430,7 +430,7 @@ struct remote_get_max_vcpus_ret {
struct remote_node_get_info_ret {
char model[32];
- hyper memory;
+ unsigned hyper memory;
int cpus;
int mhz;
int nodes;
@@ -612,7 +612,7 @@ struct remote_num_of_domains_ret {
struct remote_domain_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_domain_create_xml_ret {
@@ -657,7 +657,7 @@ struct remote_domain_shutdown_args {
struct remote_domain_reboot_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_destroy_args {
@@ -843,7 +843,7 @@ struct remote_domain_inject_nmi_args {
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
- int nvcpus;
+ unsigned int nvcpus;
};
struct remote_domain_set_vcpus_flags_args {
@@ -863,7 +863,7 @@ struct remote_domain_get_vcpus_flags_ret {
struct remote_domain_pin_vcpu_args {
remote_nonnull_domain dom;
- int vcpu;
+ unsigned int vcpu;
opaque cpumap<REMOTE_CPUMAP_MAX>;
};
@@ -1527,7 +1527,7 @@ struct remote_node_device_reset_args {
struct remote_node_device_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_node_device_create_xml_ret {
@@ -1871,7 +1871,7 @@ struct remote_domain_managed_save_remove_args {
struct remote_domain_snapshot_create_xml_args {
remote_nonnull_domain dom;
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_create_xml_ret {
@@ -1880,7 +1880,7 @@ struct remote_domain_snapshot_create_xml_ret {
struct remote_domain_snapshot_get_xml_desc_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_get_xml_desc_ret {
@@ -1889,7 +1889,7 @@ struct remote_domain_snapshot_get_xml_desc_ret {
struct remote_domain_snapshot_num_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_num_ret {
@@ -1899,7 +1899,7 @@ struct remote_domain_snapshot_num_ret {
struct remote_domain_snapshot_list_names_args {
remote_nonnull_domain dom;
int maxnames;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_list_names_ret {
@@ -1909,7 +1909,7 @@ struct remote_domain_snapshot_list_names_ret {
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_lookup_by_name_ret {
@@ -1918,7 +1918,7 @@ struct remote_domain_snapshot_lookup_by_name_ret {
struct remote_domain_has_current_snapshot_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_has_current_snapshot_ret {
@@ -1927,7 +1927,7 @@ struct remote_domain_has_current_snapshot_ret {
struct remote_domain_snapshot_current_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_current_ret {
@@ -1936,12 +1936,12 @@ struct remote_domain_snapshot_current_ret {
struct remote_domain_revert_to_snapshot_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_delete_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_open_console_args {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index fd23fa0..04f36ca 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -140,7 +140,7 @@ struct remote_get_max_vcpus_ret {
};
struct remote_node_get_info_ret {
char model[32];
- int64_t memory;
+ uint64_t memory;
int cpus;
int mhz;
int nodes;
@@ -325,7 +325,7 @@ struct remote_num_of_domains_ret {
};
struct remote_domain_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_domain_create_xml_ret {
remote_nonnull_domain dom;
@@ -359,7 +359,7 @@ struct remote_domain_shutdown_args {
};
struct remote_domain_reboot_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_destroy_args {
remote_nonnull_domain dom;
@@ -526,7 +526,7 @@ struct remote_domain_inject_nmi_args {
};
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
- int nvcpus;
+ u_int nvcpus;
};
struct remote_domain_set_vcpus_flags_args {
remote_nonnull_domain dom;
@@ -542,7 +542,7 @@ struct remote_domain_get_vcpus_flags_ret {
};
struct remote_domain_pin_vcpu_args {
remote_nonnull_domain dom;
- int vcpu;
+ u_int vcpu;
struct {
u_int cpumap_len;
char * cpumap_val;
@@ -1103,7 +1103,7 @@ struct remote_node_device_reset_args {
};
struct remote_node_device_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_node_device_create_xml_ret {
remote_nonnull_node_device dev;
@@ -1370,21 +1370,21 @@ struct remote_domain_managed_save_remove_args {
struct remote_domain_snapshot_create_xml_args {
remote_nonnull_domain dom;
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_create_xml_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_snapshot_get_xml_desc_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_get_xml_desc_ret {
remote_nonnull_string xml;
};
struct remote_domain_snapshot_num_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_num_ret {
int num;
@@ -1392,7 +1392,7 @@ struct remote_domain_snapshot_num_ret {
struct remote_domain_snapshot_list_names_args {
remote_nonnull_domain dom;
int maxnames;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_list_names_ret {
struct {
@@ -1403,32 +1403,32 @@ struct remote_domain_snapshot_list_names_ret {
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_lookup_by_name_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_has_current_snapshot_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_has_current_snapshot_ret {
int result;
};
struct remote_domain_snapshot_current_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_current_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_revert_to_snapshot_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_delete_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_open_console_args {
remote_nonnull_domain dom;
--
1.7.0.4
11:17 a.m.
New subject: [libvirt] [PATCH 6/7] Fix sign mismatches between public API, driver API and XDR protocol
On Mon, May 23, 2011 at 07:36:09PM +0200, Matthias Bolte wrote:
In most cases this affects flags parameters that are unsigned in the
public and driver API but signed in the XDR protocol. Switch the
XDR protocol to unsigned for those.
A counterexample is virNWFilterGetXMLDesc. Its flags parameter is signed
in the public API and XDR protocol, but unsigned in the driver API.
---
daemon/remote_generator.pl | 16 ----------------
src/driver.h | 2 +-
src/nwfilter/nwfilter_driver.c | 2 +-
src/remote/remote_protocol.x | 30 +++++++++++++++---------------
src/remote_protocol-structs | 30 +++++++++++++++---------------
5 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 9143b3a..ac808fb 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -896,22 +896,6 @@ elsif ($opt_k) {
$type_name .= $2;
$type_name =~ s/hyper/long/;
- if ($type_name eq "int") {
- # fix bad decisions in the xdr protocol
- if ($arg_name eq "flags" and
- $call->{ProcName} ne "DomainCoreDump" and
- $call->{ProcName} ne "DomainGetXMLDesc" and
- $call->{ProcName} ne "NetworkGetXMLDesc") {
- $type_name = "unsigned int";
- } elsif ($arg_name eq "nvcpus" and
- $call->{ProcName} eq "DomainSetVcpus") {
- $type_name = "unsigned int";
- } elsif ($arg_name eq "vcpu" and
- $call->{ProcName} eq "DomainPinVcpu") {
- $type_name = "unsigned int";
- }
- }
-
# SPECIAL: some hyper parameters map to long longs
if (($call->{ProcName} eq "DomainMigrateSetMaxDowntime"
and
$arg_name eq "downtime") or
diff --git a/src/driver.h b/src/driver.h
index 450dd53..58e8f02 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1282,7 +1282,7 @@ typedef int
typedef char *
(*virDrvNWFilterGetXMLDesc) (virNWFilterPtr nwfilter,
- unsigned int flags);
+ int flags);
typedef struct _virNWFilterDriver virNWFilterDriver;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index db3d789..d9ac17e 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -407,7 +407,7 @@ cleanup:
static char *
nwfilterGetXMLDesc(virNWFilterPtr obj,
- unsigned int flags) {
+ int flags) {
virNWFilterDriverStatePtr driver = obj->conn->nwfilterPrivateData;
virNWFilterObjPtr nwfilter;
char *ret = NULL;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index f5218cd..2b9f9de 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -430,7 +430,7 @@ struct remote_get_max_vcpus_ret {
struct remote_node_get_info_ret {
char model[32];
- hyper memory;
+ unsigned hyper memory;
int cpus;
int mhz;
int nodes;
@@ -612,7 +612,7 @@ struct remote_num_of_domains_ret {
struct remote_domain_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_domain_create_xml_ret {
@@ -657,7 +657,7 @@ struct remote_domain_shutdown_args {
struct remote_domain_reboot_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_destroy_args {
@@ -843,7 +843,7 @@ struct remote_domain_inject_nmi_args {
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
- int nvcpus;
+ unsigned int nvcpus;
};
struct remote_domain_set_vcpus_flags_args {
@@ -863,7 +863,7 @@ struct remote_domain_get_vcpus_flags_ret {
struct remote_domain_pin_vcpu_args {
remote_nonnull_domain dom;
- int vcpu;
+ unsigned int vcpu;
opaque cpumap<REMOTE_CPUMAP_MAX>;
};
@@ -1527,7 +1527,7 @@ struct remote_node_device_reset_args {
struct remote_node_device_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_node_device_create_xml_ret {
@@ -1871,7 +1871,7 @@ struct remote_domain_managed_save_remove_args {
struct remote_domain_snapshot_create_xml_args {
remote_nonnull_domain dom;
remote_nonnull_string xml_desc;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_create_xml_ret {
@@ -1880,7 +1880,7 @@ struct remote_domain_snapshot_create_xml_ret {
struct remote_domain_snapshot_get_xml_desc_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_get_xml_desc_ret {
@@ -1889,7 +1889,7 @@ struct remote_domain_snapshot_get_xml_desc_ret {
struct remote_domain_snapshot_num_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_num_ret {
@@ -1899,7 +1899,7 @@ struct remote_domain_snapshot_num_ret {
struct remote_domain_snapshot_list_names_args {
remote_nonnull_domain dom;
int maxnames;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_list_names_ret {
@@ -1909,7 +1909,7 @@ struct remote_domain_snapshot_list_names_ret {
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_lookup_by_name_ret {
@@ -1918,7 +1918,7 @@ struct remote_domain_snapshot_lookup_by_name_ret {
struct remote_domain_has_current_snapshot_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_has_current_snapshot_ret {
@@ -1927,7 +1927,7 @@ struct remote_domain_has_current_snapshot_ret {
struct remote_domain_snapshot_current_args {
remote_nonnull_domain dom;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_current_ret {
@@ -1936,12 +1936,12 @@ struct remote_domain_snapshot_current_ret {
struct remote_domain_revert_to_snapshot_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_snapshot_delete_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ unsigned int flags;
};
struct remote_domain_open_console_args {
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index fd23fa0..04f36ca 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -140,7 +140,7 @@ struct remote_get_max_vcpus_ret {
};
struct remote_node_get_info_ret {
char model[32];
- int64_t memory;
+ uint64_t memory;
int cpus;
int mhz;
int nodes;
@@ -325,7 +325,7 @@ struct remote_num_of_domains_ret {
};
struct remote_domain_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_domain_create_xml_ret {
remote_nonnull_domain dom;
@@ -359,7 +359,7 @@ struct remote_domain_shutdown_args {
};
struct remote_domain_reboot_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_destroy_args {
remote_nonnull_domain dom;
@@ -526,7 +526,7 @@ struct remote_domain_inject_nmi_args {
};
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
- int nvcpus;
+ u_int nvcpus;
};
struct remote_domain_set_vcpus_flags_args {
remote_nonnull_domain dom;
@@ -542,7 +542,7 @@ struct remote_domain_get_vcpus_flags_ret {
};
struct remote_domain_pin_vcpu_args {
remote_nonnull_domain dom;
- int vcpu;
+ u_int vcpu;
struct {
u_int cpumap_len;
char * cpumap_val;
@@ -1103,7 +1103,7 @@ struct remote_node_device_reset_args {
};
struct remote_node_device_create_xml_args {
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_node_device_create_xml_ret {
remote_nonnull_node_device dev;
@@ -1370,21 +1370,21 @@ struct remote_domain_managed_save_remove_args {
struct remote_domain_snapshot_create_xml_args {
remote_nonnull_domain dom;
remote_nonnull_string xml_desc;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_create_xml_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_snapshot_get_xml_desc_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_get_xml_desc_ret {
remote_nonnull_string xml;
};
struct remote_domain_snapshot_num_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_num_ret {
int num;
@@ -1392,7 +1392,7 @@ struct remote_domain_snapshot_num_ret {
struct remote_domain_snapshot_list_names_args {
remote_nonnull_domain dom;
int maxnames;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_list_names_ret {
struct {
@@ -1403,32 +1403,32 @@ struct remote_domain_snapshot_list_names_ret {
struct remote_domain_snapshot_lookup_by_name_args {
remote_nonnull_domain dom;
remote_nonnull_string name;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_lookup_by_name_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_has_current_snapshot_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_has_current_snapshot_ret {
int result;
};
struct remote_domain_snapshot_current_args {
remote_nonnull_domain dom;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_current_ret {
remote_nonnull_domain_snapshot snap;
};
struct remote_domain_revert_to_snapshot_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_snapshot_delete_args {
remote_nonnull_domain_snapshot snap;
- int flags;
+ u_int flags;
};
struct remote_domain_open_console_args {
remote_nonnull_domain dom;
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:20 a.m.
New subject: [libvirt] [PATCH 6/7] Fix sign mismatches between public API, driver API and XDR protocol
On Mon, May 23, 2011 at 07:36:09PM +0200, Matthias Bolte wrote:
In most cases this affects flags parameters that are unsigned in the
public and driver API but signed in the XDR protocol. Switch the
XDR protocol to unsigned for those.
A counterexample is virNWFilterGetXMLDesc. Its flags parameter is signed
in the public API and XDR protocol, but unsigned in the driver API.
For the record, the XDR code confirms this is ok. Compare the xdr_int32
with xdr_uint32 functions:
bool_t
xdr_int32_t (XDR *xdrs, int32_t *lp)
{
switch (xdrs->x_op)
{
case XDR_ENCODE:
return XDR_PUTINT32 (xdrs, lp);
case XDR_DECODE:
return XDR_GETINT32 (xdrs, lp);
case XDR_FREE:
return TRUE;
default:
return FALSE;
}
}
/* XDR 32bit unsigned integers */
bool_t
xdr_uint32_t (XDR *xdrs, uint32_t *ulp)
{
switch (xdrs->x_op)
{
case XDR_ENCODE:
return XDR_PUTINT32 (xdrs, (int32_t *) ulp);
case XDR_DECODE:
return XDR_GETINT32 (xdrs, (int32_t *) ulp);
case XDR_FREE:
return TRUE;
default:
return FALSE;
}
}
So the uint32 function just casts to an int32 and put
that on the wire. So there's no difference at all.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:36 p.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
Remove some special case code that took care of mapping hyper to the
correct C types.
Use macros for hyper to long assignments that perform overflow checks
when long is smaller than hyper. Also use such macros for the safe
hyper to longlong assignemts as this allows to keep the generator a
bit simpler.
Suggested by Eric Blake.
---
configure.ac | 1 +
daemon/remote.c | 21 ++++
daemon/remote_generator.pl | 240 ++++++++++++++++++++++++++++++++----------
src/remote/remote_driver.c | 21 ++++
src/remote/remote_protocol.x | 144 +++++++++++++------------
5 files changed, 303 insertions(+), 124 deletions(-)
diff --git a/configure.ac b/configure.ac
index e17e7af..412fdb8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -117,6 +117,7 @@ if test "x$have_cpuid" = xyes; then
fi
AC_MSG_RESULT([$have_cpuid])
+AC_CHECK_SIZEOF(long)
dnl Availability of various common functions (non-fatal if missing),
dnl and various less common threadsafe functions
diff --git a/daemon/remote.c b/daemon/remote.c
index 6ca4daa..83587c0 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -68,6 +68,27 @@
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
+#define HYPER_TO_TYPE(_type, _to, _from) \
+ do { \
+ if ((_from) != (_type)(_from)) { \
+ virNetError(VIR_ERR_INTERNAL_ERROR, \
+ _("conversion from hyper to %s overflowed"), #_type);
\
+ goto cleanup; \
+ } \
+ (_to) = (_from); \
+ } while (0)
+
+#if SIZEOF_LONG < 8
+# define HYPER_TO_LONG(_to, _from) HYPER_TO_TYPE(long, _to, _from)
+# define HYPER_TO_ULONG(_to, _from) HYPER_TO_TYPE(unsigned long, _to, _from)
+#else
+# define HYPER_TO_LONG(_to, _from) (_to) = (_from)
+# define HYPER_TO_ULONG(_to, _from) (_to) = (_from)
+#endif
+
+#define HYPER_TO_LONGLONG(_to, _from) (_to) = (_from)
+#define HYPER_TO_ULONGLONG(_to, _from) (_to) = (_from)
+
static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain
domain);
static virNetworkPtr get_nonnull_network(virConnectPtr conn, remote_nonnull_network
network);
static virInterfacePtr get_nonnull_interface(virConnectPtr conn, remote_nonnull_interface
iface);
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index ac808fb..f34f420 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -174,6 +174,62 @@ while (<PROTOCOL>) {
close(PROTOCOL);
+sub handle_multi_ret_member
+{
+ my $dst = shift;
+ my $src = shift;
+ my $ret_member = shift;
+ my $check_overflow = shift;
+
+ if ($ret_member =~ m/^(unsigned )?(char|short|int) (\S+)\[\S+\];/) {
+ return "memcpy($dst->$3, $src.$3, sizeof $dst->$3);";
+ } elsif ($ret_member =~ m/^(unsigned )?hyper
(\S+)\[\S+\];\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ # FIXME: in the simple case where hyper and the other type have the
+ # same size a simple memcpy will do, but for size mismatch
+ # this becomes more complicated and needs a for loop with
+ # possible overflow check for each item.
+ #
+ # currently the XDR protocol doesn't use hyper arrays, so the
+ # generator can lack an implementation for this at the moment
+ die "hyper arrays aren't implemented yet";
+ } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
+ } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~ m/\[\S+\];/) {
+ # just make all other array types fail
+ die "unhandled type for multi-return-value: $ret_member";
+ } elsif ($ret_member =~ m/^(unsigned )?(char|short|int) (\S+);/) {
+ return "$dst->$3 = $src.$3;";
+ } elsif ($ret_member =~ m/^(unsigned )?hyper
(\S+);\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$3) or (!$1 and $3)) {
+ die "sign mismatch in (unsigned)? hyper annotation: $ret_member";
+ }
+
+ if ($check_overflow) {
+ my $sign = ""; $sign = "U" if ($3);
+ my $macro;
+
+ # assignment from hyper to long can overflow, check for it
+ if ($4 eq "longlong") {
+ $macro = "HYPER_TO_${sign}LONGLONG";
+ } elsif ($4 eq "long") {
+ $macro = "HYPER_TO_${sign}LONG";
+ } else {
+ die "internal error";
+ }
+
+ return "$macro($dst->$2, $src.$2);";
+ } else {
+ return "$dst->$2 = $src.$2;";
+ }
+ } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
+ } else {
+ die "unhandled type for multi-return-value: $ret_member";
+ }
+}
+
#----------------------------------------------------------------------
# Output
@@ -378,12 +434,45 @@ elsif ($opt_b) {
}
push(@args_list, "args->$1");
- } elsif ($args_member =~ m/^(unsigned )?(int|hyper) (\S+);/) {
+ } elsif ($args_member =~ m/^(unsigned )?int (\S+);/) {
+ if (! @args_list) {
+ push(@args_list, "conn");
+ }
+
+ push(@args_list, "args->$2");
+ } elsif ($args_member =~ m/^(unsigned )?hyper
(\S+);\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$3) or (!$1 and $3)) {
+ die "sign mismatch in (unsigned)? hyper annotation:
$args_member";
+ }
+
if (! @args_list) {
push(@args_list, "conn");
}
- push(@args_list, "args->$3");
+ my $type_name;
+ my $arg_name = $2;
+ my $sign = ""; $sign = "U" if ($3);
+ my $macro;
+
+ $type_name = $1 if ($1);
+ $type_name .= "long";
+
+ # assignment from hyper to long can overflow, check for it
+ if ($4 eq "longlong") {
+ $type_name .= " long";
+ $macro = "HYPER_TO_${sign}LONGLONG";
+ } elsif ($4 eq "long") {
+ $macro = "HYPER_TO_${sign}LONG";
+ } else {
+ die "internal error";
+ }
+
+ push(@vars_list, "$type_name $arg_name");
+ push(@getters_list, " $macro($arg_name,
args->$arg_name);\n");
+ push(@args_list, "$arg_name");
+ } elsif ($args_member =~ m/^(unsigned )?hyper (\S+);/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$args_member";
} elsif ($args_member =~ m/^(\/)?\*/) {
# ignore comments
} else {
@@ -410,13 +499,7 @@ elsif ($opt_b) {
if ($call->{ret} ne "void") {
foreach my $ret_member (@{$call->{ret_members}}) {
if ($multi_ret) {
- if ($ret_member =~ m/^(unsigned )?(char|short|int|hyper)
(\S+)\[\S+\];/) {
- 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;");
- } else {
- die "unhandled type for multi-return-value:
$ret_member";
- }
+ push(@ret_list, handle_multi_ret_member("ret",
"tmp", $ret_member, 0));
} elsif ($ret_member =~ m/^remote_nonnull_string
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "int len");
splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
@@ -501,34 +584,60 @@ elsif ($opt_b) {
$single_ret_check = " < 0";
}
}
- } elsif ($ret_member =~ m/^(?:unsigned )?hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
+ } elsif ($ret_member =~ m/^(unsigned )?hyper
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s+(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$5) or (!$1 and $5)) {
+ die "sign mismatch in (unsigned)? hyper annotation:
$ret_member";
+ }
+
+ my $type_name;
+ $type_name = $1 if ($1);
+ $type_name .= "long";
+
+ # assignment to hyper cannot overflow, no check necessary
+ if ($6 eq "longlong") {
+ $type_name .= " long";
+ } elsif ($6 ne "long") {
+ die "internal error";
+ }
+
push(@vars_list, "int len");
- push(@ret_list, "ret->$1.$1_len = len;");
- push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
+ push(@ret_list, "ret->$2.$2_len = len;");
+ push(@free_list_on_error, "VIR_FREE(ret->$2.$2_val);");
$single_ret_var = "len";
$single_ret_by_ref = 0;
$single_ret_as_list = 1;
- $single_ret_list_name = $1;
- $single_ret_list_max_var = "max$1";
- $single_ret_list_max_define = $2;
+ $single_ret_list_name = $2;
+ $single_ret_list_max_var = "max$2";
+ $single_ret_list_max_define = $3;
if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
$single_ret_check = " <= 0";
- splice(@args_list, int($3), 0, ("(unsigned long long
*)ret->$1.$1_val"));
+ splice(@args_list, int($4), 0, ("(unsigned long long
*)ret->$2.$2_val"));
} else {
$single_ret_check = " < 0";
- splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
+ splice(@args_list, int($4), 0, ("ret->$2.$2_val"));
}
} elsif ($ret_member =~ m/^(?:unsigned )?hyper (\S+)<\S+>;/) {
# error out on unannotated arrays
- die "hyper array without insert@<offset> annotation:
$ret_member";
- } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
+ die "hyper array without insert@<offset>
(u)?(long|longlong) annotation: $ret_member";
+ } elsif ($ret_member =~ m/^(unsigned )?hyper
(\S+);\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$3) or (!$1 and $3)) {
+ die "sign mismatch in (unsigned)? hyper annotation:
$ret_member";
+ }
+
my $type_name;
my $ret_name = $2;
$type_name = $1 if ($1);
$type_name .= "long";
+ # assignment to hyper cannot overflow, no check necessary
+ if ($4 eq "longlong") {
+ $type_name .= " long";
+ } elsif ($4 ne "long") {
+ die "internal error";
+ }
+
push(@vars_list, "$type_name $ret_name");
push(@ret_list, "ret->$ret_name = $ret_name;");
$single_ret_var = $ret_name;
@@ -543,6 +652,9 @@ elsif ($opt_b) {
} else {
$single_ret_by_ref = 1;
}
+ } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
} elsif ($ret_member =~ m/^opaque
(\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
push(@vars_list, "char *$1 = NULL");
push(@vars_list, "int $1_len = 0");
@@ -888,26 +1000,38 @@ elsif ($opt_k) {
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/^(unsigned )?(int|hyper) (\S+);/) {
+ } elsif ($args_member =~ m/^(unsigned )?int (\S+);/) {
my $type_name;
- my $arg_name = $3;
+ my $arg_name = $2;
+
+ $type_name = $1 if ($1);
+ $type_name .= "int";
+
+ push(@args_list, "$type_name $arg_name");
+ push(@setters_list, "args.$arg_name = $arg_name;");
+ } elsif ($args_member =~ m/^(unsigned )?hyper
(\S+);\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$3) or (!$1 and $3)) {
+ die "sign mismatch in (unsigned)? hyper annotation:
$args_member";
+ }
+
+ my $type_name;
+ my $arg_name = $2;
$type_name = $1 if ($1);
- $type_name .= $2;
- $type_name =~ s/hyper/long/;
-
- # SPECIAL: some hyper parameters map to long longs
- if (($call->{ProcName} eq "DomainMigrateSetMaxDowntime"
and
- $arg_name eq "downtime") or
- ($call->{ProcName} eq "StorageVolUpload" and
- ($arg_name eq "offset" or $arg_name eq
"length")) or
- ($call->{ProcName} eq "StorageVolDownload" and
- ($arg_name eq "offset" or $arg_name eq
"length"))) {
+ $type_name .= "long";
+
+ # assignment to hyper cannot overflow, no check necessary
+ if ($4 eq "longlong") {
$type_name .= " long";
+ } elsif ($4 ne "long") {
+ die "internal error";
}
push(@args_list, "$type_name $arg_name");
push(@setters_list, "args.$arg_name = $arg_name;");
+ } elsif ($args_member =~ m/^(unsigned )?hyper (\S+);/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$args_member";
} elsif ($args_member =~ m/^(\/)?\*/) {
# ignore comments
} else {
@@ -960,18 +1084,7 @@ elsif ($opt_k) {
foreach my $ret_member (@{$call->{ret_members}}) {
if ($multi_ret) {
- if ($ret_member =~ m/^(unsigned )?(char|short|int|hyper)
(\S+)\[\S+\];/) {
- push(@ret_list, "memcpy(result->$3, ret.$3, sizeof
result->$3);");
- } 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 " .
- "procedure $call->{name}: $ret_member";
- } elsif ($ret_member =~ m/^(unsigned )?(char|short|int|hyper)
(\S+);/) {
- push(@ret_list, "result->$3 = ret.$3;");
- } else {
- die "unhandled type for multi-return-value for " .
- "procedure $call->{name}: $ret_member";
- }
+ push(@ret_list, handle_multi_ret_member("result",
"ret", $ret_member, 1));
} elsif ($ret_member =~ m/^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;");
@@ -1038,24 +1151,43 @@ elsif ($opt_k) {
$single_ret_var = "int rv = -1";
$single_ret_type = "int";
- } elsif ($ret_member =~ m/^unsigned hyper (\S+);/) {
- my $arg_name = $1;
+ } elsif ($ret_member =~ m/^(unsigned )?hyper
(\S+);\s*\/\*\s*(u)?(long|longlong)\s*\*\//) {
+ if (($1 and !$3) or (!$1 and $3)) {
+ die "sign mismatch in (unsigned)? hyper annotation:
$ret_member";
+ }
+
+ my $type_name;
+ my $arg_name = $2;
+ my $sign = ""; $sign = "U" if ($3);
+ my $macro;
+
+ $type_name = $1 if ($1);
+ $type_name .= "long";
+
+ # assignment from hyper to long can overflow, check for it
+ if ($4 eq "longlong") {
+ $type_name .= " long";
+ $macro = "HYPER_TO_${sign}LONGLONG";
+ } elsif ($4 eq "long") {
+ $macro = "HYPER_TO_${sign}LONG";
+ } else {
+ die "internal error";
+ }
if ($call->{ProcName} =~ m/Get(Lib)?Version/) {
- push(@args_list, "unsigned long *$arg_name");
- push(@ret_list, "if ($arg_name) *$arg_name =
ret.$arg_name;");
+ push(@args_list, "$type_name *$arg_name");
+ push(@ret_list, "if ($arg_name) $macro(*$arg_name,
ret.$arg_name);");
push(@ret_list, "rv = 0;");
$single_ret_var = "int rv = -1";
$single_ret_type = "int";
- } elsif ($call->{ProcName} eq "NodeGetFreeMemory") {
- push(@ret_list, "rv = ret.$arg_name;");
- $single_ret_var = "unsigned long long rv = 0";
- $single_ret_type = "unsigned long long";
} else {
- push(@ret_list, "rv = ret.$arg_name;");
- $single_ret_var = "unsigned long rv = 0";
- $single_ret_type = "unsigned long";
+ push(@ret_list, "$macro(rv, ret.$arg_name);");
+ $single_ret_var = "$type_name rv = 0";
+ $single_ret_type = "$type_name";
}
+ } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
} elsif ($ret_member =~ m/^(\/)?\*/) {
# ignore comments
} else {
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 3556b85..9f346ee 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -87,6 +87,27 @@
#define VIR_FROM_THIS VIR_FROM_REMOTE
+#define HYPER_TO_TYPE(_type, _to, _from) \
+ do { \
+ if ((_from) != (_type)(_from)) { \
+ remoteError(VIR_ERR_INTERNAL_ERROR, \
+ _("conversion from hyper to %s overflowed"), #_type);
\
+ goto done; \
+ } \
+ (_to) = (_from); \
+ } while (0)
+
+#if SIZEOF_LONG < 8
+# define HYPER_TO_LONG(_to, _from) HYPER_TO_TYPE(long, _to, _from)
+# define HYPER_TO_ULONG(_to, _from) HYPER_TO_TYPE(unsigned long, _to, _from)
+#else
+# define HYPER_TO_LONG(_to, _from) (_to) = (_from)
+# define HYPER_TO_ULONG(_to, _from) (_to) = (_from)
+#endif
+
+#define HYPER_TO_LONGLONG(_to, _from) (_to) = (_from)
+#define HYPER_TO_ULONGLONG(_to, _from) (_to) = (_from)
+
static int inside_daemon = 0;
struct remote_thread_call;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2b9f9de..9928b77 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -371,7 +371,11 @@ struct remote_memory_param {
*
* 'remote_CALL_ret' members that are filled via call-by-reference must be
* annotated with a insert@<offset> comment to indicate the offset in the
- * parameter list of the function to be called. */
+ * parameter list of the function to be called.
+ *
+ * 'remote_CALL_args' and 'remote_CALL_ret' members of type hyper must
be
+ * annotated with the C type they should be mapped to: long, ulong, longlong
+ * or ulonglong */
struct remote_open_args {
/* NB. "name" might be NULL although in practice you can't
@@ -394,11 +398,11 @@ struct remote_get_type_ret {
};
struct remote_get_version_ret {
- unsigned hyper hv_ver;
+ unsigned hyper hv_ver; /* ulong */
};
struct remote_get_lib_version_ret {
- unsigned hyper lib_ver;
+ unsigned hyper lib_ver; /* ulong */
};
struct remote_get_hostname_ret {
@@ -430,7 +434,7 @@ struct remote_get_max_vcpus_ret {
struct remote_node_get_info_ret {
char model[32];
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
int cpus;
int mhz;
int nodes;
@@ -449,11 +453,11 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
+ unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 ulonglong */
};
struct remote_node_get_free_memory_ret {
- unsigned hyper freeMem;
+ unsigned hyper freeMem; /* ulonglong */
};
struct remote_domain_get_scheduler_type_args {
@@ -525,11 +529,11 @@ struct remote_domain_block_stats_args {
};
struct remote_domain_block_stats_ret {
- hyper rd_req;
- hyper rd_bytes;
- hyper wr_req;
- hyper wr_bytes;
- hyper errs;
+ hyper rd_req; /* longlong */
+ hyper rd_bytes; /* longlong */
+ hyper wr_req; /* longlong */
+ hyper wr_bytes; /* longlong */
+ hyper errs; /* longlong */
};
struct remote_domain_interface_stats_args {
@@ -538,14 +542,14 @@ struct remote_domain_interface_stats_args {
};
struct remote_domain_interface_stats_ret {
- hyper rx_bytes;
- hyper rx_packets;
- hyper rx_errs;
- hyper rx_drop;
- hyper tx_bytes;
- hyper tx_packets;
- hyper tx_errs;
- hyper tx_drop;
+ hyper rx_bytes; /* longlong */
+ hyper rx_packets; /* longlong */
+ hyper rx_errs; /* longlong */
+ hyper rx_drop; /* longlong */
+ hyper tx_bytes; /* longlong */
+ hyper tx_packets; /* longlong */
+ hyper tx_errs; /* longlong */
+ hyper tx_drop; /* longlong */
};
struct remote_domain_memory_stats_args {
@@ -556,7 +560,7 @@ struct remote_domain_memory_stats_args {
struct remote_domain_memory_stat {
int tag;
- unsigned hyper val;
+ unsigned hyper val; /* longlong */
};
struct remote_domain_memory_stats_ret {
@@ -593,9 +597,9 @@ struct remote_domain_get_block_info_args {
};
struct remote_domain_get_block_info_ret {
- unsigned hyper allocation;
- unsigned hyper capacity;
- unsigned hyper physical;
+ unsigned hyper allocation; /* ulonglong */
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper physical; /* ulonglong */
};
struct remote_list_domains_args {
@@ -677,22 +681,22 @@ struct remote_domain_get_max_memory_args {
};
struct remote_domain_get_max_memory_ret {
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_max_memory_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_memory_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_memory_flags_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
unsigned int flags;
};
@@ -702,10 +706,10 @@ struct remote_domain_get_info_args {
struct remote_domain_get_info_ret {
unsigned char state;
- unsigned hyper maxMem;
- unsigned hyper memory;
+ unsigned hyper maxMem; /* ulong */
+ unsigned hyper memory; /* ulong */
unsigned short nrVirtCpu;
- unsigned hyper cpuTime;
+ unsigned hyper cpuTime; /* ulonglong */
};
struct remote_domain_save_args {
@@ -758,16 +762,16 @@ struct remote_domain_migrate_perform_args {
remote_nonnull_domain dom;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_finish_args {
remote_nonnull_string dname;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
};
struct remote_domain_migrate_finish_ret {
@@ -776,9 +780,9 @@ struct remote_domain_migrate_finish_ret {
struct remote_domain_migrate_prepare2_args {
remote_string uri_in;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -791,7 +795,7 @@ struct remote_domain_migrate_finish2_args {
remote_nonnull_string dname;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int retcode;
};
@@ -1328,9 +1332,9 @@ struct remote_storage_pool_get_info_args {
struct remote_storage_pool_get_info_ret {
unsigned char state;
- unsigned hyper capacity;
- unsigned hyper allocation;
- unsigned hyper available;
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper allocation; /* ulonglong */
+ unsigned hyper available; /* ulonglong */
};
struct remote_storage_pool_get_autostart_args {
@@ -1438,8 +1442,8 @@ struct remote_storage_vol_get_info_args {
struct remote_storage_vol_get_info_ret {
char type;
- unsigned hyper capacity;
- unsigned hyper allocation;
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper allocation; /* ulonglong */
};
struct remote_storage_vol_get_path_args {
@@ -1649,9 +1653,9 @@ struct remote_secret_lookup_by_usage_ret {
};
struct remote_domain_migrate_prepare_tunnel_args {
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -1756,20 +1760,20 @@ struct remote_domain_get_job_info_args {
struct remote_domain_get_job_info_ret {
int type;
- unsigned hyper timeElapsed;
- unsigned hyper timeRemaining;
+ unsigned hyper timeElapsed; /* ulonglong */
+ unsigned hyper timeRemaining; /* ulonglong */
- unsigned hyper dataTotal;
- unsigned hyper dataProcessed;
- unsigned hyper dataRemaining;
+ unsigned hyper dataTotal; /* ulonglong */
+ unsigned hyper dataProcessed; /* ulonglong */
+ unsigned hyper dataRemaining; /* ulonglong */
- unsigned hyper memTotal;
- unsigned hyper memProcessed;
- unsigned hyper memRemaining;
+ unsigned hyper memTotal; /* ulonglong */
+ unsigned hyper memProcessed; /* ulonglong */
+ unsigned hyper memRemaining; /* ulonglong */
- unsigned hyper fileTotal;
- unsigned hyper fileProcessed;
- unsigned hyper fileRemaining;
+ unsigned hyper fileTotal; /* ulonglong */
+ unsigned hyper fileProcessed; /* ulonglong */
+ unsigned hyper fileRemaining; /* ulonglong */
};
@@ -1780,13 +1784,13 @@ struct remote_domain_abort_job_args {
struct remote_domain_migrate_set_max_downtime_args {
remote_nonnull_domain dom;
- unsigned hyper downtime;
+ unsigned hyper downtime; /* ulonglong */
unsigned int flags;
};
struct remote_domain_migrate_set_max_speed_args {
remote_nonnull_domain dom;
- unsigned hyper bandwidth;
+ unsigned hyper bandwidth; /* ulong */
unsigned int flags;
};
@@ -1952,15 +1956,15 @@ struct remote_domain_open_console_args {
struct remote_storage_vol_upload_args {
remote_nonnull_storage_vol vol;
- unsigned hyper offset;
- unsigned hyper length;
+ unsigned hyper offset; /* ulonglong */
+ unsigned hyper length; /* ulonglong */
unsigned int flags;
};
struct remote_storage_vol_download_args {
remote_nonnull_storage_vol vol;
- unsigned hyper offset;
- unsigned hyper length;
+ unsigned hyper offset; /* ulonglong */
+ unsigned hyper length; /* ulonglong */
unsigned int flags;
};
@@ -1976,9 +1980,9 @@ struct remote_domain_get_state_ret {
struct remote_domain_migrate_begin3_args {
remote_nonnull_domain dom;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_begin3_ret {
@@ -1989,9 +1993,9 @@ struct remote_domain_migrate_begin3_ret {
struct remote_domain_migrate_prepare3_args {
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_string uri_in;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -2002,9 +2006,9 @@ struct remote_domain_migrate_prepare3_ret {
struct remote_domain_migrate_prepare_tunnel3_args {
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -2016,9 +2020,9 @@ struct remote_domain_migrate_perform3_args {
remote_nonnull_domain dom;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_perform3_ret {
@@ -2029,7 +2033,7 @@ struct remote_domain_migrate_finish3_args {
remote_nonnull_string dname;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int cancelled;
};
@@ -2041,7 +2045,7 @@ struct remote_domain_migrate_finish3_ret {
struct remote_domain_migrate_confirm3_args {
remote_nonnull_domain dom;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int cancelled;
};
--
1.7.0.4
11:23 a.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
On Mon, May 23, 2011 at 07:36:10PM +0200, Matthias Bolte wrote:
Remove some special case code that took care of mapping hyper to the
correct C types.
Use macros for hyper to long assignments that perform overflow checks
when long is smaller than hyper. Also use such macros for the safe
hyper to longlong assignemts as this allows to keep the generator a
bit simpler.
+ } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) {
+ # error out on unannotated hypers
+ die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
'hyper' in XDR world is a fixed 64 bit type, so IMHO, this
should automatically map to 'long long' in the API, without
requiring an annotation.
eg, we should only annotate if the public API uses the variable
sized 'long' / 'unsigned long' types in C.
+ } elsif ($ret_member =~ m/<\S+>;/ or $ret_member =~
m/\[\S+\];/) {
+ # just make all other array types fail
+ die "unhandled type for multi-return-value: $ret_member";
+ } elsif ($ret_member =~ m/^(unsigned )?(char|short|int) (\S+);/) {
+ return "$dst->$3 = $src.$3;";
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2b9f9de..9928b77 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -371,7 +371,11 @@ struct remote_memory_param {
*
* 'remote_CALL_ret' members that are filled via call-by-reference must be
* annotated with a insert@<offset> comment to indicate the offset in the
- * parameter list of the function to be called. */
+ * parameter list of the function to be called.
+ *
+ * 'remote_CALL_args' and 'remote_CALL_ret' members of type hyper must
be
+ * annotated with the C type they should be mapped to: long, ulong, longlong
+ * or ulonglong */
struct remote_open_args {
/* NB. "name" might be NULL although in practice you can't
@@ -394,11 +398,11 @@ struct remote_get_type_ret {
};
struct remote_get_version_ret {
- unsigned hyper hv_ver;
+ unsigned hyper hv_ver; /* ulong */
};
struct remote_get_lib_version_ret {
- unsigned hyper lib_ver;
+ unsigned hyper lib_ver; /* ulong */
};
struct remote_get_hostname_ret {
@@ -430,7 +434,7 @@ struct remote_get_max_vcpus_ret {
struct remote_node_get_info_ret {
char model[32];
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
int cpus;
int mhz;
int nodes;
@@ -449,11 +453,11 @@ struct remote_node_get_cells_free_memory_args {
};
struct remote_node_get_cells_free_memory_ret {
- unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 */
+ unsigned hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert@1 ulonglong */
};
struct remote_node_get_free_memory_ret {
- unsigned hyper freeMem;
+ unsigned hyper freeMem; /* ulonglong */
};
struct remote_domain_get_scheduler_type_args {
@@ -525,11 +529,11 @@ struct remote_domain_block_stats_args {
};
struct remote_domain_block_stats_ret {
- hyper rd_req;
- hyper rd_bytes;
- hyper wr_req;
- hyper wr_bytes;
- hyper errs;
+ hyper rd_req; /* longlong */
+ hyper rd_bytes; /* longlong */
+ hyper wr_req; /* longlong */
+ hyper wr_bytes; /* longlong */
+ hyper errs; /* longlong */
};
struct remote_domain_interface_stats_args {
@@ -538,14 +542,14 @@ struct remote_domain_interface_stats_args {
};
struct remote_domain_interface_stats_ret {
- hyper rx_bytes;
- hyper rx_packets;
- hyper rx_errs;
- hyper rx_drop;
- hyper tx_bytes;
- hyper tx_packets;
- hyper tx_errs;
- hyper tx_drop;
+ hyper rx_bytes; /* longlong */
+ hyper rx_packets; /* longlong */
+ hyper rx_errs; /* longlong */
+ hyper rx_drop; /* longlong */
+ hyper tx_bytes; /* longlong */
+ hyper tx_packets; /* longlong */
+ hyper tx_errs; /* longlong */
+ hyper tx_drop; /* longlong */
};
struct remote_domain_memory_stats_args {
@@ -556,7 +560,7 @@ struct remote_domain_memory_stats_args {
struct remote_domain_memory_stat {
int tag;
- unsigned hyper val;
+ unsigned hyper val; /* longlong */
};
struct remote_domain_memory_stats_ret {
@@ -593,9 +597,9 @@ struct remote_domain_get_block_info_args {
};
struct remote_domain_get_block_info_ret {
- unsigned hyper allocation;
- unsigned hyper capacity;
- unsigned hyper physical;
+ unsigned hyper allocation; /* ulonglong */
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper physical; /* ulonglong */
};
struct remote_list_domains_args {
@@ -677,22 +681,22 @@ struct remote_domain_get_max_memory_args {
};
struct remote_domain_get_max_memory_ret {
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_max_memory_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_memory_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
};
struct remote_domain_set_memory_flags_args {
remote_nonnull_domain dom;
- unsigned hyper memory;
+ unsigned hyper memory; /* ulong */
unsigned int flags;
};
@@ -702,10 +706,10 @@ struct remote_domain_get_info_args {
struct remote_domain_get_info_ret {
unsigned char state;
- unsigned hyper maxMem;
- unsigned hyper memory;
+ unsigned hyper maxMem; /* ulong */
+ unsigned hyper memory; /* ulong */
unsigned short nrVirtCpu;
- unsigned hyper cpuTime;
+ unsigned hyper cpuTime; /* ulonglong */
};
struct remote_domain_save_args {
@@ -758,16 +762,16 @@ struct remote_domain_migrate_perform_args {
remote_nonnull_domain dom;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_finish_args {
remote_nonnull_string dname;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
};
struct remote_domain_migrate_finish_ret {
@@ -776,9 +780,9 @@ struct remote_domain_migrate_finish_ret {
struct remote_domain_migrate_prepare2_args {
remote_string uri_in;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -791,7 +795,7 @@ struct remote_domain_migrate_finish2_args {
remote_nonnull_string dname;
opaque cookie<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int retcode;
};
@@ -1328,9 +1332,9 @@ struct remote_storage_pool_get_info_args {
struct remote_storage_pool_get_info_ret {
unsigned char state;
- unsigned hyper capacity;
- unsigned hyper allocation;
- unsigned hyper available;
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper allocation; /* ulonglong */
+ unsigned hyper available; /* ulonglong */
};
struct remote_storage_pool_get_autostart_args {
@@ -1438,8 +1442,8 @@ struct remote_storage_vol_get_info_args {
struct remote_storage_vol_get_info_ret {
char type;
- unsigned hyper capacity;
- unsigned hyper allocation;
+ unsigned hyper capacity; /* ulonglong */
+ unsigned hyper allocation; /* ulonglong */
};
struct remote_storage_vol_get_path_args {
@@ -1649,9 +1653,9 @@ struct remote_secret_lookup_by_usage_ret {
};
struct remote_domain_migrate_prepare_tunnel_args {
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -1756,20 +1760,20 @@ struct remote_domain_get_job_info_args {
struct remote_domain_get_job_info_ret {
int type;
- unsigned hyper timeElapsed;
- unsigned hyper timeRemaining;
+ unsigned hyper timeElapsed; /* ulonglong */
+ unsigned hyper timeRemaining; /* ulonglong */
- unsigned hyper dataTotal;
- unsigned hyper dataProcessed;
- unsigned hyper dataRemaining;
+ unsigned hyper dataTotal; /* ulonglong */
+ unsigned hyper dataProcessed; /* ulonglong */
+ unsigned hyper dataRemaining; /* ulonglong */
- unsigned hyper memTotal;
- unsigned hyper memProcessed;
- unsigned hyper memRemaining;
+ unsigned hyper memTotal; /* ulonglong */
+ unsigned hyper memProcessed; /* ulonglong */
+ unsigned hyper memRemaining; /* ulonglong */
- unsigned hyper fileTotal;
- unsigned hyper fileProcessed;
- unsigned hyper fileRemaining;
+ unsigned hyper fileTotal; /* ulonglong */
+ unsigned hyper fileProcessed; /* ulonglong */
+ unsigned hyper fileRemaining; /* ulonglong */
};
@@ -1780,13 +1784,13 @@ struct remote_domain_abort_job_args {
struct remote_domain_migrate_set_max_downtime_args {
remote_nonnull_domain dom;
- unsigned hyper downtime;
+ unsigned hyper downtime; /* ulonglong */
unsigned int flags;
};
struct remote_domain_migrate_set_max_speed_args {
remote_nonnull_domain dom;
- unsigned hyper bandwidth;
+ unsigned hyper bandwidth; /* ulong */
unsigned int flags;
};
@@ -1952,15 +1956,15 @@ struct remote_domain_open_console_args {
struct remote_storage_vol_upload_args {
remote_nonnull_storage_vol vol;
- unsigned hyper offset;
- unsigned hyper length;
+ unsigned hyper offset; /* ulonglong */
+ unsigned hyper length; /* ulonglong */
unsigned int flags;
};
struct remote_storage_vol_download_args {
remote_nonnull_storage_vol vol;
- unsigned hyper offset;
- unsigned hyper length;
+ unsigned hyper offset; /* ulonglong */
+ unsigned hyper length; /* ulonglong */
unsigned int flags;
};
@@ -1976,9 +1980,9 @@ struct remote_domain_get_state_ret {
struct remote_domain_migrate_begin3_args {
remote_nonnull_domain dom;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_begin3_ret {
@@ -1989,9 +1993,9 @@ struct remote_domain_migrate_begin3_ret {
struct remote_domain_migrate_prepare3_args {
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_string uri_in;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -2002,9 +2006,9 @@ struct remote_domain_migrate_prepare3_ret {
struct remote_domain_migrate_prepare_tunnel3_args {
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
remote_nonnull_string dom_xml;
};
@@ -2016,9 +2020,9 @@ struct remote_domain_migrate_perform3_args {
remote_nonnull_domain dom;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
remote_string dname;
- unsigned hyper resource;
+ unsigned hyper resource; /* ulong */
};
struct remote_domain_migrate_perform3_ret {
@@ -2029,7 +2033,7 @@ struct remote_domain_migrate_finish3_args {
remote_nonnull_string dname;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
remote_nonnull_string uri;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int cancelled;
};
@@ -2041,7 +2045,7 @@ struct remote_domain_migrate_finish3_ret {
struct remote_domain_migrate_confirm3_args {
remote_nonnull_domain dom;
opaque cookie_in<REMOTE_MIGRATE_COOKIE_MAX>;
- unsigned hyper flags;
+ unsigned hyper flags; /* ulong */
int cancelled;
};
All the 'longlong' and 'ulonglong' annotations should
be removed here IMHO, since they should be the default
for hyper/unsigned hyper.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:33 a.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
On 05/25/2011 10:23 AM, Daniel P. Berrange wrote:
On Mon, May 23, 2011 at 07:36:10PM +0200, Matthias Bolte wrote:
> Remove some special case code that took care of mapping hyper to the
> correct C types.
>
> Use macros for hyper to long assignments that perform overflow checks
> when long is smaller than hyper. Also use such macros for the safe
> hyper to longlong assignemts as this allows to keep the generator a
> bit simpler.
> + } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) {
> + # error out on unannotated hypers
> + die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
'hyper' in XDR world is a fixed 64 bit type, so IMHO, this
should automatically map to 'long long' in the API, without
requiring an annotation.
eg, we should only annotate if the public API uses the variable
sized 'long' / 'unsigned long' types in C.
All the 'longlong' and 'ulonglong' annotations
should
be removed here IMHO, since they should be the default
for hyper/unsigned hyper.
If we add a new API that uses 'long' but forget the annotation, then we
have silent type mismatch in the generated code. Since we are using
hyper for both types in C, I find it better to require an annotation on
all uses to be explicit on which type we meant, rather than defaulting
the lack of annotation as 'long long' and only requiring annotation for
'long' - that is, I find that from the maintenance aspect, having the
code generator explicitly fail because you forget an annotation (even if
the annotation is the default of llong) is safer than risking silent
code misgeneration.
But anything is better than nothing, so I won't give any further
complaints if we go with your idea of only annotating the exceptions to
the 'long long' default, rather than all uses of hyper.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4:06 p.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
2011/5/25 Eric Blake <eblake(a)redhat.com>:
On 05/25/2011 10:23 AM, Daniel P. Berrange wrote:
> On Mon, May 23, 2011 at 07:36:10PM +0200, Matthias Bolte wrote:
>> Remove some special case code that took care of mapping hyper to the
>> correct C types.
>>
>> Use macros for hyper to long assignments that perform overflow checks
>> when long is smaller than hyper. Also use such macros for the safe
>> hyper to longlong assignemts as this allows to keep the generator a
>> bit simpler.
>
>> + } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) {
>> + # error out on unannotated hypers
>> + die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
>
> 'hyper' in XDR world is a fixed 64 bit type, so IMHO, this
> should automatically map to 'long long' in the API, without
> requiring an annotation.
>
> eg, we should only annotate if the public API uses the variable
> sized 'long' / 'unsigned long' types in C.
> All the 'longlong' and 'ulonglong' annotations should
> be removed here IMHO, since they should be the default
> for hyper/unsigned hyper.
If we add a new API that uses 'long' but forget the annotation, then we
have silent type mismatch in the generated code. Since we are using
hyper for both types in C, I find it better to require an annotation on
all uses to be explicit on which type we meant, rather than defaulting
the lack of annotation as 'long long' and only requiring annotation for
'long' - that is, I find that from the maintenance aspect, having the
code generator explicitly fail because you forget an annotation (even if
the annotation is the default of llong) is safer than risking silent
code misgeneration.
But anything is better than nothing, so I won't give any further
complaints if we go with your idea of only annotating the exceptions to
the 'long long' default, rather than all uses of hyper.
I think that being more explicit here is better and would like to
stick with annotating all hyper members. This way we cannot overlook a
missing (u)long annotation in a review because the generator will
complain for us.
Matthias
5:12 a.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
On Wed, May 25, 2011 at 10:33:06AM -0600, Eric Blake wrote:
On 05/25/2011 10:23 AM, Daniel P. Berrange wrote:
> On Mon, May 23, 2011 at 07:36:10PM +0200, Matthias Bolte wrote:
>> Remove some special case code that took care of mapping hyper to the
>> correct C types.
>>
>> Use macros for hyper to long assignments that perform overflow checks
>> when long is smaller than hyper. Also use such macros for the safe
>> hyper to longlong assignemts as this allows to keep the generator a
>> bit simpler.
>
>> + } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+)\[\S+\];/) {
>> + # error out on unannotated hypers
>> + die "(unsigned)? hyper without (u)?(long|longlong) annotation:
$ret_member";
>
> 'hyper' in XDR world is a fixed 64 bit type, so IMHO, this
> should automatically map to 'long long' in the API, without
> requiring an annotation.
>
> eg, we should only annotate if the public API uses the variable
> sized 'long' / 'unsigned long' types in C.
> All the 'longlong' and 'ulonglong' annotations should
> be removed here IMHO, since they should be the default
> for hyper/unsigned hyper.
If we add a new API that uses 'long' but forget the annotation, then we
have silent type mismatch in the generated code. Since we are using
hyper for both types in C, I find it better to require an annotation on
all uses to be explicit on which type we meant, rather than defaulting
the lack of annotation as 'long long' and only requiring annotation for
'long' - that is, I find that from the maintenance aspect, having the
code generator explicitly fail because you forget an annotation (even if
the annotation is the default of llong) is safer than risking silent
code misgeneration.
But anything is better than nothing, so I won't give any further
complaints if we go with your idea of only annotating the exceptions to
the 'long long' default, rather than all uses of hyper.
My rationale for defaulting to 'long long' is that we should never
add any more APIs which take a plain 'long' anywhere. It would be
a nice idea if we could get apibuild.py or something to raise an
error if it finds any more APIs using 'long', except for a whitelist
of existing ones
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:39 a.m.
New subject: [libvirt] [PATCH 7/7] remote generator: Annotate hyper with the actual C type
On 05/27/2011 04:12 AM, Daniel P. Berrange wrote:
> But anything is better than nothing, so I won't give any
further
> complaints if we go with your idea of only annotating the exceptions to
> the 'long long' default, rather than all uses of hyper.
My rationale for defaulting to 'long long' is that we should never
add any more APIs which take a plain 'long' anywhere. It would be
a nice idea if we could get apibuild.py or something to raise an
error if it finds any more APIs using 'long', except for a whitelist
of existing ones
Yes, I like that compromise. Default to 'long long', enforce that
default in new code, and whitelist the old exceptions; then we only have
to annotate the exceptions, and don't have to worry about adding
annotations when adding new API.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:26 p.m.
2011/5/23 Matthias Bolte <matthias.bolte(a)googlemail.com>:
This series removes special case code from the generator and moves
it to annotations in the .x files. There are also cleanups of sign
mismatches between the different API layers and additional moves of
functions to generated code.
This series superseds patch 4/5 of the last series [1] and implements
Eric's suggestions.
[1] https://www.redhat.com/archives/libvir-list/2011-May/msg00991.html
configure.ac | 1 +
daemon/remote.c | 289 ++-----------------------
daemon/remote_generator.pl | 473 ++++++++++++++++++++++++++++------------
src/driver.h | 2 +-
src/nwfilter/nwfilter_driver.c | 2 +-
src/remote/remote_driver.c | 268 ++--------------------
src/remote/remote_protocol.x | 247 +++++++++++----------
src/remote_protocol-structs | 40 ++--
8 files changed, 532 insertions(+), 790 deletions(-)
Matthias
I pushed the ACKed patch 1-6 now.
Matthias
4925
days inactive
4933
days old
20 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte