connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Jussi Laakkonen <jussi.laakkonen@jolla.com>
To: Daniel Wagner <wagi@monom.org>
Cc: connman@lists.linux.dev
Subject: Re: [PATCH v2 0/2] Implement connmand state monitoring to vpnd
Date: Tue, 31 Aug 2021 10:00:40 +0300	[thread overview]
Message-ID: <a0d28b35-d3d0-8454-c1ab-159c6ccd728f@jolla.com> (raw)
In-Reply-To: <20210830170631.ni5frzfk6v6jn7s6@beryllium.lan>

Hi Daniel,

On 8/30/21 8:06 PM, Daniel Wagner wrote:
> Hi Jussi,
> 
> On Mon, Aug 30, 2021 at 12:24:03PM +0300, Jussi Laakkonen wrote:
>> Implement connmand state monitoring into vpnd to keep track of the state
>> changes in regards to connectivity and D-Bus registration. vpnd retrieves the
>> state from connmand using Manager interface when started and otherwise it
>> monitors PropertyChanged signals as well as utilizes D-Bus service monitoring
>> to get the changes.
>>
>> This change prevents connecting VPNs when connmand is not yet online or has
>> gone away from D-Bus (e.g., restart). In case VPN is connected before the state
>> has been queried a delayed connection function is utilized to connect that VPN
>> when online/ready state is reached.
>>
>> In case connmand is offline or gone from D-Bus ENOLINK is returned as an error.
>> This is handled in plugins/vpn.c and returned back to the caller as NoCarrier
>> D-Bus error message.
>>
>> Changes since V2:
>>   - Updated vpn-provider patch to v3: cleanups, reformat, remove agent checks.
>>   - Updated vpn patch to v2: do handle ENOLINK properly and reformat.
>>
>> Jussi Laakkonen (2):
>>    vpn-provider: Implement connmand online state checking
>>    vpn: Refactor connect_reply() and handle NoCarrier ->  ENOLINK error
> 
> I fixed up some small whitespace issues in the first patch and dropped
> the else block in do_connect(). Also added the 'void' argument to
> get_connman_state() (all complains from checkpatch).
> 

Ok, that is cool. Apparently my git did not find any whitespace issues 
with the patches as I do check that prior to sending, hmm.

> Also I had to fallback to my old workflow and cherrypick the patches. b4
> was not happy with the thread:
> 
>    Grabbing thread from lore.kernel.org/connman/20210830092405.30964-2-jussi.laakkonen%40jolla.com/t.mbox.gz
>    Will use the latest revision: v3
>    You can pick other revisions using the -vN flag
>    Checking attestation on all messages, may take a moment...
>    ---
>      [PATCH v3 1/2] vpn-provider: Implement connmand online state checking
>      ERROR: missing [2/2]!
>    ---
> 
> Anyway, I gave it a quick test, all looks good. Patches applied.
> 

Great, thanks. I'm curious, what kind of workflow you have now and what 
I should take account for prior to sending the patches. Does the b4 
require that all of those patches are in the same thread or otherwise 
reset the versioning when re-sending the patches as a new set?

Also, it wouldn't hurt to download Linus' checkpatch.pl :)

Cheers,
  Jussi

  reply	other threads:[~2021-08-31  7:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30  9:24 Jussi Laakkonen
2021-08-30  9:24 ` [PATCH v3 1/2] vpn-provider: Implement connmand online state checking Jussi Laakkonen
2021-08-30  9:24 ` [PATCH v2 2/2] vpn: Refactor connect_reply() and handle NoCarrier -> ENOLINK error Jussi Laakkonen
2021-08-30 17:06 ` [PATCH v2 0/2] Implement connmand state monitoring to vpnd Daniel Wagner
2021-08-31  7:00   ` Jussi Laakkonen [this message]
2021-08-31  7:22     ` Daniel Wagner
2021-08-31 10:29       ` Jussi Laakkonen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0d28b35-d3d0-8454-c1ab-159c6ccd728f@jolla.com \
    --to=jussi.laakkonen@jolla.com \
    --cc=connman@lists.linux.dev \
    --cc=wagi@monom.org \
    --subject='Re: [PATCH v2 0/2] Implement connmand state monitoring to vpnd' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).