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

Re: [libvirt] [RFC] Power Hypervisor Libvirt support



Em Qua, 2009-05-06 às 09:26 +0200, Daniel Veillard escreveu:
> 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 :-)

Hello DV,

Nice! Hope you enjoyed your vacations :)

> > > > +    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 ?

In fact I am part of the libssh team, I help them with testing. And yes,
we're planning to release a new version of libssh this week with all
these fixes. But I still need to make it sure in configure.in. I'll do
it right now.

> 
> > > > +
> > > > +/* 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.

All right, got it. Thanks for the tips :)

[]'s

-- 
Eduardo Otubo
Software Engineer
Linux Technology Center
IBM Systems & Technology Group
Mobile: +55 19 8135 0885 
otubo linux vnet ibm com


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