[Ovirt-devel] [PATCH server] Add the installer files as a subpackage of the server package

[David Lutterkort]

On Fri, 2009-01-23 at 15:19 -0500, Joey Boggs wrote:
> Some comments/questions inline
> 
> Bryan Kearney wrote:
> > I will push the change below. Joey... can you pick up these comments 
> > from David?
> >
> > -- bk
> >
> > David Lutterkort wrote:
> >> On Tue, 2009-01-20 at 16:49 -0500, Bryan Kearney wrote:
> >>> 
> selinuxdisabled wasn't around in F9 :) makes it easier now in F10

Huh ? It's on my RHEL5 machine (and it's called selinuxenabled)

> >>> +if dns_servers == "y"
> >>> +    host_lookup = Socket.getaddrinfo(ipa_host,nil)
> >>> +    hostip = host_lookup[1][3]
> >>> +    if hostip.to_s != mgmt_ipaddr.to_s
> >>> +        @cli.say("Reverse dns lookup for #{ipa_host} failed, exiting")
> >>
> >> That's a forward DNS lookup you're doing - but you should also check
> >> that looking up mgmt_ipaddr gets you ipa_host.
> I can switch this around to do reverse, just didnt catch it when I wrote 
> it. Or should both foward and reverse be there?

Yeah, I'd do both forward and reverse lookup: forward to guard against
typos, reverse to guard against broken DNS setup.

> >> Strictly speaking, this doesn't have to be on a /24 network; maybe just
> >> ask for full IP addresses ?
> >>
> start/stop would be easy to calculate but wouldn't we need to determine 
> the subnets as well or are we assuming 1 subnet?

Not sure I understand - what I was getting at was that you asked for the
first three octets for the network, and then start and stop as the
fourth octet.

There's no reason to assume that the netmask is always 255.255.255.0 -
it could be smaller or larger; in all generality, you'd want to ask for
the network in a format like '172.31.0.0/27' (or for network address and
mask, '172.31.0.0' and '255.255.255.224' in this example) and full IP
addresses for the start and stop address.

> >>> +    dhcp_domain = prompt_for_answer("Enter the dhcp domain you wish 
> >>> to use (example: example.com):", :regex => IP_OR_FQDN)
> >>
> >> Default to dnsdomainname ? (and use that for other places where we ask
> >> for a domain)
> I don't think we can trust dnsdomainname that's why it's explicity asked 
> for and hardcoded during the installation?

I only meant to use it as a reasonable default when prompting the user -
the user definitely needs a way to adjust it during installation.

> >>> +# Generate the file and output it.
> >>> +FileUtils.mkdir_p("/usr/share/ace/appliances/ovirt")
> >>> +config_file = File.new("/usr/share/ace/appliances/ovirt/ovirt.pp", 
> >>> "w")
> >>> +config_file.write(ERB.new(template, 0, "%>").result)
> >>> +config_file.close()
> >>
> >> Why is the file written to /usr/share ? It should go into /var/lib, and
> >> ideally would be configurable (so that I can run the installer as an
> >> ordinary user)
> >>
> >>
> The ace installer is looking for it in that spot, would a symlink from 
> /usr/share/ace/appliances/ovirt/ovirt.pp to var/lib/ovirt/ovirt.pp 
> suffice in this case?

No, a symlink wouldn't serve any purpose. It's simply bad practice to
write these things into /usr/share. But if ACE looks for it there and
nowhere else, it's ok to leave it there (it's unlikely that anybody
would run the installer with a readonly /usr/share)

David