Am 30.07.2014 um 04:55 hat Chunyan Liu geschrieben:
> Add nocow info in 'qemu-img info' output to show whether the file
> currently has NOCOW flag set or not.
>
> Signed-off-by: Chunyan Liu <cy...@suse.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> Resend for QEMU 2.2. Change json version comment. Add Reviewed-by.
>
> block/qapi.c | 25 +++++++++++++++++++++++++
> qapi/block-core.json | 5 ++++-
> 2 files changed, 29 insertions(+), 1 deletion(-)
Unfortunately, I have never been CCed on this patch and I saw the code
it added only now. I think it doesn't work correctly and has the wrong
design, so I would prefer to revert it for 2.2 rather than making a bad
API stable.
> diff --git a/block/qapi.c b/block/qapi.c
> index f44f6b4..aa53f19 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -28,6 +28,13 @@
> #include "qapi-visit.h"
> #include "qapi/qmp-output-visitor.h"
> #include "qapi/qmp/types.h"
> +#ifdef __linux__
> +#include <linux/fs.h>
> +#include <sys/ioctl.h>
> +#ifndef FS_NOCOW_FL
> +#define FS_NOCOW_FL 0x00800000 /* Do not cow file */
> +#endif
> +#endif
>
> BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
> {
> @@ -173,6 +180,20 @@ void bdrv_query_image_info(BlockDriverState *bs,
> Error *err = NULL;
> ImageInfo *info = g_new0(ImageInfo, 1);
>
> +#ifdef __linux__
> + int fd, attr;
> +
> + /* get NOCOW info */
> + fd = qemu_open(bs->filename, O_RDONLY | O_NONBLOCK);
> + if (fd >= 0) {
> + if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0 && (attr & FS_NOCOW_FL)) {
> + info->has_nocow = true;
> + info->nocow = true;
> + }
> + qemu_close(fd);
> + }
> +#endif
Code like this has no business in the general block layer. This belongs
in raw-posix, which already has an open fd, and nowhere else. There you
wouldn't have to rely on bs->filename, which may be empty or contain
something different from a local filename, e.g. a URL. The approach of
this patch also doesn't work if the file has been renamed or deleted
since qemu opened it (e.g. because of -snapshot).
In short: This code may happen to work in some special cases, but
generally speaking, it is wrong.
> bdrv_get_geometry(bs, &total_sectors);
>
> info->filename = g_strdup(bs->filename);
> @@ -625,4 +646,8 @@ void bdrv_image_info_dump(fprintf_function func_fprintf,
> void *f,
> func_fprintf(f, "Format specific information:\n");
> bdrv_image_info_specific_dump(func_fprintf, f,
> info->format_specific);
> }
> +
> + if (info->has_nocow && info->nocow) {
> + func_fprintf(f, "NOCOW flag: set\n");
> + }
> }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index e378653..72b4015 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -115,6 +115,8 @@
> # @format-specific: #optional structure supplying additional format-specific
> # information (since 1.7)
> #
> +# @nocow: #optional info of whether NOCOW flag is set or not. (since 2.2)
> +#
> # Since: 1.3
> #
> ##
> @@ -126,7 +128,8 @@
> '*backing-filename': 'str', '*full-backing-filename': 'str',
> '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
> '*backing-image': 'ImageInfo',
> - '*format-specific': 'ImageInfoSpecific' } }
> + '*format-specific': 'ImageInfoSpecific',
> + '*nocow': 'bool' } }
As this field makes only sense for raw-posix, it should be moved to
ImageInfoSpecific. ("format-specific" is also an unfortunate misnomer,
it should be "driver-specific", but it's too late to fix that.)
Kevin
