openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Williams <patrick@stwcx.xyz>
To: Ed Tanous <ed@tanous.net>
Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Ratan Gupta <ratagupt@linux.vnet.ibm.com>
Subject: Re: Using bios-settings-mgr for setting hypervisor network attributes
Date: Wed, 23 Sep 2020 16:26:45 -0500	[thread overview]
Message-ID: <20200923212645.GU6152@heinlein> (raw)
In-Reply-To: <CACWQX83TAW8TfAUaNSkO7UA0VrYKjut8uFnd6pF3RgcJm_EDrA@mail.gmail.com>

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

On Wed, Sep 23, 2020 at 01:51:33PM -0700, Ed Tanous wrote:
> On Wed, Sep 23, 2020 at 12:24 PM Patrick Williams <patrick@stwcx.xyz> wrote:
> >
> > On Tue, Sep 22, 2020 at 02:39:04PM +0530, Ratan Gupta wrote:
> >
> > It is unfortunate that org.freedesktop.DBus.Properties doesn't have a
> > way to set multiple properties as the analogous operation to 'GetAll'.
> 
> It was proposed we (OpenBMC) add one while back.  I think it muddies
> the water of what it means to be a method call, and what it means to
> be a property, especially for the use case that it was being proposed
> to cover.

I'm not sure why it would be considered mudding the water.  All property
Get/Set/GetAll operations really are just a method call under the covers
anyhow to org.freedesktop.DBus.Properties.  I do think that ideally we'd
get the method added directly to that interface because then the DBus
bindings will support it natively.

I forgot the mention this again, but another way to solve it is similar
to xyz.openbmc_project.Inventory.Manager where you take a fully (or
partially) formed object as a method parameter and the process which
hosts Inventory.Manager hosts the object.  Settings could be done the
same way.  The issue is, again, having other processes know when to use
this new method and when to just update properties.

> > When all of our DBus objects were serial we likely never had this issue
> > because the request to read the properties (to send to the hypervisor)
> > would come behind the signal and subsequent property updates.  Now that
> > we're moving towards more ASIO we likely will see this kind of issue
> > more often.  I don't like it but we could certainly proposal a
> > 'SetMultiple' extension to org.freedesktop or create our own interface.
> 
> If you have properties that need to be set in lockstep with one
> another to be valid, I suspect that indicates that properties are not
> the right tool.  Redfish hits this a lot, where each resource is
> expected that any property is modifiable independently, and certain
> implementations need an atomic "unit" of update.  bmcweb doesn't want
> to have to cache properties that are collectively invalid right now,
> but might become valid in the future, so there's an impasse.  Who
> keeps the state while it's invalid?  Thus Far, that falls to the
> dbus-daemons to store.

Agreed.  This has also been a general statement  we've made in reviews
for new interfaces.  "If you need to update multiple properties, use
a method; if you are just updating a single property, update the property."

> > We could define an interface to implement something like Proposal #1,
> > but we would need a new interface and not a property we tack onto
> > existing interfaces.  We'd probably need to revisit a lot of our
> > interface definitions and see which ones typicallly have multi-property
> > updates and does an intermediate state leave us in a bad situation.
> >
> > Specifically for BIOS/Hypervisor settings, I mentioned before that it
> > isn't clear to me what the proposal is for applying Pending to Current.
> > Again, this isn't general, but we could define an interface specific for
> > BIOS/Hypervisor settings which has a way to indicate 'Pending
> > transaction is complete' (set by entities like Redfish) and 'Pending
> > values applied to Current' (set by entities like PLDM).  For the current
> > settings-style values though, this requires external interfaces to
> > somehow know that the setting is associated with the Host in order to do
> > the application, since BMC-owned properties won't have or need this.
> 
> Dumb question: Does anyone actually need to know the "current" value?
> Redfish certainly would need to return  the "pending" value in all
> cases, as it's required so the restful API emulates ACID-like
> compliance to the user.  Could we just have an optional interface that
> indicates "values might not be loaded yet" and simplify the dbus API a
> little?

I think this is generally for humans in the case of BIOS settings.
   - "What is the setting my system is currently running with?"
   - "What will happen next time I reboot?"

I don't know how this is modeled in Redfish.

-- 
Patrick Williams

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

  reply	other threads:[~2020-09-23 21:28 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16 14:47 Using bios-settings-mgr for setting hypervisor network attributes manoj kiran
2020-09-16 16:26 ` Ed Tanous
2020-09-16 16:33   ` James Feist
2020-09-16 17:20 ` Patrick Williams
2020-09-16 17:44   ` Ed Tanous
2020-09-17  7:40     ` Ratan Gupta
2020-09-17 12:21       ` Deepak Kodihalli
2020-09-17 14:20       ` Thomaiyar, Richard Marian
2020-09-17 15:36       ` Patrick Williams
2020-09-19  5:41         ` Ratan Gupta
2020-09-22  9:09           ` Ratan Gupta
2020-09-22 12:08             ` Deepak Kodihalli
2020-11-05 16:48               ` Brad Bishop
2020-09-23 19:24             ` Patrick Williams
2020-09-23 20:51               ` Ed Tanous
2020-09-23 21:26                 ` Patrick Williams [this message]
2020-09-24 13:08                   ` Deepak Kodihalli
2020-09-24 15:36                   ` Ed Tanous
2020-09-30 15:05                     ` Ratan Gupta
2020-09-30 15:56                       ` Ed Tanous
2020-10-01 11:17                         ` Ratan Gupta
2020-10-16 11:40                           ` manoj kiran
2020-10-20 10:43                             ` Ratan Gupta
2020-09-24  7:30               ` Ratan Gupta

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=20200923212645.GU6152@heinlein \
    --to=patrick@stwcx.xyz \
    --cc=ed@tanous.net \
    --cc=openbmc@lists.ozlabs.org \
    --cc=ratagupt@linux.vnet.ibm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).