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(a)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