Re: [Qemu-devel] [PATCH 3/3] block/gluster: add support for multiple gluster servers

[Prasanna Kumar Kalever <pkalever@redhat.com>]

On Monday, November 9, 2015 12:34:45 PM, Peter Krempa wrote:
> On Thu, Nov 05, 2015 at 07:45:50 -0500, Prasanna Kumar Kalever wrote:
> > On Thursday, November 5, 2015 6:07:06 PM, Prasanna Kumar Kalever wrote:
> > > This patch adds a way to specify multiple volfile servers to the gluster
> > > block backend of QEMU with tcp|rdma transport types and their port
> > > numbers.
> > > 
> > > Problem:
> > > 
> > > Currently VM Image on gluster volume is specified like this:
> > 
> 
> [...]
> 
> > > @@ -345,7 +676,7 @@ static int qemu_gluster_open(BlockDriverState *bs,
> > > QDict
> > > *options,
> > >  
> > >  out:
> > >      qemu_opts_del(opts);
> > > -    qemu_gluster_gconf_free(gconf);
> > > +    qapi_free_BlockdevOptionsGluster(gconf);
> > 
> > Can some one help me please ?
> > This leads to crash in the second iteration i.e. while freeing
> > "gconf->servers->next->value"
> 
> So, prior to this you allocate a array of the data structures as:
> 
> +    gsconf = g_new0(GlusterServer, num_servers);
> +
> +    ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME);
> +    if (!ptr) {
> +        error_setg(&local_err, "Error: qemu_gluster: please provide 'volume'
> "
> +                               "option");
> +        goto out;
> +    }
> 
> Then you use the following code to fill the linked list:
> 
> +      if (gconf->servers == NULL) {
> +            gconf->servers = g_new0(GlusterServerList, 1);
> +            gconf->servers->value = &gsconf[i];
> 
> So here you set the value. For a i of 0 the '&gsconf[i]' expression will
> be a pointer with equal address to 'gsconf'. For explanation:
> 
> 'gsconf[i]' can be written as '*(gsconf + i)', so
> '&gsconf[i]' becomes basically '&(*(gsconf + i))'
> 
> This can be also simplified to:
> 'gsconf + i'. For a i of 0 this becomes the same pointer as 'gsconf'
> 
> And once you use that with free(), the whole gsconf array will be freed.
> All the other pointers that you've filled to the linked list become
> invalid, since they were pointing into the same array that was
> completely freed in the first iteration.

Thanks for the help Peter, It solves the problem.

-Prasanna 
> 
> Peter
>