[libvirt] [PATCH] Fix virsh {net,pool}-edit

[Daniel P. Berrange]

On Fri, Feb 13, 2009 at 04:58:17PM +0100, Jim Meyering wrote:
> Cole Robinson <crobinso at redhat.com> wrote:
> > The dumpxml commands for networks and pools don't support any flag
> > arguments, and in fact explictly fail if flags != 0. This is not the
> > case for vm dumpxml though, and flags were added to the base 'edit'
> > implementation in virsh recently.
> >
> > The net and pool derivatives were not addressing this difference, so any
> > net-edit or pool-edit attempt currently gives an error like:
> >
> > Network error : invalid argument in virNetworkGetXMLDesc
> >
> > The attached patch is one way to fix this. Thanks to Charles Duffy for
> > the report.
> 
> Hi Cole,
> 
> That looks good to me.
> I confirmed it fixes the bug when using the qemu-based driver:
> 
> cat <<EOF > net.xml
> <network>
>   <name>N</name>
>   <ip address="192.168.199.1" netmask="255.255.255.0"></ip>
> </network>
> EOF
> printf '#!/bin/sh\nperl -pi -e "s/199/19/" "$@"\n' > ed; chmod +x ed
> qemud/libvirtd &
> EDITOR=$PWD/ed src/virsh 'net-define net.xml; net-edit N'
> 
> However, using the test driver, my little test exposed a bug:
> 
>   EDITOR=$PWD/ed src/virsh -c test:///default 'net-define net.xml; net-edit N'
>   zsh: segmentation fault
> 
> That was because the "def = NULL" assignment (to make it so
> the upcoming "free" is a no-op) is too early, since there are
> uses of def right after that.
> 
> And the same bug appears in 3 other nearby functions.
> Here's the fix I'll commit soon, along with tests based on the above.

This is not safe in general, because after the 'AssignDef' call, the
'def' is now owned by the 'obj'. If you don't set it to NULL immediately
you have the risk of later error cleanup paths, seeing the non-NULL def 
and free'ing it when they shouldn't.

The virGetNetwork() calls should be changed to call 'obj->def->name'
instead of just 'def->name'.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|