On Tue, 2012-07-24 at 13:22 +0200, Karel Zak wrote: > 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@xxxxxxx> > > > > > > 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.) Hmm, well the original get_boot() did most of the work that fdisk_new_context_from_filename() currently does - thanks for getting rid of it BTW :) Since label probing is the last action taken in this function, wouldn't everything be set already? Perhaps I'm missing something. > > 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. Ok, I was trying to make it easier for users by doing most of the work when calling fdisk_new_context_from_filename() - you're right about loosing flexibility though. > > 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 ;-) Good point, I agree. Thanks for reviewing! Davidlohr -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
