[libvirt] [PATCH 2/7] Add virDomainBlockPull support to the remote driver. The generator can handle DomainBlockPullAll and DomainBlockPullAbort. DomainBlockPull and DomainBlockPullInfo must be written by hand.

[Adam Litke]

On 06/01/2011 12:27 PM, Matthias Bolte wrote:
> 2011/6/1 Adam Litke <agl at us.ibm.com>:
>> * src/remote/remote_protocol.x: provide defines for the new entry points
>> * src/remote/remote_driver.c daemon/remote.c: implement the client and
>>  server side
>> * daemon/remote_generator.pl: Specify the manually-written functions
>>
>> Signed-off-by: Adam Litke <agl at us.ibm.com>
>> ---
> 
> +static int remoteDomainBlockPull(virDomainPtr domain,
> +                                 const char *path,
> +                                 virDomainBlockPullInfoPtr info,
> +                                 unsigned int flags)
> +{
> +    int rv = -1;
> +    remote_domain_block_pull_args args;
> +    remote_domain_block_pull_ret ret;
> +    struct private_data *priv = domain->conn->privateData;
> +
> +    remoteDriverLock(priv);
> +
> +    make_nonnull_domain(&args.dom, domain);
> +    args.path = (char *)path;
> +    args.flags = flags;
> +
> +    if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL,
> +             (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args,
> +             (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1)
> +        goto done;
> +
> +    if (info) {
> +        info->cur = ret.info.cur;
> +        info->end = ret.info.end;
> +    }
> +    rv = 0;
> +
> +done:
> +    remoteDriverUnlock(priv);
> +    return rv;
> +}
> 
> From the generator point-of-view I would want to avoid having the info
> parameter being NULLable, as this differs from the common pattern and
> results in special case code in the generator.

>From an API user's point of view, I think it's nice to allow NULL for
the info struct.  The user may not care about the current progress
information.  I could fix this at the global libvirt level by allocating
a dummy struct to pass down to the driver implementations if the user
passed NULL.  Would that be acceptable?

>> +struct remote_domain_block_pull_info {
>> +    unsigned hyper cur;
>> +    unsigned hyper end;
>> +};
> 
>> +struct remote_domain_block_pull_ret {
>> +    remote_domain_block_pull_info info;
>> +};
> 
>> +struct remote_domain_get_block_pull_info_ret {
>> +    remote_domain_block_pull_info info;
>> +};
> 
> From the generator point-of-view I would avoid this approach of
> putting a struct in a struct because that differs from the common
> approach and results in special case code in the generator. It should
> look like this
> 
> struct remote_domain_block_pull_ret {
>     unsigned hyper cur;
>     unsigned hyper end;
> };
> 
> struct remote_domain_get_block_pull_info_ret {
>     unsigned hyper cur;
>     unsigned hyper end;
> };

Ok, I will make this change.

-- 
Adam Litke
IBM Linux Technology Center