[virt-tools-list] [PATCH] fix: do cleanupLocation only at last time for cached fetcher

[Chun Yan Liu]

>>> On 9/11/2015 at 11:22 PM, in message <55F2F19A.30302 at redhat.com>, Cole Robinson
<crobinso at redhat.com> wrote: 
> On 09/11/2015 01:12 AM, Chun Yan Liu wrote: 
> >  
> >  
> >>>> On 9/9/2015 at 08:07 PM, in message <55F020E4.40206 at redhat.com>, Cole  
> Robinson 
> > <crobinso at redhat.com> wrote:  
> >> On 09/09/2015 01:56 AM, Chunyan Liu wrote:  
> >>> Regression introduced after commit d8d6af5. Failed to install guests  
> >>> through ftp network source, see below:  
> >>>   
> >>> /usr/bin/virt-install   --name sled-12-fcs-64-pv-nnm-nete5250fdfecc94f922b5a21a  
> >>>  -p --location ftp://xen100.virt.lab.novell.com/install/sled12/x86_64  
> >>>  --extra-args "autoyast=http://192.168.123.1/install/autoyast/  
> >>> sled-12-fcs-64-pv-nnm-nete5250fdfecc94f922b5a21a"   --disk path=/var/lib/  
> >>> libvirt/images/sled-12-fcs-64-pv-nnm-nete5250fdfecc94f922b5a21a.qcow2,size=10,  
> >>> format=qcow2   --network=bridge=br0   --ram=1024   --vcpu=2   --vnc  
> >>>  --serial pty   --noautoconsole   --video cirrus  
> >>>   
> >>> ERROR    'NoneType' object has no attribute 'sendall'  
> >>> Domain installation does not appear to have been successful.  
> >>> If it was, you can restart your domain by running:  
> >>>   virsh --connect xen:/// start sled-12-fcs-64-pv-nnm-nete5250fdfecc94f922b5a21a  
> >>> otherwise, please restart your installation.  
> >>>   
> >>> Root cause is: now we use cached fetcher, but still do cleanupLocation  
> >>> each time, it will cause problem when using the fetcher next time.  
> >>> We should only do cleanupLocation at last time.  
> >>>   
> >>> Signed-off-by: Chunyan Liu <cyliu at suse.com>  
> >>> ---  
> >>>  virtinst/distroinstaller.py | 7 +++++--  
> >>>  1 file changed, 5 insertions(+), 2 deletions(-)  
> >>>   
> >>   
> >> I can't reproduce the issue, and in fact there isn't any call to 'sendall'   
>  
> >  
> > 'sendall' is a function in FTPLib. It's called from ftp.quit in  
> FTPImageFetcher. 
> > This problem is only happens with 'ftp'. No problem if using 'http'. 
> >  
>  
> Yeah I tried with an ftp link too but couldn't reproduce (I can't use the  
> link 
> you provided because it's not a public URL). I still can't see in the code  
> how 

Interesting. I did use virt-manager.git, and I tried another ftp link too,
also has this issue. 

> cleanupLocation() can be called without a matching prepareLocation so not  
> sure 
> how this problem comes up in upstream code. 

Yes, prepareLocation and cleanupLocation appear in pair. That's the problem.
Now preparelocation will use cached ftp directly, from the debugging, after
the 1st cleanupLocation, ftp.sock becomes None, so when 2nd time use that
ftp, will cross problem as reported.

-Chunyan
 
>  
> Can you reproduce with virt-manager.git and not a system installed version? 
>  From the git checkout, do ./virt-install $args 



>  
> - Cole 
>  
>  
>