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@xxxxxxxxxx> > --- > 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) { > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list