With this change, it would be nice to have a comment that explains that the ""-returning case is not trivial.
Otherwise LGTM. On Wed, Apr 9, 2014 at 9:53 AM, Jose A. Lopes <[email protected]> wrote: > As mentioned, most of the body of the function 'GenerateKvmTapName' is > being extracted to a separate function in the hypervisor base module > so it can be reused by the Xen hypervisor. > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > lib/hypervisor/hv_base.py | 41 > +++++++++++++++++++++++++++++++++++++++ > lib/hypervisor/hv_kvm/__init__.py | 32 ++---------------------------- > 2 files changed, 43 insertions(+), 30 deletions(-) > > diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py > index 70a4a50..a945ef1 100644 > --- a/lib/hypervisor/hv_base.py > +++ b/lib/hypervisor/hv_base.py > @@ -152,6 +152,47 @@ def ParamInSet(required, my_set): > return (required, fn, err, None, None) > > > +def GenerateTapName(): > + """Generate a TAP network interface name for a NIC. > + > + This helper function generates a special TAP network interface > + name for NICs that are meant to be used in instance communication. > + This function checks the existing TAP interfaces in order to find > + a unique name for the new TAP network interface. The TAP network > + interface names are of the form 'gnt.com.%d', where '%d' is a > + unique number within the node. > + > + @rtype: string > + @return: TAP network interface name, or the empty string if the > + NIC is not used in instance communication > + > + """ > + result = utils.RunCmd(["ip", "tuntap", "list"]) > + > + if result.failed: > + raise errors.HypervisorError("Failed to list TUN/TAP interfaces") > + > + idxs = set() > + > + for line in result.output.splitlines(): > + parts = line.split(": ", 1) > + > + if len(parts) < 2: > + raise errors.HypervisorError("Failed to parse TUN/TAP interfaces") > + > + r = re.match(r"gnt\.com\.([0-9]+)", parts[0]) > + > + if r is not None: > + idxs.add(int(r.group(1))) > + > + if idxs: > + idx = max(idxs) + 1 > + else: > + idx = 0 > + > + return "gnt.com.%d" % idx > + > + > class HvInstanceState(object): > RUNNING = 0 > SHUTDOWN = 1 > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > index 2e1dd47..9cbf7e8 100644 > --- a/lib/hypervisor/hv_kvm/__init__.py > +++ b/lib/hypervisor/hv_kvm/__init__.py > @@ -1465,12 +1465,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > def _GenerateKvmTapName(nic): > """Generate a TAP network interface name for a NIC. > > - This helper function generates a special TAP network interface > - name for NICs that are meant to be used in instance communication. > - This function checks the existing TAP interfaces in order to find > - a unique name for the new TAP network interface. The TAP network > - interface names are of the form 'gnt.com.%d', where '%d' is a > - unique number within the node. > + See L{hv_base.GenerateTapName}. > > @type nic: ganeti.objects.NIC > @param nic: NIC object for the name should be generated > @@ -1484,30 +1479,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > nic.name.startswith(constants.INSTANCE_COMMUNICATION_NIC_PREFIX): > return "" > > - result = utils.RunCmd(["ip", "tuntap", "list"]) > - > - if result.failed: > - raise errors.HypervisorError("Failed to list TUN/TAP interfaces") > - > - idxs = set() > - > - for line in result.output.splitlines(): > - parts = line.split(": ", 1) > - > - if len(parts) < 2: > - raise errors.HypervisorError("Failed to parse TUN/TAP interfaces") > - > - r = re.match(r"gnt\.com\.([0-9]+)", parts[0]) > - > - if r is not None: > - idxs.add(int(r.group(1))) > - > - if idxs: > - idx = max(idxs) + 1 > - else: > - idx = 0 > - > - return "gnt.com.%d" % idx > + return hv_base.GenerateTapName() > > # too many local variables > # pylint: disable=R0914 > -- > 1.9.1.423.g4596e3a > >
