Re: [PATCH v2 05/12] getstats: rearrange blockinfo gathering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/16/2014 06:54 AM, Peter Krempa wrote:
> On 12/16/14 09:04, Eric Blake wrote:
>> We want to avoid read()ing a file while qemu is running.  We still
>> have to open() block devices to determine their physical size, but
>> that is safer.  This patch rearranges code to make it easier to
>> skip the metadata collection when possible.
>>
>> * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Check for empty
>> disk up front.  Perform stat first.  Place metadata reading next
>> to use.
>>

>> +        if (stat(disk->src->path, &sb) < 0) {
>> +            virReportSystemError(errno,
>> +                                 _("cannot stat file '%s'"), disk->src->path);
>>              goto endjob;
>>          }
> 
> Um this will cause problems on NFS. The code after that that you are
> moving uses the FD to do the stat() call. The fd is opened by the helper
> that opens the file with correct perms.

Oh, you're right.  I'll see how hairy it is to pull this patch out of
the current series, and save the rework of avoiding open()ing the files
to later (I may end up with a situation that is less efficient, by
possibly open()ing some files twice for offline domains, once for
capacity, and once for determining the last offset of a block device,
instead of the current code that tries to do both from a single fd but
becomes harder to untangle).  1-4 are pushed, and I'm seeing what
happens to the rest of the series if I leave this out for now...

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]