11:48 a.m.
When the storage is pre-created on the destination, it might have
been allocated as sparse on the source. Let's keep it that way if
we can.
Michal Privoznik (3):
qemuMigrationPrecreateStorage: Fix debug message
qemuMigrationCookieNBD: Turn some items into their own struct
qemuMigrationPrecreateDisk: Preserve sparse files
src/qemu/qemu_migration.c | 49 +++++++++++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 12 deletions(-)
--
2.0.5
11:48 a.m.
New subject: [libvirt] [PATCH 1/3] qemuMigrationPrecreateStorage: Fix debug message
When pre-creating storage for domains, we need to find corresponding
disk in the XML on the destination (domain XML may differ there, e.g.
disk is accessible under different path). For better debugging, I'm
printing all info I received on a disk. But there was a typo when
printing the disk capacity: "%lluu" instead of "%llu".
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8c45415..b20ede8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1579,7 +1579,7 @@ qemuMigrationPrecreateStorage(virConnectPtr conn,
int indx;
const char *diskSrcPath;
- VIR_DEBUG("Looking up disk target '%s' (capacity=%lluu)",
+ VIR_DEBUG("Looking up disk target '%s' (capacity=%llu)",
nbd->disks[i].target, nbd->disks[i].capacity);
if ((indx = virDomainDiskIndexByName(vm->def,
--
2.0.5
11:48 a.m.
New subject: [libvirt] [PATCH 2/3] qemuMigrationCookieNBD: Turn some items into their own struct
It will be more future proof if we just pass a pointer to a single NBD
disk representation to the qemuMigrationPrecreateDisk() instead of
copying every single item onto the stack.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_migration.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b20ede8..3adb949 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -139,16 +139,20 @@ struct _qemuMigrationCookieNetwork {
qemuMigrationCookieNetDataPtr net;
};
+typedef struct _qemuMigrationCookieNBDDisk qemuMigrationCookieNBDDisk;
+typedef qemuMigrationCookieNBDDisk *qemuMigrationCookieNBDDiskPtr;
+struct _qemuMigrationCookieNBDDisk {
+ char *target; /* Disk target */
+ unsigned long long capacity; /* And its capacity */
+};
+
typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD;
typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr;
struct _qemuMigrationCookieNBD {
int port; /* on which port does NBD server listen for incoming data */
size_t ndisks; /* Number of items in @disk array */
- struct {
- char *target; /* Disk target */
- unsigned long long capacity; /* And its capacity */
- } *disks;
+ qemuMigrationCookieNBDDiskPtr disks;
};
typedef struct _qemuMigrationCookie qemuMigrationCookie;
@@ -1458,7 +1462,7 @@ qemuMigrationRestoreDomainState(virConnectPtr conn, virDomainObjPtr
vm)
static int
qemuMigrationPrecreateDisk(virConnectPtr conn,
virDomainDiskDefPtr disk,
- unsigned long long capacity)
+ qemuMigrationCookieNBDDiskPtr nbd)
{
int ret = -1;
virStoragePoolPtr pool = NULL;
@@ -1534,7 +1538,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn,
virBufferAddLit(&buf, "<volume>\n");
virBufferAdjustIndent(&buf, 2);
virBufferEscapeString(&buf, "<name>%s</name>\n", volName);
- virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n",
capacity);
+ virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n",
nbd->capacity);
virBufferAddLit(&buf, "<target>\n");
virBufferAdjustIndent(&buf, 2);
virBufferAsprintf(&buf, "<format type='%s'/>\n",
format);
@@ -1601,7 +1605,7 @@ qemuMigrationPrecreateStorage(virConnectPtr conn,
VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath));
- if (qemuMigrationPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0)
+ if (qemuMigrationPrecreateDisk(conn, disk, nbd->disks + i) < 0)
goto cleanup;
}
--
2.0.5
11:48 a.m.
New subject: [libvirt] [PATCH 3/3] qemuMigrationPrecreateDisk: Preserve sparse files
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);
}
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;
+ }
}
}
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);
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) {
--
2.0.5
5:53 a.m.
New subject: [libvirt] [PATCH 3/3] qemuMigrationPrecreateDisk: Preserve sparse files
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) {
3672
days inactive
3679
days old
4 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik