[PATCH] Use libvirt to get StorageVolume Path to set SV InstanceID
# HG changeset patch # User Sharad Mishra <snmishra@us.ibm.com> # Date 1268096483 28800 # Node ID fa15816439a7ef1e9ff212cf1833cafa7b1fd704 # Parent 4ee7c4354bc550780bc02ef1f69fbda387b3fe13 Use libvirt to get StorageVolume Path to set SV InstanceID. InstanceID of a StorageVolume was set using the 'Path' field of StorageVolume RASD. If 'Path' was not set by the user, the InstanceID would be invalid. This patch fixed the issue by using libvirt to get the storage volume path. The path returned by libvirt is used to set the InstanceID. Signed-off-by: Sharad Mishra <snmishra@us.ibm.com> diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.c Mon Mar 08 17:01:23 2010 -0800 @@ -353,12 +353,12 @@ } #if VIR_USE_LIBVIRT_STORAGE -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type) { - int ret = 0; + char *path = NULL; virStoragePoolPtr ptr = NULL; virStorageVolPtr vptr = NULL; @@ -376,14 +376,19 @@ goto out; } - ret = 1; + path = virStorageVolGetPath(vptr); + if (path == NULL) { + CU_DEBUG("Unable to get storage volume path"); + goto out; + } + } out: virStoragePoolFree(ptr); virStorageVolFree(vptr); - return ret; + return path; } int delete_resource(virConnectPtr conn, @@ -414,13 +419,13 @@ return ret; } #else -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type) { CU_DEBUG("Creating resources within libvirt pools not supported"); - return 0; + return NULL; } int delete_resource(virConnectPtr conn, diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.h --- a/libxkutil/pool_parsing.h Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.h Mon Mar 08 17:01:23 2010 -0800 @@ -90,7 +90,7 @@ int define_pool(virConnectPtr conn, const char *xml, int res_type); int destroy_pool(virConnectPtr conn, const char *name, int res_type); -int create_resource(virConnectPtr conn, const char *pname, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type); int delete_resource(virConnectPtr conn, const char *rname, int res_type); diff -r 4ee7c4354bc5 -r fa15816439a7 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Tue Mar 02 15:23:45 2010 -0800 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Mar 08 17:01:23 2010 -0800 @@ -832,6 +832,7 @@ { virConnectPtr conn; CMPIInstance *inst = NULL; + char *path = NULL; conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); if (conn == NULL) { @@ -839,7 +840,8 @@ return NULL; } - if (create_resource(conn, res->pool_id, xml, res->type) == 0) { + path = create_resource(conn, res->pool_id, xml, res->type); + if (path == NULL) { virt_set_status(_BROKER, s, CMPI_RC_ERR_FAILED, conn, @@ -855,6 +857,8 @@ "Failed to lookup resulting resource"); } + CMSetProperty(inst, "InstanceID", (CMPIValue *)path, CMPI_chars); + out: virConnectClose(conn);
Sharad Mishra wrote:
# HG changeset patch # User Sharad Mishra <snmishra@us.ibm.com> # Date 1268096483 28800 # Node ID fa15816439a7ef1e9ff212cf1833cafa7b1fd704 # Parent 4ee7c4354bc550780bc02ef1f69fbda387b3fe13 Use libvirt to get StorageVolume Path to set SV InstanceID.
InstanceID of a StorageVolume was set using the 'Path' field of StorageVolume RASD. If 'Path' was not set by the user, the InstanceID would be invalid. This patch fixed the issue by using libvirt to get the storage volume path. The path returned by libvirt is used to set the InstanceID.
The patch assigns the Path to the InstanceID. AFAIK, the InstanceID has always been name, for example for DiskPool it would be Poolname or for NetworkPool it would be the NetworkPool name. So, I think we should have the Volume Name as part of the InsanceID. Is there a DMTF which describes about this, which you referred before fixing this ? Between is there a plan to enumerate StorageVolumeRASD like the way we have DiskRASD and NetRASD?
Signed-off-by: Sharad Mishra <snmishra@us.ibm.com>
diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.c Mon Mar 08 17:01:23 2010 -0800 @@ -353,12 +353,12 @@ }
#if VIR_USE_LIBVIRT_STORAGE -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type) { - int ret = 0; + char *path = NULL; virStoragePoolPtr ptr = NULL; virStorageVolPtr vptr = NULL;
@@ -376,14 +376,19 @@ goto out; }
- ret = 1; + path = virStorageVolGetPath(vptr); + if (path == NULL) { + CU_DEBUG("Unable to get storage volume path"); + goto out; + } + }
out: virStoragePoolFree(ptr); virStorageVolFree(vptr);
- return ret; + return path; }
int delete_resource(virConnectPtr conn, @@ -414,13 +419,13 @@ return ret; } #else -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type) { CU_DEBUG("Creating resources within libvirt pools not supported"); - return 0; + return NULL; }
int delete_resource(virConnectPtr conn, diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.h --- a/libxkutil/pool_parsing.h Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.h Mon Mar 08 17:01:23 2010 -0800 @@ -90,7 +90,7 @@ int define_pool(virConnectPtr conn, const char *xml, int res_type); int destroy_pool(virConnectPtr conn, const char *name, int res_type);
-int create_resource(virConnectPtr conn, const char *pname, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type);
int delete_resource(virConnectPtr conn, const char *rname, int res_type); diff -r 4ee7c4354bc5 -r fa15816439a7 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Tue Mar 02 15:23:45 2010 -0800 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Mar 08 17:01:23 2010 -0800 @@ -832,6 +832,7 @@ { virConnectPtr conn; CMPIInstance *inst = NULL; + char *path = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); if (conn == NULL) { @@ -839,7 +840,8 @@ return NULL; }
- if (create_resource(conn, res->pool_id, xml, res->type) == 0) { + path = create_resource(conn, res->pool_id, xml, res->type); + if (path == NULL) { virt_set_status(_BROKER, s, CMPI_RC_ERR_FAILED, conn, @@ -855,6 +857,8 @@ "Failed to lookup resulting resource"); }
+ CMSetProperty(inst, "InstanceID", (CMPIValue *)path, CMPI_chars); + out: virConnectClose(conn);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Thanks and Regards, Deepti B. Kalakeri IBM Linux Technology Center deeptik@linux.vnet.ibm.com
I tested volume creation, but I don't really have a way to check if this code work, since I can't directly access a StorageVolume to check the InstanceID. :-/ Em 08-03-2010 22:10, Sharad Mishra escreveu:
# HG changeset patch # User Sharad Mishra<snmishra@us.ibm.com> # Date 1268096483 28800 # Node ID fa15816439a7ef1e9ff212cf1833cafa7b1fd704 # Parent 4ee7c4354bc550780bc02ef1f69fbda387b3fe13 Use libvirt to get StorageVolume Path to set SV InstanceID.
InstanceID of a StorageVolume was set using the 'Path' field of StorageVolume RASD. If 'Path' was not set by the user, the InstanceID would be invalid. This patch fixed the issue by using libvirt to get the storage volume path. The path returned by libvirt is used to set the InstanceID.
Signed-off-by: Sharad Mishra<snmishra@us.ibm.com>
diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.c Mon Mar 08 17:01:23 2010 -0800 @@ -353,12 +353,12 @@ }
#if VIR_USE_LIBVIRT_STORAGE -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
All function parameters must be aligned
{ - int ret = 0; + char *path = NULL; virStoragePoolPtr ptr = NULL; virStorageVolPtr vptr = NULL;
@@ -376,14 +376,19 @@ goto out; }
- ret = 1; + path = virStorageVolGetPath(vptr); + if (path == NULL) { + CU_DEBUG("Unable to get storage volume path"); + goto out; + } + }
out: virStoragePoolFree(ptr); virStorageVolFree(vptr);
- return ret; + return path; }
int delete_resource(virConnectPtr conn, @@ -414,13 +419,13 @@ return ret; } #else -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
Align those parameters too
{ CU_DEBUG("Creating resources within libvirt pools not supported"); - return 0; + return NULL; }
int delete_resource(virConnectPtr conn, diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.h --- a/libxkutil/pool_parsing.h Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.h Mon Mar 08 17:01:23 2010 -0800 @@ -90,7 +90,7 @@ int define_pool(virConnectPtr conn, const char *xml, int res_type); int destroy_pool(virConnectPtr conn, const char *name, int res_type);
-int create_resource(virConnectPtr conn, const char *pname, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type);
int delete_resource(virConnectPtr conn, const char *rname, int res_type); diff -r 4ee7c4354bc5 -r fa15816439a7 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Tue Mar 02 15:23:45 2010 -0800 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Mar 08 17:01:23 2010 -0800 @@ -832,6 +832,7 @@ { virConnectPtr conn; CMPIInstance *inst = NULL; + char *path = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); if (conn == NULL) { @@ -839,7 +840,8 @@ return NULL; }
- if (create_resource(conn, res->pool_id, xml, res->type) == 0) { + path = create_resource(conn, res->pool_id, xml, res->type); + if (path == NULL) { virt_set_status(_BROKER, s, CMPI_RC_ERR_FAILED, conn, @@ -855,6 +857,8 @@ "Failed to lookup resulting resource"); }
+ CMSetProperty(inst, "InstanceID", (CMPIValue *)path, CMPI_chars); +
Don't you need to free the path pointer as well?
out: virConnectClose(conn);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com
Richard, Apart from the test, does the code look okay? I have tested it and Deepti has tested it too. So if the code review is good, then please ack it. Thanks Sharad Mishra System x Enablement Linux Technology Center IBM Richard Maciel <rmaciel@linux.vn et.ibm.com> To Sent by: libvirt-cim@redhat.com libvirt-cim-bounc cc es@redhat.com Subject Re: [Libvirt-cim] [PATCH] Use 03/19/10 03:50 PM libvirt to get StorageVolume Path to set SV InstanceID Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redh at.com> I tested volume creation, but I don't really have a way to check if this code work, since I can't directly access a StorageVolume to check the InstanceID. :-/ Em 08-03-2010 22:10, Sharad Mishra escreveu:
# HG changeset patch # User Sharad Mishra<snmishra@us.ibm.com> # Date 1268096483 28800 # Node ID fa15816439a7ef1e9ff212cf1833cafa7b1fd704 # Parent 4ee7c4354bc550780bc02ef1f69fbda387b3fe13 Use libvirt to get StorageVolume Path to set SV InstanceID.
InstanceID of a StorageVolume was set using the 'Path' field of StorageVolume RASD. If 'Path' was not set by the user, the InstanceID would be invalid. This patch fixed the issue by using libvirt to get the storage volume path. The path returned by libvirt is used to set the InstanceID.
Signed-off-by: Sharad Mishra<snmishra@us.ibm.com>
diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.c Mon Mar 08 17:01:23 2010 -0800 @@ -353,12 +353,12 @@ }
#if VIR_USE_LIBVIRT_STORAGE -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
All function parameters must be aligned
{ - int ret = 0; + char *path = NULL; virStoragePoolPtr ptr = NULL; virStorageVolPtr vptr = NULL;
@@ -376,14 +376,19 @@ goto out; }
- ret = 1; + path = virStorageVolGetPath(vptr); + if (path == NULL) { + CU_DEBUG("Unable to get storage volume path"); + goto out; + } + }
out: virStoragePoolFree(ptr); virStorageVolFree(vptr);
- return ret; + return path; }
int delete_resource(virConnectPtr conn, @@ -414,13 +419,13 @@ return ret; } #else -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
Align those parameters too
{ CU_DEBUG("Creating resources within libvirt pools not supported"); - return 0; + return NULL; }
int delete_resource(virConnectPtr conn, diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.h --- a/libxkutil/pool_parsing.h Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.h Mon Mar 08 17:01:23 2010 -0800 @@ -90,7 +90,7 @@ int define_pool(virConnectPtr conn, const char *xml, int res_type); int destroy_pool(virConnectPtr conn, const char *name, int res_type);
-int create_resource(virConnectPtr conn, const char *pname, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type);
int delete_resource(virConnectPtr conn, const char *rname, int res_type); diff -r 4ee7c4354bc5 -r fa15816439a7 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Tue Mar 02 15:23:45 2010 -0800 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Mar 08 17:01:23 2010 -0800 @@ -832,6 +832,7 @@ { virConnectPtr conn; CMPIInstance *inst = NULL; + char *path = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); if (conn == NULL) { @@ -839,7 +840,8 @@ return NULL; }
- if (create_resource(conn, res->pool_id, xml, res->type) == 0) { + path = create_resource(conn, res->pool_id, xml, res->type); + if (path == NULL) { virt_set_status(_BROKER, s, CMPI_RC_ERR_FAILED, conn, @@ -855,6 +857,8 @@ "Failed to lookup resulting resource"); }
+ CMSetProperty(inst, "InstanceID", (CMPIValue *)path, CMPI_chars); +
Don't you need to free the path pointer as well?
out: virConnectClose(conn);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
I didn't ack because there are changes to be made in the code. Please, check the comments. Em 19-03-2010 21:19, Sharad Mishra escreveu:
Richard,
Apart from the test, does the code look okay? I have tested it and Deepti has tested it too. So if the code review is good, then please ack it.
Thanks Sharad Mishra System x Enablement Linux Technology Center IBM
Inactive hide details for Richard Maciel ---03/19/2010 03:54:31 PM---I tested volume creation, but I don't really have a way toRichard Maciel ---03/19/2010 03:54:31 PM---I tested volume creation, but I don't really have a way to check if this
*Richard Maciel <rmaciel@linux.vnet.ibm.com>* Sent by: libvirt-cim-bounces@redhat.com
03/19/10 03:50 PM Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
Subject
Re: [Libvirt-cim] [PATCH] Use libvirt to get StorageVolume Path to set SV InstanceID
I tested volume creation, but I don't really have a way to check if this code work, since I can't directly access a StorageVolume to check the InstanceID. :-/
Em 08-03-2010 22:10, Sharad Mishra escreveu:
# HG changeset patch # User Sharad Mishra<snmishra@us.ibm.com> # Date 1268096483 28800 # Node ID fa15816439a7ef1e9ff212cf1833cafa7b1fd704 # Parent 4ee7c4354bc550780bc02ef1f69fbda387b3fe13 Use libvirt to get StorageVolume Path to set SV InstanceID.
InstanceID of a StorageVolume was set using the 'Path' field of StorageVolume RASD. If 'Path' was not set by the user, the InstanceID would be invalid. This patch fixed the issue by using libvirt to get the storage volume path. The path returned by libvirt is used to set the InstanceID.
Signed-off-by: Sharad Mishra<snmishra@us.ibm.com>
diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.c Mon Mar 08 17:01:23 2010 -0800 @@ -353,12 +353,12 @@ }
#if VIR_USE_LIBVIRT_STORAGE -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
All function parameters must be aligned
{ - int ret = 0; + char *path = NULL; virStoragePoolPtr ptr = NULL; virStorageVolPtr vptr = NULL;
@@ -376,14 +376,19 @@ goto out; }
- ret = 1; + path = virStorageVolGetPath(vptr); + if (path == NULL) { + CU_DEBUG("Unable to get storage volume path"); + goto out; + } + }
out: virStoragePoolFree(ptr); virStorageVolFree(vptr);
- return ret; + return path; }
int delete_resource(virConnectPtr conn, @@ -414,13 +419,13 @@ return ret; } #else -int create_resource(virConnectPtr conn, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type)
Align those parameters too
{ CU_DEBUG("Creating resources within libvirt pools not supported"); - return 0; + return NULL; }
int delete_resource(virConnectPtr conn, diff -r 4ee7c4354bc5 -r fa15816439a7 libxkutil/pool_parsing.h --- a/libxkutil/pool_parsing.h Tue Mar 02 15:23:45 2010 -0800 +++ b/libxkutil/pool_parsing.h Mon Mar 08 17:01:23 2010 -0800 @@ -90,7 +90,7 @@ int define_pool(virConnectPtr conn, const char *xml, int res_type); int destroy_pool(virConnectPtr conn, const char *name, int res_type);
-int create_resource(virConnectPtr conn, const char *pname, +char *create_resource(virConnectPtr conn, const char *pname, const char *xml, int res_type);
int delete_resource(virConnectPtr conn, const char *rname, int res_type); diff -r 4ee7c4354bc5 -r fa15816439a7 src/Virt_ResourcePoolConfigurationService.c --- a/src/Virt_ResourcePoolConfigurationService.c Tue Mar 02 15:23:45 2010 -0800 +++ b/src/Virt_ResourcePoolConfigurationService.c Mon Mar 08 17:01:23 2010 -0800 @@ -832,6 +832,7 @@ { virConnectPtr conn; CMPIInstance *inst = NULL; + char *path = NULL;
conn = connect_by_classname(_BROKER, CLASSNAME(ref), s); if (conn == NULL) { @@ -839,7 +840,8 @@ return NULL; }
- if (create_resource(conn, res->pool_id, xml, res->type) == 0) { + path = create_resource(conn, res->pool_id, xml, res->type); + if (path == NULL) { virt_set_status(_BROKER, s, CMPI_RC_ERR_FAILED, conn, @@ -855,6 +857,8 @@ "Failed to lookup resulting resource"); }
+ CMSetProperty(inst, "InstanceID", (CMPIValue *)path, CMPI_chars); +
Don't you need to free the path pointer as well?
out: virConnectClose(conn);
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com
participants (3)
-
Deepti B Kalakeri -
Richard Maciel -
Sharad Mishra