Re: [libvirt] [PATCH 3/5] remote: implement remote protocol for virConnectListAllDomains()
On 05/22/2012 01:30 AM, Eric Blake wrote:
On 05/20/2012 09:56 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.
> ---
> daemon/remote.c | 50 +++++++++++++++++++++++++++++++
> src/remote/remote_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++
> src/remote/remote_protocol.x | 14 ++++++++-
> src/remote_protocol-structs | 12 +++++++
> 4 files changed, 141 insertions(+), 1 deletions(-)
>
> +
> +struct remote_connect_list_all_domains_ret {
> + remote_nonnull_domain domains<>;
This is an unbounded array; we aren't using any of these anywhere else.
I wonder if reusing REMOTE_DOMAIN_ID_LIST_MAX would be reasonable
instead. Then again, we are returning more information per domain: a
name, uuid, and id, so maybe REMOTE_DOMAIN_NAME_LIST_MAX is the best we
can do (currently 1024, but then again there is the pending patch to
raise that limit). The name element may make us run into RPC limits
even if we don't otherwise enforce a limit.
I chose the unbounded array intentionaly as I don't see any point in
applying two limits on the size of the RPC message. I agree that the
global message size limit is good for avoiding DoS attacks. On the other
side, I don't find limiting the maximum count any useful (as you stated,
there's the domain name that is variable length) and we still have to
scale the other limits when we rise the global one.
I'll add the limit to conform with the rest of the code and hopefuly
nobody will need more than 1024 machines soon.
Peter