Change in vdsm[master]: Signed-off-by: Dan Yasny <[e-mail]>

Dan Yasny has uploaded a new change for review.

Change subject: Signed-off-by: Dan Yasny 
......................................................................

Signed-off-by: Dan Yasny

Cisco VM-FEX support vdsm hooks

Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Signed-off-by: Dan Yasny 
---
A vdsm_hooks/vmfex/Makefile.am
A vdsm_hooks/vmfex/README
A vdsm_hooks/vmfex/before_vm_migrate_destination.py
A vdsm_hooks/vmfex/before_vm_start.py
4 files changed, 394 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/7547/1

diff --git a/vdsm_hooks/vmfex/Makefile.am b/vdsm_hooks/vmfex/Makefile.am
new file mode 100644
index 0000000..b921275
--- /dev/null
+++ b/vdsm_hooks/vmfex/Makefile.am
@@ -0,0 +1,36 @@
+#
+# Copyright 2011 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+EXTRA_DIST = \
+	before_vm_migrate_destination.py \
+	before_vm_start.py \
+
+install-data-local: 
+	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_start
+	$(INSTALL_SCRIPT) $(srcdir)/before_vm_start.py \
+		$(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_vmfex
+	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination
+	$(INSTALL_SCRIPT) $(srcdir)/before_vm_migrate_destination.py \
+		$(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination/50_vmfex
+
+uninstall-local: uninstall-data-sudoers
+	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_vmfex
+	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination/50_vmfex
+
diff --git a/vdsm_hooks/vmfex/README b/vdsm_hooks/vmfex/README
new file mode 100644
index 0000000..1eb7629
--- /dev/null
+++ b/vdsm_hooks/vmfex/README
@@ -0,0 +1,61 @@
+vmfex vdsm hook
+===================
+
+Add Cisco VM-FEX Port Profile to Virtual Machine
+Sample:
+    vmfex=NIC1:profile1,NIC2:profile2...
+
+    Will add 2 virtual nics attached to profile1 and profile2 using the vnic
+    MAC addresses specified, replacing the actual NICs assigned to the VM.
+    The mapping is by order of NIC<number> in the parameters, mapped to the 
+    order of the "interface" elements in the devices of the VM xml, as well 
+    as the NICs defined for a VM in the RHEV-M admin GUI.
+
+Libvirt internals:
+Replace the existing interface xml:
+    <interface type="bridge">
+        <mac address="<mac>"/>
+        <model type="virtio"/>
+        <source bridge="<logical network>"/>
+    </interface>
+    
+with the following interface xml:
+    <interface type='network'>
+      <mac address='<mac>'/>
+      <source network='direct-pool'/>
+      <virtualport type='802.1Qbh'>
+          <parameters profileid='<Port Profile>'/>
+      </virtualport>
+      <model type='virtio'/>
+    </interface>
+    
+Dynamic network with libvirt (define a NIC pool, so libvirt can assign VMs to NICs dynamically):
+
+      <network>
+        <name>direct-pool</name>
+        <forward mode="passthrough">
+          <interface dev="eth3"/>
+          <interface dev="eth4"/>
+          <interface dev="eth5"/>
+          <interface dev="eth6"/>
+          <interface dev="eth7"/>
+          <interface dev="eth8"/>
+          <interface dev="eth9"/>
+          <interface dev="eth10"/>
+          <interface dev="eth11"/>
+        </forward>
+      </network>
+
+Using libvirt, the network is defined like this:
+
+   virsh net-define /tmp/direct-pool.xml
+   virsh net-start direct-pool
+   virsh net-autostart direct-pool
+
+(where /tmp/direct-pool.xml contains the xml above)
+
+(everything else is autogenerated, and shouldn't be specified
+when defining a guest (but whatever is there after definition
+should be left in place, e.g. the PCI address)). Note that these
+interface definitions are completely static - you never need to modify
+them due to migration, or starting up/shutting down the guest.
diff --git a/vdsm_hooks/vmfex/before_vm_migrate_destination.py b/vdsm_hooks/vmfex/before_vm_migrate_destination.py
new file mode 100755
index 0000000..a80df7f
--- /dev/null
+++ b/vdsm_hooks/vmfex/before_vm_migrate_destination.py
@@ -0,0 +1,109 @@
+#!/usr/bin/python
+
+import os
+import subprocess
+import sys
+import hooking
+import traceback
+import libvirtconnection
+
+'''
+Placed in before_vm_migrate_destination
+
+vmfex hook on migration destination:
+
+Set up a dynamic NIC pool for incoming migrations to use
+
+      <network>
+        <name>direct-pool</name>
+        <forward mode="passthrough">
+          <interface dev="eth3"/>
+          <interface dev="eth4"/>
+          <interface dev="eth5"/>
+          <interface dev="eth6"/>
+          <interface dev="eth7"/>
+          <interface dev="eth8"/>
+          <interface dev="eth9"/>
+          <interface dev="eth10"/>
+          <interface dev="eth11"/>
+        </forward>
+      </network>
+
+Using libvirt, the network is defined like this:
+
+   virsh net-define /tmp/direct-pool.xml
+   virsh net-start direct-pool
+   virsh net-autostart direct-pool
+
+(where /tmp/direct-pool.xml contains the xml above)
+
+(everything else is autogenerated, and shouldn't be specified
+when defining a guest (but whatever is there after definition
+should be left in place, e.g. the PCI address)). Note that these
+interface definitions are completely static - you never need to modify
+them due to migration, or starting up/shutting down the guest.
+
+'''
+
+def getUsableNics():
+    # Scan localhost for physical NICs and return list of physical nics 
+    # that have all zeroes MAC
+    # Example ['eth0','eth1']
+    nics = []
+    cmd  = "find /sys/devices/pci* | grep '/net/' |grep address"
+    p = subprocess.Popen(cmd,stdout=subprocess.PIPE,shell=True)
+    s = p.stdout.read().split()        
+    for i in s:
+        mac = open(i).read().strip()
+        if mac == '00:00:00:00:00:00':
+            nics.append(i.split('/')[-2])
+    return nics
+
+def createDirectPool(conn):
+    xmlstr = """<network> \n <name>direct-pool</name> \n <forward mode="passthrough"> \n """
+    for i in getUsableNics():
+        s = '<interface dev="' + str(i) + '"/> \n'
+        xmlstr += s
+    
+    xmlstr += """ </forward> \n </network> """
+    conn.networkDefineXML(xmlstr)
+    dpool = conn.networkLookupByName('direct-pool')
+    dpool.setAutostart(1)
+    dpool.create()
+    
+
+
+if os.environ.has_key('vmfex'):
+    try:
+        #connect to libvirtd
+        conn = libvirtconnection.get('qemu:///system')
+        
+        #check for running VMs. If there are VMs running, skip creating a dNIC pool
+        if len(conn.listDomainsID()) == 0:
+            #check if direct-pool is created
+            if 'direct-pool' not in conn.listNetworks():
+                createDirectPool(conn)
+            else: 
+                if conn.networkLookupByName('direct-pool').isActive() == 0:
+                    conn.networkLookupByName('direct-pool').create()
+                    conn.networkLookupByName('direct-pool').setAutostart(1)
+                
+                #check if the defined dNIC pool didn't change
+                dpool = conn.networkLookupByName('direct-pool')
+                definedNics = []
+                for i in dpool.split():
+                    if 'dev=' in i:
+                        definedNics.append(str(i.split('=')[1].split('/')[0].strip("'")))
+                #if the defined NIC pool doesn't match the usable NIC list, recreate the network        
+                if set(definedNics) != set(getUsableNics()):
+                    conn.networkLookupByName('direct-pool').destroy()
+                    conn.networkLookupByName('direct-pool').undefine()
+                    createDirectPool(conn)
+        
+        if 'direct-pool' not in conn.listNetworks():
+            createDirectPool(conn)
+
+    except:
+        sys.stderr.write('vmfex: [unexpected error]: %s\n' % traceback.format_exc())
+        sys.exit(2)
+
diff --git a/vdsm_hooks/vmfex/before_vm_start.py b/vdsm_hooks/vmfex/before_vm_start.py
new file mode 100755
index 0000000..9002e3f
--- /dev/null
+++ b/vdsm_hooks/vmfex/before_vm_start.py
@@ -0,0 +1,188 @@
+#!/usr/bin/python
+
+import os
+import subprocess
+import sys
+import xml
+import hooking
+import traceback
+from xml.dom import minidom
+import libvirtconnection
+
+'''
+Placed in before_vm_start
+
+vmfex hook:
+Add Cisco VM-FEX Port Profile to Virtual Machine
+Sample:
+    vmfex=NIC1:profile1,NIC2:profile2...
+
+    Will add 2 virtual nics attached to profile1 and profile2 using the vnic
+    MAC addresses specified, replacing the actual NICs assigned to the VM.
+    The mapping is by order of NIC<number> in the parameters, mapped to the 
+    order of the "interface" elements in the devices of the VM xml, as well 
+    as the NICs defined for a VM in the RHEV-M admin GUI.
+
+Libvirt internals:
+Replace the existing interface xml:
+    <interface type="bridge">
+        <mac address="<mac>"/>
+        <model type="virtio"/>
+        <source bridge="<logical network>"/>
+    </interface>
+    
+with the following interface xml:
+    <interface type='network'>
+      <mac address='<mac>'/>
+      <source network='direct-pool'/>
+      <virtualport type='802.1Qbh'>
+          <parameters profileid='<Port Profile>'/>
+      </virtualport>
+      <model type='virtio'/>
+    </interface>
+    
+Dynamic network with libvirt (define a NIC pool, so libvirt can assign VMs to NICs dynamically):
+
+      <network>
+        <name>direct-pool</name>
+        <forward mode="passthrough">
+          <interface dev="eth3"/>
+          <interface dev="eth4"/>
+          <interface dev="eth5"/>
+          <interface dev="eth6"/>
+          <interface dev="eth7"/>
+          <interface dev="eth8"/>
+          <interface dev="eth9"/>
+          <interface dev="eth10"/>
+          <interface dev="eth11"/>
+        </forward>
+      </network>
+
+Using libvirt, the network is defined like this:
+
+   virsh net-define /tmp/direct-pool.xml
+   virsh net-start direct-pool
+   virsh net-autostart direct-pool
+
+(where /tmp/direct-pool.xml contains the xml above)
+
+(everything else is autogenerated, and shouldn't be specified
+when defining a guest (but whatever is there after definition
+should be left in place, e.g. the PCI address)). Note that these
+interface definitions are completely static - you never need to modify
+them due to migration, or starting up/shutting down the guest.
+
+'''
+
+def getUsableNics():
+    # Scan localhost for physical NICs and return list of physical nics 
+    # that have all zeroes MAC
+    # Example ['eth0','eth1']
+    nics = []
+    cmd  = "find /sys/devices/pci* | grep '/net/' |grep address"
+    p = subprocess.Popen(cmd,stdout=subprocess.PIPE,shell=True)
+    s = p.stdout.read().split()        
+    for i in s:
+        mac = open(i).read().strip()
+        if mac == '00:00:00:00:00:00':
+            nics.append(i.split('/')[-2])
+    return nics
+
+def enumNics(devices):
+    # Given the domxml of VM devices, return the NIC elements numbered 1 through len()+1
+    vnicmap = {}
+    i = 1
+    for interface in devices.getElementsByTagName('interface'):
+        vnicmap[i] = interface
+        i += 1
+    return vnicmap
+
+def createDirectPool(conn):
+    xmlstr = """<network> \n <name>direct-pool</name> \n <forward mode="passthrough"> \n """
+    for i in getUsableNics():
+        s = '<interface dev="' + str(i) + '"/> \n'
+        xmlstr += s
+    
+    xmlstr += """ </forward> \n </network> """
+    conn.networkDefineXML(xmlstr)
+    dpool = conn.networkLookupByName('direct-pool')
+    dpool.setAutostart(1)
+    dpool.create()
+    
+
+
+if os.environ.has_key('vmfex'):
+    try:
+        #connect to libvirtd
+        conn = libvirtconnection.get('qemu:///system')
+        
+        #check for running VMs. If there are VMs running, skip creating a dNIC pool
+        if len(conn.listDomainsID()) == 0:
+            #check if direct-pool is created
+            if 'direct-pool' not in conn.listNetworks():
+                createDirectPool(conn)
+            else: 
+                if conn.networkLookupByName('direct-pool').isActive() == 0:
+                    conn.networkLookupByName('direct-pool').create()
+                    conn.networkLookupByName('direct-pool').setAutostart(1)
+                
+                #check if the defined dNIC pool didn't change
+                dpool = conn.networkLookupByName('direct-pool')
+                definedNics = []
+                for i in dpool.split():
+                    if 'dev=' in i:
+                        definedNics.append(str(i.split('=')[1].split('/')[0].strip("'")))
+                #if the defined NIC pool doesn't match the usable NIC list, recreate the network        
+                if set(definedNics) != set(getUsableNics()):
+                    conn.networkLookupByName('direct-pool').destroy()
+                    conn.networkLookupByName('direct-pool').undefine()
+                    createDirectPool(conn)
+
+        if 'direct-pool' not in conn.listNetworks():
+            createDirectPool(conn)
+        
+    except:
+        pass    
+        
+    
+    try:
+        #Get the vmfex line
+        vmfex = os.environ['vmfex']
+        
+        #Get the VM's xml definition
+        domxml = hooking.read_domxml()
+        devices = domxml.getElementsByTagName('devices')[0]
+        vnicmap = enumNics(devices)
+        
+        counter = 0
+        
+        for entry in vmfex.split(','):
+            sys.stderr.write('vmfex: adding interface to profile: %s\n' % entry)
+            try: 
+                nicnum = int(entry.split(':')[0][3:])
+                profile = entry.split(':')[1]
+                mynic = vnicmap[nicnum]
+                #replace type="bridge" with "network"
+                mynic.setAttribute('type','network')
+
+                source = mynic.getElementsByTagName('source')[0]
+                source.removeAttribute('bridge')
+                source.setAttribute('network', 'direct-pool')
+                
+                virtualport = domxml.createElement('virtualport')
+                virtualport.setAttribute('type', '802.1Qbh')
+                mynic.appendChild(virtualport)
+
+                parameters = domxml.createElement('parameters')
+                parameters.setAttribute('profileid', profile)
+                virtualport.appendChild(parameters)
+                
+            except:
+                sys.stderr.write('vmfex: Error in vmfex custom property line: %s\n' % entry)
+                sys.exit(2)
+
+        hooking.write_domxml(domxml)
+    except:
+        sys.stderr.write('vmfex: [unexpected error]: %s\n' % traceback.format_exc())
+        sys.exit(2)
+

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Signed-off-by: Dan Yasny 
......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/732/ : ABORTED

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: hook: Cisco VM-FEX support

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: hook: Cisco VM-FEX support
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/742/ : ABORTED

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: hook: Cisco VM-FEX support

Antoni Segura Puimedon has posted comments on this change.

Change subject: hook: Cisco VM-FEX support
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(10 inline comments)

Not a final review yet, but some things to fix for both files (mostly readability and one hiccup that might not exist with direct-pools libvirt network object). I'll check the before_vm_start in more detail later.

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 58:             nics.append(i.split('/')[-2])
Line 59:     return nics
Line 60: 
Line 61: def createDirectPool(conn):
Line 62:     xmlstr = """<network> \n <name>direct-pool</name> \n <forward mode="passthrough"> \n """
The line is too long to pass PEP8. Other people might disagree, but I find xml embedding in the code much easier to read when it is something like:
xmlstr = """<network>
    <name>direct-pool</name>
    <forward mode="passthrough">
"""

Alternatively, just use implicit string concatenation by splitting the line towards the end before it reaches the PEP8 critical point.
Line 63:     for i in getUsableNics():
Line 64:         s = '<interface dev="' + str(i) + '"/> \n'
Line 65:         xmlstr += s
Line 66:

Line 61: def createDirectPool(conn):
Line 62:     xmlstr = """<network> \n <name>direct-pool</name> \n <forward mode="passthrough"> \n """
Line 63:     for i in getUsableNics():
Line 64:         s = '<interface dev="' + str(i) + '"/> \n'
Line 65:         xmlstr += s
The variable 's' is not necessary in my opinion.
you could just append to xmlstr and use string formatting for increased readability.
Line 66: 
Line 67:     xmlstr += """ </forward> \n </network> """
Line 68:     conn.networkDefineXML(xmlstr)
Line 69:     dpool = conn.networkLookupByName('direct-pool')

Line 63:     for i in getUsableNics():
Line 64:         s = '<interface dev="' + str(i) + '"/> \n'
Line 65:         xmlstr += s
Line 66: 
Line 67:     xmlstr += """ </forward> \n </network> """
I don't think triple quote is necessary in this case.
Line 68:     conn.networkDefineXML(xmlstr)
Line 69:     dpool = conn.networkLookupByName('direct-pool')
Line 70:     dpool.setAutostart(1)
Line 71:     dpool.create()

Line 69:     dpool = conn.networkLookupByName('direct-pool')
Line 70:     dpool.setAutostart(1)
Line 71:     dpool.create()
Line 72: 
Line 73: if os.environ.has_key('vmfex'):
I suggest using the more pythonic:
if 'vmfex' in os.environ:
Line 74:     try:
Line 75:         #connect to libvirtd
Line 76:         conn = libvirtconnection.get('qemu:///system')
Line 77:

Line 75:         #connect to libvirtd
Line 76:         conn = libvirtconnection.get('qemu:///system')
Line 77: 
Line 78:         #check for running VMs. If there are VMs running, skip creating a dNIC pool
Line 79:         if len(conn.listDomainsID()) == 0:
it's simpler to just test:
if not conn.listDomainsID():
Line 80:             #check if direct-pool is created
Line 81:             if 'direct-pool' not in conn.listNetworks():
Line 82:                 createDirectPool(conn)
Line 83:             else:

Line 80:             #check if direct-pool is created
Line 81:             if 'direct-pool' not in conn.listNetworks():
Line 82:                 createDirectPool(conn)
Line 83:             else:
Line 84:                 if conn.networkLookupByName('direct-pool').isActive() == 0:
Again, better to test 'not' than compare to zero.
Line 85:                     conn.networkLookupByName('direct-pool').create()
Line 86:                     conn.networkLookupByName('direct-pool').setAutostart(1)
Line 87: 
Line 88:                 #check if the defined dNIC pool didn't change

Line 87: 
Line 88:                 #check if the defined dNIC pool didn't change
Line 89:                 dpool = conn.networkLookupByName('direct-pool')
Line 90:                 definedNics = []
Line 91:                 for i in dpool.split():
I didn't try with a directpool network, but for me conn.networkLookupByName('default') does not return me an object in which I can do split().
Line 92:                     if 'dev=' in i:
Line 93:                         definedNics.append(str(i.split('=')[1].split('/')[0].strip("'")))
Line 94:                 #if the defined NIC pool doesn't match the usable NIC list, recreate the network
Line 95:                 if set(definedNics) != set(getUsableNics()):

Line 93:                         definedNics.append(str(i.split('=')[1].split('/')[0].strip("'")))
Line 94:                 #if the defined NIC pool doesn't match the usable NIC list, recreate the network
Line 95:                 if set(definedNics) != set(getUsableNics()):
Line 96:                     conn.networkLookupByName('direct-pool').destroy()
Line 97:                     conn.networkLookupByName('direct-pool').undefine()
can't we use dpool instead of repeating the lookups?
Line 98:                     createDirectPool(conn)
Line 99: 
Line 100:         if 'direct-pool' not in conn.listNetworks():
Line 101:             createDirectPool(conn)

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 38:       </virtualport>
Line 39:       <model type='virtio'/>
Line 40:     </interface>
Line 41: 
Line 42: Dynamic network with libvirt (define a NIC pool, so libvirt can assign VMs to NICs dynamically):
Too long for PEP8.
Line 43: 
Line 44:       <network>
Line 45:         <name>direct-pool</name>
Line 46:         <forward mode="passthrough">

Line 143: 
Line 144:     try:
Line 145:         #Get the vmfex line
Line 146:         vmfex = os.environ['vmfex']
Line 147:         
Whitespaces alert.
Line 148:         #Get the VM's xml definition
Line 149:         domxml = hooking.read_domxml()
Line 150:         devices = domxml.getElementsByTagName('devices')[0]
Line 151:         vnicmap = enumNics(devices)

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(8 inline comments)

I have several stylistic comments inside. The most difficult issue is to avoid concurrently running createDirectPool. I think createDirectPool should look like

with file('/var/run/vdsm/hook-vmfex.lock', 'w') as f:
    fcntl.flock(f.fileno(), fcntl.LOCK_EX)
    try:
       # something that should happen only once, host-wide
    finally:
        fcntl.flock(f.fileno(), fcntl.LOCK_UN)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 53:     nics = []
Line 54:     cmd = "find /sys/devices/pci* | grep '/net/' |grep address"
Line 55:     p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
Line 56:     s = p.stdout.read().split()
Line 57:     for i in s:
I think using glob would be quicker and safer

for i in glob.iglob('/sys/devices/pci0000:00/*/net/*/address'):
Line 58:         mac = open(i).read().strip()
Line 59:         if mac == '00:00:00:00:00:00':
Line 60:             nics.append(i.split('/')[-2])
Line 61:     return nics

Line 65:     xmlstr = '''<network>
Line 66:        <name>direct-pool</name>
Line 67:        <forward mode="passthrough">
Line 68:     '''
Line 69:     for i in getUsableNics():
"i" seems a bad naming for nicName. or nic.
Line 70:         xmlstr += '<interface dev="' + str(i) + '"/> \n'
Line 71: 
Line 72:     xmlstr += ' </forward> \n </network> '
Line 73:     conn.networkDefineXML(xmlstr)

Line 83:         #connect to libvirtd
Line 84:         conn = libvirtconnection.get('qemu:///system')
Line 85: 
Line 86:         #check for running VMs. If there are VMs running, skip
Line 87:         #creating a dNIC pool
the code is pretty clear for doing this skip. what I'm missing is the reason WHY we should not create the pool with VMs up.
Line 88:         if not conn.listDomainsID():
Line 89:             #check if direct-pool is created
Line 90:             if 'direct-pool' not in conn.listNetworks():
Line 91:                 createDirectPool(conn)

Line 89:             #check if direct-pool is created
Line 90:             if 'direct-pool' not in conn.listNetworks():
Line 91:                 createDirectPool(conn)
Line 92:             else:
Line 93:                 if not conn.networkLookupByName('direct-pool').isActive():
I think you could set "dpool" right here, and save a couple of libvirt calls.
Line 94:                     conn.networkLookupByName('direct-pool').create()
Line 95:                     conn.networkLookupByName('direct-pool').setAutostart(1)
Line 96: 
Line 97:                 #check if the defined dNIC pool didn't change

Line 96: 
Line 97:                 #check if the defined dNIC pool didn't change
Line 98:                 dpool = conn.networkLookupByName('direct-pool')
Line 99:                 definedNics = []
Line 100:                 for i in dpool.XMLDesc(0).split():
parsing xml yourself is clearly calling for troubles. (think: what if the nic name has "dev=" in it?)

you'd better use minidom.parseString() to obtain an xml object, call its getElementsByTagName('interface'), and extract the device from there.

This can sit in its own, easily testable, function.
Line 101:                     if 'dev=' in i:
Line 102:                         definedNics.append(str(i.split('=')[1].split('/')[0].strip("'")))
Line 103:                 #if the defined NIC pool doesn't match the usable NIC list,
Line 104:                 #recreate the network

Line 107:                     dpool.undefine()
Line 108:                     createDirectPool(conn)
Line 109: 
Line 110:         if 'direct-pool' not in conn.listNetworks():
Line 111:             createDirectPool(conn)
just a second - you create the dpool again, regardless if a VM is up? I'm confused.
Line 112:     except:
Line 113:         sys.stderr.write('vmfex: [unexpected error]: %s\n' % traceback.format_exc())

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 99:         i += 1
Line 100:     return vnicmap
Line 101: 
Line 102: 
Line 103: def createDirectPool(conn):
there is a lot of code duplication between this script and the one the I reviewed earlier. It would be safer to share the code in a single module (that could be copied to two places on deployment).
Line 104:     xmlstr = '''<network>
Line 105:        <name>direct-pool</name>
Line 106:        <forward mode="passthrough">
Line 107:     '''

Line 120: if 'vmfex' in os.environ:
Line 121:     try:
Line 122:         sys.stderr.write('vmfex: starting to edit VM \n')
Line 123:         #connect to libvirtd
Line 124:         conn = libvirtconnection.get('qemu:///system')
you should not pass anything to libvirtconnection.get()
Line 125:         # Check for running VMs. If there are VMs running, skip
Line 126:         # creating a dNIC pool
Line 127:         if not conn.listDomainsID():
Line 128:

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 5: (11 inline comments)

Both hooks have almost same code.
Could you please move all identical code to some new modul and just use it in both hooks

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 82: 
Line 83: 
Line 84: def getUsableNics():
Line 85:     # Scan localhost for physical NICs and return list of physical nics
Line 86:     # that have all zeroes MAC
why?
Line 87:     # Example ['eth0','eth1']
Line 88:     #TODO: switch to glob in the future
Line 89:     nics = []
Line 90:     cmd = "find /sys/devices/pci* | grep '/net/' |grep address"

Line 84: def getUsableNics():
Line 85:     # Scan localhost for physical NICs and return list of physical nics
Line 86:     # that have all zeroes MAC
Line 87:     # Example ['eth0','eth1']
Line 88:     #TODO: switch to glob in the future
why not now?   it's too simple

glob.glob('/sys/devices/pci*/*/*/*/net/*/address')
Line 89:     nics = []
Line 90:     cmd = "find /sys/devices/pci* | grep '/net/' |grep address"
Line 91:     p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
Line 92:     for nic in p.stdout.read().split():

Line 104:                <name>direct-pool</name>
Line 105:                <forward mode="passthrough">
Line 106:             '''
Line 107:             for i in getUsableNics():
Line 108:                 xmlstr += '<interface dev="' + str(i) + '"/> \n'
no need str(), it's already strings
Line 109: 
Line 110:             xmlstr += ' </forward> \n </network> '
Line 111:             conn.networkDefineXML(xmlstr)
Line 112:             dpool = conn.networkLookupByName('direct-pool')

Line 107:             for i in getUsableNics():
Line 108:                 xmlstr += '<interface dev="' + str(i) + '"/> \n'
Line 109: 
Line 110:             xmlstr += ' </forward> \n </network> '
Line 111:             conn.networkDefineXML(xmlstr)
What will happens if this direct-pool already exists?
For example if hook run for several VMs.
Do you want to swallow this exception?
Line 112:             dpool = conn.networkLookupByName('direct-pool')
Line 113:             dpool.setAutostart(1)
Line 114:             dpool.create()
Line 115:             sys.stderr.write('vmfex: creating Direct-Pool Network \n')

Line 123:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 124:         try:
Line 125:             dpool = conn.networkLookupByName('direct-pool')
Line 126:             dpool.destroy()
Line 127:             dpool.undefine()
as above, just remove case
Line 128:             sys.stderr.write('vmfex: removing direct-pool \n')
Line 129:         finally:
Line 130:             fcntl.flock(f.fileno(), fcntl.LOCK_UN)
Line 131:

Line 134:     for vm in conn.listDomainsID():
Line 135:         domxml = minidom.parseString(conn.lookupByID(vm).XMLDesc(0))
Line 136:         for interface in domxml.getElementsByTagName('interface'):
Line 137:             for attr in interface.getElementsByTagName('virtualport'):
Line 138:                 if attr.attributes.items()[0][1] == '802.1Qbh':
is it should be a simple
getAttribute('type') == '802.1Qbh'
Line 139:                     return True
Line 140:     return False
Line 141: 
Line 142:

Line 155: 
Line 156: 
Line 157: def handleDirectPool(conn):
Line 158:     #TODO: take this part and everything it uses out and into a separate
Line 159:     #      package
module, not package
Line 160: 
Line 161:     #is direct-pool defined?
Line 162:     if 'direct-pool' in conn.listNetworks():
Line 163:         #are there VMs running?

Line 157: def handleDirectPool(conn):
Line 158:     #TODO: take this part and everything it uses out and into a separate
Line 159:     #      package
Line 160: 
Line 161:     #is direct-pool defined?
IMHO the bellow looks better:

if 'direct-pool'  not in conn.listNetworks():
  createDirectPool(conn)
  return
Line 162:     if 'direct-pool' in conn.listNetworks():
Line 163:         #are there VMs running?
Line 164:         if conn.listDomainsID():
Line 165:             #does any of the running VMs havd a Qbh nic?

....................................................
File vdsm_hooks/vmfex/Makefile.am
Line 21: EXTRA_DIST = \
Line 22: 	before_vm_migrate_destination.py \
Line 23: 	before_vm_start.py \
Line 24: 
Line 25: install-data-local: 
trailing spases ?????
Line 26: 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_start
Line 27: 	$(INSTALL_SCRIPT) $(srcdir)/before_vm_start.py \
Line 28: 		$(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_vmfex
Line 29: 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination

Line 29: 	$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination
Line 30: 	$(INSTALL_SCRIPT) $(srcdir)/before_vm_migrate_destination.py \
Line 31: 		$(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination/50_vmfex
Line 32: 
Line 33: uninstall-local: 
again
Line 34: 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_start/50_vmfex
Line 35: 	$(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_migrate_destination/50_vmfex

....................................................
File vdsm_hooks/vmfex/README
Line 17:         <mac address="<mac>"/>
Line 18:         <model type="virtio"/>
Line 19:         <source bridge="<logical network>"/>
Line 20:     </interface>
Line 21:     
trailing spaces
Line 22: with the following interface xml:
Line 23:     <interface type='network'>
Line 24:       <mac address='<mac>'/>
Line 25:       <source network='direct-pool'/>

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 6: I would prefer that you didn't submit this

Look at my comments in patch set 5

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Hook: Cisco VM-FEX support

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Hook: Cisco VM-FEX support
......................................................................

Patch Set 7: I would prefer that you didn't submit this

Same comments as in patch set 5

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Dan Yasny has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 5: (8 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 82: 
Line 83: 
Line 84: def getUsableNics():
Line 85:     # Scan localhost for physical NICs and return list of physical nics
Line 86:     # that have all zeroes MAC
Done
Line 87:     # Example ['eth0','eth1']
Line 88:     #TODO: switch to glob in the future
Line 89:     nics = []
Line 90:     cmd = "find /sys/devices/pci* | grep '/net/' |grep address"

Line 84: def getUsableNics():
Line 85:     # Scan localhost for physical NICs and return list of physical nics
Line 86:     # that have all zeroes MAC
Line 87:     # Example ['eth0','eth1']
Line 88:     #TODO: switch to glob in the future
glob fails, but I used os.walk() and it seems to not only be working, but also faster than glob.glob
Line 89:     nics = []
Line 90:     cmd = "find /sys/devices/pci* | grep '/net/' |grep address"
Line 91:     p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True)
Line 92:     for nic in p.stdout.read().split():

Line 104:                <name>direct-pool</name>
Line 105:                <forward mode="passthrough">
Line 106:             '''
Line 107:             for i in getUsableNics():
Line 108:                 xmlstr += '<interface dev="' + str(i) + '"/> \n'
Done
Line 109: 
Line 110:             xmlstr += ' </forward> \n </network> '
Line 111:             conn.networkDefineXML(xmlstr)
Line 112:             dpool = conn.networkLookupByName('direct-pool')

Line 107:             for i in getUsableNics():
Line 108:                 xmlstr += '<interface dev="' + str(i) + '"/> \n'
Line 109: 
Line 110:             xmlstr += ' </forward> \n </network> '
Line 111:             conn.networkDefineXML(xmlstr)
If the pool exists and there are other VMs already running, we will not enter this function
Line 112:             dpool = conn.networkLookupByName('direct-pool')
Line 113:             dpool.setAutostart(1)
Line 114:             dpool.create()
Line 115:             sys.stderr.write('vmfex: creating Direct-Pool Network \n')

Line 123:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 124:         try:
Line 125:             dpool = conn.networkLookupByName('direct-pool')
Line 126:             dpool.destroy()
Line 127:             dpool.undefine()
as above - If the pool exists and there are other VMs already running, we will not enter this function
Line 128:             sys.stderr.write('vmfex: removing direct-pool \n')
Line 129:         finally:
Line 130:             fcntl.flock(f.fileno(), fcntl.LOCK_UN)
Line 131:

Line 134:     for vm in conn.listDomainsID():
Line 135:         domxml = minidom.parseString(conn.lookupByID(vm).XMLDesc(0))
Line 136:         for interface in domxml.getElementsByTagName('interface'):
Line 137:             for attr in interface.getElementsByTagName('virtualport'):
Line 138:                 if attr.attributes.items()[0][1] == '802.1Qbh':
Done
Line 139:                     return True
Line 140:     return False
Line 141: 
Line 142:

Line 155: 
Line 156: 
Line 157: def handleDirectPool(conn):
Line 158:     #TODO: take this part and everything it uses out and into a separate
Line 159:     #      package
Done
Line 160: 
Line 161:     #is direct-pool defined?
Line 162:     if 'direct-pool' in conn.listNetworks():
Line 163:         #are there VMs running?

Line 157: def handleDirectPool(conn):
Line 158:     #TODO: take this part and everything it uses out and into a separate
Line 159:     #      package
Line 160: 
Line 161:     #is direct-pool defined?
Done
Line 162:     if 'direct-pool' in conn.listNetworks():
Line 163:         #are there VMs running?
Line 164:         if conn.listDomainsID():
Line 165:             #does any of the running VMs havd a Qbh nic?

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: I would prefer that you didn't submit this

(5 inline comments)

I have only one major question, about code repetition in handleDirectPool().

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 8:     import libvirtconnection
Line 9: except ImportError:
Line 10:     #3.1 compat
Line 11:     from vdsm import libvirtconnection
Line 12: import fcntl
minor: standard modules should be imported first
Line 13: from xml.dom import minidom
Line 14: 
Line 15: '''
Line 16: Placed in before_vm_migrate_destination

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 92:     # Example ['eth0','eth1']
Line 93:     nics = []
Line 94:     for root, dirs, names in os.walk('/sys/devices/'):
Line 95:         if 'address' in names and 'pci' in root:
Line 96:             f = open(root + '/address', 'r')
with file(root + '/address', 'r') as f:
  mac = f.readlines()[0].strip()

would have been slightly nicer
Line 97:             mac = f.readlines()[0].strip()
Line 98:             f.close()
Line 99:             if mac == '00:00:00:00:00:00':
Line 100:                 eth = root.split('/')[-1]

Line 148: def compareDPoolToUsableNics(conn):
Line 149:     #return True if currently defined direct-pool matches the set
Line 150:     #returned by getUsableNics()
Line 151:     dpool = conn.networkLookupByName('direct-pool')
Line 152:     definedNics = []
minor: this could be a set() to begin with.
Line 153:     dpoolxml = minidom.parseString(dpool.XMLDesc(0))
Line 154:     for iface in dpoolxml.getElementsByTagName('interface'):
Line 155:         definedNics.append(iface.getAttribute('dev'))
Line 156:     if set(definedNics) == set(getUsableNics()):

Line 158:     else:
Line 159:         return False
Line 160: 
Line 161: 
Line 162: def handleDirectPool(conn):
minor: this is a very vague name. I think that updateDirectPool is clearer.
Line 163:     #TODO: take this part and everything it uses out and into a separate
Line 164:     # module
Line 165: 
Line 166:     #is direct-pool defined? If not, just create it and move on

Line 168:         createDirectPool(conn)
Line 169:         return
Line 170:     #Now we know for sure that the pool exists...
Line 171:     #are there VMs running?
Line 172:     if conn.listDomainsID():
if there is no Domain running, qbhInUse() would return False.
so this block can be simplified to

if not qbhInUse(conn):
            #no VMs use the existing pool, it can be checked and updated
                #is the pool the same as list of available dNICs?
                if not compareDPoolToUsableNics(conn):
                #not the same, something changed, recreating
                    removeDirectPool(conn)
                    createDirectPool(conn)

eliminating the repeated code. Am I right?
Line 173:         #does any of the running VMs havd a Qbh nic?
Line 174:         #if yes, can't touch the existing pool
Line 175:         if not qbhInUse(conn):
Line 176:         #no VMs use the existing pool, it can be checked and updated

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Dan Yasny has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: (4 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 8:     import libvirtconnection
Line 9: except ImportError:
Line 10:     #3.1 compat
Line 11:     from vdsm import libvirtconnection
Line 12: import fcntl
Done
Line 13: from xml.dom import minidom
Line 14: 
Line 15: '''
Line 16: Placed in before_vm_migrate_destination

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 92:     # Example ['eth0','eth1']
Line 93:     nics = []
Line 94:     for root, dirs, names in os.walk('/sys/devices/'):
Line 95:         if 'address' in names and 'pci' in root:
Line 96:             f = open(root + '/address', 'r')
According to what people are saying on python forums, file() is not a good idea and one should stick to open() instead. There's even talk of deprecating file() completely in the future.
Line 97:             mac = f.readlines()[0].strip()
Line 98:             f.close()
Line 99:             if mac == '00:00:00:00:00:00':
Line 100:                 eth = root.split('/')[-1]

Line 158:     else:
Line 159:         return False
Line 160: 
Line 161: 
Line 162: def handleDirectPool(conn):
I don't "update" here, this is a logical wrapper on update/check/verify/... all the actions around directPool. I am thinking of actually moving the rest of the directPool functions into this one, somehow, but that's not as urgent as taking this entire set of functions into a separate module, IMO
Line 163:     #TODO: take this part and everything it uses out and into a separate
Line 164:     # module
Line 165: 
Line 166:     #is direct-pool defined? If not, just create it and move on

Line 168:         createDirectPool(conn)
Line 169:         return
Line 170:     #Now we know for sure that the pool exists...
Line 171:     #are there VMs running?
Line 172:     if conn.listDomainsID():
Makes sense... 
Done.
Line 173:         #does any of the running VMs havd a Qbh nic?
Line 174:         #if yes, can't touch the existing pool
Line 175:         if not qbhInUse(conn):
Line 176:         #no VMs use the existing pool, it can be checked and updated

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: (2 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 92:     # Example ['eth0','eth1']
Line 93:     nics = []
Line 94:     for root, dirs, names in os.walk('/sys/devices/'):
Line 95:         if 'address' in names and 'pci' in root:
Line 96:             f = open(root + '/address', 'r')
you use file() elsewhere in this code...

but I did not mean file() or open() - but not needing to call close() if only you use "with"
Line 97:             mac = f.readlines()[0].strip()
Line 98:             f.close()
Line 99:             if mac == '00:00:00:00:00:00':
Line 100:                 eth = root.split('/')[-1]

Line 158:     else:
Line 159:         return False
Line 160: 
Line 161: 
Line 162: def handleDirectPool(conn):
"handle" is a sign of unclarity. if you do not have a better name, fine. I'm not a name nazi.
Line 163:     #TODO: take this part and everything it uses out and into a separate
Line 164:     # module
Line 165: 
Line 166:     #is direct-pool defined? If not, just create it and move on

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: (1 inline comment)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 7:     #3.0 compat
Line 8:     import libvirtconnection
Line 9: except ImportError:
Line 10:     #3.1 compat
Line 11:     from vdsm import libvirtconnection
we have an issue here, as pyflakes does not understand that this is a valid second attempt to import the module.
Line 12: import fcntl
Line 13: from xml.dom import minidom
Line 14: 
Line 15: '''

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Dan Yasny has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 9: (3 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 7:     #3.0 compat
Line 8:     import libvirtconnection
Line 9: except ImportError:
Line 10:     #3.1 compat
Line 11:     from vdsm import libvirtconnection
ideas? 
https://github.com/kevinw/pyflakes/issues/13
and 
http://stackoverflow.com/questions/5033727/how-do-i-get-pyflakes-to-ignore-a...
pretty much provide the same idea and state this is a pyflakes bug
Line 12: import fcntl
Line 13: from xml.dom import minidom
Line 14: 
Line 15: '''

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 92:     # Example ['eth0','eth1']
Line 93:     nics = []
Line 94:     for root, dirs, names in os.walk('/sys/devices/'):
Line 95:         if 'address' in names and 'pci' in root:
Line 96:             f = open(root + '/address', 'r')
You're right, gotta fix that (it's your code I copied after all)
Line 97:             mac = f.readlines()[0].strip()
Line 98:             f.close()
Line 99:             if mac == '00:00:00:00:00:00':
Line 100:                 eth = root.split('/')[-1]

Line 158:     else:
Line 159:         return False
Line 160: 
Line 161: 
Line 162: def handleDirectPool(conn):
I'd rather leave it for now, until vdsm takes the hook under it's wing and makes everything right
Line 163:     #TODO: take this part and everything it uses out and into a separate
Line 164:     # module
Line 165: 
Line 166:     #is direct-pool defined? If not, just create it and move on

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 11: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Igor Lvovsky]

Igor Lvovsky has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 11: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 12: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 95:             dpool.create()
Line 96:             sys.stderr.write('vmfex: created Direct-Pool Net \n')
Line 97:             sys.stderr.write(xmlstr + '\n')
Line 98:     except:  # Need a traceback on ANY error, thus no specification
Line 99:         sys.stderr.write('vmfex: Failed createDirectPool \n')
I don't understand why you've added this try-block. The write() at the end of this file already catches and prints this traceback.

If your point here was to ignore various libvirt errors - go ahead an enumerate them. Catch-all "except" is dangerous - if the pool was not created, you don't want to go on starting the VM.
Line 100:         sys.stderr.write('vmfex: ERROR: %s\n' % traceback.format_exc())
Line 101:     finally:
Line 102:         fcntl.flock(f.fileno(), fcntl.LOCK_UN)
Line 103:

Line 98:     except:  # Need a traceback on ANY error, thus no specification
Line 99:         sys.stderr.write('vmfex: Failed createDirectPool \n')
Line 100:         sys.stderr.write('vmfex: ERROR: %s\n' % traceback.format_exc())
Line 101:     finally:
Line 102:         fcntl.flock(f.fileno(), fcntl.LOCK_UN)
this is wrong here, as "f" may not be defined here. it was correct in the former patch.
Line 103: 
Line 104: 
Line 105: def qbhInUse(conn):
Line 106:     for vm in conn.listDomainsID():

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Dan Yasny has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 12: (2 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 95:             dpool.create()
Line 96:             sys.stderr.write('vmfex: created Direct-Pool Net \n')
Line 97:             sys.stderr.write(xmlstr + '\n')
Line 98:     except:  # Need a traceback on ANY error, thus no specification
Line 99:         sys.stderr.write('vmfex: Failed createDirectPool \n')
Done
Line 100:         sys.stderr.write('vmfex: ERROR: %s\n' % traceback.format_exc())
Line 101:     finally:
Line 102:         fcntl.flock(f.fileno(), fcntl.LOCK_UN)
Line 103:

Line 98:     except:  # Need a traceback on ANY error, thus no specification
Line 99:         sys.stderr.write('vmfex: Failed createDirectPool \n')
Line 100:         sys.stderr.write('vmfex: ERROR: %s\n' % traceback.format_exc())
Line 101:     finally:
Line 102:         fcntl.flock(f.fileno(), fcntl.LOCK_UN)
Done
Line 103: 
Line 104: 
Line 105: def qbhInUse(conn):
Line 106:     for vm in conn.listDomainsID():

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 13: I would prefer that you didn't submit this

(3 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 71: def createDirectPool(conn):
Line 72:     with open('/var/run/vdsm/hook-vmfex.lock', 'w') as f:
Line 73:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 74:         try:
Line 75:             #singleton - doublecheck after taking lock
I'm a bit puzzled by this comment and this code.. I would prefer if you called removeDirectPool() (and drop the locking there if it is a problem)
Line 76:             #to avoid a race between two threads
Line 77:             if 'direct-pool' in conn.listNetworks():
Line 78:                 dpool = conn.networkLookupByName('direct-pool')
Line 79:                 #destroy and undefine direct-pool

Line 132:         return
Line 133:     #Now we know for sure that the pool exists...
Line 134:     #are there VMs running, and are they using 802.1Qbh?
Line 135:     #if yes, can't touch the existing pool
Line 136:     if not qbhInUse(conn):
there's still a race hiding here: a VM using qbh may be started right after this check.

I believe we should take the lock outside this function :-(
Line 137:     #no VMs use the existing pool, it can be checked and updated
Line 138:         #is the pool the same as list of available dNICs?
Line 139:         if not compareDPoolToUsableNics(conn):
Line 140:         #not the same, something changed, recreating

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 105: def createDirectPool(conn):
Line 106:     with open('/var/run/vdsm/hook-vmfex.lock', 'w') as f:
Line 107:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 108:         try:
Line 109:             #singleton - doublecheck after taking lock
isn't it lovely to repeat each change twice?
Line 110:             #to avoid a race between two threads
Line 111:             if 'direct-pool' in conn.listNetworks():
Line 112:                 dpool = conn.networkLookupByName('direct-pool')
Line 113:                 #destroy and undefine direct-pool

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Dan Yasny has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 13: (2 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 71: def createDirectPool(conn):
Line 72:     with open('/var/run/vdsm/hook-vmfex.lock', 'w') as f:
Line 73:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 74:         try:
Line 75:             #singleton - doublecheck after taking lock
There seemed to be a race between two threads where:
-thread A would run removeDirectPool() and release lock
- thread B would come in, check for direct-pool existence and since it doesn't exist, try to create it
- thread A comes back, takes lock on createDirectPool, creates the network, releases lock
- thread B come into createDirectPool() and crashes on libvirtError - network already exists.

When bulk starting/migrating VMs this was causing a small percentage of VMs not to start/migrate. The solution, as suggested by Livnat, was to 
1 check for direct-pool existence
2 if not exist, take lock
3 check again for existence
4 if not exist - create

With this solution, thread B would fail on second check and will skip creating direct-pool, just starting the VM. Apparently this is a classic CS solution called "singleton" (according to Livnat).

Indeed, with this implementation, I successfully migrate/start 50 VMs with VMFEX NICs, and with the direct-pool network in various different shapes to begin with.

I also had a long chat with apuimedo regarding this, and he confirmed this is a valid solution. If you have a better one, please let me know (I'd rather discuss this live, and asap)
Line 76:             #to avoid a race between two threads
Line 77:             if 'direct-pool' in conn.listNetworks():
Line 78:                 dpool = conn.networkLookupByName('direct-pool')
Line 79:                 #destroy and undefine direct-pool

Line 132:         return
Line 133:     #Now we know for sure that the pool exists...
Line 134:     #are there VMs running, and are they using 802.1Qbh?
Line 135:     #if yes, can't touch the existing pool
Line 136:     if not qbhInUse(conn):
going to fly out on libvirt error again, removeNetwork won't work if the network is in use, and create will fail if the network already exists. Ugly, but seems safe to me.
Line 137:     #no VMs use the existing pool, it can be checked and updated
Line 138:         #is the pool the same as list of available dNICs?
Line 139:         if not compareDPoolToUsableNics(conn):
Line 140:         #not the same, something changed, recreating

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Antoni Segura Puimedon has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 13: (2 inline comments)

....................................................
File vdsm_hooks/vmfex/before_vm_migrate_destination.py
Line 132:         return
Line 133:     #Now we know for sure that the pool exists...
Line 134:     #are there VMs running, and are they using 802.1Qbh?
Line 135:     #if yes, can't touch the existing pool
Line 136:     if not qbhInUse(conn):
It would be safer to hold the lock on creation and usage. In fact the ideal thing would be to acquire some dedicated libvirt lock, but as d.yasny pointed out that lock probably does not exist in libvirt.
Line 137:     #no VMs use the existing pool, it can be checked and updated
Line 138:         #is the pool the same as list of available dNICs?
Line 139:         if not compareDPoolToUsableNics(conn):
Line 140:         #not the same, something changed, recreating

....................................................
File vdsm_hooks/vmfex/before_vm_start.py
Line 105: def createDirectPool(conn):
Line 106:     with open('/var/run/vdsm/hook-vmfex.lock', 'w') as f:
Line 107:         fcntl.flock(f.fileno(), fcntl.LOCK_EX)
Line 108:         try:
Line 109:             #singleton - doublecheck after taking lock
Would it be possible to move some of this to hooky.py or however the common hooks library is called?
Line 110:             #to avoid a race between two threads
Line 111:             if 'direct-pool' in conn.listNetworks():
Line 112:                 dpool = conn.networkLookupByName('direct-pool')
Line 113:                 #destroy and undefine direct-pool

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 14:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/139/ (2/2)

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 14:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/105/ (1/2)

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 14: I would prefer that you didn't submit this

Build Unstable

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/105/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/139/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/106/ (1/2)

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/140/ (2/2)

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/106/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/140/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15: Looks good to me, approved

seems good to me (but so was a previous version...)
please verify with a barrage of VM creation/migration.

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Livnat Peer 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

Meni Yakove has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15: Verified

Run 50 VMs at once with vmfex hook
Migrate 50 vms at once with vmfex hook

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Livnat Peer 
Gerrit-Reviewer: Meni Yakove 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Patch Set 15:

thanks!

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Livnat Peer 
Gerrit-Reviewer: Meni Yakove 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: Cisco VM-FEX support vdsm hooks

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Cisco VM-FEX support vdsm hooks
......................................................................

Cisco VM-FEX support vdsm hooks

Moved the lock to handleDirectPool

Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Author: Dan Yasny 
Signed-off-by: Dan Yasny 
---
A vdsm_hooks/vmfex/Makefile.am
A vdsm_hooks/vmfex/README
A vdsm_hooks/vmfex/before_vm_migrate_destination.py
A vdsm_hooks/vmfex/before_vm_start.py
4 files changed, 440 insertions(+), 0 deletions(-)

Approvals:
  Meni Yakove: Verified
  Dan Kenigsberg: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/7547
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I45a7fa46919bb39a648dff190c40618395990e91
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Yasny 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dan Yasny 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Igor Lvovsky 
Gerrit-Reviewer: Livnat Peer 
Gerrit-Reviewer: Meni Yakove 
Gerrit-Reviewer: oVirt Jenkins CI Server