On 2013年02月04日 22:03, John Ferlan wrote:
On 02/01/2013 12:24 PM, Osier Yang wrote:It iterates over all the domain disks, and translate the source of all the disks of 'volume' type from storage pool/volume to the real underlying source. Disks of type 'file', 'block', and 'dir' are supported now. Network type is not supported yet, it will be another patch. src/storage/storage_driver.c: * New helper storagePoolObjFindByDiskSource to find the specified pool by name. * Implement translateDiskSourcePool as storageTranslateDomainDiskSourcePool --- src/storage/storage_driver.c | 106 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 106 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index e98c18c..a2c3a1a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -47,6 +47,8 @@ #include "virlog.h" #include "virfile.h" #include "fdstream.h" +#include "domain_conf.h" +#include "domain_storage.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2367,6 +2369,104 @@ storageListAllPools(virConnectPtr conn, return ret; } +static virStoragePoolObjPtr +storagePoolObjFindByDiskSource(virConnectPtr conn, + const char *name) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolObjPtr pool = NULL; + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, name); + storageDriverUnlock(driver); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching name '%s'"), + name); + goto error; + } + + if (!virStoragePoolObjIsActive(pool)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("The specified pool '%s' is not active"), + name); + goto error; + } + + return pool; +error: + if (pool) + virStoragePoolObjUnlock(pool); + return NULL; +} + +static int +storageTranslateDomainDiskSourcePool(virConnectPtr conn, + virDomainDefPtr def) +{ + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + int i; + int ret = -1; + + for (i = 0; i< def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->type != VIR_DOMAIN_DISK_TYPE_VOLUME) + continue; + + if (!(pool = storagePoolObjFindByDiskSource(conn, disk->srcpool->pool))) + goto cleanup; + + if (!(vol = virStorageVolDefFindByName(pool, disk->srcpool->volume))) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage volume of pool '%s' with " + "matching name '%s'"), + disk->srcpool->pool, + disk->srcpool->volume); + goto cleanup; + } + + switch (vol->type) { + case VIR_STORAGE_VOL_FILE: + case VIR_STORAGE_VOL_BLOCK: + case VIR_STORAGE_VOL_DIR: + if (!vol->target.path&& + disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM&& + disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("target path of the specified volume '%s' " + "(pool = '%s') is empty"), + disk->srcpool->volume + disk->srcpool->pool); + goto cleanup; + } + + if (vol->target.path&& + !(disk->src = strdup(vol->target.path))) { + virReportOOMError(); + goto cleanup; + }Just checking - if !vol->target.path, then disk->src will be empty, is that expected at this point? You've added the first check since the initial pass at this and I want to make sure you've covered your cases. Without comments in the code I'm not sure what is expected.
CD-ROM and Floppy disk allow the disk source be empty. So as long as the code flows to here. It means the disk is either a CD-ROM or a Floppy, as we checked it before. But I agreed with you that adding a comment here will be better. Will add it when pushing.
+ break; + case VIR_STORAGE_VOL_NETWORK: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Using network volume as disk source is not supported")); + goto cleanup; + } + + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + ret = 0; + +cleanup: + if (pool) + virStoragePoolObjUnlock(pool); + return ret; +} + static virStorageDriver storageDriver = { .name = "storage", .open = storageOpen, /* 0.4.0 */ @@ -2423,8 +2523,14 @@ static virStateDriver stateDriver = { .reload = storageDriverReload, }; +static virDomainStorageDriver domainStorageDriver = { + .translateDiskSourcePool = storageTranslateDomainDiskSourcePool, +}; + + int storageRegister(void) { virRegisterStorageDriver(&storageDriver); virRegisterStateDriver(&stateDriver); + virRegisterDomainStorageDriver(&domainStorageDriver); return 0; }ACK (as long as you feel you've covered your cases for the disk storage pool) John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list