Re: [PATCH 4/9 v2] Implement translateDiskSourcePool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> +            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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]