art of the two VMs are exactly the same
> except the network, because they’re all created by OpenStack.
>
> The two VM booted on the same system, on the same NIC card, linked to the
> same switch……
>
> ** **
>
> ** **
>
> *From:* Reid Price [mailto:rpr...@nicira.
Apologies for the long answer. Skip to the last two paragraphs for
suggestions.
> "%s: %s %s" % (self.name, title, msg)
I've narrowed this down a bit. The minimal reproduction is:
# python -c '"%s%s" % ("\xc2", u"")'
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 0:
ord
Hi chen,
What Jesse is pointing out is that your problem seems unrelated to OVS. In
particular, the low throughput you observe is probably related to something
else in the system, be it drivers, VLAN offloading, intermediate network,
etc.
The point is that you already have verified for yourself
Hi Luigi,
At some level it is a logical error to create flows that:
- have the same priority
- have overlapping match criteria
- have different actions
You are already in trouble if you have
priority=100, nw_src=1.2.3.0/24,actions=1
priority=100, nw_dst=5.6.7.0/24,actions=2
Since the action
It seems as though there might be use for a flag to allow
using more of the disk somehow, or to tar up existing files
rather than copying them. I assume in most cases the user
can make that decision themselves by manually removing
older files or other things on the FS.
-Reid
On Tue, Sep 24, 2
I am also a bit concerned by issues that might arise
from a user thinking that this is always accurate, rather
than hints. Aaron, I think you had said something
regarding this when we chatted off-list, but I don't
recall the details.
-Reid
On Fri, Aug 9, 2013 at 2:50 PM, Reid Price
Or you could keep the original function behavior the same and expose this
as a separate function
def foo(...):
def run(...):
return self.foo(...)[0]
where foo is a better function name - update? run_details?
run_with_changes? run_diff? _run? No opinion there.
-Reid
On Fri, Au
I like this one better. I am surprised that test-ovsdb has the only
invocations of idl.run(), but if that is true, LGTM.
On Tue, Aug 6, 2013 at 3:14 PM, Aaron Rosen wrote:
> This patch changes what is being returned from Idl.run() to a tuple
> (changed, changes) so one can determine what chang
>
> Compacting the database loses tons of information that I often find
> valuable.
+1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Yes, especially if it passed the unit test, looks correct.
On Wed, Jun 19, 2013 at 3:26 PM, Ben Pfaff wrote:
> Thanks. Is that a review?
>
> On Tue, Jun 18, 2013 at 09:27:18PM -0700, Reid Price wrote:
> > Thanks, that seems like the right amount of information
> >
>
s confusing. This commit improves the error message to:
> cannot delete port br0 because it is the local port for bridge br0
> (deleting this port requires deleting the entire bridge)
>
> Bug #17994.
> Reported-by: Reid Price
> Signed-off-by: Ben Pfaff
> ---
> tests/
Looks good to me, tested.
On Wed, Jun 12, 2013 at 11:35 AM, Ben Pfaff wrote:
> This feature wasn't used and removing it slightly simplifies the code.
>
> Signed-off-by: Ben Pfaff
> ---
> build-aux/extract-ofp-errors | 10 +++---
> 1 files changed, 3 insertions(+), 7 deletions(-)
>
> dif
Couple notes inline
On Wed, May 22, 2013 at 4:48 PM, Ben Pfaff wrote:
> This feature wasn't used and removing it slightly simplifies the code.
>
> Signed-off-by: Ben Pfaff
> ---
> build-aux/extract-ofp-errors | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a
One nit, there is only one line between the 'Bonds' section and 'Controllers'
section. Everywhere else in the file you use 2 lines, with the (accidental)
exception of between 'General' and 'Releases', which has 3.
-Reid
On Thu, Mar 21, 2013 at 3:22 PM, Ben Pfaff wrote:
> CC: Jesse Gross
> Si
LGTM. Thanks for fixes
-Reid
PS: One parting shot, I assume there's no constant for "syslog"
elsewhere in the code =)
On Wed, Feb 27, 2013 at 7:20 PM, Andy Zhou wrote:
>
> vlog.py currently generates the same log messages, starts with the time stamp
> information, for console, syslog and fi
Seems reasonable. A few suggestions:
> syslog_message = ("%s|%s|%s|%s"
>% (Vlog.__msg_num, self.name, level, message))
Fix indent. It looks like you should be able to line up % under the
first " still without going over 80.
> message = syslog_message;
Drop the semicolon =)
> m
Looks good, one style nit. I often see people bevel the first character, don't
know if that is canonical though.
On Mon, Feb 25, 2013 at 5:30 PM, Ethan Jackson wrote:
> Acked-by: Ethan Jackson
>
> This has the added benefit of making it really obvious when the switch
> restarted in the logs. F
Python seems good, nice use of int(X, 0)
One question on
> +print "{ -1, -1, -1 }, /* %s */" % enum
If you moved the print outside this and used the same print (below) for both
> +print "{ %#8x, %2d, %3d }, /* %s */" % (vendor,
> type_, co
Looks good. I assume there is no way to have no literals?
For what it's worth,
english = 'one of %s, %s, or %s' % (literals[0],
', '.join(literals[1:-1]),
literals[-1])
could also be written as
english = 'one o
> +the in-band support as the following.
Perhaps change 'as the following' to 'with the following command'
-Reid
On Thu, Jan 31, 2013 at 2:49 AM, YAMAMOTO Takashi
wrote:
> From: YAMAMOTO Takashi
>
> Signed-off-by: YAMAMOTO Takashi
> ---
> INSTALL.userspace | 6 +--
> lib/netdev-bsd.c |
s/ofporot/ofproto/
On Tue, Nov 20, 2012 at 1:49 PM, Ansis Atteka wrote:
> The usage message for this command was wrong, because it did not
> specify priority as one of its arguments.
>
> Signed-off-by: Ansis Atteka
> ---
> ofproto/ofproto-dpif.c |2 +-
> 1 file changed, 1 insertion(+), 1 d
Thanks Justin!
Signed-off-by: Reid Price
On Tue, Oct 16, 2012 at 5:29 PM, Justin Pettit wrote:
> Commit 8cc820 (python/ovs/stream: teach stream.py tcp socket) made a
> change that used class decorators. Unfortunately, they were not
> introduced until Python 2.6. XenServer uses P
Nice! Few notes inline
On Mon, Oct 15, 2012 at 3:22 PM, Ethan Jackson wrote:
> The new ovs-backtrace-parse utility makes the output of ovs-appctl
> backtrace more human readable by removing duplicate traces and
> converting addresses to function names.
>
> Signed-off-by: Ethan Jackson
> ---
>
reasonable and clean, if a little overkill
compared to a conditional in open(). Thanks!
-Reid
On Mon, Oct 15, 2012 at 11:55 AM, Reid Price wrote:
>
> Sorry, was out of town on Sept 27th, this was still in my mail queue. I'll
> look over this immediately.
>
>
> On Mon, O
Sorry, was out of town on Sept 27th, this was still in my mail queue. I'll
look over this immediately.
On Mon, Oct 15, 2012 at 10:13 AM, Ben Pfaff wrote:
> The only blocker is that I was hoping for reviews from Reid. Since
> it's been a while and none has arrived, I'll review them myself.
>
>
+else:
+if type(json) == list:
+json = [self._substitute_uuids(elem) for elem in json]
+else:
+json = tuple(self._substitute_uuids(elem) for elem in
json)
tuple/list are treated identically, based upon the
retur
If by chance you haven't pushed yet, s/preresentation//
On Wed, Sep 12, 2012 at 8:48 PM, Ben Pfaff wrote:
> On Thu, Sep 13, 2012 at 11:27:30AM +0900, Isaku Yamahata wrote:
> > On Wed, Sep 12, 2012 at 11:52:09AM -0700, Ben Pfaff wrote:
> > > I agree about raising an error if both are provided.
>
Patches 1/5, 2/5, 5/5 look fine to me.
On Sep 11, 2555 BE, at 23:17, Isaku Yamahata wrote:
> In some cases getattr(Row instance, attrname) doesn't raise AttributeError,
> but TypeError
>
>> File "python/ovs/db/idl.py", line 554, in __getattr__
>>datum = self._data[column_name]
>> TypeError:
Contextless phone review here also.
Perhaps raise a value error if location and schema are both provided?
Isaku/Ben, are there any side effects of keeping this data in memory before it
is used, and removing entirely it after the 'schema =' line? Before it loaded
it on demand. Isn't necessarily
One note inline
On Sep 11, 2555 BE, at 23:17, Isaku Yamahata wrote:
> Since Transaction._substitute doesn't substitute elements of list/tuple,
> setting list references results in transaction error. Teach it such case.
>
> Example:
> {"op": "update",
> "row":{"bridges":["set",[["uuid",
>
Thanks for humoring me, looks good.
On Sep 5, 2555 BE, at 3:38, Isaku Yamahata wrote:
> eventlet/gevent doesn't work well with select.poll because select.poll blocks
> python interpreter as a whole instead of switching from the current thread
> which is about to block to other runnable thread.
>
Seems reasonable. Some notes inline, mostly nits and questions.
On Tue, Sep 4, 2012 at 10:09 PM, Isaku Yamahata wrote:
> eventlet/gevent doesn't work well with select.poll because select.poll
> blocks
> python interpreter as a whole instead of switching from the current thread
> which is about t
Hi Marco,
You will want to provide much more information in order to get a useful
response. OVS typically uses near-zero CPU when it is in an idle state.
Perhaps look at the instructions for reporting bugs to get an idea of what
information to collect. I would start with at least
# ovs-dpctl
Noted in passing:
s/nubmer/number/
Did not review rest
On Aug 22, 2555 BE, at 3:05, Isaku Yamahata wrote:
> Signed-off-by: Isaku Yamahata
> ---
> python/ovs/socket_util.py | 40
> 1 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/
On Wed, Jul 11, 2012 at 9:53 AM, Arun Sharma wrote:
>
> +if len(filters):
> +filter = ",".join(filters)
> +else:
> +filter = None
> +
>
Pythonic to do
if filters:
otherwise LGTM, thanks!
-Reid
___
dev mailing list
dev@open
Thanks for the fixes! A couple nitpick comments inline. v2 and your
responses in v1 resolved all of my previous concerns.
On Tue, Jul 10, 2012 at 11:40 PM, Arun Sharma wrote:
> +# Filter out ovs relevant information if --ovs option passed
> +# else collect all information
> +if onl
A few comments inline
On Tue, Jul 10, 2012 at 12:07 PM, Arun Sharma wrote:
>
> diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/
> ovs-bugtool.in
> index 3bafa13..af2bb6e 100755
> --- a/utilities/bugtool/ovs-bugtool.in
> +++ b/utilities/bugtool/ovs-bugtool.in
> @@ -404,6 +404,10 @
On Fri, Jun 29, 2012 at 10:45:47PM -0700, Ansis Atteka wrote:
> ovs-l3ping is similar to ovs-test, but the main difference
> is that it does not require administrator to open firewall
> holes for the XML/RPC control connection. This is achieved
> by encapsulating the Control Connection over the L3
Glanced at python, seems good
On Tue, Mar 27, 2012 at 5:00 PM, Ben Pfaff wrote:
> The previous interface was just bizarre.
>
> Signed-off-by: Ben Pfaff
> ---
> tests/test-ovsdb.py | 14 ++
>
>
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index b0e42a3..77b3a2c 1
Looks good to me, thanks
On Mon, Mar 26, 2012 at 3:45 PM, Ben Pfaff wrote:
> On Mon, Mar 26, 2012 at 02:46:19PM -0700, Reid Price wrote:
> > On Mon, Mar 26, 2012 at 1:59 PM, Ben Pfaff wrote:
> >
> > > From: Simon Horman
> > > @@ -254,12 +304,17 @@ static enum
Glanced at python. One note inline, rest seemed correct
On Mon, Mar 26, 2012 at 1:59 PM, Ben Pfaff wrote:
> From: Simon Horman
>
>
> @@ -254,12 +304,17 @@ static enum ofperr
> %s_decode(uint16_t type, uint16_t code)
> {
> switch ((type << 16) | code) {""" % name
> +found = []
This looks good to me, thanks. Few nits that don't effect correctness:
Unnecessary lines in try block, move this
path_entries.append((path, s))
to outside the block
>From a style nit perspective, there are a few line continuations and
indentations that seem non-standard. google style doesn't
On Fri, Mar 23, 2012 at 1:43 PM, Ben Pfaff wrote:
> On Fri, Mar 23, 2012 at 01:37:53PM -0700, Raju Subramanian wrote:
> > On Fri, Mar 23, 2012 at 1:28 PM, Ben Pfaff wrote:
> >
> > > On Fri, Mar 23, 2012 at 01:22:54PM -0700, Raju Subramanian wrote:
> > > > > The tree_output function is documented
; +Raju Subramanianrsubraman...@nicira.com
> Ravi Kerur ravi.ke...@telekom.com
> Reid Price r...@nicira.com
> Rob Hoesrob.h...@citrix.com
> diff --git a/utilities/bugtool/ovs-bugtool.in
> b/utilities/bugtool/ovs-bugtool.in
> index f
On Wed, Mar 21, 2012 at 3:57 PM, Ethan Jackson wrote:
> This removes some boilerplate from the bridge.
>
> Signed-off-by: Ethan Jackson
> ---
>
> It actually turned out to be less difficult than I expected to do a binary
> search so I went ahead and did it.
>
> ---
> ovsdb/ovsdb-idlc.in | 50
Looks nice, thanks
On Wed, Mar 21, 2012 at 1:48 PM, Ben Pfaff wrote:
> On Tue, Mar 20, 2012 at 06:27:31PM -0700, Ethan Jackson wrote:
> > This removes some boilerplate from the bridge.
> >
> > Signed-off-by: Ethan Jackson
>
> I like this very much. Thank you!
>
> The keys are guaranteed to be
>
> +# String Map Helpers.
> +for columnName, column in sorted(table.columns.iteritems()):
> +if not is_string_map(column):
> +continue
> +
> +print
> +print "const char *"
> +print "%(s)s_get_%(c)s_value(const struct %
and what you're suggesting. Could you
> > please give an example of a piece of code that should change?
> >
> > Based on my reading of the code, when there's an error we return a
> > non-zero errno. The errno is always an integer type. Sometimes in
> > a
One other note, are you deliberately allowing None (along with other python
expressions that are False as booleans) to equate to a 0 return code? You
may just want to check for integer type at the beginning of the function,
since
things compare across type without error (for error-free sorting pur
Diff looks good to me. I assume that vals always contains every key you care
about.
-Reid
On Jan 9, 2012, at 18:54, Justin Pettit wrote:
> ovs-monitor-ipsec wakes up when the Interface table is modified. To
> prevent needless reconfiguration, it maintains a dictionary of the
> currently im
On Wed, Dec 7, 2011 at 8:19 PM, Ben Pfaff wrote:
> On Tue, Nov 29, 2011 at 10:36:35PM -0800, Ben Pfaff wrote:
> > This commit switches from using the actual protocol values of error codes
> > internally in Open vSwitch, to using abstract values that are translated
> to
> > and from protocol value
n Thu, Oct 27, 2011 at 18:33, Reid Price wrote:
> > On Thu, Oct 27, 2011 at 5:05 PM, Ethan Jackson wrote:
> >>
> >> This patch fixes the issue and adds a test which would have caught
> >> it.
> >>
> >> Reported-by: Michael Mao
> >> Bug #
On Thu, Oct 27, 2011 at 5:05 PM, Ethan Jackson wrote:
> This patch fixes the issue and adds a test which would have caught
> it.
>
> Reported-by: Michael Mao
> Bug #8045.
@@ -50,6 +51,7 @@ NXT_FLOW_MOD: ADD table:255
> actions=bundle(symmetric_l4,60,hrw,ofport,slaves:)
> NXT_FLOW_MOD: ADD
Looks obviously correct, though you might want to fix
"Map from interface name to "
while you're there
On Fri, Sep 23, 2011 at 5:21 PM, Ethan Jackson wrote:
> Pleases pep8.
> ---
> .../usr_share_openvswitch_scripts_ovs-xapi-sync| 30
>
> 1 files changed, 24 insert
On Wed, Sep 21, 2011 at 12:30 PM, Ben Pfaff wrote:
> On Wed, Sep 21, 2011 at 11:41:58AM -0700, Reid Price wrote:
> > On Wed, Sep 21, 2011 at 10:58 AM, Ben Pfaff wrote:
> >
> > > I wish there was a simple way to just test for "iterable" and
> > > &quo
On Wed, Sep 21, 2011 at 10:58 AM, Ben Pfaff wrote:
> On Tue, Sep 20, 2011 at 04:55:36PM -0700, Reid Price wrote:
> > > for uuid_string, row_update in table_update.iteritems():
> > > -if not ovs.ovsuuid.UUID.is_valid_string(uuid_string):
> >
On Tue, Sep 20, 2011 at 5:04 PM, Ethan Jackson wrote:
> > I'm getting enough deja vu to know I've said this before =/
> > If it wasn't for your dastardly-ly informative error messages, this would
> be
> > better-written as
> > uuid = ovs.<...>.from_string(uuid_string)
> > and letting that funct
Some notes inline. One typo.
On Mon, Sep 19, 2011 at 11:18 AM, Ben Pfaff wrote:
> This class only caused unnecessary confusion. This commit changes all of
> its methods into top-level functions.
> ---
> python/ovs/db/data.py |6 ++--
> python/ovs/db/idl.py |4 +-
> python/ovs/db/ty
Text LGTM, thanks. I'm useless at grok-ing the markup, assume it is correct
=)
On Mon, Sep 19, 2011 at 1:22 PM, Ben Pfaff wrote:
> Suggestion #7370.
> Suggested-by: Reid Price
> ---
> utilities/ovs-ofctl.8.in |7 ++-
> 1 files changed, 6 insertions(+), 1 deletions
LGTM
On Fri, Sep 16, 2011 at 4:49 PM, Ethan Jackson wrote:
> The python style guide requires two newlines between top level
> definitions. This patch also removes some trailing whitespace.
> ---
> python/ovs/daemon.py | 34 +-
> 1 files changed, 29 insertions(
LGTM
On Fri, Sep 16, 2011 at 4:49 PM, Ethan Jackson wrote:
> By convention, unused arguments should be named "_" and top level
> definitions should be separated by two spaces.
> ---
> tests/test-daemon.py |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/tests/te
Seems reasonable.
If you are working entirely in python, you could switch some of this errno
passing into something more exception-based, it might look a little more
natural.
On Fri, Sep 16, 2011 at 4:49 PM, Ethan Jackson wrote:
> Pychecker complains about __read_pidfile() having too may return
Didn't look at the entire file, but the patch seems obviously correct.
On Fri, Sep 16, 2011 at 6:28 PM, Ethan Jackson wrote:
> Pychecker considers it bad style.
> ---
>
> The original version of this patch that I sent out broke the unit tests.
> Please review this version.
>
> ---
> python/ovs/
On Wed, Aug 24, 2011 at 11:59 AM, Ben Pfaff wrote:
> On Wed, Aug 24, 2011 at 12:28:00AM -0700, Reid Price wrote:
> > On Tue, Aug 23, 2011 at 2:05 PM, Ben Pfaff wrote:
> >
> > > Suggested-by: Reid Price
> > > ---
> > > lib/ovsdb-types.c |3
Modulo my replies to 02/31, 06/31, and 23/31, looks good to me, thanks!
On Tue, Aug 23, 2011 at 2:04 PM, Ben Pfaff wrote:
> These fixes are based on Reid's review of the initial Python code
> back in October:
> http://openvswitch.org/pipermail/dev/2010-October/003927.html
>
> Ben Pfaff (31):
>
On Tue, Aug 23, 2011 at 2:05 PM, Ben Pfaff wrote:
> Suggested-by: Reid Price
> ---
> lib/ovsdb-types.c |3 +--
> python/ovs/db/data.py |2 +-
> python/ovs/db/parser.py |4 ++--
> python/ovs/db/types.py |8 +++-
> 4 files changed, 7 insert
On Tue, Aug 23, 2011 at 2:05 PM, Ben Pfaff wrote:
> Suggested-by: Reid Price
> ---
> python/ovs/json.py | 10 +-
> 1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/python/ovs/json.py b/python/ovs/json.py
> index 67470fc..6362830 100644
>
On Tue, Aug 23, 2011 at 2:04 PM, Ben Pfaff wrote:
> Reported-by: Reid Price
> ---
> python/ovs/db/data.py |3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
> index bfdc11c..a5fcb76 100644
> ---
27;ve lost the message that I'm replying to, it's archived here:
> http://openvswitch.org/pipermail/dev/2010-October/003927.html
>
> On Wed, Oct 06, 2010 at 03:10:12AM -0700, Reid Price wrote:
> > +def read_pidfile(pidfile):
> > +""&quo
--- a/lib/vlog.h
+++ b/lib/vlog.h
@@ -33,7 +33,7 @@ extern "C" {
* The following log levels, in descending order of importance, are enabled
by
* default:
*
- * - EMER: Not currently used.
+ * - OFF: Not currently used.
*
* - ERR: A high-level operation or a subsystem failed. Attention i
LGTM too, not sure if you added the new empty line on purpose
On Mon, Jul 11, 2011 at 6:31 PM, Ethan Jackson wrote:
> Looks Good.
>
> Ethan
>
> On Thu, Jun 30, 2011 at 14:57, Ben Pfaff wrote:
> > ovs-bugtool's output is potentially sensitive, so it seems best not to
> > allow anyone but the own
Sorry, last note on expected behavior. One other seemingly reasonable
option
for the example below would be to delete that external id from tap, as
opposed
to just ignoring that they are different.
On Wed, Jun 22, 2011 at 9:44 PM, Reid Price wrote:
> Inline.
>
> On Wed, Jun 22, 2011 a
Inline.
On Wed, Jun 22, 2011 at 3:42 PM, Ethan Jackson wrote:
> Thanks for the review Reid, comments inline.
>
>
> > This might be slightly more obvious as
> >
> > col = 'external-ids:"%s"="%s"' % (key, value)
>
> I agree, that would be clearer. I'm just doing an indentation change
> on this
Trivial notes inline
On Wed, Jun 22, 2011 at 2:18 PM, Ethan Jackson wrote:
> Commit 400430 "xenserver: Give tap devices iface-ids." copies the
> iface-id from vifs to their related tap device. It turns out this
> is not sufficient, so this commit copies all relevant external_ids
> over.
>
> Req
Some notes in passing, please forgive any ignorance
On Thu, Jun 16, 2011 at 5:08 PM, Andrew Evans wrote:
> On Thu, 2011-06-16 at 14:12 -0700, Ben Pfaff wrote:
> > On Wed, Jun 15, 2011 at 06:55:38PM -0700, Andrew Evans wrote:
> > > When an error is encountered while parsing flows from a file, ovs
75 matches
Mail list logo
