On 13:48 Thu 16 Dec     , Guido Trotter wrote:
> On Mon, Dec 13, 2010 at 3:22 PM, Apollon Oikonomopoulos
> > Minor modifications are made to _ExecKVMRuntime to handle tap device
> > initialization. NIC <-> tap associations are stored under a new directory,
> > _ROOT_DIR/nic in a file-per-nic fashion.
> >
> > The script generated by _WriteNetScript must be moved to live under
> > _KVM_NET_SCRIPT as the default network configuration script.
> >
> 
> Currently _KVM_NET_SCRIPT is an optional override for the net script.
> You make it something the user must provide without documenting the
> requirement or giving an example.
> Either we ship/install an example, or even better we should provide a
> script in /usr/lib/ganeti/ and make it optionally overridable (or make
> it possible to optionally override parts of it) from /etc.
> Requiring the user to do more work for the "simple" setup is not a
> good idea. Also the way to override it should change from
> all/or/nothing to hooks and various parts. :)

Fixed to keep the previous semantics: we ship the script previously 
written out by WriteNetScript as /usr/lib/ganeti/kvm-ifup and allow 
overriding it using _KVM_NET_SCRIPT as before. We will convert to hooks 
at a later stage probably (together with the introduction of "down" 
actions?).

Updated patches will follow.

> > +  def _InstanceNICFile(cls, instance_name, seq):
> > +    """Returns the name of the file containing the tap device for a given 
> > NIC
> > +
> > +    """
> > +    return utils.PathJoin(cls._NICS_DIR, "%s-%d" % (instance_name, seq))
> > +
> > + �...@classmethod
> > +  def _InstanceNICFiles(cls, instance_name):
> > +    """Returns all existing NIC filenames
> > +
> > +    """
> > +    return glob.glob(utils.PathJoin(cls._NICS_DIR, "%s-[0-9]" % 
> > instance_name))
> > +
> 
> Ouch, these two functions differ by a single "s" at the end? Not good,
> we'll end up confusing them.
> _AllInstanceNICFiles(...) or _InstanceAllNICFiles(....) vs
> _InstanceNICFile(...) would be better.
Ack.

Thanks,
Apollon

Reply via email to