On Sun, Apr 04, 2010 at 11:24:56AM +0200, Daniel Veillard wrote:
On Fri, Apr 02, 2010 at 05:29:58PM -0400, Chris Lalancette wrote:
On 04/02/2010 04:56 PM, Daniel Veillard wrote:
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f86c7f1..0374f9a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1657,6 +1657,10 @@ struct remote_domain_has_managed_save_image_args { unsigned flags; };
+struct remote_domain_has_managed_save_image_ret { + int ret; +}; +
Hm, I don't think this is necessary. I think the return value is always going to be an int, so you should just be able to return -1, 0, or 1 in the remote driver as necessary.
My initial reaction was the same, then I looked at GetMaxVcpus and other examples and converted the code accordingly.
At least, that's how all of the other things that return numbers (such as virDomainNumDefinedDomains) work.
In the cases I checked and looked for it seems the network call() return values is always 0 or -1, and looking at virDomainGetMaxVcpus() it does use
struct remote_domain_get_max_vcpus_ret { int num; };
same for virDomainNumDomains()
and I also see
struct remote_num_of_defined_domains_ret { int num; };
in the src/remote/remote_protocol.x right now, remoteNumOfDefinedDomains( does use remote_num_of_defined_domains_ret ret; and remoteDispatchNumOfDefinedDomains() do use a remote_num_of_defined_domains_ret *ret argument, so I'm wondering if we are really looking at the same code.
In the case we just return 0 for success and -1 in case of error, we clearly don't need the return structure, but all examples I checked for an full int reurn used a structure. So I assume the change is needed, or at least it's safe :-)
That is correct. The success vs fail error code in the wire protocol is a simple enum enum remote_message_status { /* Status is always REMOTE_OK for calls. * For replies, indicates no error. */ REMOTE_OK = 0, /* For replies, indicates that an error happened, and a struct * remote_error follows. */ REMOTE_ERROR = 1, /* For streams, indicates that more data is still expected */ REMOTE_CONTINUE = 2 }; Thus, it can't cope with any data that needs to be returned. So in this case, you need the 'int ret' return value for the positive case - this is very like the virDomainIsPersistent() method. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|