[libvirt] [PATCH v2 3/3] qemuMigrationPrecreateDisk: Preserve sparse files

[Peter Krempa]

On Fri, Apr 10, 2015 at 17:25:41 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=817700

This patch certainly does not resolve this bug. See below ...

> 
> When pre-creating a disk on the destination, a volume XML is
> constructed. The XML is then passed to virStorageVolCreateXML() which
> does the work. But, since there's no <allocation/> in the XML, the
> disk are fully allocated. This possibly breaks sparse allocation user
> has on the migration source.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_migration.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index d4757e4..7a40548 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -144,6 +144,7 @@ typedef qemuMigrationCookieNBDDisk *qemuMigrationCookieNBDDiskPtr;
>  struct _qemuMigrationCookieNBDDisk {
>      char *target;                   /* Disk target */
>      unsigned long long capacity;    /* And its capacity */
> +    unsigned long long allocation;  /* And its allocation */
>  };
>  
>  typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD;
> @@ -593,6 +594,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig,
>                         disk->dst) < 0)
>              goto cleanup;
>          mig->nbd->disks[mig->nbd->ndisks].capacity = entry->capacity;
> +        mig->nbd->disks[mig->nbd->ndisks].allocation = entry->wr_highest_offset;

This "allocation" value works only for thin-provisioned LVM with qcow2
inside. Qemu docs state the following:

    - "wr_highest_offset": Highest offset of a sector written since the
                           BlockDriverState has been opened (json-int)

As the documentation hints this information doesn't make much sense for
regular files. The file may (and certainly will) be sparse in between of
the start and the highest offset.

Additionally since the docs state that the value is "since it has been
opened" the number may actually be lower than the count of blocks that
are used.


>          mig->nbd->ndisks++;
>      }
>  

...

> @@ -1541,6 +1560,8 @@ qemuMigrationPrecreateDisk(virConnectPtr conn,
>      virBufferAdjustIndent(&buf, 2);
>      virBufferEscapeString(&buf, "<name>%s</name>\n", volName);
>      virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n", nbd->capacity);
> +    if (nbd->allocation)
> +        virBufferAsprintf(&buf, "<allocation>%llu</allocation>\n", nbd->allocation);

You add this to the snippet that is used to pre-create the file, but ..

>      virBufferAddLit(&buf, "<target>\n");
>      virBufferAdjustIndent(&buf, 2);
>      virBufferAsprintf(&buf, "<format type='%s'/>\n", format);

The @flags variable in this function is never touched for raw files. For
qcow2 files, full preallocation is not supported. This means that the
@allocation info is never used.


> @@ -1585,8 +1606,9 @@ qemuMigrationPrecreateStorage(virConnectPtr conn,
>          int indx;
>          const char *diskSrcPath;
>  
> -        VIR_DEBUG("Looking up disk target '%s' (capacity=%llu)",
> -                  nbd->disks[i].target, nbd->disks[i].capacity);
> +        VIR_DEBUG("Looking up disk target '%s' (capacity=%llu allocation=%llu)",
> +                  nbd->disks[i].target, nbd->disks[i].capacity,
> +                  nbd->disks[i].allocation);
>  
>          if ((indx = virDomainDiskIndexByName(vm->def,
>                                               nbd->disks[i].target, false)) < 0) {

The bugreport states that the user actually pre-created the file on the
destination as sparse so everything in this patch actually won't fix the
original bug.

The problem is that the block-copy job in qemu copies the entire disk
even with blocks that were not touched. For the reporter the fix would
probably be to use qcow2 as it should only copy the blocks that were
touched by the VM since qemu has the information.

As for this patch: NACK, the code you add doesn't do anything useful.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150413/5a460d0a/attachment-0001.sig>