[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] [UPDATED] Add host_device support to qemu-img.
From: |
malc |
Subject: |
Re: [Qemu-devel] [PATCH] [UPDATED] Add host_device support to qemu-img. |
Date: |
Thu, 2 Apr 2009 05:20:11 +0400 (MSD) |
On Wed, 1 Apr 2009, Nolan wrote:
> This patch allows the use a host_device as the destination for "qemu-img
> convert".
>
> I added a ->bdrv_create function host_device. It merely verifies that
> the device exists and is large enough.
>
> A check is needed in the qemu-img convert loop to ensure that we write
> out all 0 sectors to the host_device. Otherwise they end up with stale
> garbage where all zero sectors were expected.
>
> I also made the check against bdrv_is_allocated enabled for everything
> _except_ host devices, since there is no point in making the block
> backend write a bunch of zeros just so that we can memcmp them
> immediately afterwards. Host devices can't benefit from this because
> there is no way to differentiate between a sector being unallocated
> because it was never written, or because it was written with all zeros
> and then made a trip through qemu-img convert.
>
> Finally, there is an unrelated fix for a typo in the error message
> printed if the destination device does not support ->bdrv_create.
>
> Signed-off-by: Nolan Leake <nolan <at> sigbus.net>
>
> Index: block-raw-posix.c
> ===================================================================
> --- block-raw-posix.c (revision 6963)
> +++ block-raw-posix.c (working copy)
> @@ -1378,11 +1378,47 @@
> }
> #endif /* !linux && !FreeBSD */
>
> +#if defined(__linux__) || defined(__FreeBSD__)
> +static int hdev_create(const char *filename, int64_t total_size,
> + const char *backing_file, int flags)
> +{
> + int fd;
> + int ret = 0;
> + struct stat stat_buf;
> +
> + if (flags || backing_file)
> + return -ENOTSUP;
> +
> + fd = open(filename, O_WRONLY | O_BINARY);
> + if (fd < 0)
> + return -EIO;
> +
> + if (fstat(fd, &stat_buf) < 0)
> + ret = -EIO;
> + else if (!S_ISBLK(stat_buf.st_mode))
> + ret = -EIO;
> + else if (lseek(fd, 0, SEEK_END) < total_size * 512)
> + ret = -ENOSPC;
This leaks fescriptors doesn't it, even if only user is qemu-img which
just exits on errors this still looks fragile.
> +
> + close(fd);
> + return ret;
> +}
> +
> +#else /* !(linux || freebsd) */
> +
> +static int hdev_create(const char *filename, int64_t total_size,
> + const char *backing_file, int flags)
> +{
> + return -ENOTSUP;
> +}
> +#endif
> +
> BlockDriver bdrv_host_device = {
> .format_name = "host_device",
> .instance_size = sizeof(BDRVRawState),
> .bdrv_open = hdev_open,
> .bdrv_close = raw_close,
> + .bdrv_create = hdev_create,
> .bdrv_flush = raw_flush,
>
> #ifdef CONFIG_AIO
> Index: qemu-img.c
> ===================================================================
> --- qemu-img.c (revision 6963)
> +++ qemu-img.c (working copy)
> @@ -493,7 +493,7 @@
> ret = bdrv_create(drv, out_filename, total_sectors, out_baseimg, flags);
> if (ret < 0) {
> if (ret == -ENOTSUP) {
> - error("Formatting not supported for file format '%s'", fmt);
> + error("Formatting not supported for file format '%s'", out_fmt);
> } else {
> error("Error while formatting '%s'", out_filename);
> }
> @@ -592,18 +592,17 @@
> if (n > bs_offset + bs_sectors - sector_num)
> n = bs_offset + bs_sectors - sector_num;
>
> - /* If the output image is being created as a copy on write image,
> - assume that sectors which are unallocated in the input image
> - are present in both the output's and input's base images (no
> - need to copy them). */
> - if (out_baseimg) {
> - if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, n,
> &n1)) {
> - sector_num += n1;
> - continue;
> - }
> - /* The next 'n1' sectors are allocated in the input image.
> Copy
> - only those as they may be followed by unallocated sectors.
> */
> - n = n1;
> + if (drv != &bdrv_host_device) {
> + if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
> + n, &n1)) {
> + sector_num += n1;
> + continue;
> + }
> + /* The next 'n1' sectors are allocated in the input image.
> Copy
> + only those as they may be followed by unallocated
> sectors. */
> + n = n1;
> + } else {
> + n1 = n;
> }
>
> if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) < 0)
> @@ -615,8 +614,13 @@
> while (n > 0) {
> /* If the output image is being created as a copy on write
> image,
> copy all sectors even the ones containing only NUL bytes,
> - because they may differ from the sectors in the base
> image. */
> - if (out_baseimg || is_allocated_sectors(buf1, n, &n1)) {
> + because they may differ from the sectors in the base
> image.
> +
> + If the output is to a host device, we also write out
> + sectors that are entirely 0, since whatever data was
> + already there is garbage, not 0s. */
> + if (drv == &bdrv_host_device || out_baseimg ||
> + is_allocated_sectors(buf1, n, &n1)) {
> if (bdrv_write(out_bs, sector_num, buf1, n1) < 0)
> error("error while writing");
> }
>
>
>
>
--
mailto:address@hidden