I think is better to squash the two patches together but leave
the vif-ganeti script out for now. And then another patch can exctract
common code out of kvm-ifup and add vif-ganeti to the tree.
I'll send the first one right away and the second as soon as
possible.

Thanks,
dimara

* Guido Trotter <[email protected]> [2013-05-17 14:58:42 +0100]:

> On Fri, May 17, 2013 at 2:23 PM, Dimitris Aragiorgis <[email protected]>wrote:
> 
> > For every NIC write down a file that contains NIC's
> > info (MAC, mode, link, network details), that can
> > be sourced by any vif script.  The file location is:
> > /var/run/ganeti/xen-hypervisor/nic/<domname>/<nicidx>.
> >
> > This file is created upon cfg file creation and before
> > starting the instance.
> >
> > Additionally a custom vif script is provided, vif-ganeti
> > that is stored in /usr/lib/ganeti/vif-ganeti. This
> > script is executed instead of the one defined in
> > xend-config.sxp. Include this script in build process.
> > TODO: debian/links should include
> > a symlink of that file within /etc/xen/scripts/.
> >
> > Cleanup NIC dir when removing config files. Upon Xen
> > configuration file removal, remove NIC dir as well.
> >
> > Signed-off-by: Dimitris Aragiorgis <[email protected]>
> > ---
> >  Makefile.am              |    7 +++++
> >  lib/hypervisor/hv_xen.py |   74
> > ++++++++++++++++++++++++++++++++++++++++++++--
> >  tools/vif-ganeti.in      |   47 +++++++++++++++++++++++++++++
> >  3 files changed, 126 insertions(+), 2 deletions(-)
> >  create mode 100755 tools/vif-ganeti.in
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 037cf53..4e46d8f 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -192,6 +192,7 @@ CLEANFILES = \
> >         $(man_MANS) \
> >         $(manhtml) \
> >         tools/kvm-ifup \
> > +       tools/vif-ganeti \
> >         tools/users-setup \
> >         tools/vcluster-setup \
> >         stamp-directories \
> > @@ -904,6 +905,7 @@ nodist_pkglib_python_scripts = \
> >  myexeclib_SCRIPTS = \
> >         daemons/daemon-util \
> >         tools/kvm-ifup \
> > +       tools/vif-ganeti \
> >         $(pkglib_python_scripts) \
> >         $(HS_MYEXECLIB_PROGS)
> >
> > @@ -939,6 +941,7 @@ EXTRA_DIST = \
> >         devel/upload \
> >         devel/webserver \
> >         tools/kvm-ifup.in \
> > +       tools/vif-ganeti.in \
> >         tools/users-setup.in \
> >         tools/vcluster-setup.in \
> >         $(docinput) \
> > @@ -1337,6 +1340,10 @@ tools/kvm-ifup: tools/kvm-ifup.in$(REPLACE_VARS_SED)
> >         sed -f $(REPLACE_VARS_SED) < $< > $@
> >         chmod +x $@
> >
> > +tools/vif-ganeti: tools/vif-ganeti.in $(REPLACE_VARS_SED)
> > +       sed -f $(REPLACE_VARS_SED) < $< > $@
> > +       chmod +x $@
> > +
> >  tools/users-setup: tools/users-setup.in $(REPLACE_VARS_SED)
> >         sed -f $(REPLACE_VARS_SED) < $< > $@
> >         chmod +x $@
> > diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> > index 653f6b2..9c0c86e 100644
> > --- a/lib/hypervisor/hv_xen.py
> > +++ b/lib/hypervisor/hv_xen.py
> > @@ -25,6 +25,7 @@
> >
> >  import logging
> >  import string # pylint: disable=W0402
> > +import shutil
> >  from cStringIO import StringIO
> >
> >  from ganeti import constants
> > @@ -41,6 +42,8 @@ XEND_CONFIG_FILE =
> > utils.PathJoin(pathutils.XEN_CONFIG_DIR, "xend-config.sxp")
> >  XL_CONFIG_FILE = utils.PathJoin(pathutils.XEN_CONFIG_DIR, "xen/xl.conf")
> >  VIF_BRIDGE_SCRIPT = utils.PathJoin(pathutils.XEN_CONFIG_DIR,
> >                                     "scripts/vif-bridge")
> > +VIF_GANETI_SCRIPT = utils.PathJoin(pathutils.XEN_CONFIG_DIR,
> > +                                   "scripts/vif-ganeti")
> >  _DOM0_NAME = "Domain-0"
> >  _DISK_LETTERS = string.ascii_lowercase
> >
> > @@ -310,6 +313,8 @@ class XenHypervisor(hv_base.BaseHypervisor):
> >    CAN_MIGRATE = True
> >    REBOOT_RETRY_COUNT = 60
> >    REBOOT_RETRY_INTERVAL = 10
> > +  _ROOT_DIR = pathutils.RUN_DIR + "/xen-hypervisor"
> > +  _NICS_DIR = _ROOT_DIR + "/nic" # contains instances nic <-> tap
> > associations
> >
> >    ANCILLARY_FILES = [
> >      XEND_CONFIG_FILE,
> > @@ -373,12 +378,68 @@ class XenHypervisor(hv_base.BaseHypervisor):
> >      return utils.PathJoin(self._cfgdir, instance_name)
> >
> >    @classmethod
> > +  def _WriteNICInfoFile(cls, instance_name, idx, nic):
> > +    """Write the Xen config file for the instance.
> > +
> > +    This version of the function just writes the config file from static
> > data.
> > +
> > +    """
> > +    utils.EnsureDirs([(cls._InstanceNICDir(instance_name),
> > +                     constants.RUN_DIRS_MODE)])
> > +    cfg_file = cls._InstanceNICFile(instance_name, idx)
> > +    data = StringIO()
> > +
> > +    if nic.netinfo:
> > +      netinfo = objects.Network.FromDict(nic.netinfo)
> > +      data.write("NETWORK_NAME=%s\n" % netinfo.name)
> > +      if netinfo.network:
> > +        data.write("NETWORK_SUBNET=%s\n" % netinfo.network)
> > +      if netinfo.gateway:
> > +        data.write("NETWORK_GATEWAY=%s\n" % netinfo.gateway)
> > +      if netinfo.network6:
> > +        data.write("NETWORK_SUBNET6=%s\n" % netinfo.network6)
> > +      if netinfo.gateway6:
> > +        data.write("NETWORK_GATEWAY6=%s\n" % netinfo.gateway6)
> > +      if netinfo.mac_prefix:
> > +        data.write("NETWORK_MAC_PREFIX=%s\n" % netinfo.mac_prefix)
> > +      if netinfo.tags:
> > +        data.write("NETWORK_TAGS=%s\n" % "\ ".join(netinfo.tags))
> > +
> > +    data.write("MAC=%s\n" % nic.mac)
> > +    data.write("IP=%s\n" % nic.ip)
> > +    data.write("MODE=%s\n" % nic.nicparams[constants.NIC_MODE])
> > +    data.write("LINK=%s\n" % nic.nicparams[constants.NIC_LINK])
> > +
> > +    try:
> > +      utils.WriteFile(cfg_file, data=data.getvalue())
> > +    except EnvironmentError, err:
> > +      raise errors.HypervisorError("Cannot write Xen instance
> > configuration"
> > +                                   " file %s: %s" % (cfg_file, err))
> > +
> > +  @classmethod
> > +  def _InstanceNICDir(cls, instance_name):
> > +    """Returns the name of the directory holding the tap device files for
> > a
> > +    given instance.
> > +
> > +    """
> > +    return utils.PathJoin(cls._NICS_DIR, instance_name)
> > +
> > +  @classmethod
> > +  def _InstanceNICFile(cls, instance_name, seq):
> > +    """Returns the name of the file containing the tap device for a given
> > NIC
> > +
> > +    """
> > +    return utils.PathJoin(cls._InstanceNICDir(instance_name), str(seq))
> > +
> > +
> > +  @classmethod
> >    def _GetConfig(cls, instance, startup_memory, block_devices):
> >      """Build Xen configuration for an instance.
> >
> >      """
> >      raise NotImplementedError
> >
> > +
> >
> 
> Nack to the extra empty line.
> 
> 
> >    def _WriteConfigFile(self, instance_name, data):
> >      """Write the Xen config file for the instance.
> >
> > @@ -413,6 +474,11 @@ class XenHypervisor(hv_base.BaseHypervisor):
> >
> >      """
> >      utils.RemoveFile(self._ConfigFileName(instance_name))
> > +    try:
> > +      shutil.rmtree(self._InstanceNICDir(instance_name))
> > +    except OSError, err:
> > +      if err.errno != errno.ENOENT:
> > +        raise
> >
> >
> Ack, but we should probably stash this file away for debugging in case of
> failing to start an instance (see balazs' super-recent patch).
> 
> 
> >    def _GetXmList(self, include_node):
> >      """Wrapper around module level L{_GetXmList}.
> > @@ -840,14 +906,16 @@ class XenPvmHypervisor(XenHypervisor):
> >      config.write("name = '%s'\n" % instance.name)
> >
> >      vif_data = []
> > -    for nic in instance.nics:
> > +    for idx, nic in enumerate(instance.nics):
> >        nic_str = "mac=%s" % (nic.mac)
> >        ip = getattr(nic, "ip", None)
> >        if ip is not None:
> >          nic_str += ", ip=%s" % ip
> >        if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
> >          nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK]
> > +      nic_str += ", script=%s" % VIF_GANETI_SCRIPT
> >        vif_data.append("'%s'" % nic_str)
> > +      cls._WriteNICInfoFile(instance.name, idx, nic)
> >
> >      disk_data = \
> >        _GetConfigFileDiskData(block_devices,
> > hvp[constants.HV_BLOCKDEV_PREFIX])
> > @@ -999,14 +1067,16 @@ class XenHvmHypervisor(XenHypervisor):
> >        # parameter 'model' is only valid with type 'ioemu'
> >        nic_type_str = ", model=%s, type=%s" % \
> >          (nic_type, constants.HT_HVM_VIF_IOEMU)
> > -    for nic in instance.nics:
> > +    for idx, nic in enumerate(instance.nics):
> >        nic_str = "mac=%s%s" % (nic.mac, nic_type_str)
> >        ip = getattr(nic, "ip", None)
> >        if ip is not None:
> >          nic_str += ", ip=%s" % ip
> >        if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
> >          nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK]
> > +      nic_str += ", script=%s" % VIF_GANETI_SCRIPT
> >        vif_data.append("'%s'" % nic_str)
> > +      cls._WriteNICInfoFile(instance.name, idx, nic)
> >
> >      config.write("vif = [%s]\n" % ",".join(vif_data))
> >
> > diff --git a/tools/vif-ganeti.in b/tools/vif-ganeti.in
> > new file mode 100755
> > index 0000000..a8eebea
> > --- /dev/null
> > +++ b/tools/vif-ganeti.in
> > @@ -0,0 +1,47 @@
> > +#!/bin/bash
> > +
> > +if [ -x "@SYSCONFDIR@/xen/scripts/vif-custom" ]; then
> > +       exec @SYSCONFDIR@/xen/scripts/vif-custom $*
> > +fi
> > +
> > +dir=$(dirname "$0")
> > +. "$dir"/vif-common.sh
> > +
> > +# taken from older vif-common.sh
> > +dev=$vif
> > +dev_=${dev#vif}
> > +domid=${dev_%.*}
> > +devid=${dev_#*.}
> > +domname=$(xm domname $domid)
> > +
> > +GANETI_NIC_DIR=/var/run/ganeti/xen-hypervisor/nic
> > +INTERFACE=$dev
> > +INSTANCE=$domname
> > +
> > +source $GANETI_NIC_DIR/$domname/$devid
> > +
> > +function setup_routed_mode {
> > +
> > +  ;;
> > +
> > +}
> > +
> > +
> > +function setup_bridged_mode {
> > +
> > +  ;;
> > +
> > +}
> > +
> > +ip link set $INTERFACE up
> > +
> > +if [ "$MODE" == "routed" ]; then
> > +  setup_routed_mode
> > +elif [ "$MODE" == "bridged" ]; then
> > +  BRIDGE=$(xenstore_read_default "$XENBUS_PATH/bridge" "$LINK")
> > +  brctl addif $BRIDGE $INTERFACE
> > +  setup_bridged_mode
> > +fi
> > +
> > +success
> >
> 
> Couldn't we share some of this between the xen and kvm script?
> Also, could we not support openvswitch (the same way as we do in the kvm
> one).
> 
> Thanks,
> 
> Guido

Attachment: signature.asc
Description: Digital signature

Reply via email to