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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list