[libvirt] [PATCH 7/7] VolumeCreateXMLFrom FS storage backend implementation.

[Daniel P. Berrange]

On Mon, May 04, 2009 at 01:43:02PM -0400, Cole Robinson wrote:
> Add an extra 'inputvol' parameter to the file volume building routines. This
> allows us to easily reuse the duplicate functionality.
> 

> @@ -1032,15 +1046,68 @@ static int createRaw(virConnectPtr conn,
>          virReportSystemError(conn, errno,
>                               _("cannot extend file '%s'"),
>                               vol->target.path);
> -        close(fd);
> -        return -1;
> +        goto cleanup;
> +    }
> +
> +    remain = vol->capacity;
> +
> +    if (inputfd != -1) {
> +        size_t bytes = 1024 * 1024;
> +        int sparse_interval = 512;
> +        int amtread = -1;
> +
> +        while (amtread != 0) {
> +            char buf[bytes];

Preferrable not to allocate 1 MB on the stack, since not all OS
default to such large stacks as Linux does for threads.

> +            char *tmp;
> +            int newamt;
> +
> +            if (remain < bytes)
> +                bytes = remain;
> +
> +            if ((amtread = saferead(inputfd, buf, bytes)) < 0) {
> +                virReportSystemError(conn, errno,
> +                                     _("failed reading from file '%s'"),
> +                                     inputvol->target.path);
> +                goto cleanup;
> +            }
> +            remain -= amtread;
> +
> +            /* Loop over amt read in 512 byte increments, looking for sparse
> +             * blocks */
> +            tmp = buf;
> +            newamt = amtread;
> +            do {
> +                int interval = ((sparse_interval > newamt) ? newamt
> +                                                           : sparse_interval );
> +                int sparse_counter = interval;
> +
> +                while (sparse_counter-- && *(tmp+sparse_counter) == '\0')
> +                    continue;
> +
> +                if (sparse_counter < 0) {
> +                    if (lseek(fd, interval, SEEK_CUR) < 0) {
> +                        virReportSystemError(conn, errno,
> +                                             _("cannot extend file '%s'"),
> +                                             vol->target.path);
> +                        goto cleanup;
> +                    }
> +                } else if (safewrite(fd, tmp, interval) < 0) {
> +                    virReportSystemError(conn, errno,
> +                                         _("failed writing to file '%s'"),
> +                                         vol->target.path);
> +                    goto cleanup;
> +
> +                }
> +
> +                tmp += interval;
> +            } while ((newamt -= sparse_interval) > 0);
> +        }

I'm finding this bit of code a little hard to follow. Since you're scanning
in fixed 512 byte chunks, how about just doing

       char buf[512];
       memset(buf, 0, sizeof buf);

and then just doing a simple loop using memcmp(tmp, buf) over the 1 MB
data. Sure, the current code allows for detection of chunks of zeros
< 512 in size, but since data is allocated in underlying FS in at least
that size chunks, its not worth trying to detect zeros < 512  in length

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|