openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Emit interface added signal
@ 2021-03-18 10:33 Ratan Gupta
  2021-03-18 14:57 ` Patrick Williams
  0 siblings, 1 reply; 3+ messages in thread
From: Ratan Gupta @ 2021-03-18 10:33 UTC (permalink / raw)
  To: Brad Bishop, Ed Tanous, openbmc, Patrick Williams

Hi Team,

I faced the following issue in some of the D-Bus services.

Generally D-Bus services emit the interface added signal once the D-Bus 
object gets created.

D-Bus objects can be created in two scenario

1) Actual creation of D-Bus object due to some action.

2) Restore the old D-Bus objects reading from the file system.

    e.g. Restore the dump objects / Error log objects etc.

We are having some use cases where we have to send the Rest/Redfish 
event to the external clients once the dbus object gets added and for 
the same we watch for the interface added signal.

Due to the 2) we send the false events to the external clients as the 
D-Bus service gets restarted due to any reason(BMC reboot / Genuine 
Failure).

Proposal: Don't emit the interface added signal by the D-bus service if 
we are restoring the D-Bus objects from file system.

Do we see any concern here? or any other way to fix the above issue?

Ratan












^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Emit interface added signal
  2021-03-18 10:33 Emit interface added signal Ratan Gupta
@ 2021-03-18 14:57 ` Patrick Williams
  2021-03-19  8:37   ` Ratan Gupta
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Williams @ 2021-03-18 14:57 UTC (permalink / raw)
  To: Ratan Gupta; +Cc: openbmc, Brad Bishop, Ed Tanous

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

On Thu, Mar 18, 2021 at 04:03:21PM +0530, Ratan Gupta wrote:

> Proposal: Don't emit the interface added signal by the D-bus service if 
> we are restoring the D-Bus objects from file system.
> 
> Do we see any concern here? or any other way to fix the above issue?

Yes, this is pretty dangerous.  If you do this, mapper will not know
about your objects and they will not show up in queries.  This is
certainly not what you want.  (Mapper will delete your objects from its
cache when the service exits, and not know that you have new ones
because it did not get the signal.)

This isn't just about mapper though.  Anytime you do not emit the
signals, you then require applications to poll your service for
knowledge of your objects.

In general, you should not work around an issue in one application
(bmcweb in this case) by changing your dbus behavior.  This isn't the
first time that I've seen a similar request.

Having said this, there is one option for you.  This is completely valid,
and done by other dbus services we have, and arguably more correct than
what it sounds like you're currently doing.

   - Defer the service name registration until just before entering your
     dbus processing loop.

   - Create your restored dbus objects before this service name
     registration and use the `emitSignal=false` argument to prevent
     the signal from being emitted.

This pattern is your way of indicating "these aren't new dynamic
objects, but objects I'm creating at the beginning of my service."
When mapper sees the NameOwnerChanged signal, it will query your service
for objects and add them to its cache.  `bmcweb` won't see the
ObjectManager.InterfacesAdded signals for these static / restored
objects, but it will see them for new / dynamic objects.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Emit interface added signal
  2021-03-18 14:57 ` Patrick Williams
@ 2021-03-19  8:37   ` Ratan Gupta
  0 siblings, 0 replies; 3+ messages in thread
From: Ratan Gupta @ 2021-03-19  8:37 UTC (permalink / raw)
  To: Patrick Williams; +Cc: openbmc, Brad Bishop, Ed Tanous

Thanks Patrick for your response.

On 3/18/21 8:27 PM, Patrick Williams wrote:
> On Thu, Mar 18, 2021 at 04:03:21PM +0530, Ratan Gupta wrote:
>
>> Proposal: Don't emit the interface added signal by the D-bus service if
>> we are restoring the D-Bus objects from file system.
>>
>> Do we see any concern here? or any other way to fix the above issue?
> Yes, this is pretty dangerous.  If you do this, mapper will not know
> about your objects and they will not show up in queries.  This is
> certainly not what you want.  (Mapper will delete your objects from its
> cache when the service exits, and not know that you have new ones
> because it did not get the signal.)
>
> This isn't just about mapper though.  Anytime you do not emit the
> signals, you then require applications to poll your service for
> knowledge of your objects.
>
> In general, you should not work around an issue in one application
> (bmcweb in this case) by changing your dbus behavior.  This isn't the
> first time that I've seen a similar request.

My plan was not to modify anything in bmcweb, My plan was exactly as you 
described below

The dbus applications which is restoring the Dbus objects from the file 
system needs to be modified if they are restoring the Dbus objects from 
the file system and that was the intent to send this mail as the changes 
lies in other repo.

e.g. 
https://github.com/openbmc/phosphor-debug-collector/blob/master/dump_manager_main.cpp#L60


>
> Having said this, there is one option for you.  This is completely valid,
> and done by other dbus services we have, and arguably more correct than
> what it sounds like you're currently doing.
>
>     - Defer the service name registration until just before entering your
>       dbus processing loop.
>
>     - Create your restored dbus objects before this service name
>       registration and use the `emitSignal=false` argument to prevent
>       the signal from being emitted.
>
> This pattern is your way of indicating "these aren't new dynamic
> objects, but objects I'm creating at the beginning of my service."
> When mapper sees the NameOwnerChanged signal, it will query your service
> for objects and add them to its cache.  `bmcweb` won't see the
> ObjectManager.InterfacesAdded signals for these static / restored
> objects, but it will see them for new / dynamic objects.
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-19  8:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 10:33 Emit interface added signal Ratan Gupta
2021-03-18 14:57 ` Patrick Williams
2021-03-19  8:37   ` Ratan Gupta

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).