Re: [libvirt] [PATCH 4/6] Implement virDomainGetBlockInfo in QEMU driver
Daniel P. Berrange wrote:
* src/qemu/qemu_driver.c: Implementation of virDomainGetBlockInfo
* src/util/storage_file.h: Add DEV_BSIZE
* src/storage/storage_backend.c: Remove DEV_BSIZE
...
Hi Dan,
This whole series looks fine.
+static int qemuDomainGetBlockInfo(virDomainPtr dom,
It's slightly better to format with both the function name and the "{"
in the first column. That makes it easier for certain function-locating
tools (and even humans who are used to that) to find the beginning of
a function. It also saves horizontal space, which can be important with
some of these very long function names.
static int
qemuDomainGetBlockInfo(virDomainPtr dom,
...
{
+ const char *path,
+ virDomainBlockInfoPtr info,
+ unsigned int flags) {
+ struct qemud_driver *driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ }
...
+ /* Probe for magic formats */
+ memset(&meta, 0, sizeof(meta));
+ if (virStorageFileGetMetadataFromFD(path, fd, &meta) < 0)
+ goto cleanup;
+
+ /* Get info for normal formats */
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("cannot stat file '%s'"), path);
+ goto cleanup;
+ }
+
+ if (S_ISREG(sb.st_mode)) {
+#ifndef __MINGW32__
+ info->physical = (unsigned long long)sb.st_blocks *
+ (unsigned long long)DEV_BSIZE;
+#else
+ info->physical = sb.st_size;
+#endif
+ /* Regular files may be sparse, so logical size (capacity) is not same
+ * as actual physical above
+ */
+ info->capacity = sb.st_size;
+ } else {
It might be worthwhile to change this to handle S_ISBLK
specifically:
} else if (S_ISBLK(sb.st_mode)) {
and to add a final "else" so that the code diagnoses
a non-regular, non-BLOCK file. Then, we can give a better
diagnostic about the file being of a non-seekable type.
On the other hand, its's not very likely we'll encounter such a
file here, so no big deal either way.
+ /* NB. Because we configure with AC_SYS_LARGEFILE, off_t
should
+ * be 64 bits on all platforms.
+ */
+ end = lseek (fd, 0, SEEK_END);
+ if (end == (off_t)-1) {
+ virReportSystemError(errno,
+ _("failed to seek to end of %s"), path);
+ goto cleanup;
+ }
+ info->physical = end;
+ info->capacity = end;
+ }
+
+ /* If the file we probed has a capacity set, then override
+ * what we calculated from file/block extents */
+ if (meta.capacity)
+ info->capacity = meta.capacity;
+
+ /* XXX allocation will need to be pulled from QEMU for
+ * the qcow inside LVM case */
+ info->allocation = info->physical;
+
+ ret = 0;
+
+cleanup:
+ if (fd != -1)
+ close(fd);
+ if (vm)
+ virDomainObjUnlock(vm);
+ return ret;
+}