[Libvir] PATCH: 14/16: disk partition backend

[Daniel P. Berrange]

On Fri, Feb 15, 2008 at 07:52:37PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > +static int virStorageBackendDiskCreateVol(virConnectPtr conn,
> > +                                          virStoragePoolObjPtr pool,
> > +                                          virStorageVolDefPtr vol)
> > +{
> > +    int i;
> > +    char start[100], end[100];
> > +    unsigned long long startOffset, endOffset, smallestSize = 0, smallestExtent = -1;
> 
> [Past column 80 -> harder to spot]
> unsigned... = -1; ?
> 
> How about this instead, since smallestExtent only ever
> gets set to values stored in "i" (an int)?
> 
>     int smallestExtent = -1;

Yes, it should just be an int.

> 
> > +    virStoragePoolSourceDevicePtr dev = &pool->def->source.devices[0];
> > +    /* XXX customizable partition types */
> > +    const char *cmdargv[] = {
> > +        PARTED,
> > +        pool->def->source.devices[0].path,
> > +        "mkpart",
> > +        "--script",
> > +        "ext2",
> > +        start,
> > +        end,
> > +        NULL
> > +    };
> > +
> > +    for (i = 0 ; i < dev->nfreeExtent ; i++) {
> > +        unsigned long long size =
> > +            dev->freeExtents[i].end -
> > +            dev->freeExtents[i].start;
> 
> Shouldn't size be 1 larger?  i.e.,
> 
>         unsigned long long size =
>             dev->freeExtents[i].end -
>             dev->freeExtents[i].start + 1;

I don't think so - the +1  adjustment is already done in the libvirt_parthelper 
program, but I'll double-check

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|