Re: [libvirt] [PATCH v2 3/3] qemuMigrationPrecreateDisk: Preserve sparse files
On Fri, Apr 10, 2015 at 17:25:41 +0200, Michal Privoznik wrote:
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)