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

Re: [libvirt] [PATCH REPOST] esx: Add read-only storage volume access



On Mon, Aug 23, 2010 at 12:25:19AM +0200, Matthias Bolte wrote:
> This allows to list existing volumes and to retrieve information
> about them.
> ---
>  src/esx/esx_driver.c           |  112 +----------
>  src/esx/esx_storage_driver.c   |  433 +++++++++++++++++++++++++++++++++++++++-
>  src/esx/esx_vi.c               |  279 ++++++++++++++++++++++++++
>  src/esx/esx_vi.h               |    9 +
>  src/esx/esx_vi_generator.input |    7 +
>  5 files changed, 724 insertions(+), 116 deletions(-)
> +static char *
> +esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags)
> +{
> +    esxPrivate *priv = volume->conn->storagePrivateData;
> +    esxVI_String *propertyNameList = NULL;
> +    esxVI_ObjectContent *datastore = NULL;
> +    esxVI_DynamicProperty *dynamicProperty = NULL;
> +    esxVI_DatastoreInfo *datastoreInfo = NULL;
> +    virStoragePoolDef pool;
> +    char *datastorePath = NULL;
> +    esxVI_FileInfo *fileInfo = NULL;
> +    esxVI_VmDiskFileInfo *vmDiskFileInfo = NULL;
> +    esxVI_IsoImageFileInfo *isoImageFileInfo = NULL;
> +    esxVI_FloppyImageFileInfo *floppyImageFileInfo = NULL;
> +    virStorageVolDef def;
> +    char *xml = NULL;
> +
> +    virCheckFlags(0, NULL);
> +
> +    memset(&pool, 0, sizeof (pool));
> +    memset(&def, 0, sizeof (def));
> +
> +    if (esxVI_EnsureSession(priv->primary) < 0) {
> +        return NULL;
> +    }
> +
> +    /* Lookup storage pool type */
> +    if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 ||
> +        esxVI_LookupDatastoreByName(priv->primary, volume->pool,
> +                                    propertyNameList, &datastore,
> +                                    esxVI_Occurrence_RequiredItem) < 0) {
> +        goto cleanup;
> +    }
> +
> +    for (dynamicProperty = datastore->propSet; dynamicProperty != NULL;
> +         dynamicProperty = dynamicProperty->_next) {
> +        if (STREQ(dynamicProperty->name, "info")) {
> +            if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val,
> +                                                    &datastoreInfo) < 0) {
> +                goto cleanup;
> +            }
> +
> +            break;
> +        }
> +    }
> +
> +    if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
> +        pool.type = VIR_STORAGE_POOL_DIR;
> +    } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
> +        pool.type = VIR_STORAGE_POOL_NETFS;
> +    } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) {
> +        pool.type = VIR_STORAGE_POOL_FS;
> +    } else {
> +        ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> +                  _("DatastoreInfo has unexpected type"));
> +        goto cleanup;
> +    }
> +
> +    /* Lookup file info */
> +    if (virAsprintf(&datastorePath, "[%s] %s", volume->pool, volume->name) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    if (esxVI_LookupFileInfoByDatastorePath(priv->primary, datastorePath,
> +                                            &fileInfo,
> +                                            esxVI_Occurrence_RequiredItem) < 0) {
> +        goto cleanup;
> +    }
> +
> +    vmDiskFileInfo = esxVI_VmDiskFileInfo_DynamicCast(fileInfo);
> +    isoImageFileInfo = esxVI_IsoImageFileInfo_DynamicCast(fileInfo);
> +    floppyImageFileInfo = esxVI_FloppyImageFileInfo_DynamicCast(fileInfo);
> +
> +    def.name = volume->name;
> +    def.key = datastorePath;

I know that the main libvirt storage driver uses a path for 'key'
currently, but if you have a choice with ESX, it would be desirable
to use a 'better' unique identifier for key. The idea is that 'key'
is trying to provide a unique identifier that is stable even if the
volume is moved or renamed, but still points at the same underlying
data. A path is fine as the catchall fallback case, but if there's a
a UUID, or SCSI/iSCSI LUN WWID then that's better.

ACK, to the patch anyway since we can change this later if desired.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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