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; +}