[libvirt] [PATCH v3 0/4] Precreate storage on migration
diff to v2: -nits found by Peter fixed Michal Privoznik (4): storage: Introduce storagePoolLookupByTargetPath qemuMonitorJSONBlockStatsUpdateCapacity: Don't skip disks qemu_migration: Send disk sizes to the other side qemu_migration: Precreate missing storage src/libvirt-domain.c | 3 + src/qemu/qemu_migration.c | 319 ++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor_json.c | 14 +- src/storage/storage_driver.c | 42 ++++++ src/storage/storage_driver.h | 4 + 5 files changed, 358 insertions(+), 24 deletions(-) -- 2.0.4
While this could be exposed as a public API, it's not done yet as there's no demand for that yet. Anyway, this is just preparing the environment for easier volume creation on the destination. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 4 ++++ 2 files changed, 46 insertions(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 88dea34..5ff63cd 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1466,6 +1466,48 @@ storageVolLookupByPath(virConnectPtr conn, return ret; } +virStoragePoolPtr +storagePoolLookupByTargetPath(virConnectPtr conn, + const char *path) +{ + size_t i; + virStoragePoolPtr ret = NULL; + char *cleanpath; + + cleanpath = virFileSanitizePath(path); + if (!cleanpath) + return NULL; + + storageDriverLock(); + for (i = 0; i < driver->pools.count && !ret; i++) { + virStoragePoolObjPtr pool = driver->pools.objs[i]; + + virStoragePoolObjLock(pool); + + if (!virStoragePoolObjIsActive(pool)) { + virStoragePoolObjUnlock(pool); + continue; + } + + if (STREQ(path, pool->def->target.path)) { + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + } + + virStoragePoolObjUnlock(pool); + } + storageDriverUnlock(); + + if (!ret) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage pool with matching target path '%s'"), + path); + } + + VIR_FREE(cleanpath); + return ret; +} + static int storageVolDeleteInternal(virStorageVolPtr obj, diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index b805ddd..74da9bb 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -57,6 +57,10 @@ int virStorageFileGetMetadata(virStorageSourcePtr src, int virStorageTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def); +virStoragePoolPtr +storagePoolLookupByTargetPath(virConnectPtr conn, + const char *path); + int storageRegister(void); #endif /* __VIR_STORAGE_DRIVER_H__ */ -- 2.0.4
On 12/02/14 17:13, Michal Privoznik wrote:
While this could be exposed as a public API, it's not done yet as there's no demand for that yet. Anyway, this is just preparing the environment for easier volume creation on the destination.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_driver.h | 4 ++++ 2 files changed, 46 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 88dea34..5ff63cd 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1466,6 +1466,48 @@ storageVolLookupByPath(virConnectPtr conn, return ret; }
+virStoragePoolPtr +storagePoolLookupByTargetPath(virConnectPtr conn, + const char *path) +{ + size_t i; + virStoragePoolPtr ret = NULL; + char *cleanpath; + + cleanpath = virFileSanitizePath(path); + if (!cleanpath) + return NULL; + + storageDriverLock(); + for (i = 0; i < driver->pools.count && !ret; i++) { + virStoragePoolObjPtr pool = driver->pools.objs[i]; + + virStoragePoolObjLock(pool); + + if (!virStoragePoolObjIsActive(pool)) { + virStoragePoolObjUnlock(pool); + continue; + } + + if (STREQ(path, pool->def->target.path)) { + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, + NULL, NULL); + } + + virStoragePoolObjUnlock(pool); + } + storageDriverUnlock(); + + if (!ret) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage pool with matching target path '%s'"), + path); + } + + VIR_FREE(cleanpath); + return ret; +} +
static int storageVolDeleteInternal(virStorageVolPtr obj, diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index b805ddd..74da9bb 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -57,6 +57,10 @@ int virStorageFileGetMetadata(virStorageSourcePtr src, int virStorageTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def);
+virStoragePoolPtr +storagePoolLookupByTargetPath(virConnectPtr conn, + const char *path);
ATTRIBUTE_NONNULL is still missing
+ int storageRegister(void);
#endif /* __VIR_STORAGE_DRIVER_H__ */
ACK if and only if you add the attribute ;) Peter
The function queries the block devices visible to qemu ('query-block') and parses the qemu's output. The info is returned in a hash table which is expected to be pre-filled by qemuMonitorJSONGetAllBlockStatsInfo(). However, in the next patch we are not going to call the latter function at all, so we should make the former function add devices into the hash table if not found there. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6e251b3..162579b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1889,15 +1889,21 @@ int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, if (STRPREFIX(dev_name, QEMU_DRIVE_HOST_PREFIX)) dev_name += strlen(QEMU_DRIVE_HOST_PREFIX); - /* ignore missing info */ - if (!(bstats = virHashLookup(stats, dev_name))) - continue; - /* drive may be empty */ if (!(inserted = virJSONValueObjectGet(dev, "inserted")) || !(image = virJSONValueObjectGet(inserted, "image"))) continue; + if (!(bstats = virHashLookup(stats, dev_name))) { + if (VIR_ALLOC(bstats) < 0) + goto cleanup; + + if (virHashAddEntry(stats, dev_name, bstats) < 0) { + VIR_FREE(bstats); + goto cleanup; + } + } + if (virJSONValueObjectGetNumberUlong(image, "virtual-size", &bstats->capacity) < 0) continue; -- 2.0.4
On 12/02/14 17:13, Michal Privoznik wrote:
The function queries the block devices visible to qemu ('query-block') and parses the qemu's output. The info is returned in a hash table which is expected to be pre-filled by qemuMonitorJSONGetAllBlockStatsInfo(). However, in the next patch we are not going to call the latter function at all, so we should make the former function add devices into the hash table if not found there.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
ACK
Up 'til now, users need to precreate non-shared storage on migration themselves. This is not very friendly requirement and we should do something about it. In this patch, the migration cookie is extended, so that <nbd/> section does not only contain NBD port, but info on disks being migrated. This patch sends a list of pairs of: <disk target; disk size> to the destination. The actual storage allocation is left for next commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_migration.c | 160 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f19e68c..36b7e43 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -141,6 +141,12 @@ 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; }; typedef struct _qemuMigrationCookie qemuMigrationCookie; @@ -206,6 +212,18 @@ qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network) } +static void qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) +{ + if (!nbd) + return; + + while (nbd->ndisks) + VIR_FREE(nbd->disks[--nbd->ndisks].target); + VIR_FREE(nbd->disks); + VIR_FREE(nbd); +} + + static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) { if (!mig) @@ -213,13 +231,13 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) qemuMigrationCookieGraphicsFree(mig->graphics); qemuMigrationCookieNetworkFree(mig->network); + qemuMigrationCookieNBDFree(mig->nbd); VIR_FREE(mig->localHostname); VIR_FREE(mig->remoteHostname); VIR_FREE(mig->name); VIR_FREE(mig->lockState); VIR_FREE(mig->lockDriver); - VIR_FREE(mig->nbd); VIR_FREE(mig->jobInfo); VIR_FREE(mig); } @@ -525,20 +543,64 @@ qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, static int qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, - virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virQEMUDriverPtr driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr stats = NULL; + size_t i; + int ret = -1; /* It is not a bug if there already is a NBD data */ if (!mig->nbd && VIR_ALLOC(mig->nbd) < 0) return -1; + if (vm->def->ndisks && + VIR_ALLOC_N(mig->nbd->disks, vm->def->ndisks) < 0) + return -1; + mig->nbd->ndisks = 0; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuBlockStats *entry; + + if (!stats) { + if (!(stats = virHashCreate(10, virHashValueFree))) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitor(driver, vm); + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domain exited meanwhile")); + goto cleanup; + } + } + + if (!disk->info.alias || + !(entry = virHashLookup(stats, disk->info.alias))) + continue; + + if (VIR_STRDUP(mig->nbd->disks[mig->nbd->ndisks].target, + disk->dst) < 0) + goto cleanup; + mig->nbd->disks[mig->nbd->ndisks].capacity = entry->capacity; + mig->nbd->ndisks++; + } + mig->nbd->port = priv->nbdPort; mig->flags |= QEMU_MIGRATION_COOKIE_NBD; - return 0; + ret = 0; + cleanup: + virHashFree(stats); + return ret; } @@ -763,7 +825,20 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virBufferAddLit(buf, "<nbd"); if (mig->nbd->port) virBufferAsprintf(buf, " port='%d'", mig->nbd->port); - virBufferAddLit(buf, "/>\n"); + if (mig->nbd->ndisks) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + 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); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nbd>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } } if (mig->flags & QEMU_MIGRATION_COOKIE_STATS && mig->jobInfo) @@ -891,6 +966,64 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) } +static qemuMigrationCookieNBDPtr +qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) +{ + qemuMigrationCookieNBDPtr ret = NULL; + char *port = NULL, *capacity = NULL; + size_t i; + int n; + xmlNodePtr *disks = NULL; + xmlNodePtr save_ctxt = ctxt->node; + + if (VIR_ALLOC(ret) < 0) + goto error; + + port = virXPathString("string(./nbd/@port)", ctxt); + if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed nbd port '%s'"), + port); + goto error; + } + + /* Now check if source sent a list of disks to prealloc. We might be + * talking to an older server, so it's not an error if the list is + * missing. */ + if ((n = virXPathNodeSet("./nbd/disk", ctxt, &disks)) > 0) { + if (VIR_ALLOC_N(ret->disks, n) < 0) + goto error; + ret->ndisks = n; + + for (i = 0; i < n; i++) { + ctxt->node = disks[i]; + VIR_FREE(capacity); + + ret->disks[i].target = virXPathString("string(./@target)", ctxt); + capacity = virXPathString("string(./@capacity)", ctxt); + if (virStrToLong_ull(capacity, NULL, 10, + &ret->disks[i].capacity) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed disk capacity: '%s'"), + capacity); + goto error; + } + } + } + + cleanup: + VIR_FREE(port); + VIR_FREE(capacity); + VIR_FREE(disks); + ctxt->node = save_ctxt; + return ret; + error: + qemuMigrationCookieNBDFree(ret); + ret = NULL; + goto cleanup; +} + + static qemuDomainJobInfoPtr qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) { @@ -1123,22 +1256,9 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, goto error; if (flags & QEMU_MIGRATION_COOKIE_NBD && - virXPathBoolean("boolean(./nbd)", ctxt)) { - char *port; - - if (VIR_ALLOC(mig->nbd) < 0) - goto error; - - port = virXPathString("string(./nbd/@port)", ctxt); - if (port && virStrToLong_i(port, NULL, 10, &mig->nbd->port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Malformed nbd port '%s'"), - port); - VIR_FREE(port); - goto error; - } - VIR_FREE(port); - } + virXPathBoolean("boolean(./nbd)", ctxt) && + (!(mig->nbd = qemuMigrationCookieNBDXMLParse(ctxt)))) + goto error; if (flags & QEMU_MIGRATION_COOKIE_STATS && virXPathBoolean("boolean(./statistics)", ctxt) && -- 2.0.4
On 12/02/14 17:13, Michal Privoznik wrote:
Up 'til now, users need to precreate non-shared storage on migration themselves. This is not very friendly requirement and we should do something about it. In this patch, the migration cookie is extended, so that <nbd/> section does not only contain NBD port, but info on disks being migrated. This patch sends a list of pairs of:
<disk target; disk size>
to the destination. The actual storage allocation is left for next commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_migration.c | 160 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 140 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f19e68c..36b7e43 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c
@@ -891,6 +966,64 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) }
+static qemuMigrationCookieNBDPtr +qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) +{ + qemuMigrationCookieNBDPtr ret = NULL; + char *port = NULL, *capacity = NULL; + size_t i; + int n; + xmlNodePtr *disks = NULL; + xmlNodePtr save_ctxt = ctxt->node; + + if (VIR_ALLOC(ret) < 0) + goto error; + + port = virXPathString("string(./nbd/@port)", ctxt); + if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed nbd port '%s'"), + port); + goto error; + } + + /* Now check if source sent a list of disks to prealloc. We might be + * talking to an older server, so it's not an error if the list is + * missing. */ + if ((n = virXPathNodeSet("./nbd/disk", ctxt, &disks)) > 0) { + if (VIR_ALLOC_N(ret->disks, n) < 0) + goto error; + ret->ndisks = n; + + for (i = 0; i < n; i++) { + ctxt->node = disks[i]; + VIR_FREE(capacity); + + ret->disks[i].target = virXPathString("string(./@target)", ctxt);
Note here that the existence of the @target element is not checked in case the cookie XML contains <nbd><disk .. entries. In 4/4 you use the field unconditionally which could lead to a crash with a specially crafted migration cookie. I think a check and a quick error message wouldn't hurt here.
+ capacity = virXPathString("string(./@capacity)", ctxt); + if (virStrToLong_ull(capacity, NULL, 10,
if (!capacity ||
+ &ret->disks[i].capacity) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed disk capacity: '%s'"), + capacity);
NULLSTR(capacity)
+ goto error; + } + } + } + + cleanup: + VIR_FREE(port); + VIR_FREE(capacity); + VIR_FREE(disks); + ctxt->node = save_ctxt; + return ret; + error: + qemuMigrationCookieNBDFree(ret); + ret = NULL; + goto cleanup; +} + + static qemuDomainJobInfoPtr qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) {
ACK with NULL-hardening added. Peter
Based on previous commit, we can now precreate missing volumes. While digging out the functionality from storage driver would be nicer, if you've seen the code it's nearly impossible. So I'm going from the other end: 1) For given disk target, disk path is looked up. 2) For the disk path, storage pool is looked up, a volume XML is constructed and then passed to virStorageVolCreateXML() which has all the knowledge how to create raw images, (encrypted) qcow(2) images, etc. One of the advantages of this approach is, we don't have to care about image conversion - qemu does that for us. So for instance, users can transform qcow2 into raw on migration (if the correct XML is passed to the migration API). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 3 + src/qemu/qemu_migration.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 01bd9e4..8333183 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -4312,6 +4312,9 @@ virDomainMigrateToURI(virDomainPtr domain, * If you want to copy non-shared storage within migration you * can use either VIR_MIGRATE_NON_SHARED_DISK or * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * As of 1.2.11 disks of some types ('file' and 'volume') are + * precreated automatically, if there's a pool defined on the + * destination for the disk path. * * If a hypervisor supports changing the configuration of the guest * during migration, the @dxml parameter specifies the new config diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 36b7e43..11e2a2c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -58,6 +58,7 @@ #include "virtypedparam.h" #include "virprocess.h" #include "nwfilter_conf.h" +#include "storage/storage_driver.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1451,6 +1452,161 @@ qemuMigrationRestoreDomainState(virConnectPtr conn, virDomainObjPtr vm) return ret; } + +static int +qemuMigrationPrecreateDisk(virConnectPtr conn, + virDomainDiskDefPtr disk, + unsigned long long capacity) +{ + int ret = -1; + virStoragePoolPtr pool = NULL; + virStorageVolPtr vol = NULL; + char *volName = NULL, *basePath = NULL; + char *volStr = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *format = NULL; + unsigned int flags = 0; + + VIR_DEBUG("Precreate disk type=%s", virStorageTypeToString(disk->src->type)); + + switch ((virStorageType) disk->src->type) { + case VIR_STORAGE_TYPE_FILE: + if (!virDomainDiskGetSource(disk)) { + VIR_DEBUG("Dropping sourceless disk '%s'", + disk->dst); + return 0; + } + + if (VIR_STRDUP(basePath, disk->src->path) < 0) + goto cleanup; + + if (!(volName = strrchr(basePath, '/'))) { + virReportError(VIR_ERR_INVALID_ARG, + _("malformed disk path: %s"), + disk->src->path); + goto cleanup; + } + + *volName = '\0'; + volName++; + + if (!(pool = storagePoolLookupByTargetPath(conn, basePath))) + goto cleanup; + format = virStorageFileFormatTypeToString(disk->src->format); + if (disk->src->format == VIR_STORAGE_FILE_QCOW2) + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + break; + + case VIR_STORAGE_TYPE_VOLUME: + if (!(pool = virStoragePoolLookupByName(conn, disk->src->srcpool->pool))) + goto cleanup; + format = virStorageFileFormatTypeToString(disk->src->format); + volName = disk->src->srcpool->volume; + if (disk->src->format == VIR_STORAGE_FILE_QCOW2) + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + break; + + case VIR_STORAGE_TYPE_BLOCK: + case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_NETWORK: + case VIR_STORAGE_TYPE_NONE: + case VIR_STORAGE_TYPE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot precreate storage for disk type '%s'"), + virStorageTypeToString(disk->src->type)); + goto cleanup; + break; + } + + if ((vol = virStorageVolLookupByName(pool, volName))) { + VIR_DEBUG("Skipping creation of already existing volume of name '%s'", + volName); + ret = 0; + goto cleanup; + } + + virBufferAddLit(&buf, "<volume>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", volName); + virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n", capacity); + virBufferAddLit(&buf, "<target>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<format type='%s'/>\n", format); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</target>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</volume>\n"); + + if (!(volStr = virBufferContentAndReset(&buf))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to create volume XML")); + goto cleanup; + } + + if (!(vol = virStorageVolCreateXML(pool, volStr, flags))) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(basePath); + VIR_FREE(volStr); + if (vol) + virStorageVolFree(vol); + if (pool) + virStoragePoolFree(pool); + return ret; +} + + +static int +qemuMigrationPrecreateStorage(virConnectPtr conn, + virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + qemuMigrationCookieNBDPtr nbd) +{ + int ret = -1; + size_t i = 0; + + if (!nbd || !nbd->ndisks) + return 0; + + for (i = 0; i < nbd->ndisks; i++) { + virDomainDiskDefPtr disk; + int indx; + const char *diskSrcPath; + + VIR_DEBUG("Looking up disk target '%s' (capacity=%lluu)", + nbd->disks[i].target, nbd->disks[i].capacity); + + if ((indx = virDomainDiskIndexByName(vm->def, + nbd->disks[i].target, false)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to find disk by target: %s"), + nbd->disks[i].target); + goto cleanup; + } + + disk = vm->def->disks[indx]; + diskSrcPath = virDomainDiskGetSource(disk); + + if (disk->src->shared || disk->src->readonly || + (diskSrcPath && virFileExists(diskSrcPath))) { + /* Skip shared, read-only and already existing disks. */ + continue; + } + + VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath)); + + if (qemuMigrationPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + + /** * qemuMigrationStartNBDServer: * @driver: qemu driver @@ -2835,6 +2991,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto cleanup; } + if (qemuMigrationPrecreateStorage(dconn, driver, vm, mig->nbd) < 0) + goto cleanup; + if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PREPARE); -- 2.0.4
On 12/02/14 17:13, Michal Privoznik wrote:
Based on previous commit, we can now precreate missing volumes. While digging out the functionality from storage driver would be nicer, if you've seen the code it's nearly impossible. So I'm going from the other end:
1) For given disk target, disk path is looked up. 2) For the disk path, storage pool is looked up, a volume XML is constructed and then passed to virStorageVolCreateXML() which has all the knowledge how to create raw images, (encrypted) qcow(2) images, etc.
One of the advantages of this approach is, we don't have to care about image conversion - qemu does that for us. So for instance, users can transform qcow2 into raw on migration (if the correct XML is passed to the migration API).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-domain.c | 3 + src/qemu/qemu_migration.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 01bd9e4..8333183 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -4312,6 +4312,9 @@ virDomainMigrateToURI(virDomainPtr domain, * If you want to copy non-shared storage within migration you * can use either VIR_MIGRATE_NON_SHARED_DISK or * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. + * As of 1.2.11 disks of some types ('file' and 'volume') are + * precreated automatically, if there's a pool defined on the + * destination for the disk path. * * If a hypervisor supports changing the configuration of the guest * during migration, the @dxml parameter specifies the new config diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 36b7e43..11e2a2c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -58,6 +58,7 @@ #include "virtypedparam.h" #include "virprocess.h" #include "nwfilter_conf.h" +#include "storage/storage_driver.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -1451,6 +1452,161 @@ qemuMigrationRestoreDomainState(virConnectPtr conn, virDomainObjPtr vm) return ret; }
+ +static int +qemuMigrationPrecreateDisk(virConnectPtr conn, + virDomainDiskDefPtr disk, + unsigned long long capacity) +{ + int ret = -1; + virStoragePoolPtr pool = NULL; + virStorageVolPtr vol = NULL; + char *volName = NULL, *basePath = NULL; + char *volStr = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *format = NULL; + unsigned int flags = 0; + + VIR_DEBUG("Precreate disk type=%s", virStorageTypeToString(disk->src->type)); + + switch ((virStorageType) disk->src->type) { + case VIR_STORAGE_TYPE_FILE: + if (!virDomainDiskGetSource(disk)) { + VIR_DEBUG("Dropping sourceless disk '%s'", + disk->dst); + return 0; + } + + if (VIR_STRDUP(basePath, disk->src->path) < 0) + goto cleanup; + + if (!(volName = strrchr(basePath, '/'))) { + virReportError(VIR_ERR_INVALID_ARG, + _("malformed disk path: %s"), + disk->src->path); + goto cleanup; + } + + *volName = '\0'; + volName++; + + if (!(pool = storagePoolLookupByTargetPath(conn, basePath))) + goto cleanup; + format = virStorageFileFormatTypeToString(disk->src->format); + if (disk->src->format == VIR_STORAGE_FILE_QCOW2) + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + break; + + case VIR_STORAGE_TYPE_VOLUME: + if (!(pool = virStoragePoolLookupByName(conn, disk->src->srcpool->pool))) + goto cleanup; + format = virStorageFileFormatTypeToString(disk->src->format); + volName = disk->src->srcpool->volume; + if (disk->src->format == VIR_STORAGE_FILE_QCOW2) + flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + break; + + case VIR_STORAGE_TYPE_BLOCK: + case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_NETWORK: + case VIR_STORAGE_TYPE_NONE: + case VIR_STORAGE_TYPE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot precreate storage for disk type '%s'"), + virStorageTypeToString(disk->src->type)); + goto cleanup; + break; + } + + if ((vol = virStorageVolLookupByName(pool, volName))) { + VIR_DEBUG("Skipping creation of already existing volume of name '%s'", + volName); + ret = 0; + goto cleanup; + } + + virBufferAddLit(&buf, "<volume>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", volName); + virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n", capacity); + virBufferAddLit(&buf, "<target>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<format type='%s'/>\n", format); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</target>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</volume>\n"); + + if (!(volStr = virBufferContentAndReset(&buf))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to create volume XML")); + goto cleanup; + } + + if (!(vol = virStorageVolCreateXML(pool, volStr, flags))) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(basePath); + VIR_FREE(volStr); + if (vol) + virStorageVolFree(vol); + if (pool) + virStoragePoolFree(pool);
I just ACKed John's series that forbids the virStorageVolFree and virStoragePoolFree functions in our codebase. You'll need to change them to virObjectUnref().
+ return ret; +} + + +static int +qemuMigrationPrecreateStorage(virConnectPtr conn, + virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + qemuMigrationCookieNBDPtr nbd) +{ + int ret = -1; + size_t i = 0; + + if (!nbd || !nbd->ndisks) + return 0; + + for (i = 0; i < nbd->ndisks; i++) { + virDomainDiskDefPtr disk; + int indx; + const char *diskSrcPath; + + VIR_DEBUG("Looking up disk target '%s' (capacity=%lluu)", + nbd->disks[i].target, nbd->disks[i].capacity);
Following up on 3/4: here and below you use the "target" field without verifying that it was provided. Sounds like bikeshedding, but if we can be robust here, we should be. I think the check should be in 3/4
+ + if ((indx = virDomainDiskIndexByName(vm->def, + nbd->disks[i].target, false)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to find disk by target: %s"), + nbd->disks[i].target); + goto cleanup; + } + + disk = vm->def->disks[indx]; + diskSrcPath = virDomainDiskGetSource(disk); + + if (disk->src->shared || disk->src->readonly || + (diskSrcPath && virFileExists(diskSrcPath))) { + /* Skip shared, read-only and already existing disks. */ + continue; + } + + VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath)); + + if (qemuMigrationPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + + /** * qemuMigrationStartNBDServer: * @driver: qemu driver
ACK if you change the virStorage*Free funcs. Peter
participants (2)
-
Michal Privoznik -
Peter Krempa