[ovs-dev] [PATCH 1/1] xenserver: Fix iteration of dictionary

[Ben Pfaff]

On Wed, Feb 15, 2012 at 05:38:28PM +0000, Dominic Curran wrote:
> > -----Original Message-----
> > From: Ben Pfaff [mailto:blp at nicira.com]
> > Sent: 15 February 2012 17:24
> > To: Rob Hoes
> > Cc: dev at openvswitch.org; Dominic Curran; Rob Hoes
> > Subject: Re: [ovs-dev] [PATCH 1/1] xenserver: Fix iteration of
> > dictionary
> > 
> > On Wed, Feb 15, 2012 at 05:17:25PM +0000, Rob Hoes wrote:
> > > From: Dominic Curran <dominic.curran at citrix.com>
> > >
> > > Fix bug in commit 3249bb907a1dab9b0
> > >
> > > Incorrectly assume that get_all_records_where() returned a list.
> > > It infact returns a disctionary and the list iteratory needs to
> > change
> > > to account for this.
> > > Thanks to Nicira for pointing this out.
> > >
> > > NIC-454.
> > >
> > > Signed-off-by: Rob Hoes <rob.hoes at citrix.com>
> > > Reported-by: David Tsai <dtsai at nicira.com>
> > > Signed-off-by: Dominic Curran <Dominic.curran at citrix.com>
> > 
> > Now I'm confused.  First, Dominic, I understand <gigamuffin at gmail.com>
> > to be your email address, but you've attached Rob Hoes's name to it in
> > your email headers.  Second, who actually wrote this patch?  Dominic,
> > did you write it or did Rob?
> > 
> > Thanks,
> > 
> > Ben.
> 
> I know, I know, I've screwed up. Hang head in shame.
> 
> Nicira suggested bug fix.  
> I implemented/wrote the patch.  
> RobH has OK'd the change.
> 
> I thought 'git send-email' was going to put 'From: Rob Hoes
> <rob.hoes at citrix.com>' where you told me to put it.  I'm sorry.  I
> certainly didn't think it was going to mangle my email with his name.

I understand now.  Thanks.

You're the author of the patch, so there's no need for a From: line in
the body, since it can come from your email headers directly.

RobH did not write the patch and the patch did not flow through him, so
I converted the Signed-off-by: to an Acked-by:.  (I think that there is
some provision in the system for some kind of centralized sign-off for
an organization, but I think that an Acked-by: represents what happened
here acceptably well.)

I applied the following to master and branch-1.[345]:

--8<--------------------------cut here-------------------------->8--

From: Dominic Curran <dominic.curran at citrix.com>
Date: Wed, 15 Feb 2012 17:17:25 +0000
Subject: [PATCH] xenserver: Fix iteration of dictionary.

Fix bug in commit 3249bb907a1dab9b0, which incorrectly assumed that
get_all_records_where() returned a list.  It in fact returns a
dictionary and the list iteratory needs to change to account for this.

Thanks to Nicira for pointing this out.

NIC-454.

Reported-by: David Tsai <dtsai at nicira.com>
Acked-by: Rob Hoes <rob.hoes at citrix.com>
Signed-off-by: Dominic Curran <dominic.curran at citrix.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 AUTHORS                                            |    2 +-
 .../etc_xapi.d_plugins_openvswitch-cfg-update      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index cd7c9d3..58f7ecc 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -16,6 +16,7 @@ Dan Wendlandt           dan at nicira.com
 Daniel Roman            droman at nicira.com
 David Erickson          derickso at stanford.edu
 Devendra Naga           devendra.aaru at gmail.com
+Dominic Curran          dominic.curran at citrix.com
 Edward Tomasz Napierała trasz at freebsd.org
 Ethan Jackson           ethan at nicira.com
 Gaetano Catalli         gaetano.catalli at gmail.com
@@ -76,7 +77,6 @@ Bryan Osoro             bosoro at nicira.com
 Cedric Hobbs            cedric at nicira.com
 Dave Walker             DaveWalker at ubuntu.com
 Derek Cormier           derek.cormier at lab.ntt.co.jp
-Dominic Curran          dominic.curran at citrix.com
 Duffie Cooley           dcooley at nicira.com
 DK Moon                 dkmoon at nicira.com
 Edwin Chiu              echiu at nicira.com
diff --git a/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update b/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
index a9a10e8..ef4d11a 100755
--- a/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
+++ b/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
@@ -100,7 +100,7 @@ def update(session, args):
     pool_mgmt_macs = {}
     if new_controller:
         recs = session.xenapi.PIF.get_all_records_where('field "management"="true"')
-        for rec in recs:
+        for rec in recs.itervalues():
             pool_mgmt_macs[rec.get('MAC')] = rec.get('device')
 
     dib_changed = False
-- 
1.7.2.5