5:53 a.m.
On 04/02/2015 12:48 PM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=817700
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 | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3adb949..a2f68ed 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;
@@ -591,6 +592,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;
mig->nbd->ndisks++;
}
@@ -831,8 +833,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver,
for (i = 0; i < mig->nbd->ndisks; i++) {
virBufferEscapeString(buf, "<disk target='%s'",
mig->nbd->disks[i].target);
- virBufferAsprintf(buf, " capacity='%llu'/>\n",
- mig->nbd->disks[i].capacity);
+ virBufferAsprintf(buf, " capacity='%llu'
allocation='%llu'/>\n",
+ mig->nbd->disks[i].capacity,
+ mig->nbd->disks[i].allocation);
Here you're printing it here regardless of value (-1, 0?)...
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</nbd>\n");
@@ -970,7 +973,7 @@ static qemuMigrationCookieNBDPtr
qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt)
{
qemuMigrationCookieNBDPtr ret = NULL;
- char *port = NULL, *capacity = NULL;
+ char *port = NULL, *capacity = NULL, *allocation = NULL;
size_t i;
int n;
xmlNodePtr *disks = NULL;
@@ -998,6 +1001,7 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt)
for (i = 0; i < n; i++) {
ctxt->node = disks[i];
VIR_FREE(capacity);
+ VIR_FREE(allocation);
if (!(ret->disks[i].target =
virXPathString("string(./@target)", ctxt))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1014,12 +1018,26 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt)
NULLSTR(capacity));
goto error;
}
+
+ allocation = virXPathString("string(./@allocation)", ctxt);
+ if (allocation) {
+ if (virStrToLong_ull(allocation, NULL, 10,
+ &ret->disks[i].allocation) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed disk allocation:
'%s'"),
+ allocation);
+ goto error;
+ }
+ } else {
+ ret->disks[i].allocation = -1;
This would seem to be dangerous - while we don't have a disk that takes
up that much space, why not just leave it at zero for a "marker" of
sorts? Or set it to capacity like virStorageVolDefParseXML would if
allocation wasn't in the XML
+ }
}
}
cleanup:
VIR_FREE(port);
VIR_FREE(capacity);
+ VIR_FREE(allocation);
VIR_FREE(disks);
ctxt->node = save_ctxt;
return ret;
@@ -1539,6 +1557,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 != -1)
+ virBufferAsprintf(&buf,
"<allocation>%llu</allocation>\n", nbd->allocation);
Here of course you compare to -1 and don't print...
While it's perhaps not good to assume allocation == capacity, it's
perhaps no worse than today which would essentially allocate up to
capacity, right? Mixing ULL and -1 just seems dangerous
John
virBufferAddLit(&buf, "<target>\n");
virBufferAdjustIndent(&buf, 2);
virBufferAsprintf(&buf, "<format type='%s'/>\n",
format);
@@ -1583,8 +1603,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) {