[libvirt] [PATCH 1/2] conf: make disk source pool translation generic

Currently, qemu driver uses qemuTranslateDiskSourcePool() to translate disk volume information. This function is general enough and could be used for other drivers as well, so move it to conf/domain_conf.c along with its helpers. - qemuTranslateDiskSourcePool: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePool, - qemuAddISCSIPoolSourceHost: move to conf/domain_conf.c and rename to virDomainAddISCSIPoolSourceHost, - qemuTranslateDiskSourcePoolAuth: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePoolAuth, - Expose virDomainTranslateDiskSourcePool through libvirt_private.syms, - Update users of virDomainTranslateDiskSourcePool to use a new name. --- src/conf/domain_conf.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 243 ---------------------------------------------- src/qemu/qemu_conf.h | 3 - src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 4 +- tests/qemuxml2argvtest.c | 2 +- 9 files changed, 256 insertions(+), 253 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5c762fa..d5f80e0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20045,6 +20045,251 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) } +static int +virDomainAddISCSIPoolSourceHost(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + int ret = -1; + char **tokens = NULL; + + /* Only support one host */ + if (pooldef->source.nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + } + + /* iscsi pool only supports one host */ + def->src->nhosts = 1; + + if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0) + goto cleanup; + + if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0) + goto cleanup; + + if (virAsprintf(&def->src->hosts[0].port, "%d", + pooldef->source.hosts[0].port ? + pooldef->source.hosts[0].port : + 3260) < 0) + goto cleanup; + + /* iscsi volume has name like "unit:0:0:1" */ + if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0))) + goto cleanup; + + if (virStringListLength(tokens) != 4) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected iscsi volume name '%s'"), + def->src->srcpool->volume); + goto cleanup; + } + + /* iscsi pool has only one source device path */ + if (virAsprintf(&def->src->path, "%s/%s", + pooldef->source.devices[0].path, + tokens[3]) < 0) + goto cleanup; + + /* Storage pool have not supported these 2 attributes yet, + * use the defaults. + */ + def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + def->src->hosts[0].socket = NULL; + + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + + ret = 0; + + cleanup: + virStringFreeList(tokens); + return ret; +} + + +static int +virDomainTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, + virStoragePoolSourcePtr source) +{ + int ret = -1; + + /* Only necessary when authentication set */ + if (!source->auth) { + ret = 0; + goto cleanup; + } + def->src->auth = virStorageAuthDefCopy(source->auth); + if (!def->src->auth) + goto cleanup; + ret = 0; + + cleanup: + return ret; +} + + +int +virDomainTranslateDiskSourcePool(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + virStoragePoolDefPtr pooldef = NULL; + virStoragePoolPtr pool = NULL; + virStorageVolPtr vol = NULL; + char *poolxml = NULL; + virStorageVolInfo info; + int ret = -1; + virErrorPtr savedError = NULL; + + if (def->src->type != VIR_STORAGE_TYPE_VOLUME) + return 0; + + if (!def->src->srcpool) + return 0; + + if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool))) + return -1; + + if (virStoragePoolIsActive(pool) != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage pool '%s' containing volume '%s' " + "is not active"), + def->src->srcpool->pool, def->src->srcpool->volume); + goto cleanup; + } + + if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume))) + goto cleanup; + + if (virStorageVolGetInfo(vol, &info) < 0) + goto cleanup; + + if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) + goto cleanup; + + if (!(pooldef = virStoragePoolDefParseString(poolxml))) + goto cleanup; + + def->src->srcpool->pooltype = pooldef->type; + def->src->srcpool->voltype = info.type; + + if (def->src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk source mode is only valid when " + "storage pool is of iscsi type")); + goto cleanup; + } + + VIR_FREE(def->src->path); + virStorageNetHostDefFree(def->src->nhosts, def->src->hosts); + virStorageAuthDefFree(def->src->auth); + + switch ((virStoragePoolType) pooldef->type) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_NETFS: + case VIR_STORAGE_POOL_LOGICAL: + case VIR_STORAGE_POOL_DISK: + case VIR_STORAGE_POOL_SCSI: + case VIR_STORAGE_POOL_ZFS: + if (!(def->src->path = virStorageVolGetPath(vol))) + goto cleanup; + + if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + + switch (info.type) { + case VIR_STORAGE_VOL_FILE: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; + break; + + case VIR_STORAGE_VOL_DIR: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; + break; + + case VIR_STORAGE_VOL_BLOCK: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + break; + + case VIR_STORAGE_VOL_NETWORK: + case VIR_STORAGE_VOL_NETDIR: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage volume type '%s' " + "for storage pool type '%s'"), + virStorageVolTypeToString(info.type), + virStoragePoolTypeToString(pooldef->type)); + goto cleanup; + } + + break; + + case VIR_STORAGE_POOL_ISCSI: + if (def->startupPolicy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + switch (def->src->srcpool->mode) { + case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT: + case VIR_STORAGE_SOURCE_POOL_MODE_LAST: + def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; + /* fallthrough */ + case VIR_STORAGE_SOURCE_POOL_MODE_HOST: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + if (!(def->src->path = virStorageVolGetPath(vol))) + goto cleanup; + break; + + case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + + if (virDomainTranslateDiskSourcePoolAuth(def, &pooldef->source) < 0) + goto cleanup; + + if (virDomainAddISCSIPoolSourceHost(def, pooldef) < 0) + goto cleanup; + break; + } + break; + + case VIR_STORAGE_POOL_MPATH: + case VIR_STORAGE_POOL_RBD: + case VIR_STORAGE_POOL_SHEEPDOG: + case VIR_STORAGE_POOL_GLUSTER: + case VIR_STORAGE_POOL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("using '%s' pools for backing 'volume' disks " + "isn't yet supported"), + virStoragePoolTypeToString(pooldef->type)); + goto cleanup; + } + + ret = 0; + cleanup: + if (ret < 0) + savedError = virSaveLastError(); + if (pool) + virStoragePoolFree(pool); + if (vol) + virStorageVolFree(vol); + if (savedError) { + virSetError(savedError); + virFreeError(savedError); + } + + VIR_FREE(poolxml); + virStoragePoolDefFree(pooldef); + return ret; +} + + char * virDomainObjGetMetadata(virDomainObjPtr vm, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ff7d640..d745072 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2716,6 +2716,9 @@ int virDomainDefFindDevice(virDomainDefPtr def, bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) ATTRIBUTE_NONNULL(1); +int virDomainTranslateDiskSourcePool(virConnectPtr conn, + virDomainDiskDefPtr def); + void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); char *virDomainObjGetMetadata(virDomainObjPtr vm, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..975c414 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -407,6 +407,7 @@ virDomainTPMBackendTypeToString; virDomainTPMDefFree; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; +virDomainTranslateDiskSourcePool; virDomainVcpuPinAdd; virDomainVcpuPinDefArrayFree; virDomainVcpuPinDefCopy; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 238d2b1..eef5be1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1225,249 +1225,6 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver) return virAtomicIntInc(&driver->nextvmid); } -static int -qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def, - virStoragePoolDefPtr pooldef) -{ - int ret = -1; - char **tokens = NULL; - - /* Only support one host */ - if (pooldef->source.nhost != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Expected exactly 1 host for the storage pool")); - goto cleanup; - } - - /* iscsi pool only supports one host */ - def->src->nhosts = 1; - - if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0) - goto cleanup; - - if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0) - goto cleanup; - - if (virAsprintf(&def->src->hosts[0].port, "%d", - pooldef->source.hosts[0].port ? - pooldef->source.hosts[0].port : - 3260) < 0) - goto cleanup; - - /* iscsi volume has name like "unit:0:0:1" */ - if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0))) - goto cleanup; - - if (virStringListLength(tokens) != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected iscsi volume name '%s'"), - def->src->srcpool->volume); - goto cleanup; - } - - /* iscsi pool has only one source device path */ - if (virAsprintf(&def->src->path, "%s/%s", - pooldef->source.devices[0].path, - tokens[3]) < 0) - goto cleanup; - - /* Storage pool have not supported these 2 attributes yet, - * use the defaults. - */ - def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - def->src->hosts[0].socket = NULL; - - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - ret = 0; - - cleanup: - virStringFreeList(tokens); - return ret; -} - -static int -qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, - virStoragePoolSourcePtr source) -{ - int ret = -1; - - /* Only necessary when authentication set */ - if (!source->auth) { - ret = 0; - goto cleanup; - } - def->src->auth = virStorageAuthDefCopy(source->auth); - if (!def->src->auth) - goto cleanup; - ret = 0; - - cleanup: - return ret; -} - - -int -qemuTranslateDiskSourcePool(virConnectPtr conn, - virDomainDiskDefPtr def) -{ - virStoragePoolDefPtr pooldef = NULL; - virStoragePoolPtr pool = NULL; - virStorageVolPtr vol = NULL; - char *poolxml = NULL; - virStorageVolInfo info; - int ret = -1; - virErrorPtr savedError = NULL; - - if (def->src->type != VIR_STORAGE_TYPE_VOLUME) - return 0; - - if (!def->src->srcpool) - return 0; - - if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool))) - return -1; - - if (virStoragePoolIsActive(pool) != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("storage pool '%s' containing volume '%s' " - "is not active"), - def->src->srcpool->pool, def->src->srcpool->volume); - goto cleanup; - } - - if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume))) - goto cleanup; - - if (virStorageVolGetInfo(vol, &info) < 0) - goto cleanup; - - if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) - goto cleanup; - - if (!(pooldef = virStoragePoolDefParseString(poolxml))) - goto cleanup; - - def->src->srcpool->pooltype = pooldef->type; - def->src->srcpool->voltype = info.type; - - if (def->src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk source mode is only valid when " - "storage pool is of iscsi type")); - goto cleanup; - } - - VIR_FREE(def->src->path); - virStorageNetHostDefFree(def->src->nhosts, def->src->hosts); - virStorageAuthDefFree(def->src->auth); - - switch ((virStoragePoolType) pooldef->type) { - case VIR_STORAGE_POOL_DIR: - case VIR_STORAGE_POOL_FS: - case VIR_STORAGE_POOL_NETFS: - case VIR_STORAGE_POOL_LOGICAL: - case VIR_STORAGE_POOL_DISK: - case VIR_STORAGE_POOL_SCSI: - case VIR_STORAGE_POOL_ZFS: - if (!(def->src->path = virStorageVolGetPath(vol))) - goto cleanup; - - if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - goto cleanup; - } - - - switch (info.type) { - case VIR_STORAGE_VOL_FILE: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; - break; - - case VIR_STORAGE_VOL_DIR: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; - break; - - case VIR_STORAGE_VOL_BLOCK: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; - break; - - case VIR_STORAGE_VOL_NETWORK: - case VIR_STORAGE_VOL_NETDIR: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected storage volume type '%s' " - "for storage pool type '%s'"), - virStorageVolTypeToString(info.type), - virStoragePoolTypeToString(pooldef->type)); - goto cleanup; - } - - break; - - case VIR_STORAGE_POOL_ISCSI: - if (def->startupPolicy) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - goto cleanup; - } - - switch (def->src->srcpool->mode) { - case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT: - case VIR_STORAGE_SOURCE_POOL_MODE_LAST: - def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; - /* fallthrough */ - case VIR_STORAGE_SOURCE_POOL_MODE_HOST: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; - if (!(def->src->path = virStorageVolGetPath(vol))) - goto cleanup; - break; - - case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - if (qemuTranslateDiskSourcePoolAuth(def, &pooldef->source) < 0) - goto cleanup; - - if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0) - goto cleanup; - break; - } - break; - - case VIR_STORAGE_POOL_MPATH: - case VIR_STORAGE_POOL_RBD: - case VIR_STORAGE_POOL_SHEEPDOG: - case VIR_STORAGE_POOL_GLUSTER: - case VIR_STORAGE_POOL_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("using '%s' pools for backing 'volume' disks " - "isn't yet supported"), - virStoragePoolTypeToString(pooldef->type)); - goto cleanup; - } - - ret = 0; - cleanup: - if (ret < 0) - savedError = virSaveLastError(); - if (pool) - virStoragePoolFree(pool); - if (vol) - virStorageVolFree(vol); - if (savedError) { - virSetError(savedError); - virFreeError(savedError); - } - - VIR_FREE(poolxml); - virStoragePoolDefFree(pooldef); - return ret; -} - int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED, diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 90aebef..3276412 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -307,9 +307,6 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev); int qemuDriverAllocateID(virQEMUDriverPtr driver); virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); -int qemuTranslateDiskSourcePool(virConnectPtr conn, - virDomainDiskDefPtr def); - int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, virDomainSnapshotDiskDefPtr def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac0717c..e0411b1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6635,7 +6635,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, virCapsPtr caps = NULL; int ret = -1; - if (qemuTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainTranslateDiskSourcePool(conn, disk) < 0) goto end; if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) @@ -12581,7 +12581,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, return -1; if (!active) { - if (qemuTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainTranslateDiskSourcePool(conn, disk) < 0) return -1; if (qemuDomainSnapshotPrepareDiskExternalBackingInactive(disk) < 0) @@ -12639,7 +12639,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, if (active) return 0; - if (qemuTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainTranslateDiskSourcePool(conn, disk) < 0) return -1; actualType = virStorageSourceGetActualType(disk->src); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f7e223a..1d54de6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -722,7 +722,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, goto end; } - if (qemuTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainTranslateDiskSourcePool(conn, disk) < 0) goto end; if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 13c396f..4ceb54e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3321,7 +3321,7 @@ qemuProcessReconnect(void *opaque) for (i = 0; i < obj->def->ndisks; i++) { virDomainDeviceDef dev; - if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0) + if (virDomainTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0) goto error; /* XXX we should be able to restore all data from XML in the future */ @@ -4000,7 +4000,7 @@ int qemuProcessStart(virConnectPtr conn, * cgroup and security setting. */ for (i = 0; i < vm->def->ndisks; i++) { - if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) + if (virDomainTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) goto cleanup; } diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 62b969c..7c3eb7c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -351,7 +351,7 @@ static int testCompareXMLToArgvFiles(const char *xml, } for (i = 0; i < vmdef->ndisks; i++) { - if (qemuTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0) + if (virDomainTranslateDiskSourcePool(conn, vmdef->disks[i]) < 0) goto out; } -- 1.9.0

Update bhyveBuildDiskArgStr to support volumes: - Make virBhyveProcessBuildBhyveCmd take virConnectPtr as the first argument instead of bhyveConnPtr as virConnectPtr is needed for virDomainTranslateDiskSourcePool, - Add virDomainTranslateDiskSourcePool call to virBhyveProcessBuildBhyveCmd, - Allow disks of type VIR_STORAGE_TYPE_VOLUME --- src/bhyve/bhyve_command.c | 8 ++++++-- src/bhyve/bhyve_command.h | 5 +++-- src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 2 +- tests/bhyvexml2argvtest.c | 5 ++++- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index e2940e8..e9fe1b7 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -184,7 +184,8 @@ bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return -1; } - if (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) { + if ((virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) && + (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_VOLUME)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk type")); return -1; @@ -209,7 +210,7 @@ bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, } virCommandPtr -virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, +virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainDefPtr def, bool dryRun) { /* @@ -263,6 +264,9 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, for (i = 0; i < def->ndisks; i++) { virDomainDiskDefPtr disk = def->disks[i]; + if (virDomainTranslateDiskSourcePool(conn, disk) < 0) + goto error; + if (bhyveBuildDiskArgStr(def, disk, cmd) < 0) goto error; } diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h index 31de97a..5b0045c 100644 --- a/src/bhyve/bhyve_command.h +++ b/src/bhyve/bhyve_command.h @@ -29,8 +29,9 @@ # define BHYVE_CONFIG_FORMAT_ARGV "bhyve-argv" -virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr, - virDomainDefPtr def, bool dryRun); +virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, + virDomainDefPtr def, + bool dryRun); virCommandPtr virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver, diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index eb8f9af..dcaf847 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -692,7 +692,7 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, if (!(loadcmd = virBhyveProcessBuildLoadCmd(privconn, def))) goto cleanup; - if (!(cmd = virBhyveProcessBuildBhyveCmd(privconn, def, true))) + if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true))) goto cleanup; virBufferAdd(&buf, virCommandToString(loadcmd), -1); diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 168202e..03c72e1 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -137,7 +137,7 @@ virBhyveProcessStart(virConnectPtr conn, goto cleanup; /* Call bhyve to start the VM */ - if (!(cmd = virBhyveProcessBuildBhyveCmd(driver, + if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vm->def, false))) goto cleanup; diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 408c73a..b9be378 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -23,8 +23,11 @@ static int testCompareXMLToArgvFiles(const char *xml, virDomainDefPtr vmdef = NULL; virDomainObj vm; virCommandPtr cmd = NULL; + virConnectPtr conn; int ret = -1; + if (!(conn = virGetConnect())) + goto out; if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, 1 << VIR_DOMAIN_VIRT_BHYVE, @@ -33,7 +36,7 @@ static int testCompareXMLToArgvFiles(const char *xml, vm.def = vmdef; - if (!(cmd = virBhyveProcessBuildBhyveCmd(&driver, vmdef, false))) + if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false))) goto out; if (!(actualargv = virCommandToString(cmd))) -- 1.9.0

On Thu, Aug 14, 2014 at 08:22:07PM +0400, Roman Bogorodskiy wrote:
Update bhyveBuildDiskArgStr to support volumes:
- Make virBhyveProcessBuildBhyveCmd take virConnectPtr as the first argument instead of bhyveConnPtr as virConnectPtr is needed for virDomainTranslateDiskSourcePool, - Add virDomainTranslateDiskSourcePool call to virBhyveProcessBuildBhyveCmd, - Allow disks of type VIR_STORAGE_TYPE_VOLUME --- src/bhyve/bhyve_command.c | 8 ++++++-- src/bhyve/bhyve_command.h | 5 +++-- src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 2 +- tests/bhyvexml2argvtest.c | 5 ++++- 5 files changed, 15 insertions(+), 7 deletions(-)
I can't build with bhyve support, but ACK, Martin

On Thu, Aug 14, 2014 at 08:22:06PM +0400, Roman Bogorodskiy wrote:
Currently, qemu driver uses qemuTranslateDiskSourcePool() to translate disk volume information. This function is general enough and could be used for other drivers as well, so move it to conf/domain_conf.c along with its helpers.
- qemuTranslateDiskSourcePool: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePool, - qemuAddISCSIPoolSourceHost: move to conf/domain_conf.c and rename to virDomainAddISCSIPoolSourceHost, - qemuTranslateDiskSourcePoolAuth: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePoolAuth, - Expose virDomainTranslateDiskSourcePool through libvirt_private.syms, - Update users of virDomainTranslateDiskSourcePool to use a new name. --- src/conf/domain_conf.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 243 ---------------------------------------------- src/qemu/qemu_conf.h | 3 - src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 4 +- tests/qemuxml2argvtest.c | 2 +- 9 files changed, 256 insertions(+), 253 deletions(-)
The only problem with this patch is that I can't build when I apply it. libvirt_lxc (the binary) does not get built because unresolved dependencies in the libs. I cameup with a fix, but I'm pretty sure that's not what we want. However, it works nice with it: diff --git i/src/Makefile.am w/src/Makefile.am index f69923f..0c4c8ae 100644 --- i/src/Makefile.am +++ w/src/Makefile.am @@ -2568,6 +2568,7 @@ libvirt_lxc_SOURCES = \ libvirt_lxc_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ + -lvirt \ $(NULL) libvirt_lxc_LDADD = \ $(FUSE_LIBS) \ -- Martin

On 08/15/14 15:35, Martin Kletzander wrote:
On Thu, Aug 14, 2014 at 08:22:06PM +0400, Roman Bogorodskiy wrote:
Currently, qemu driver uses qemuTranslateDiskSourcePool() to translate disk volume information. This function is general enough and could be used for other drivers as well, so move it to conf/domain_conf.c along with its helpers.
- qemuTranslateDiskSourcePool: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePool, - qemuAddISCSIPoolSourceHost: move to conf/domain_conf.c and rename to virDomainAddISCSIPoolSourceHost, - qemuTranslateDiskSourcePoolAuth: move to conf/domain_conf.c and rename to virDomainTranslateDiskSourcePoolAuth, - Expose virDomainTranslateDiskSourcePool through libvirt_private.syms, - Update users of virDomainTranslateDiskSourcePool to use a new name. --- src/conf/domain_conf.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 243 ---------------------------------------------- src/qemu/qemu_conf.h | 3 - src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 4 +- tests/qemuxml2argvtest.c | 2 +- 9 files changed, 256 insertions(+), 253 deletions(-)
The only problem with this patch is that I can't build when I apply it. libvirt_lxc (the binary) does not get built because unresolved dependencies in the libs. I cameup with a fix, but I'm pretty sure that's not what we want. However, it works nice with it:
diff --git i/src/Makefile.am w/src/Makefile.am index f69923f..0c4c8ae 100644 --- i/src/Makefile.am +++ w/src/Makefile.am @@ -2568,6 +2568,7 @@ libvirt_lxc_SOURCES = \ libvirt_lxc_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ + -lvirt \ $(NULL) libvirt_lxc_LDADD = \ $(FUSE_LIBS) \ --
In that case it will be better to move the code to the storage driver so that we don't pull too much stuff into libvirt_lxc Peter
participants (3)
-
Martin Kletzander
-
Peter Krempa
-
Roman Bogorodskiy