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

Re: [libvirt] [PATCHv2 3/9] remote: implement remote protocol for virConnectListAllDomains()



On 06/05/2012 07:19 AM, Peter Krempa wrote:
> This patch wires up the RPC protocol handlers for
> virConnectListAllDomains(). The RPC generator has no support for the way
> how virConnectListAllDomains() returns the results so the handler code
> had to be done manually.
> 
> The new api is handled by REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS, with
> number 271 and marked with high priority.
> ---
> Diff to v1:
> Add the NULL element at the end of the list.

I missed this in my earlier review:


> +++ b/daemon/remote.c
> @@ -996,6 +996,58 @@ no_memory:
>  }
> 
>  static int
> +remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED,

> +    } else {
> +        ret->domains.domains_len = 0;
> +        ret->domains.domains_val = NULL;
> +    }
> +
> +    rv = ndomains;

Here, you want:

ret->ret = ndomains;
rv = 0;

> +++ b/src/remote/remote_driver.c
> @@ -1265,6 +1265,68 @@ done:
>      return rv;
>  }
> 
> +static int
> +remoteConnectListAllDomains(virConnectPtr conn,

> +    if (call (conn,
> +              priv,
> +              0,
> +              REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS,
> +              (xdrproc_t) xdr_remote_connect_list_all_domains_args,
> +              (char *) &args,
> +              (xdrproc_t) xdr_remote_connect_list_all_domains_ret,
> +              (char *) &ret) == -1)
> +        goto done;
> +
> +    if (domains) {
> +        if (VIR_ALLOC_N(doms, ret.domains.domains_len + 1) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        for (i = 0; i < ret.domains.domains_len; i++) {
> +            doms[i] = get_nonnull_domain(conn, ret.domains.domains_val[i]);
> +            if (!doms[i]) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +        }
> +        *domains = doms;
> +        doms = NULL;
> +    }
> +
> +    rv = ret.domains.domains_len;

and here, you want:

rv = ret.ret;

> +++ b/src/remote/remote_protocol.x
> @@ -2463,6 +2463,16 @@ struct remote_domain_get_disk_errors_ret {
>      int nerrors;
>  };
> 
> +struct remote_connect_list_all_domains_args {
> +    bool need_results;
> +    unsigned int flags;
> +};
> +
> +struct remote_connect_list_all_domains_ret {
> +    remote_nonnull_domain domains<>;
> +    unsigned int ret;

That's because the daemon/remote.c doesn't do any special treatment for
positive return values; to specifically pass a positive count, you have
to pass it through the remote_*_ret.ret member.  If need_results was 0
in _args, then ret.domains.domains_len will be 0 in reply, even though
ret should be non-zero for the count of domains.

-- 
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]