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

Eric Blake eblake at redhat.com
Wed Aug 27 21:02:57 UTC 2014


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140827/a927df9a/attachment-0001.sig>


More information about the libvir-list mailing list