The changed condition was always false because the function was always
called with boundary values 0.

Use the free extent's start value to get its start offset from the
cylinder boundary and determine if the needed size for allocation
needs to be expanded too in case the offset doesn't fit within extra
bytes for alignment.

This fixes an issue where vol-create-from will call qemu-img convert
to create a destination volume of same capacity as the source volume
and qemu-img will error 'Cannot grow device files' due to the partition
being too small for the source although both destination partition and
source volume have the same capacity.

Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
---
 src/storage/storage_backend_disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index a6d4e41220..ec0679d353 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -691,7 +691,7 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool,
          if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) {
              /* align to cylinder boundary */
              neededSize += extraBytes;
-             if ((*start % cylinderSize) > extraBytes) {
+             if ((dev->freeExtents[i].start % cylinderSize) > extraBytes) {
                  /* add an extra cylinder if the offset can't fit within
                     the extra bytes we have */
                  neededSize += cylinderSize;
-- 
2.25.2

PING

On Sat, Aug 29, 2020 at 2:49 AM Sebastian Mitterle <smitterl@redhat.com>
wrote:

> The changed condition was always false because the function was always
> called with boundary values 0.
>
> Use the free extent's start value to get its start offset from the
> cylinder boundary and determine if the needed size for allocation
> needs to be expanded too in case the offset doesn't fit within extra
> bytes for alignment.
>
> This fixes an issue where vol-create-from will call qemu-img convert
> to create a destination volume of same capacity as the source volume
> and qemu-img will error 'Cannot grow device files' due to the partition
> being too small for the source although both destination partition and
> source volume have the same capacity.
>
> Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
> ---
>  src/storage/storage_backend_disk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/storage/storage_backend_disk.c
> b/src/storage/storage_backend_disk.c
> index a6d4e41220..ec0679d353 100644
> --- a/src/storage/storage_backend_disk.c
> +++ b/src/storage/storage_backend_disk.c
> @@ -691,7 +691,7 @@
> virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool,
>           if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) {
>               /* align to cylinder boundary */
>               neededSize += extraBytes;
> -             if ((*start % cylinderSize) > extraBytes) {
> +             if ((dev->freeExtents[i].start % cylinderSize) > extraBytes)
> {
>                   /* add an extra cylinder if the offset can't fit within
>                      the extra bytes we have */
>                   neededSize += cylinderSize;
> --
> 2.25.2
>
>

-- 
Best,
Sebastian

On 8/29/20 2:49 AM, Sebastian Mitterle wrote:
> The changed condition was always false because the function was always
> called with boundary values 0.
> 
> Use the free extent's start value to get its start offset from the
> cylinder boundary and determine if the needed size for allocation
> needs to be expanded too in case the offset doesn't fit within extra
> bytes for alignment.
> 
> This fixes an issue where vol-create-from will call qemu-img convert
> to create a destination volume of same capacity as the source volume
> and qemu-img will error 'Cannot grow device files' due to the partition
> being too small for the source although both destination partition and
> source volume have the same capacity.
> 
> Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
> ---
>   src/storage/storage_backend_disk.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal