[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]