[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [ANNOUNCE] libblkio v0.1.0 preview release
From: |
Stefan Hajnoczi |
Subject: |
Re: [ANNOUNCE] libblkio v0.1.0 preview release |
Date: |
Wed, 5 May 2021 17:19:25 +0100 |
On Tue, May 04, 2021 at 03:44:23PM +0200, Kevin Wolf wrote:
> Am 30.04.2021 um 17:49 hat Stefan Hajnoczi geschrieben:
> > On Thu, Apr 29, 2021 at 05:51:16PM +0200, Kevin Wolf wrote:
> > > Am 29.04.2021 um 16:05 hat Stefan Hajnoczi geschrieben:
> There is one more thing I'm wondering right now: Why do we have separate
> states for connecting to the backend (created) and configuring it
> (initialized)? The property setters check for the right state, but they
> don't really do anything that wouldn't be possible in the other state.
> A state machine exposed as two boolean rather than a tristate enum feels
> a bit awkward, too, but even more so if two states could possibly be
> enough.
>
> The reason why I'm asking is that in order to address my points, it
> would be best to have separate property accessors for each state, and
> two pairs of accessors would make property declarations more managable
> than three pairs.
There is no need to have separate boolean properties, it's just how I
implemented it. There could be a single state property. For example, a
string that is "uninitialized", "initialized", and "started".
> > > Alternatives in QEMU are qdev properties (which are internally QOM
> > > properties, but provide default implementations and are at least
> > > automatically read-only after realize, avoiding that whole class of
> > > bugs) and QAPI.
> > > If this was QEMU code, I would of course go for QAPI, but a library is
> > > something different and adding the code generator would probably be a
> > > bit too much anyway. But the idea in the resulting code would be dealing
> > > with native structs instead of a bunch of function calls. This would
> > > probably be the least error prone way for the implementation, but of
> > > course, it would make binary compatibility a bit harder when adding new
> > > properties.
> >
> > An alternative I considered was the typestate and builder patterns:
> >
> > /* Create a new io_uring driver in the uninitialized state */
> > struct blkio_iou_uninit *blkio_new_io_uring(void);
> >
> > /* Uninitialized state property setters */
> > int blkio_iou_uninit_set_path(struct blkio_iou_uninit *u,
> > const char *path);
> > int blkio_iou_uninit_set_direct(struct blkio_iou_uninit *u,
> > bool o_direct);
> > ...
> >
> > /* Transition to initialized state. Frees u on success. */
> > struct blkio_iou_init *blkio_iou_init(struct blkio_iou_uninit *u);
> >
> > /* Initialized state property setters/getters */
> > int blkio_iou_init_get_capacity(struct blkio_iou_init *i,
> > uint64_t *capacity);
> > ...
> >
> > /* Transition to started state. Frees i on success. */
> > struct blkio_iou_started *blkio_iou_start(struct blkio_iou_init *i);
> >
> > ...
> >
> > /* Transition back to initialized state. Frees s on success. */
> > struct blkio_iou_init *blkio_iou_stop(struct blkio_iou_started *s);
> >
> > On the plus side:
> >
> > - No state checks are needed because an API won't even exist if it's
> > unavailable in a given state (uninitialized/initialized/started).
> >
> > - State structs come with pre-initialized default values, so the caller
> > only needs to set non-default values. For example O_DIRECT is false by
> > default and callers happy with that don't need to set the property.
> >
> > - ABI compatibility is easy since the state structs are opaque (their
> > size is not defined) and new properties can be added at any time.
> >
> > On the minus side:
> >
> > - Completely static. Hard to introspect and requires a dedicated call
> > site for each property (applications cannot simply assign a property
> > string given to them on the command-line). This means every single
> > property must be explicitly coded into every application :(.
>
> How are you going to deal with this for QEMU integration, by the way?
> Put all the properties that we know into the QAPI schema and then some
> way of passing key/value pairs for the rest?
In QEMU's case let's define each property explicitly instead of passing
them through. That's due to QAPI's philosophy rather than libblkio.
> > - So many functions! This makes understanding the API harder.
> >
> > - Very verbose. The function and type names get long and there is a lot
> > of repetition in the API.
>
> I think it wouldn't be too bad if all drivers exposed the same
> properties, but you're explicitly expecting driver-specific properties.
> If drivers add an external APIs that just fail for other drivers, it
> would indeed make understanding the API much harder.
>
> We could consider a mix where you would first create a configuration
> object, then use the generic property functions to set options for it
> and finally have a separate blkio_initialize() function where you turn
> that config into a struct blkio that is needed to actually do I/O (and
> also supports generic property functions for runtime option updates).
>
> I'm not sure it provides much except making the state machine more
> prominent than just two random bool properties.
I prefer to keep the configuration public API as it is. We can change
the properties.rs implementation however we want though.
Do you think the public API should be a typestate API instead with
struct blkio_init_info, struct blkio_start_info, and struct blkio
expressing the 3 states instead?
Stefan
signature.asc
Description: PGP signature
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Kevin Wolf, 2021/05/04
- Re: [ANNOUNCE] libblkio v0.1.0 preview release,
Stefan Hajnoczi <=
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Kevin Wolf, 2021/05/05
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Stefan Hajnoczi, 2021/05/06
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Kevin Wolf, 2021/05/06
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Stefan Hajnoczi, 2021/05/06
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Stefan Hajnoczi, 2021/05/13
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Kevin Wolf, 2021/05/14
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Stefan Hajnoczi, 2021/05/17
- Re: [ANNOUNCE] libblkio v0.1.0 preview release, Kevin Wolf, 2021/05/18