On 09/30/2014 08:20 PM, Tomoki Sekiyama wrote:
Add daemon and driver code to (de-)serialize virDomainFSInfo.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama(a)hds.com>
---
daemon/remote.c | 117 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 87 +++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 32 +++++++++++
src/remote_protocol-structs | 21 ++++++++
src/rpc/gendispatch.pl | 1
5 files changed, 257 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index c93c97a..d775434 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -6340,6 +6340,123 @@ remoteDispatchNodeAllocPages(virNetServerPtr server
ATTRIBUTE_UNUSED,
}
+static int
+remoteDispatchDomainGetFSInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_fsinfo_args *args,
+ remote_domain_get_fsinfo_ret *ret)
+{
+ int rv = -1;
+ size_t i, j;
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+ virDomainFSInfoPtr *info = NULL;
+ virDomainPtr dom = NULL;
+ remote_domain_fsinfo *dst;
+ char **alias;
+ int ninfo = 0, ndisk;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+ goto cleanup;
+
+ if ((ninfo = virDomainGetFSInfo(dom, &info, args->flags)) < 0)
+ goto cleanup;
+
+ if (ninfo > REMOTE_DOMAIN_FSINFO_MAX) {
+ virReportError(VIR_ERR_RPC,
+ _("Too many mountpoints in fsinfo: %d for limit %d"),
+ ninfo, REMOTE_DOMAIN_FSINFO_MAX);
+ goto cleanup;
+ }
+
+ if (info && ninfo) {
Because you compare 'info' to NULL here, Coverity gets grumpy later on [1]
+ if (VIR_ALLOC_N(ret->info.info_val, ninfo) < 0)
+ goto cleanup;
+
+ ret->info.info_len = ninfo;
+
+ for (i = 0; i < ninfo; i++) {
+ dst = &ret->info.info_val[i];
+ if (VIR_STRDUP(dst->mountpoint, info[i]->mountpoint) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(dst->name, info[i]->name) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(dst->type, info[i]->type) < 0)
+ goto cleanup;
+
+ ndisk = 0;
+ for (alias = info[i]->devAlias; alias && *alias; alias++)
+ ndisk++;
+
+ if (ndisk > REMOTE_DOMAIN_FSINFO_DISKS_MAX) {
+ virReportError(VIR_ERR_RPC,
+ _("Too many disks in fsinfo: %d for limit
%d"),
+ ndisk, REMOTE_DOMAIN_FSINFO_DISKS_MAX);
+ goto cleanup;
+ }
+
+ if (ndisk > 0) {
+ if (VIR_ALLOC_N(dst->dev_aliases.dev_aliases_val, ndisk) < 0)
+ goto cleanup;
+
+ for (j = 0; j < ndisk; j++) {
+ if (VIR_STRDUP(dst->dev_aliases.dev_aliases_val[j],
+ info[i]->devAlias[j]) < 0)
+ goto cleanup;
+ }
+
+ dst->dev_aliases.dev_aliases_len = ndisk;
+ } else {
+ dst->dev_aliases.dev_aliases_val = NULL;
+ dst->dev_aliases.dev_aliases_len = 0;
+ }
+ }
+
+ } else {
+ ret->info.info_len = 0;
+ ret->info.info_val = NULL;
Because we won't get here if virDomainGetFSInfo() fails, then the loop
below [2] will cause a problem...
The good news is if you move this to above the virDomainGetFSInfo()
call, then you avoid the Coverity complaint.
+ }
+
+ ret->ret = ninfo;
+
+ rv = 0;
+
+ cleanup:
+ if (rv < 0) {
+ virNetMessageSaveError(rerr);
+
+ if (ret->info.info_val) {
+ for (i = 0; i < ninfo; i++) {
[2]
Coverity complains when virDomainGetFSInfo() fails leaving ninfo == -1
+ dst = &ret->info.info_val[i];
+ VIR_FREE(dst->mountpoint);
+ if (dst->dev_aliases.dev_aliases_val) {
+ for (j = 0; j < dst->dev_aliases.dev_aliases_len; j++)
+ VIR_FREE(dst->dev_aliases.dev_aliases_val[j]);
+ VIR_FREE(dst->dev_aliases.dev_aliases_val);
+ }
+ }
+ VIR_FREE(ret->info.info_val);
+ }
+ }
+ if (dom)
+ virDomainFree(dom);
+ if (ninfo >= 0)
You'll need the { at the end (clarity) and because of [1], make it
if (ninfo >= 0 && info) {
+ for (i = 0; i < ninfo; i++)
[1]
And this is where Coverity gets grumpy indicating 'info' could be NULL
and this would be a NULL deref. Yes, a false positive when you know the
code, but nonetheless
+ virDomainFSInfoFree(info[i]);
and the corresponding }
+ VIR_FREE(info);
+
+ return rv;
+}
+
+
/*----- Helpers. -----*/
/* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 6c49e49..aa1866a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7897,6 +7897,92 @@ remoteNodeAllocPages(virConnectPtr conn,
}
+static int
+remoteDomainGetFSInfo(virDomainPtr dom,
+ virDomainFSInfoPtr **info,
+ unsigned int flags)
+{
+ int rv = -1;
+ size_t i, j, len;
+ struct private_data *priv = dom->conn->networkPrivateData;
+ remote_domain_get_fsinfo_args args;
+ remote_domain_get_fsinfo_ret ret;
+ remote_domain_fsinfo *src;
+ virDomainFSInfoPtr *info_ret = NULL;
+
+ remoteDriverLock(priv);
+
+ make_nonnull_domain(&args.dom, dom);
+
+ args.flags = flags;
+
+ memset(&ret, 0, sizeof(ret));
+
+ if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_FSINFO,
+ (xdrproc_t)xdr_remote_domain_get_fsinfo_args, (char *)&args,
+ (xdrproc_t)xdr_remote_domain_get_fsinfo_ret, (char *)&ret) == -1)
+ goto done;
+
+ if (ret.info.info_len > REMOTE_DOMAIN_FSINFO_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Too many mountpoints in fsinfo: %d for limit %d"),
+ ret.info.info_len, REMOTE_DOMAIN_FSINFO_MAX);
+ goto cleanup;
+ }
+
+ if (info) {
+ if (ret.info.info_len &&
+ VIR_ALLOC_N(info_ret, ret.info.info_len + 1) < 0)
I see no reason for the + 1 since you're returning a count.
+ goto cleanup;
+
+ for (i = 0; i < ret.info.info_len; i++) {
+ src = &ret.info.info_val[i];
+
+ if (VIR_ALLOC(info_ret[i]) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(info_ret[i]->mountpoint, src->mountpoint) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(info_ret[i]->name, src->name) < 0)
+ goto cleanup;
+
+ if (VIR_STRDUP(info_ret[i]->type, src->type) < 0)
+ goto cleanup;
+
+ len = src->dev_aliases.dev_aliases_len;
+ if (len &&
+ VIR_ALLOC_N(info_ret[i]->devAlias, len + 1) < 0)
+ goto cleanup;
+
+ for (j = 0; j < len; j++) {
+ if (VIR_STRDUP(info_ret[i]->devAlias[j],
+ src->dev_aliases.dev_aliases_val[j]) < 0)
+ goto cleanup;
+ }
+ }
+
+ *info = info_ret;
+ info_ret = NULL;
+ }
+
+ rv = ret.ret;
+
+ cleanup:
+ if (info_ret) {
+ for (i = 0; i < ret.info.info_len; i++)
+ virDomainFSInfoFree(info_ret[i]);
+ VIR_FREE(info_ret);
+ }
+ xdr_free((xdrproc_t)xdr_remote_domain_get_fsinfo_ret,
+ (char *) &ret);
+
+ done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
+
/* get_nonnull_domain and get_nonnull_network turn an on-wire
* (name, uuid) pair into virDomainPtr or virNetworkPtr object.
* These can return NULL if underlying memory allocations fail,
@@ -8239,6 +8325,7 @@ static virDriver remote_driver = {
.connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */
.connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */
.nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */
+ .domainGetFSInfo = remoteDomainGetFSInfo, /* 1.2.10 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index db12cda..bf261e1 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -250,6 +250,12 @@ const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = 4096;
/* Upper limit of message size for tunable event. */
const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 2048;
+/* Upper limit on number of mountpoints in fsinfo */
+const REMOTE_DOMAIN_FSINFO_MAX = 256;
+
+/* Upper limit on number of disks per mountpoint in fsinfo */
+const REMOTE_DOMAIN_FSINFO_DISKS_MAX = 256;
+
/* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -3111,6 +3117,24 @@ struct remote_connect_get_all_domain_stats_args {
struct remote_connect_get_all_domain_stats_ret {
remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>;
};
+
+struct remote_domain_fsinfo {
+ remote_nonnull_string mountpoint;
+ remote_nonnull_string name;
+ remote_nonnull_string type;
+ remote_nonnull_string dev_aliases<REMOTE_DOMAIN_FSINFO_DISKS_MAX>; /* (const
char **) */
+};
+
+struct remote_domain_get_fsinfo_args {
+ remote_nonnull_domain dom;
+ unsigned int flags;
+};
+
+struct remote_domain_get_fsinfo_ret {
+ remote_domain_fsinfo info<REMOTE_DOMAIN_FSINFO_MAX>;
+ unsigned int ret;
+};
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -5505,5 +5529,11 @@ enum remote_procedure {
* @generate: none
* @acl: connect:write
*/
- REMOTE_PROC_NODE_ALLOC_PAGES = 347
+ REMOTE_PROC_NODE_ALLOC_PAGES = 347,
+
+ /**
+ * @generate: none
+ * @acl: domain:read
+ */
+ REMOTE_PROC_DOMAIN_GET_FSINFO = 348
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 362baf9..e002b2a 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2579,6 +2579,26 @@ struct remote_connect_get_all_domain_stats_ret {
remote_domain_stats_record * retStats_val;
} retStats;
};
+struct remote_domain_fsinfo {
+ remote_nonnull_string mountpoint;
+ remote_nonnull_string name;
+ remote_nonnull_string type;
+ struct {
+ u_int dev_aliases_len;
+ remote_nonnull_string * dev_aliases_val;
+ } dev_aliases;
+};
+struct remote_domain_get_fsinfo_args {
+ remote_nonnull_domain dom;
+ u_int flags;
+};
+struct remote_domain_get_fsinfo_ret {
+ struct {
+ u_int info_len;
+ remote_domain_fsinfo * info_val;
+ } leases;
+ u_int ret;
+};
make check fails...
s/leases/info
resolves it.
John
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -2927,4 +2947,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_BLOCK_COPY = 345,
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346,
REMOTE_PROC_NODE_ALLOC_PAGES = 347,
+ REMOTE_PROC_DOMAIN_GET_FSINFO = 348,
};
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 27093d2..d2cfffb 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -66,6 +66,7 @@ sub fixup_name {
$name =~ s/Fstrim$/FSTrim/;
$name =~ s/Fsfreeze$/FSFreeze/;
$name =~ s/Fsthaw$/FSThaw/;
+ $name =~ s/Fsinfo$/FSInfo/;
$name =~ s/Scsi/SCSI/;
$name =~ s/Wwn$/WWN/;
$name =~ s/Dhcp$/DHCP/;
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list