Re: [PATCH 02/10] fdisk: API: add to label operations to context

[Karel Zak <kzak@redhat.com>]

On Tue, Jul 24, 2012 at 11:41:31AM +0200, Petr Uzel wrote:
> On Sun, Jul 22, 2012 at 07:05:01PM +0200, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso <dave@gnu.org>
> > 
> > The context structure is the fdisk API's main data type as it keeps all data together. Add the
> > label structure to it, so that the pt-specific operations can be called from the context.

 Applied with changes, thanks.

> > The internal probing function is updated so that if a label is not probed, for example when a
> > disk is not formated, it will default to use either sun or dos label operations. This avoids

 This is not so simple. You have to apply geometry and another
 settings (sector size, ...) from user *before* you create a new disk
 label.  (See the original code, get_boot() and get_geometry()
 functions.)

 It means that we should not create any default disklabel in
 fdisk_new_context_from_filename(). I think it will be better to make
 the API less complex and split this task into two steps:

    cxt = fdisk_new_context_from_filename(devname, 0);

    /* --- here apply your setting to the new context -- */

    if (!fdisk_dev_has_disklabel(cxt))
        fdisk_create_disklabel(cxt, NULL);      /* NULL means default */


 It should be our common rule that the API is well fragmented to small
 usable functions.

 Maybe one day we will create

     fdisk_new_context()
     fdisk_context_assign_device()

 because the current fdisk_new_context_from_filename() is already too
 complex for some scenarios -- for example if you want to add some
 callbacks to the context.  We will see ;-)

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com