On 2024/11/01 20:44, Daniel P. Berrangé wrote:
On Thu, Oct 31, 2024 at 04:21:53PM +0900, Akihiko Odaki wrote:
On 2024/10/29 1:50, Daniel P. Berrangé wrote:
On Tue, Oct 22, 2024 at 01:50:39PM +0900, Akihiko Odaki wrote:
DEFINE_PROP_ON_OFF_AUTO_BIT64() corresponds to DEFINE_PROP_ON_OFF_AUTO()
as DEFINE_PROP_BIT64() corresponds to DEFINE_PROP_BOOL(). The difference
is that DEFINE_PROP_ON_OFF_AUTO_BIT64() exposes OnOffAuto instead of
bool.
Again, same feedback as last time.
With this design, existing users of DEFINE_PROP_BIT64 that
get converted to DEFINE_PROP_ON_OFF_AUTO_BIT64, in addition
to gaining the desired "auto" value, also gain redundant
'on' and 'off' values as side-effects.
In the next patch, the stated problem is that virtio code
needs to distinguish between bits that are user set, and
bits that are set based on available host backend features.
That doesn't seem to require us to accept any new values
from the user. It should be sufficient to track user
specified features, separately from user specified values.
ie when parsing user input for bitfields, we need to parse
into a pair of fields
uint64_t has_user_features; /* which bits were specified */
uint64_t user_features; /* values of specified bits*/
Properties also have getters. We don't know what to return with them without
the new value.
The virtio changes in the next patch are just accessing the bitsets
directly. A getter could just be made to return false for unset
values, on the assumption that any caller should be checking the
has_user_features bits beforehand.
It means this construct is specialized for the case when the getter will
never be called for the default value. I think it is difficult to
convince others to introduce such a new mechanism when we already have
OnOffAuto as a generic solution. There is even DEFINE_PROP_ON_OFF_AUTO()
for qdev properties.
The question is: when we have BOOL, BIT64, and ON_OFF_AUTO, shouldn't we
have ON_OFF_AUTO_BIT64? Whether OnOffAuto is good or not is not a
problem here unless we are going to deprecate DEFINE_PROP_ON_OFF_AUTO().
Regards,
Akihiko Odaki
