[libvirt] [RFC] Power Hypervisor Libvirt support

[Daniel Veillard]

On Mon, May 04, 2009 at 05:50:03PM -0300, Eduardo Otubo wrote:
> Hello DV, 

  Hi Eduardo,

sorry for the delay I took a few days of vacations :-)

> I agree with your suggestions and everything is fixed now. But I would
> like to discuss some points.

 good !

> > 
> > > +    while (channel && channel_is_open(channel)) {
> > > +        ret = channel_read(channel, readbuf, 0, 0);
> > > +        if (ret < 0) {
> > > +            (*exit_status) = SSH_CHANNEL_READ_ERR;
> > > +            break;
> > > +        }
> > > +
> > > +        if (ret == 0) {
> > > +            channel_send_eof(channel);
> > > +            while(channel_get_exit_status(channel) == -1){
> > > +                channel_poll(channel,0);
> > > +                usleep(50);
> > > +            }
> > 
> >    It's always a bit nasty to sleep like that. Is there really now way
> > to use something like poll or select instead ? In average you're gonna
> > be waken up multiple time while waiting for your answer while the kernel
> > could instead wake you up exactly when you have the data.
> 
> This was a libssh issue This was the only way I could make it work. But
> now the libssh is fixed and this function is a little different -
> http://pastebin.com/fbf84426

  Looks fine now, but how can you make sure libssh used will have the
fix, is there a new version released and the associated test in
configure.in ?

> > > +
> > > +/* return the lpar_id given a name and a managed system name */
> > > +static int
> > > +phypGetLparID(SSH_SESSION * ssh_session, const char *managed_system,
> > > +              const char *name)
> > > +{
> > > +    int exit_status = 0;
> > > +    virBuffer cmd = VIR_BUFFER_INITIALIZER;
> > > +
> > > +    virBufferVSprintf(&cmd,
> > > +                      "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> > > +                      managed_system, name);
> > > +    const char *tex_ret =
> > > +        __inner_exec_command(ssh_session, virBufferContentAndReset(&cmd),
> > > +                             &exit_status);
> > > +
> > > +    virBufferContentAndReset(&cmd);
> > 
> >    Huh ? you're supposed to get the resulting char *, and then free it
> >    later once you're done with the data. Here youre just leaking memory
> >    I'm afraid
> > 
> >  same thing for most of the commands in that file.
> 
> Here, I just would like to free the Buffer, and this was the best way I
> find since I couldn't find any better function to manipulate this. How
> do I simply free a buffer using the internal virBuffer* API?

  Well you didn't allocate the buffer structure since it's on the heap
and initilized with the VIR_BUFFER_INITIALIZER macro. So technically
you don't need to free it, but you need to free the string in the
buffer. That string is returned by virBufferContentAndReset(), so you
need to save that to a local variable and free it once done.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/