Re: [PATCH v2 1/4] util: new function virFileLength()

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

 



On 11/24/2016 09:36 AM, Andrea Bolognani wrote:
On Mon, 2016-11-21 at 00:01 -0500, Laine Stump wrote:
This new function just calls stat() and returns st_size (or -1 if
there is an error). We may decide we want this function to be more
complex, and handle things like block devices - this is a placeholder
(that works) for any more complicated funtion.
s/funtion/function/

NB: virFileLength() takes a path rather than an fd because it needs to
be called for files that can't be opened (due to permissions).
---
New in "V2"
src/libvirt_private.syms | 1 +
   src/util/virfile.c       | 13 +++++++++++++
   src/util/virfile.h       |  2 ++
   3 files changed, 16 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ac6a1e1..1c0b912 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1578,6 +1578,7 @@ virFileIsLink;
   virFileIsMountPoint;
   virFileIsSharedFS;
   virFileIsSharedFSType;
+virFileLength;
   virFileLinkPointsTo;
   virFileLock;
   virFileLoopDeviceAssociate;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a45279a..11b6027 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1735,6 +1735,19 @@ virFileActivateDirOverride(const char *argv0)
       }
   }
+
+off_t
+virFileLength(const char *file)
I find the lack of documentation disturbing.

+{
+    struct stat s;
+
+    if (stat(file, &s) < 0)
+        return -1;
+
+    return s.st_size;
+}
+
+
   bool
   virFileIsDir(const char *path)
   {
diff --git a/src/util/virfile.h b/src/util/virfile.h
index b4ae6ea..a0c646d 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -179,6 +179,8 @@ char *virFileFindResourceFull(const char *filename,
   void virFileActivateDirOverride(const char *argv0)
       ATTRIBUTE_NONNULL(1);
+off_t virFileLength(const char *file) ATTRIBUTE_NONNULL(1);
+
   bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
How did a space end up here? Weird :)

   bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
   bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
As you mention in the commit message, this simple
implementation might not handle all cases; on the other
hand, its addition to virFile might encourage others to
use it.

So I guess my question would be: is adding this function,
in its current form, worth it? Would it be better to just
call stat() in qemuDomainDeviceCalculatePCIConnectFlags(),
and replace that later with a call to a more fleshed-out
virFileLength() that can be used not just in that specific
spot, but hopefully in a bunch other places?

I was going in the other direction - add a simple implementation of the function that will encourage people to enhance it (if necessary) for their own uses rather than continuing to add calls to stat() "until someone implements a full featured virFileLength()" (which may never happen).

--
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]
  Powered by Linux