Re: [Evms-devel] EVMS engine

[Steve Dobbelstein]

Andrew Claussen wrote:

> Hi all,
>
> I've been really busy with Uni, so I thought it would be better
> to read EVMS code than coming to the conference call without
> knowing what's happening.  So, I've been doing that ;-)

Thanks a lot for looking at the code and giving us feedback.  I was
beginning to wonder if anyone out there cared much about our Engine
work.  ;-)  Your involvement is appreciated.

> First, I really don't like the layout of evms-0.1.0/engine/include.
> IMHO, it should be arranged in terms of objects.  There should be
> storagecontainer.h, etc.  rather than evms_structs.h.
> storagecontainer.h would have all structs and functions relevant
> to storage containers.

The structure of the headers may not be apparent, especially since they
lack comments.  :(  Allow me to explain the current structure.  (My
apologies if you discerned this already.  At least it will document it
in the mailing list.)

At the bottom level we have files that do the real work.

evms_defines.h Basic #defines and macros useful for all of the Engine
evms_datatypes.h    Basic typdefs, data structures, enums, used in the
Engine
evms_plugin_IDs.h   Definitions for plug-in IDs and macros to manipulate
them

evms_appAPI.h  Function prototypes for the APIs exported by the Engine
evms_appstructs.h   Structures needed for the Engine's external API

evms_structs.h Structures used internally in the Engine and by plug-ins
evms_plugfucns.h    Function prototypes for the interfaces between the
Engine
               and the plug-ins

evms_frontend.h     Top level header file that can be included by any user
of
               the Engine's external API.  It includes all the header files
               that an Engine user would need.

evms_plugin.h  Top level header file that can be used by a plug-in.  It
               includes all the header files that a plug-in would need.

evms_engine.h  Top level header file for the Engine itself.  It basically
               includes evms_frontend.h and evms_plugin.h since the Engine
               has to provide the external API and talk to the plug-ins.

We debated on whether to keep prototypes and the structures they need in
the
same file or to specify functions in one file and structures in another.
In
the end we decided to keep the functions and structures in separate files,
although there was not any strong reason to do so.  It seemed more
convenient
to keep them separate so that a change to a structure or a change to a
prototype
would only hit one file and be more localized and a bit more obvious as to
what
changed.  A weak reason, yes.  On the other hand, having the prototypes and
structures in one file keeps the entire interface definition in one file.
If you need to see what changed in the latest drop there is always diff.
We're open on how to structure the header files.

I'm not sure I like the idea of breaking out all the structures into
separate
header files (e.g. one header file for disk segments, one header file for
storage regions, etc.) if that is what you are advocating.  That would make
a
lot of little files.  Most users of the Engine, applications or plug-ins,
would need most of those files anyway.  The top level file would include
them
all.  Yes, it would isolate the things pertaining to different structures
to
different files which might aid in understanding and maintenance.  On the
other
hand , having the structures in one file give the user one place to look
and
one file to maintain.

I now realize that I have used on set of reasons to argue for keeping the
prototypes and structures in separate files and yet used the counter
arguments
for keeping all the structures in one file.  Needless to say, this is a
work
in progress.  I'm happy to change to code to be consistent and provide the
best
structure.

> BTW, prefixing headers with evms is superfluous, because you can make
> the direct evms, (or evms/engine), and set -I to the directory
> under evms (include).  This is also nice, because when you install
include/evms/* into
> /usr/include, everyone can
>
>          #include <evms/storagecontainer.h>
>
> without any magic.

Agreed.  I'm getting tired of typing "evms_" in front of all the header
files anyway.  ;-)

> Anyway, back to objects.  Functions should almost always act
> on objects.  (Exceptions: constructors - which create objects,
> and things that act globally, which should be rare.  Eg:
> error handling).
>
> This should be made explicit.
>
> For example, take a look at this:
>
>          int Shrink(Object_Handle thing,
>                     DLIST plugin_init_buffers);
>
> What kind of objects does Shrink act on?  Shrinking involves
> changing the sizes of regions, right?  So, how is this communicated
> (the new size?  or is it a (start, end) tuple?  What's an init buffer?)
>
> The prototype says nothing about what it does.

True.  The files could use some comments to explain what the function does.
In this
particular case, the first parameter, "thing", is expressly kept nebulous
because
Shrink could be invoked on a variety of things -- a volume, an object, a
region...
The Engine knows what the thing is by nature of the handle and will invoke
the
appropriate internal functions.  We should have comments to explain that.

The would plugin-init-buffers need explaining too, but I believe that
parameter
is going away as we do some redesigning as we take into account how the UIs
will
interface with the Engine.

> Compare this to a similar function in libparted:
>
> int ped_file_system_resize (PedFileSystem* fs, const PedGeometry* geom);
>
> It's obvious what's going on here.  And if you don't know what a
> PedGeometry is, it's easy to find out.  (It's a (disk, start, end)
> tuple)
>
> So, my complaint is: the design isn't apparant in the headers,
> and it should be.
>
> More random stuff:
> * from "struct logical_volume", there is:
>
>          Plugin_Record            * file_system_manager;
>
> Don't you think it should be an FS_Plugin?  If the volume
> knows that it's a file system it has under it, then it
> should know how to do operations on it.  Presumebly,
> the filesystem provides special operations that other
> plugins don't provide (and vice-versa... it doesn't
> provide other stuff).

The EVMS design has a thing called a Filesystem Interface Module
(FIM or FSIM.  I'm not sure if we have settled on which acronym.
FIM is easier to say.  FSIM is more explicit.).  The FSIM is an
EVMS plug-in.  It is not the filesystem itself.  The FSIM plugs
into EVMS and knows how to tell the filesystem to do the things
requested by EVMS.  EVMS does not talk to filesystems.  The
file_system_manager in struct logical_volume really is a pointer
to a Plugin_Record since FSIMs are EVMS plug-ins.

> * I think using Handles are fine, but I suggest you
> typedef Handles to each type.  eg:
>
>          LogicalVolumeHandle
>
> very self documenting ;-)

Good idea.

> * flags should be enums, not #define's, for the same
> reason.  (As a bonus, gdb can present you the meaning
> of the enums, not just the values ;-)

The problem with making them enums is that you can't OR them together
unless you give explicit values to the enum elements.  If you define the
values for the enum elements, that's a lot like using #defines, except
that you can also get type checking if you want.  To me, the concept of
ORing enum values doesn't seem to be an appropriate use of an enum.
An enum is a list of states for a variable.  I wouldn't expect to combine
two states together, e.g.,

engine mode = EVMS_Mode_closed | EVMS_Mode_readonly

doesn't make sense.  Maybe programmers are using enums for flags nowadays,
but it looks odd to me.

(Will gdb give you the list of enum values if multiple enum elements are
ORed into the flags variable?)

> * I'm not sure I like your disk segment abstraction.
> I think their should be an abstract logical segment,
> separated from partitions, or whatever.
>
> i.e. keep your existing abstraction, but make it
> use another abstraction.
>
> Basically, like what I did with PedGeometry and PedPartition:
>
> struct _PedGeometry {
>         PedDisk*                disk;
>         PedSector               start;
>         PedSector               length;
>         PedSector               end;
> };
>
> struct _PedPartition {
>         PedPartition*           prev;
>         PedPartition*           next;
>
>         PedGeometry             geom;
>         int                     num;
>
>         PedPartitionType        type;
>         const PedFileSystemType* fs_type;
>         PedPartition*           part_list;      /* for extended
partitions */
>
>         void*                   disk_specific;
> };
>
> I've found it really useful to separate these out.  For
> example, when you deal with constraints, you only care about
> the numbers (and the fact that you're on the same device).
>
> I'm not sure how your scheme maps onto partitions, but
> I suspect you'll want to share a PedGeometry equivalent,
> but not the "partition".
>
> Perhaps it would be best to make it polymorphic, so that
> Segments == PedGeometries, and Regions use (abstract) segments.
> And partitions, and whatever LVMS use are types of segments.
> (i.e. a derived class, in OO speak)
>
> But, I'm still not sure how you plan the existing system to
> fit together.  Can you please explain how a normal partitioned
> system would look (I think I get this one...), and how an EVMS
> / LVM system looks?  What about an LVM system, built on partition
> tables?

The Engine design is a little complex to explain in a paragraph or
two.  Let me try to address it fully in a separate post.  Ben Rafanello
wrote up a post describing the basic architecture.  It should be on
the old EVMS mailing list at address@hidden  I'll make a
few statements here that may help address your comments.

The design is a merger of the partitioned view of the world and the
LVM view of the world.  The structure is supposed to accommodate both
views.

In the Engine lingo, a "segment" is a set of physically contiguous
sectors on a disk.  It is analogous to a partition.  A "region" is
a set of logically contiguous sectors.

For segments (partitions)that are volumes, there is a region that
maps to the segment.

For LVM, the LVM "region manager" takes in segments and produces
regions.


> Thanks,
> Andrew Clausen
>
> P.S. do you think you will be able to use libparted for partition
> table stuff?  I have a bit of a dilemma here:
> * I'd like to continue to develop Parted (for the moment)
> * I'd like libparted and the EVMS code to be shared.
>
> I can't see any alternative, but to fork libparted (i.e.
> cut&paste code from libparted into EVMS), and keep porting
> changes back&forth, until libparted is obsolete.

I think it would be great to get some of the libparted technology
into EVMS.  No sense reinventing the wheel.  The trick, as you
noted, is to get EVMS to functionally do what libparted does so
that users can migrate from libparted to EVMS when EVMS is up and
running.

> I think writing wrappers is ugly.  Maybe it is a good interim
> solution, though.  (We can port when EVMS is is "ready")
>
> Perhaps this could be discussed on IRC?  (I generally
> prefer IRC to telephone conferences, BTW, because you can
> have more than issue happening at once.  I find I often
> can't get a word in on telephone calls, which means sometimes
> small ideas get missed.  This is made worse by the fact that
> you can't hear anything when you're talking, so you can't
> interrupt - or get interupted.  Very annoying)
>
> I'm usually on #parted on irc.openprojects, just timezones...
> Anyway, it would be good if all EVMS people hung out on #evms,
> whenever available ;-)
>
> _______________________________________________
> Evms-devel mailing list
> address@hidden
> http://lists.sourceforge.net/lists/listinfo/evms-devel

Steve Dobbelstein