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

Re: [libvirt] 1.2.0 segfault on Centos 6



On Wed, Feb 05, 2014 at 07:02:38 -0500, John Ferlan wrote:
> 
> 
> On 02/04/2014 05:23 PM, Jiri Denemark wrote:
> > On Tue, Feb 04, 2014 at 17:02:41 +0100, Franky Van Liedekerke wrote:
> >> Hi,
> >>
> >> using libvirt 1.2.0 on a up-to-date Centos6.5 machine leads to 
> >> occasional segmentation faults (see below).
> >> Sometimes it runs for 5 minutes, sometimes for an hour, but after that 
> >> the result is always the same: segfault after some weird qom-list, that 
> >> apparently the qemu version on centos doesn't know. Has 1.2.1 a known 
> >> fix for this?
> > 
> > I believe the following patch should fix the crash. I'll do some testing
> > tomorrow and send it as a proper patch afterwards:
> > 
> > diff --git i/src/qemu/qemu_monitor.c w/src/qemu/qemu_monitor.c
> > index a968901..cdd817f 100644
> > --- i/src/qemu/qemu_monitor.c
> > +++ w/src/qemu/qemu_monitor.c
> > @@ -1019,7 +1019,9 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> >                                   virDomainObjPtr vm,
> >                                   const char *curpath)
> >  {
> > -    size_t i, j, npaths = 0, nprops = 0;
> > +    size_t i, j;
> > +    int npaths = 0;
> > +    int nprops = 0;
> 
> Reading into what the contention of the issue is, then these are perhaps
> all that's needed since the conflict is I assume the difference in size
> between 'int' and 'size_t'. Perhaps something Eric Blake understands
> best. My simple testing shows while difference in size (4 bytes vs. 8
> bytes), the compiler seems to have done the right thing on the return
> value (eg, assigning 'size_t' value to a function returning int of -1).
> 
> I know when I implemented this
> 
>  http://www.redhat.com/archives/libvir-list/2013-July/msg00770.html
> 
> that I had tested without qom-list available.
> 
> Curious to know if the issue was seen in a provided or built libvirt.
> It should be very simple to create a small C program that exhibits the
> issue - just have a variable of size_t initialized to 0 that gets set by
> a function that returns int, then print the result.
> 
> 
> >      int ret = 0;
> >      char *nextpath = NULL;
> >      qemuMonitorJSONListPathPtr *paths = NULL;
> > @@ -1045,6 +1047,8 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> >      VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
> >  
> >      npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
> > +    if (npaths < 0)
> > +        return -1;
> >  
> >      for (i = 0; i < npaths && ret == 0; i++) {
> 
> We wouldn't enter the loop in the 0 < -1 case, but would if 0 < 0x0000ffff

Sure, we wouldn't but then we would just return 0 from this function,
which would be wrong.
> 
> >  
> > @@ -1061,6 +1065,11 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> >               * then this version of qemu/kvm does not support the feature.
> >               */
> >              nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops);
> > +            if (nprops < 0) {
> > +                ret = -1;
> > +                goto cleanup;
> > +            }
> > +
> >              for (j = 0; j < nprops; j++) {
> 
> same here.

And here we would overwrite the error from
qemuMonitorJSONGetObjectListPaths with a different one.

Jirka


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