[libvirt] [PATCH 8/9] Simplified version of volume zeroing based on feedback from the list.

Dave Allan dallan at redhat.com
Wed Mar 3 21:40:36 UTC 2010


On 03/02/2010 06:44 PM, Eric Blake wrote:
> According to David Allan on 3/2/2010 3:13 PM:
>> +    ret = ftruncate(fd, st->st_size);
>> +    if (ret == -1) {
>> +        virReportSystemError(ret,
>> +                             _("Failed to truncate volume with "
>> +                               "path '%s' to %llu bytes: '%s'\n"),
>> +                             vol->target.path, st->st_size,
>
> off_t is not guaranteed to be the same as long long.  You need a cast.
>
>> +                             virStrerror(errno, errbuf, sizeof(errbuf)));
>> +    }
>> +
>> +out:
>> +    return ret;
>> +}
>> +
>> +
>> +static int
>> +storageZeroExtent(virStorageVolDefPtr vol,
>> +                  struct stat *st,
>> +                  int fd,
>> +                  size_t extent_start,
>> +                  size_t extent_length,
>> +                  char *writebuf,
>> +                  size_t *bytes_zeroed)
>
> Is size_t the right type for the extent, or do we want off_t?
>
>> +    size_t remaining, write_size;
> ...
>> +        if (written<  0) {
>> +            virReportSystemError(written,
>> +                                 _("Failed to write to storage volume with "
>> +                                   "path '%s': '%s' "
>> +                                   "(attempted to write %d bytes)"),
>> +                                 vol->target.path,
>> +                                 virStrerror(errno, errbuf, sizeof(errbuf)),
>> +                                 write_size);
>
> %d and write_size are not compatible.
>
>> +    memset(&st, 0, sizeof(st));
>> +
>> +    if (fstat(fd,&st) == -1) {
>
> The memset is wasted work.  stat() is sufficient for initializing st.
>

Good catches.  Attached is an incremental patch that addresses all your 
comments.  The  funky indentation you pointed out is created by rpcgen, 
not our generation scripts, so I'm not going to attempt to address it at 
the moment.

Dave
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Fixes-per-Eric-Blake.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100303/cd48812e/attachment-0001.ksh>


More information about the libvir-list mailing list