[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] [2/4] Implement remote protocol for managed save



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]