[libvirt] [PATCHv2 2/5] remote: protocol implementation for virDomainCreateWithFlags

[Eric Blake]

On 06/11/2010 07:33 AM, Daniel P. Berrange wrote:
> On Thu, Jun 10, 2010 at 12:21:20PM -0600, Eric Blake wrote:
>> Define the wire format for the new virDomainCreateWithFlags
>> API, and implement client and server side of marshaling code.
>>
>> * daemon/remote.c (remoteDispatchDomainCreateWithFlags): Add
>> server side dispatch for virDomainCreateWithFlags.
>> * src/remote/remote_driver.c (remoteDomainCreateWithFlags)
>> (remote_driver): Client side serialization.
>> * src/remote/remote_protocol.x
>> (remote_domain_create_with_flags_args)
>> (remote_domain_create_with_flags_ret)
>> (REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS): Define wire format.
>> * daemon/remote_dispatch_args.h: Regenerate.
>> * daemon/remote_dispatch_prototypes.h: Likewise.
>> * daemon/remote_dispatch_table.h: Likewise.
>> * src/remote/remote_protocol.c: Likewise.
>> * src/remote/remote_protocol.h: Likewise.
>> * src/remote_protocol-structs: Likewise.
>> ---
>>
>> diff from v1: use right type in .x, then rerun 'make rpcgen'.

diff from v2:

diff --git i/daemon/remote.c w/daemon/remote.c
index 88a5494..1e33066 100644
--- i/daemon/remote.c
+++ w/daemon/remote.c
@@ -1234,7 +1234,8 @@ remoteDispatchDomainCreateWithFlags (struct
qemud_server *server ATTRIBUTE_UNUSE
         remoteDispatchConnError(rerr, conn);
         return -1;
     }
-    ret->dom.id = dom->id;
+
+    make_nonnull_domain (&ret->dom, dom);
     virDomainFree(dom);
     return 0;
 }
diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs
index 2e67931..43af89e 100644
--- i/src/remote_protocol-structs
+++ w/src/remote_protocol-structs
@@ -408,7 +408,7 @@ struct remote_domain_create_args {
 };
 struct remote_domain_create_with_flags_args {
 	remote_nonnull_domain      dom;
-	int                        flags;
+	u_int                      flags;
 };
 struct remote_domain_create_with_flags_ret {
 	remote_nonnull_domain      dom;


>> +    if (virDomainCreateWithFlags (dom, args->flags) == -1) {
>> +        virDomainFree(dom);
>> +        remoteDispatchConnError(rerr, conn);
>> +        return -1;
>> +    }
>> +    ret->dom.id = dom->id;
> 
> Although its only the 'id' value the client cares about, since we
> have a full 'remote_domain' object on the wire, we should initialize
> all the fields just in case we need it in the future. So just call
> into  make_nonnull_domain() instead of setting dom.id directly.

Done.

>> +++ b/src/remote_protocol-structs
>> @@ -406,6 +406,13 @@ struct remote_num_of_defined_domains_ret {
>>  struct remote_domain_create_args {
>>  	remote_nonnull_domain      dom;
>>  };
>> +struct remote_domain_create_with_flags_args {
>> +	remote_nonnull_domain      dom;
>> +	int                        flags;
>> +};
> 
> I think this needs updating to 'unsigned' to match the changed .x file
> too

Yep, make check eventually called me on it - it stemmed from me not
re-running 'make check' after 'make rpcgen' prior to posting the series.
 I'm wondering if we should make the 'make rpcgen' target smart enough
to also update remote_protocol-structs, thus making this one of the
generated files, or whether keeping the manual step in the loop is nice
for forcing the developer to remember that the patch is intentionally
adjusting the wire protocol.

Given your ACKs on the rest of the series, and the small extent of the
v3 diff listed above to address your comments, I have pushed the series.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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