Bug 1462838 - virt-install does not find distribution files when using --location with an FTP URL
virt-install does not find distribution files when using --location with an F...
Status: CLOSED UPSTREAM
Product: Virtualization Tools
Classification: Community
Component: virt-manager (Show other bugs)
unspecified
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Cole Robinson
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-19 12:38 EDT by Kevin Castner
Modified: 2017-08-17 16:20 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-08-17 16:20:27 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
virt-install debug output (11.16 KB, text/plain)
2017-06-19 12:38 EDT, Kevin Castner
no flags Details

  None (edit)
Description Kevin Castner 2017-06-19 12:38:06 EDT
Created attachment 1289152 [details]
virt-install debug output

Description of problem:
When running virt-install with a --location URL using ftp virt-install an error stating the location must be the root of an install tree is returned


Version-Release number of selected component (if applicable):
virt-manager-common.noarch 1.4.0-2

How reproducible:
Run virt-install with --location=ftp://

Steps to Reproduce:
1. virt-install -n test-dc-02 -r 2048 --vcpus=2 --nographics --disk pool=sstor1_vmds01_dc,size=8 --network network=virbr1 --location=ftp://repo/centos/7.3.1611/os/x86_64/ --os-type linux --extra-args=console=ttyS0 --debug --os-variant centos7.0


Actual results:
At approximately line 65 of the output is shown that virt-install searches for 
URL + images/pxeboot/vmlinuz and does not find it.  This file exists on the FTP server.

Expected results:

Additional info:
I've validated the correct setup of the ftp server by using a wget command as follows:

[root@c1-n1 ~]# wget ftp://repo/centos/7.3.1611/os/x86_64/images/pxeboot/vmlinuz
--2017-06-19 12:27:58--  ftp://repo/centos/7.3.1611/os/x86_64/images/pxeboot/vmlinuz
           => ‘vmlinuz’
Resolving repo (repo)... 192.168.1.72
Connecting to repo (repo)|192.168.1.72|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD (1) /centos/7.3.1611/os/x86_64/images/pxeboot ... done.
==> SIZE vmlinuz ... 5392080
==> PASV ... done.    ==> RETR vmlinuz ... done.
Length: 5392080 (5.1M) (unauthoritative)

100%[=======================================================================================================================================================================>] 5,392,080   --.-K/s   in 0.06s

2017-06-19 12:27:58 (82.8 MB/s) - ‘vmlinuz’ saved [5392080]
Comment 1 Cole Robinson 2017-06-20 16:13:31 EDT
Seems to work with for example ftp://mirrors.ocf.berkeley.edu/centos/7.3.1611/os/x86_64/ . Does that URL work for you?

There is a newer virt-manager release. Please try git code:

git clone git://github.com/virt-manager/virt-manager
cd virt-manager
./virt-install ...
Comment 2 Kevin Castner 2017-07-11 11:57:00 EDT
I have not yet tried the newer virt-manager you mentioned but I will do so and report back.  

However, using the same version as previously, the URL ftp://mirrors.ocf.berkeley.edu/centos/7.3.1611/os/x86_64 does work.

The ftp site that is not working is a local mirror so it would appear that the issue is there.  I cannot see any difference in the files being served by FTP between the two sites.  The local mirror also works fine for yum updates and delivering the kickstart file.  

Specifically the local ftp server has .treeinfo and it is identical to .treeinfo on the Berkeley FTP server.  

Is there a more verbose debug level that can be enabled in virt-install?

Thank you,
Kevin
Comment 3 Cole Robinson 2017-07-12 15:50:45 EDT
No, there isn't any higher debug level than --debug. You can try poking in the code in virtinst/urlfetcher.py, but better to do it from the git directory. Maybe it's some configuration setting of the FTP server
Comment 4 Kevin Castner 2017-07-14 21:34:02 EDT
I traced the issue to the function _FTPURLFetcher._grabber()

When attempting to get the size of the .treeinfo file from the FTP server (proftpd) the server is returning a "550 SIZE not allowed in ASCII mode."

As the lower function is wrapped by _URLFetcher._grabURL and inside a try I would have expected that the exception be raised saying the file could not be acquired but that is not the behavior I'm seeing.  It appears that the call

"size = self._ftp.size(urlparse.urlparse(url)[2])"

does not set the value of size and does not raise any exception that is caught.  

Doing some googling on the FTP error and there are good reasons why SIZE should not be allowed on ASCII files.  However the ftpd configuration is valid and virt-install should probably be detecting this failure mode and giving a more accurate error since the file is available.

From RFC3659 (Extensions to FTP) Section 4.2 "The presence of the 550 error response to a SIZE command MUST NOT be taken by the client as an indication that the file cannot be transferred in the current MODE and TYPE. A server may generate this error for other reasons -- for instance if the processing overhead is considered too great."

A possibly simple workaround is for virt-install to request .treeinfo as a binary file assuming that the different line end conventions of the files will not prevent .treeinfo from being processed.  Since the proftpd is local to my environment I was able to changed the "DefaultTransferMode" to binary and virt-install is working as expected.  It doesn't appear the line end conventions are a problem.

I appreciate your assistance with this issue.

Kevin
Comment 5 Cole Robinson 2017-07-15 12:58:21 EDT
Cool thanks for digging into this. So your server is defaulting to ascii mode which explains it. Does this patch fix things for you?

iff --git a/virtinst/urlfetcher.py b/virtinst/urlfetcher.py
index 5e10bb52..152b076a 100644
--- a/virtinst/urlfetcher.py
+++ b/virtinst/urlfetcher.py
@@ -224,6 +224,8 @@ class _FTPURLFetcher(_URLFetcher):
             self._ftp = ftplib.FTP()
             self._ftp.connect(parsed.hostname, parsed.port)
             self._ftp.login()
+            # Force binary mode
+            self._ftp.voidcmd("TYPE I")
         except Exception as e:
             raise ValueError(_("Opening URL %s failed: %s.") %
                               (self.location, str(e)))
Comment 6 Cole Robinson 2017-08-07 18:37:12 EDT
Kevin does that patch fix things for you? I'd like confirmation before I push it to virt-manager git
Comment 7 Kevin Castner 2017-08-17 15:57:38 EDT
I was finally able to test this and it works as advertised.

Very sorry for the long delay in responding and thank you so much for the help.

Kevin
Comment 8 Cole Robinson 2017-08-17 16:20:27 EDT
Thanks for confirming, that change is pushed upstream now

commit e902fa5550cc09538429b0fa828fb31aa2706d01 (HEAD -> master)
Author: Cole Robinson <crobinso@redhat.com>
Date:   Thu Aug 17 16:10:46 2017 -0400

    urlfetcher: force binary mode with FTP servers (bz #1462838)

Note You need to log in before you can comment on or make changes to this bug.