On Tue, Oct 18, 2016 at 10:38 AM, Ed Bartosh <ed.bart...@linux.intel.com> wrote:
> On Tue, Oct 18, 2016 at 10:37:22AM +0200, Maciej Borzęcki wrote:
>> On Tue, Oct 18, 2016 at 9:31 AM, Ed Bartosh <ed.bart...@linux.intel.com> 
>> wrote:
>> > On Mon, Oct 17, 2016 at 04:46:00PM +0200, Maciej Borzęcki wrote:
>> >> On Mon, Oct 17, 2016 at 3:22 PM, Ed Bartosh <ed.bart...@linux.intel.com> 
>> >> wrote:
>> >> > Hi Maciej,
>> >> >
>> >> > There is already --size and --extra-space options.
>> >> > Can we get the same or similar result by just using them? Do we really
>> >> > need new option for similar purpose?
>> >>
>> >> --reserved-size serves a different purpose, it establishes an upper
>> >> bound on the size of a partition during layout. Unlike
>> >> --size/--extra-space does not depend on the size of the filesystem
>> >> image.
>> >>
>> >> For instance, assume I'm creating an image for SD card/eMMC with a
>> >> fixed partition layout (something simple: boot partition, primary &
>> >> secondary rootfs partitions, some data partition). Because future
>> >> system updates are delivered as filesystem image, I want to make sure
>> >> that there is exactly xxx MBs for my current and future rootfs images
>> >> (regardless of current image size). Neither --size nor --extra-space
>> >> can do that. I could use, say `--size 200 --overhead-factor 1`, but
>> >> this will needlessly create a 200MB rootfs image and if I happen to
>> >> cross the 200MB boundary I will not get an error.
>> >>
>> >> I had a private patch that added --fixed-size to enforce --size, but
>> >> that would still end up creating filesystem image to fill the whole
>> >> space.
>> > I didn't get the difference between enforcing partition size and below
>> > implementation. Can you elaborate a bit?
>>
>> `--fixed-size` was something that I had added to my fork back in 2014,
>> even before `--overhead-factor` came in. The problem is that depending
>> on a project you might want to have more control over how partitions
>> are laid out, or even need to have a fixed layout. Adding
>> `--fixed-size` would had a similar effect to what `--overhead-factor
>> 1` does right now. Combined with `--size` would ensure that rootfs is
>> say, 200MB large. The downside was that wic would actually create a
>> 200MB rootfs, something that is not really necessary. In fact, I only
>> wanted to have 200MB gap so that I have some spare space for future
>> updates (where update is just a rootfs image you dd to the partition).
>>
> Thanks for the explanations. Now I got it - reserved size is not a part
> of partition, it's a gap between partitions.

I might have not been clear enough when explaining. It's not a gap,
it's just a container of size --reserved-size listed in MBR/GPT.
There's probably a filesystem inside but not necessarily.
Graphically it looks as like this:

                         --reserved-size
                      |----------------------|
                      v                      v
    +-----------------+----------------------+---------------------+
    |..... stuff .....|xxxxxxxxxx            |..... stuff .........|
    +-----------------+----------------------+---------------------+
                      ^         ^            ^
                      |---------|------------|
                       --size    --extra-space


>
> What's the advantage of creating unusable gap over creating partition of
> the same size that can be used?

Just convenience.

> Even if that space is not needed it doesn't harm to have it, does it?

I have not seen any negative side effects.

>
>> >> >>
>> >> >> +         --reserved-size: This option specifies that there should be
>> >> >> +                          at least that many bytes reserved for
>> > According to the code if partition size > reserved size wic will throw
>> > an error. This somehow conflicts with 'at least that many bytes' part of 
>> > the
>> > description in my opinion.
>> >
>> > As far as I uderstood your code this parameter specifies hard upper
>> > limit for partition size, right?
>>
>> Thanks, the wording is a bit off. I should rephrase it to 'that many
>> bytes' or just state that it's an upper limit.
>>
> Upper limit is a wrong term here. I'm sorry for confusing you.
> I proposed it because I misunderstood that it's not a partition size but a 
> reserved
> space after partition. Anyway, I think above description should be
> modified to explain the option in a clear way. Using ascii picture of
> the partition layout would probably help to show what this option means.
>
>> >> >>              msger.debug("Assigned %s to %s%d, sectors range %d-%d 
>> >> >> size %d "
>> >> >> -                        "sectors (%d bytes)." \
>> >> >> +                        "sectors (%d bytes), reserved %d bytes." \
>> >> >>                              % (part['mountpoint'], part['disk_name'], 
>> >> >> part['num'],
>> >> >> -                               part['start'], part['start'] + 
>> >> >> part['size'] - 1,
>> >> >> -                               part['size'], part['size'] * 
>> >> >> self.sector_size))
>> >> >> +                               part['start'], disk['offset'] - 1,
>> >> >> +                               part['size'], part['size'] * 
>> >> >> self.sector_size,
>> >> >> +                               part['reserved_size'] * 
>> >> >> self.sector_size))
>> > This will produce incorrect output about partition size as in reality 
>> > partition size will
>> > be set to part['reserved_size'].
>>
>> I tried to avoid confusing the reader here. Partition size is one
>> thing and reservation is another. That is why reservation is logged
>> explicitly. Logging just the size would raise questions about why the
>> partition has grown.
>>
>> Perhaps it makes sense to log reservation like this instead?
>>
> What about using 'reserved %d bytes of space after partition' or
> something like that?
>
>>    part['reserved_size'] * self.sector_size if part['reserved_size']
>> else part['size'] * self.sector_size
>>
>> >
>> > "reserved 0 bytes" message will be produced if --reserved-size option
>> > is not used. It's a bit confusing as 'reserved' is too generic word. Can
>> > we use 'reserved size' instead?
>>
>> Yes, 'reserved size' sounds much better.
> We can also skip this part if reserved size is 0.
>
>> >> >> @@ -288,17 +303,21 @@ class Image():
>> >> >>                  # Type for ext2/ext3/ext4/btrfs
>> >> >>                  parted_fs_type = "ext2"
>> >> >>
>> >> >> +            psize = part['size']
>> >> >> +            if part['reserved_size']:
>> >> >> +                psize = part['reserved_size']
>> >> >> +
>> > Here I'm getting confused. How this new parameters differs from --size if 
>> > it behaves
>> > the same way, i.e. partition size is set to its value.
>>
>> That's intentional, Partition.prepare() is not be affected by the
>> reservation. I just want to reserve some space during layout, but not
>> necessarily enlarge the rootfs right now.
>
> This made the whole thing clear for me! Thank you.
>
>> > The only
>> > difference I see is that if rootfs size is greater than reserved size
>> > wic will fail to create partition. Would it make sense to name it
>> > --max-size in this case?
>>
>> Just wording, reservation seems more fitting, but I'm open to suggestions.
>>
> I agree. --max-size would suggest that wic will make partition with this
> size.
>
> I'm still wondering what's the benefit of having unused space between
> partitions though.
>
>> >> >>              self.__create_partition(disk['disk'].device, part['type'],
>> >> >> -                                    parted_fs_type, part['start'], 
>> >> >> part['size'])
>> >> >> +                                    parted_fs_type, part['start'], 
>> >> >> psize)
>> >> >>
>> >> >>              if part['part_type']:
>> >> >>                  msger.debug("partition %d: set type UID to %s" % \
>> >
>> > Can you cover this functionality(and may be other size-related
>> > functionality too) with tests? You can add testcases to existing
>> > oe-selftest wic module.
>>
>> I'll take a look at it.
> Thanks.
>
> --
> Regards,
> Ed



-- 
Maciej Borzecki
RnDity
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Reply via email to