Re: [ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread Aaron Conole
Frode Nordahl  writes:

> On Fri, Nov 22, 2019 at 11:46 AM Frode Nordahl
>  wrote:
>>
>> On Fri, Nov 22, 2019 at 11:12 AM Frode Nordahl
>>  wrote:
>> >
>> > This is weird, the patches apply on an up-to-date ovn repo with git am
>> > on blobs from the list.
>> >
>> > I noticed the mailing list thread order is in reverse with patch
>> > number two preceding patch number one, could that be the issue?
>>
>> It may also be me editing the output from ``git format-patch`` prior to
>> submitting, but cannot reproduce the error reported by the bot locally
>> with blobs gleaned from list, so would love to hear your views on
>> what's going on here.
>
> Ah yes that must be it, the commit msg is part of the commit sha1
> and apparently I have tampered with it, so this is my bad.
>
> Sorry about the noise, I'll submit an untampered v2.

Glad it worked out :)

In the future, you can try and grab the patch from the patchwork
instance (https://patchwork.ozlabs.org/project/openvswitch/) and check
that it matches and applies.

I'll try to improve the output of the bot as well to include base commit
information (so at least we can try to reproduce the errors).

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread Frode Nordahl
On Fri, Nov 22, 2019 at 11:46 AM Frode Nordahl
 wrote:
>
> On Fri, Nov 22, 2019 at 11:12 AM Frode Nordahl
>  wrote:
> >
> > This is weird, the patches apply on an up-to-date ovn repo with git am
> > on blobs from the list.
> >
> > I noticed the mailing list thread order is in reverse with patch
> > number two preceding patch number one, could that be the issue?
>
> It may also be me editing the output from ``git format-patch`` prior to
> submitting, but cannot reproduce the error reported by the bot locally
> with blobs gleaned from list, so would love to hear your views on
> what's going on here.

Ah yes that must be it, the commit msg is part of the commit sha1
and apparently I have tampered with it, so this is my bad.

Sorry about the noise, I'll submit an untampered v2.

-- 
Frode Nordahl

> --
> Frode Nordahl
>
> > --
> > Frode Nordahl
> >
> > On Fri, Nov 22, 2019 at 10:59 AM 0-day Robot  wrote:
> > >
> > > Bleep bloop.  Greetings Frode Nordahl, I am a robot and I have tried out 
> > > your patch.
> > > Thanks for your contribution.
> > >
> > > I encountered some error that I wasn't expecting.  See the details below.
> > >
> > >
> > > git-am:
> > > fatal: sha1 information is lacking or useless (tests/ovn-northd.at).
> > > Repository lacks necessary blobs to fall back on 3-way merge.
> > > Cannot fall back to three-way merge.
> > > Patch failed at 0001 northd: Improve handling of pause and resume
> > > The copy of the patch that failed is found in:
> > >
> > > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
> > > When you have resolved this problem, run "git am --resolved".
> > > If you prefer to skip this patch, run "git am --skip" instead.
> > > To restore the original branch and stop patching, run "git am --abort".
> > >
> > >
> > > Please check this out.  If you feel there has been an error, please email 
> > > acon...@redhat.com
> > >
> > > Thanks,
> > > 0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread Frode Nordahl
On Fri, Nov 22, 2019 at 11:12 AM Frode Nordahl
 wrote:
>
> This is weird, the patches apply on an up-to-date ovn repo with git am
> on blobs from the list.
>
> I noticed the mailing list thread order is in reverse with patch
> number two preceding patch number one, could that be the issue?

It may also be me editing the output from ``git format-patch`` prior to
submitting, but cannot reproduce the error reported by the bot locally
with blobs gleaned from list, so would love to hear your views on
what's going on here.

-- 
Frode Nordahl

> --
> Frode Nordahl
>
> On Fri, Nov 22, 2019 at 10:59 AM 0-day Robot  wrote:
> >
> > Bleep bloop.  Greetings Frode Nordahl, I am a robot and I have tried out 
> > your patch.
> > Thanks for your contribution.
> >
> > I encountered some error that I wasn't expecting.  See the details below.
> >
> >
> > git-am:
> > fatal: sha1 information is lacking or useless (tests/ovn-northd.at).
> > Repository lacks necessary blobs to fall back on 3-way merge.
> > Cannot fall back to three-way merge.
> > Patch failed at 0001 northd: Improve handling of pause and resume
> > The copy of the patch that failed is found in:
> >
> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
> > When you have resolved this problem, run "git am --resolved".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> >
> > Please check this out.  If you feel there has been an error, please email 
> > acon...@redhat.com
> >
> > Thanks,
> > 0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread Frode Nordahl
This is weird, the patches apply on an up-to-date ovn repo with git am
on blobs from the list.

I noticed the mailing list thread order is in reverse with patch
number two preceding patch number one, could that be the issue?

-- 
Frode Nordahl

On Fri, Nov 22, 2019 at 10:59 AM 0-day Robot  wrote:
>
> Bleep bloop.  Greetings Frode Nordahl, I am a robot and I have tried out your 
> patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> git-am:
> fatal: sha1 information is lacking or useless (tests/ovn-northd.at).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 northd: Improve handling of pause and resume
> The copy of the patch that failed is found in:
>
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
> Please check this out.  If you feel there has been an error, please email 
> acon...@redhat.com
>
> Thanks,
> 0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread 0-day Robot
Bleep bloop.  Greetings Frode Nordahl, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
fatal: sha1 information is lacking or useless (tests/ovn-northd.at).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 northd: Improve handling of pause and resume
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn 2/2] northd: Improve handling of pause and resume

2019-11-22 Thread Frode Nordahl
Move paused state to ``struct northd_context`` and pass the
context to paused and status command handlers.

On pause release the OVSDB lock on SB DB.

Re-instante the lock on resume.

Status command will now provide accurate information for 'paused',
'active' and 'standby' states.

Merge separate status command test into the pause and resume tests.

Signed-off-by: Frode Nordahl 
---
 northd/ovn-northd.8.xml |  9 +++--
 northd/ovn-northd.c | 87 +++--
 tests/ovn-northd.at | 24 +++-
 3 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 9734e79e6..c6d5d96b9 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -74,13 +74,15 @@
   pause
   
 Pauses the ovn-northd operation from processing any Northbound and
-Southbound database changes.
+Southbound database changes.  This will also instruct ovn-northd to
+drop any lock on SB DB.
   
 
   resume
   
 Resumes the ovn-northd operation to process Northbound and
-Southbound database contents and generate logical flows.
+Southbound database contents and generate logical flows.  This will
+also instruct ovn-northd to aspire for the lock on SB DB.
   
 
   is-paused
@@ -91,7 +93,8 @@
   status
   
 Prints this server's status.  Status will be "active" if ovn-northd has
-acquired OVSDB lock on NB DB, "standby" otherwise.
+acquired OVSDB lock on SB DB, "standby" if it has not or "paused" if
+this instance is paused.
   
   
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index a943e1037..e19515d14 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -65,6 +65,7 @@ struct northd_context {
 struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name;
 struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp;
 struct ovsdb_idl_index *sbrec_ip_mcast_by_dp;
+bool paused;
 };
 
 /* An IPv4 or IPv6 address */
@@ -10836,6 +10837,8 @@ add_column_noalert(struct ovsdb_idl *idl,
 ovsdb_idl_omit_alert(idl, column);
 }
 
+#define OVN_NORTHD_SB_DB_LOCK_NAME "ovn_northd"
+
 int
 main(int argc, char *argv[])
 {
@@ -10843,8 +10846,10 @@ main(int argc, char *argv[])
 struct unixctl_server *unixctl;
 int retval;
 bool exiting;
-bool paused;
 bool had_lock;
+struct northd_context ctx;
+
+memset(, 0, sizeof(ctx));
 
 fatal_ignore_sigpipe();
 ovs_cmdl_proctitle_init(argc, argv);
@@ -10866,11 +10871,11 @@ main(int argc, char *argv[])
 exit(EXIT_FAILURE);
 }
 unixctl_command_register("exit", "", 0, 0, ovn_northd_exit, );
-unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, );
-unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, );
+unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, );
+unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, );
 unixctl_command_register("is-paused", "", 0, 0, ovn_northd_is_paused,
- );
-unixctl_command_register("status", "", 0, 0, ovn_northd_status, _lock);
+ );
+unixctl_command_register("status", "", 0, 0, ovn_northd_status, );
 
 daemonize_complete();
 
@@ -11075,23 +11080,21 @@ main(int argc, char *argv[])
 /* Ensure that only a single ovn-northd is active in the deployment by
  * acquiring a lock called "ovn_northd" on the southbound database
  * and then only performing DB transactions if the lock is held. */
-ovsdb_idl_set_lock(ovnsb_idl_loop.idl, "ovn_northd");
+ovsdb_idl_set_lock(ovnsb_idl_loop.idl, OVN_NORTHD_SB_DB_LOCK_NAME);
 
 /* Main loop. */
 exiting = false;
-paused = false;
+ctx.paused = false;
 had_lock = false;
 while (!exiting) {
-if (!paused) {
-struct northd_context ctx = {
-.ovnnb_idl = ovnnb_idl_loop.idl,
-.ovnnb_txn = ovsdb_idl_loop_run(_idl_loop),
-.ovnsb_idl = ovnsb_idl_loop.idl,
-.ovnsb_txn = ovsdb_idl_loop_run(_idl_loop),
-.sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name,
-.sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp,
-.sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp,
-};
+if (!ctx.paused) {
+ctx.ovnnb_idl = ovnnb_idl_loop.idl;
+ctx.ovnnb_txn = ovsdb_idl_loop_run(_idl_loop);
+ctx.ovnsb_idl = ovnsb_idl_loop.idl;
+ctx.ovnsb_txn = ovsdb_idl_loop_run(_idl_loop);
+ctx.sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name;
+ctx.sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp;
+ctx.sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp;
 
 if (!had_lock && ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
 VLOG_INFO("ovn-northd lock