All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
@ 2018-05-08 12:27 Ivan Ren
  2018-05-10 15:04 ` Ivan Ren
  2018-05-10 15:57 ` Eric Blake
  0 siblings, 2 replies; 4+ messages in thread
From: Ivan Ren @ 2018-05-08 12:27 UTC (permalink / raw)
  To: mreitz, kwolf; +Cc: qemu-block, qemu-devel

Create a qcow2 directly on bare block device with
"-o preallocation=metadata" option. When read this qcow2, it will
return dirty data of block device. This patch add QCOW_OFLAG_ZERO
for all preallocated l2 entry if the underlying device is a bare
block device.

Signed-off-by: Ivan Ren <ivanren@tencent.com>
---
 block/qcow2-cluster.c |  5 +++--
 block/qcow2.c         | 19 ++++++++++++++++---
 block/qcow2.h         |  3 ++-
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1aee726..b9e0ceb 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -919,7 +919,8 @@ fail:
     return ret;
 }
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+                                uint64_t flags)
 {
     BDRVQcow2State *s = bs->opaque;
     int i, j = 0, l2_index, ret;
@@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
         }
 
         l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
-                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
+                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
      }
 
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f36e63..093735c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2044,7 +2044,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
         while (l2meta != NULL) {
             QCowL2Meta *next;
 
-            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
+            ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
             if (ret < 0) {
                 goto fail;
             }
@@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void *opaque)
     uint64_t host_offset = 0;
     unsigned int cur_bytes;
     int ret;
+    struct stat st;
     QCowL2Meta *meta;
 
     qemu_co_mutex_lock(&s->lock);
@@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void *opaque)
         while (meta) {
             QCowL2Meta *next = meta->next;
 
-            ret = qcow2_alloc_cluster_link_l2(bs, meta);
+            /* Check the underlying device type.
+             * If the underlying device is a block device, we add
+             * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore dirty
+             * data on block device.
+             * If the underlying device can't be used with stat(return < 0),
+             * treat it as a regular file.
+             */
+            if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
+                ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
+            } else {
+                ret = qcow2_alloc_cluster_link_l2(bs, meta, QCOW_OFLAG_ZERO);
+            }
+
             if (ret < 0) {
                 qcow2_free_any_clusters(bs, meta->alloc_offset,
                                         meta->nb_clusters, QCOW2_DISCARD_NEVER);
@@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
             };
             qemu_co_queue_init(&allocation.dependent_requests);
 
-            ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
+            ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Failed to update L2 tables");
                 qcow2_free_clusters(bs, host_offset,
diff --git a/block/qcow2.h b/block/qcow2.h
index adf5c39..9a59602 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -617,7 +617,8 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                          uint64_t offset,
                                          int compressed_size);
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
+                                uint64_t flags);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, enum qcow2_discard_type type,
                           bool full_discard);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
  2018-05-08 12:27 [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device Ivan Ren
@ 2018-05-10 15:04 ` Ivan Ren
  2018-05-10 15:57 ` Eric Blake
  1 sibling, 0 replies; 4+ messages in thread
From: Ivan Ren @ 2018-05-10 15:04 UTC (permalink / raw)
  To: mreitz, kwolf; +Cc: qemu-block, qemu-devel

ping for review

On Tue, May 8, 2018 at 8:27 PM Ivan Ren <renyime@gmail.com> wrote:

> Create a qcow2 directly on bare block device with
> "-o preallocation=metadata" option. When read this qcow2, it will
> return dirty data of block device. This patch add QCOW_OFLAG_ZERO
> for all preallocated l2 entry if the underlying device is a bare
> block device.
>
> Signed-off-by: Ivan Ren <ivanren@tencent.com>
> ---
>  block/qcow2-cluster.c |  5 +++--
>  block/qcow2.c         | 19 ++++++++++++++++---
>  block/qcow2.h         |  3 ++-
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 1aee726..b9e0ceb 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -919,7 +919,8 @@ fail:
>      return ret;
>  }
>
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +                                uint64_t flags)
>  {
>      BDRVQcow2State *s = bs->opaque;
>      int i, j = 0, l2_index, ret;
> @@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
> QCowL2Meta *m)
>          }
>
>          l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
> -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> +                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
>       }
>
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2f36e63..093735c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2044,7 +2044,7 @@ static coroutine_fn int
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
>          while (l2meta != NULL) {
>              QCowL2Meta *next;
>
> -            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> +            ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
>              if (ret < 0) {
>                  goto fail;
>              }
> @@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void *opaque)
>      uint64_t host_offset = 0;
>      unsigned int cur_bytes;
>      int ret;
> +    struct stat st;
>      QCowL2Meta *meta;
>
>      qemu_co_mutex_lock(&s->lock);
> @@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void
> *opaque)
>          while (meta) {
>              QCowL2Meta *next = meta->next;
>
> -            ret = qcow2_alloc_cluster_link_l2(bs, meta);
> +            /* Check the underlying device type.
> +             * If the underlying device is a block device, we add
> +             * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore
> dirty
> +             * data on block device.
> +             * If the underlying device can't be used with stat(return <
> 0),
> +             * treat it as a regular file.
> +             */
> +            if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
> +                ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
> +            } else {
> +                ret = qcow2_alloc_cluster_link_l2(bs, meta,
> QCOW_OFLAG_ZERO);
> +            }
> +
>              if (ret < 0) {
>                  qcow2_free_any_clusters(bs, meta->alloc_offset,
>                                          meta->nb_clusters,
> QCOW2_DISCARD_NEVER);
> @@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs,
> int64_t offset,
>              };
>              qemu_co_queue_init(&allocation.dependent_requests);
>
> -            ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
> +            ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
>              if (ret < 0) {
>                  error_setg_errno(errp, -ret, "Failed to update L2
> tables");
>                  qcow2_free_clusters(bs, host_offset,
> diff --git a/block/qcow2.h b/block/qcow2.h
> index adf5c39..9a59602 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -617,7 +617,8 @@ uint64_t
> qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
>                                           uint64_t offset,
>                                           int compressed_size);
>
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +                                uint64_t flags);
>  int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
>                            uint64_t bytes, enum qcow2_discard_type type,
>                            bool full_discard);
> --
> 1.8.3.1
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
  2018-05-08 12:27 [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device Ivan Ren
  2018-05-10 15:04 ` Ivan Ren
@ 2018-05-10 15:57 ` Eric Blake
  2018-05-11 10:46   ` Ivan Ren
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Blake @ 2018-05-10 15:57 UTC (permalink / raw)
  To: Ivan Ren, mreitz, kwolf; +Cc: qemu-devel, qemu-block

On 05/08/2018 07:27 AM, Ivan Ren wrote:
> Create a qcow2 directly on bare block device with
> "-o preallocation=metadata" option. When read this qcow2, it will
> return dirty data of block device.

Yes, reading garbage is expected.

> This patch add QCOW_OFLAG_ZERO
> for all preallocated l2 entry if the underlying device is a bare
> block device.

This says what the patch does, but not why.  What is the actual use case 
scenario where changing semantics to have the qcow2 overwrite the 
garbage to read 0 instead of any pre-existing garbage, when dealing with 
portions of the disk that have not yet been written by the guest?  Are 
you trying to prevent a security leak of previous information that may 
be resident on the block device?

> 
> Signed-off-by: Ivan Ren <ivanren@tencent.com>
> ---
>   block/qcow2-cluster.c |  5 +++--
>   block/qcow2.c         | 19 ++++++++++++++++---
>   block/qcow2.h         |  3 ++-
>   3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 1aee726..b9e0ceb 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -919,7 +919,8 @@ fail:
>       return ret;
>   }
>   
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +                                uint64_t flags)
>   {
>       BDRVQcow2State *s = bs->opaque;
>       int i, j = 0, l2_index, ret;
> @@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
>           }
>   
>           l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
> -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> +                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED | flags);
>        }
>   
>   
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2f36e63..093735c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2044,7 +2044,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
>           while (l2meta != NULL) {
>               QCowL2Meta *next;
>   
> -            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> +            ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
>               if (ret < 0) {
>                   goto fail;
>               }
> @@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void *opaque)
>       uint64_t host_offset = 0;
>       unsigned int cur_bytes;
>       int ret;
> +    struct stat st;
>       QCowL2Meta *meta;
>   
>       qemu_co_mutex_lock(&s->lock);
> @@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void *opaque)
>           while (meta) {
>               QCowL2Meta *next = meta->next;
>   
> -            ret = qcow2_alloc_cluster_link_l2(bs, meta);
> +            /* Check the underlying device type.
> +             * If the underlying device is a block device, we add
> +             * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore dirty
> +             * data on block device.
> +             * If the underlying device can't be used with stat(return < 0),
> +             * treat it as a regular file.
> +             */
> +            if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {

Won't work.  You cannot guarantee that bs->filename is a local file; it 
could be a remote protocol, such as NBD or gluster.  If you need to know 
whether bs->filename has a property that it reads as all zeroes when 
first initialized, we already have 
BlockDriverInfo::unallocated_blocks_are_zero, which is set to true for 
regular files and false for block devices.

> +                ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
> +            } else {
> +                ret = qcow2_alloc_cluster_link_l2(bs, meta, QCOW_OFLAG_ZERO);

Why would we not want to pass QCOW_OFLAG_ZERO always, rather than 
special-casing it based on what the underlying protocol might already 
have in place?

> +            }
> +
>               if (ret < 0) {
>                   qcow2_free_any_clusters(bs, meta->alloc_offset,
>                                           meta->nb_clusters, QCOW2_DISCARD_NEVER);
> @@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
>               };
>               qemu_co_queue_init(&allocation.dependent_requests);
>   
> -            ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
> +            ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
>               if (ret < 0) {
>                   error_setg_errno(errp, -ret, "Failed to update L2 tables");
>                   qcow2_free_clusters(bs, host_offset,
> diff --git a/block/qcow2.h b/block/qcow2.h
> index adf5c39..9a59602 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -617,7 +617,8 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
>                                            uint64_t offset,
>                                            int compressed_size);
>   
> -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
> +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> +                                uint64_t flags);
>   int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
>                             uint64_t bytes, enum qcow2_discard_type type,
>                             bool full_discard);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device
  2018-05-10 15:57 ` Eric Blake
@ 2018-05-11 10:46   ` Ivan Ren
  0 siblings, 0 replies; 4+ messages in thread
From: Ivan Ren @ 2018-05-11 10:46 UTC (permalink / raw)
  To: eblake; +Cc: mreitz, kwolf, qemu-devel, qemu-block

> This says what the patch does, but not why.  What is the actual use case
> scenario where changing semantics to have the qcow2 overwrite the
> garbage to read 0 instead of any pre-existing garbage, when dealing with
> portions of the disk that have not yet been written by the guest?  Are
> you trying to prevent a security leak of previous information that may
> be resident on the block device?

Yes, any pre-existing data on the device should not be read, otherwise it
may lead security leak in cloud environment.
For metadata preallocated qcow2 on bare block device, I think
QCOW_OFLAG_ZERO is more suitable to prevent this problem than other(such as
do "dd" before create a preallocated qcow2).

> > +            /* Check the underlying device type.
> > +             * If the underlying device is a block device, we add
> > +             * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore
dirty
> > +             * data on block device.
> > +             * If the underlying device can't be used with stat(return
< 0),
> > +             * treat it as a regular file.
> > +             */
> > +            if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
>
> Won't work.  You cannot guarantee that bs->filename is a local file; it
> could be a remote protocol, such as NBD or gluster.  If you need to know
> whether bs->filename has a property that it reads as all zeroes when
> first initialized, we already have
> BlockDriverInfo::unallocated_blocks_are_zero, which is set to true for
> regular files and false for block devices.

Yes, BlockDriverInfo::unallocated_blocks_are_zero is enough and better to
distinguish the device.

> > +                ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
> > +            } else {
> > +                ret = qcow2_alloc_cluster_link_l2(bs, meta,
QCOW_OFLAG_ZERO);
>
> Why would we not want to pass QCOW_OFLAG_ZERO always, rather than
> special-casing it based on what the underlying protocol might already
> have in place?

Yea, it sounds good. Always pass QCOW_OFLAG_ZERO in preallocation has no
problem and can guarantee no garbage will be read when preallcate metadata
for qcow2 on any underlying device.

I will send a v2 patch.

Thanks.

On Thu, May 10, 2018 at 11:57 PM Eric Blake <eblake@redhat.com> wrote:

> On 05/08/2018 07:27 AM, Ivan Ren wrote:
> > Create a qcow2 directly on bare block device with
> > "-o preallocation=metadata" option. When read this qcow2, it will
> > return dirty data of block device.
>
> Yes, reading garbage is expected.
>
> > This patch add QCOW_OFLAG_ZERO
> > for all preallocated l2 entry if the underlying device is a bare
> > block device.
>
> This says what the patch does, but not why.  What is the actual use case
> scenario where changing semantics to have the qcow2 overwrite the
> garbage to read 0 instead of any pre-existing garbage, when dealing with
> portions of the disk that have not yet been written by the guest?  Are
> you trying to prevent a security leak of previous information that may
> be resident on the block device?
>
> >
> > Signed-off-by: Ivan Ren <ivanren@tencent.com>
> > ---
> >   block/qcow2-cluster.c |  5 +++--
> >   block/qcow2.c         | 19 ++++++++++++++++---
> >   block/qcow2.h         |  3 ++-
> >   3 files changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> > index 1aee726..b9e0ceb 100644
> > --- a/block/qcow2-cluster.c
> > +++ b/block/qcow2-cluster.c
> > @@ -919,7 +919,8 @@ fail:
> >       return ret;
> >   }
> >
> > -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
> > +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> > +                                uint64_t flags)
> >   {
> >       BDRVQcow2State *s = bs->opaque;
> >       int i, j = 0, l2_index, ret;
> > @@ -969,7 +970,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState
> *bs, QCowL2Meta *m)
> >           }
> >
> >           l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
> > -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> > +                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED |
> flags);
> >        }
> >
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 2f36e63..093735c 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -2044,7 +2044,7 @@ static coroutine_fn int
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
> >           while (l2meta != NULL) {
> >               QCowL2Meta *next;
> >
> > -            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> > +            ret = qcow2_alloc_cluster_link_l2(bs, l2meta, 0);
> >               if (ret < 0) {
> >                   goto fail;
> >               }
> > @@ -2534,6 +2534,7 @@ static void coroutine_fn preallocate_co(void
> *opaque)
> >       uint64_t host_offset = 0;
> >       unsigned int cur_bytes;
> >       int ret;
> > +    struct stat st;
> >       QCowL2Meta *meta;
> >
> >       qemu_co_mutex_lock(&s->lock);
> > @@ -2552,7 +2553,19 @@ static void coroutine_fn preallocate_co(void
> *opaque)
> >           while (meta) {
> >               QCowL2Meta *next = meta->next;
> >
> > -            ret = qcow2_alloc_cluster_link_l2(bs, meta);
> > +            /* Check the underlying device type.
> > +             * If the underlying device is a block device, we add
> > +             * QCOW_OFLAG_ZERO for all preallocated l2 entry to ignore
> dirty
> > +             * data on block device.
> > +             * If the underlying device can't be used with stat(return
> < 0),
> > +             * treat it as a regular file.
> > +             */
> > +            if (stat(bs->filename, &st) < 0 || !S_ISBLK(st.st_mode)) {
>
> Won't work.  You cannot guarantee that bs->filename is a local file; it
> could be a remote protocol, such as NBD or gluster.  If you need to know
> whether bs->filename has a property that it reads as all zeroes when
> first initialized, we already have
> BlockDriverInfo::unallocated_blocks_are_zero, which is set to true for
> regular files and false for block devices.
>
> > +                ret = qcow2_alloc_cluster_link_l2(bs, meta, 0);
> > +            } else {
> > +                ret = qcow2_alloc_cluster_link_l2(bs, meta,
> QCOW_OFLAG_ZERO);
>
> Why would we not want to pass QCOW_OFLAG_ZERO always, rather than
> special-casing it based on what the underlying protocol might already
> have in place?
>
> > +            }
> > +
> >               if (ret < 0) {
> >                   qcow2_free_any_clusters(bs, meta->alloc_offset,
> >                                           meta->nb_clusters,
> QCOW2_DISCARD_NEVER);
> > @@ -3458,7 +3471,7 @@ static int qcow2_truncate(BlockDriverState *bs,
> int64_t offset,
> >               };
> >               qemu_co_queue_init(&allocation.dependent_requests);
> >
> > -            ret = qcow2_alloc_cluster_link_l2(bs, &allocation);
> > +            ret = qcow2_alloc_cluster_link_l2(bs, &allocation, 0);
> >               if (ret < 0) {
> >                   error_setg_errno(errp, -ret, "Failed to update L2
> tables");
> >                   qcow2_free_clusters(bs, host_offset,
> > diff --git a/block/qcow2.h b/block/qcow2.h
> > index adf5c39..9a59602 100644
> > --- a/block/qcow2.h
> > +++ b/block/qcow2.h
> > @@ -617,7 +617,8 @@ uint64_t
> qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
> >                                            uint64_t offset,
> >                                            int compressed_size);
> >
> > -int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
> > +int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m,
> > +                                uint64_t flags);
> >   int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
> >                             uint64_t bytes, enum qcow2_discard_type type,
> >                             bool full_discard);
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-11 10:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 12:27 [Qemu-devel] [PATCH] qcow2: fix preallocation with metadata on bare block device Ivan Ren
2018-05-10 15:04 ` Ivan Ren
2018-05-10 15:57 ` Eric Blake
2018-05-11 10:46   ` Ivan Ren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.