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;
> +}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list