Re: [libvirt] [PATCHv3 3/5] remote: Implement bulk domain stats APIs in the remote driver

On 08/27/2014 12:25 PM, Peter Krempa wrote: > Implement the remote driver support for shuffling the domain stats > around. > --- > +static int > +remoteDispatchConnectGetAllDomainStats(virNetServerPtr server ATTRIBUTE_UNUSED, > + virNetServerClientPtr client, > + virNetMessagePtr msg ATTRIBUTE_UNUSED, > + virNetMessageErrorPtr rerr, > + remote_connect_get_all_domain_stats_args *args, > + remote_connect_get_all_domain_stats_ret *ret) > +{ > + int rv = -1; > + if (nrecords) { > + ret->retStats.retStats_len = nrecords; > + > + if (VIR_ALLOC_N(ret->retStats.retStats_val, nrecords) < 0) > + goto cleanup; > + I'd switch these for safety (I don't know if the rpc cleanup code will try to dereference a NULL retStats.retStats_val if retStats_len is non-zero; while setting the length after the allocation should always be safe). > + for (i = 0; i < nrecords; i++) { > + remote_domain_stats_record *dst = ret->retStats.retStats_val + i; > + > + make_nonnull_domain(&dst->dom, retStats[i]->dom); [Not your fault, and certainly not for this patch, but we REALLY ought to fix make_nonnull_domain and friends to properly return errors on OOM, instead of silently breaking things] > +++ b/src/remote/remote_driver.c > @@ -7717,6 +7717,89 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net, > } > > > +static int > +remoteConnectGetAllDomainStats(virConnectPtr conn, > + virDomainPtr *doms, > + unsigned int ndoms, > + unsigned int stats, > + virDomainStatsRecordPtr **retStats, > + unsigned int flags) > +{ > + struct private_data *priv = conn->networkPrivateData; > + int rv = -1; > + size_t i; > + remote_connect_get_all_domain_stats_args args; > + remote_connect_get_all_domain_stats_ret ret; > + > + virDomainStatsRecordPtr *tmpret = NULL; > + > + if (ndoms) { > + if (VIR_ALLOC_N(args.doms.doms_val, ndoms) < 0) > + goto cleanup; > + > + for (i = 0; i < ndoms; i++) > + make_nonnull_domain(args.doms.doms_val + i, doms[i]); [same complaint here about silent OOM bugs] ACK with the statement reorder.