Re: [libvirt] [PATCHv2 2/5] remote: protocol implementation for virDomainCreateWithFlags
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)