[Libvirt-cim] [PATCH] Use libvirt to get StorageVolume Path to set SV InstanceID

[Richard Maciel]

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 at linux.vnet.ibm.com>*
>                         Sent by: libvirt-cim-bounces at redhat.com
>
>                         03/19/10 03:50 PM
>                         Please respond to
>                         List for discussion and development of libvirt
>                         CIM <libvirt-cim at redhat.com>
>
> 	
>
> To
> 	
> libvirt-cim at 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 at 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 at 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 at redhat.com
>  > https://www.redhat.com/mailman/listinfo/libvirt-cim
>
>
> --
> Richard Maciel, MSc
> IBM Linux Technology Center
> rmaciel at linux.vnet.ibm.com
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>
>
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim


-- 
Richard Maciel, MSc
IBM Linux Technology Center
rmaciel at linux.vnet.ibm.com