Re: [libvirt] [PATCH 09/11] storage: Use the internal API to get the secret value instead

2013-06-20 Thread Osier Yang 

On 07/06/13 01:26, John Ferlan wrote:

On 05/28/2013 02:39 AM, Osier Yang wrote:

Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no
way to get the value of private secret. And error out if the
secret value is not found.
---
  src/storage/storage_backend_rbd.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)


Is this patch separatable?  That is - is it required for this set of
changes or is it out of band enough to be its own patch.


separatable, it's the thing I found when coding on the way.






diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 953a8ee..d66d3f9 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -23,6 +23,7 @@
  
  #include config.h
  
+#include datatypes.h

  #include virerror.h
  #include storage_backend_rbd.h
  #include storage_conf.h
@@ -88,7 +89,17 @@ static int 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr,
  goto cleanup;
  }
  
-secret_value = virSecretGetValue(secret, secret_value_size, 0);

+secret_value = conn-secretDriver-secretGetValue(secret, 
secret_value_size, 0,
+  
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+

There are callers to this function that have set ATTRIBUTE_UNUSED on the
'conn' parameter.  Now this code uses it - so it seems you have some
more checking to do.

See virStorageBackendRBDRefreshPool() and virStorageBackendRBDResizeVol()

Using the same logic as before I see that storage_driver.c and
storageDriverAutostart() will call the backend-refreshPool with NULL
and that will cause you issues in this code.


Okay.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 09/11] storage: Use the internal API to get the secret value instead

2013-06-06 Thread John Ferlan 
On 05/28/2013 02:39 AM, Osier Yang wrote:
 Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no
 way to get the value of private secret. And error out if the
 secret value is not found.
 ---
  src/storage/storage_backend_rbd.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)
 

Is this patch separatable?  That is - is it required for this set of
changes or is it out of band enough to be its own patch.



 diff --git a/src/storage/storage_backend_rbd.c 
 b/src/storage/storage_backend_rbd.c
 index 953a8ee..d66d3f9 100644
 --- a/src/storage/storage_backend_rbd.c
 +++ b/src/storage/storage_backend_rbd.c
 @@ -23,6 +23,7 @@
  
  #include config.h
  
 +#include datatypes.h
  #include virerror.h
  #include storage_backend_rbd.h
  #include storage_conf.h
 @@ -88,7 +89,17 @@ static int 
 virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr,
  goto cleanup;
  }
  
 -secret_value = virSecretGetValue(secret, secret_value_size, 0);
 +secret_value = conn-secretDriver-secretGetValue(secret, 
 secret_value_size, 0,
 +  
 VIR_SECRET_GET_VALUE_INTERNAL_CALL);
 +

There are callers to this function that have set ATTRIBUTE_UNUSED on the
'conn' parameter.  Now this code uses it - so it seems you have some
more checking to do.

See virStorageBackendRBDRefreshPool() and virStorageBackendRBDResizeVol()

Using the same logic as before I see that storage_driver.c and
storageDriverAutostart() will call the backend-refreshPool with NULL
and that will cause you issues in this code.

John

 +if (!secret_value) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(could not get the value of the secret 
 + for username %s),
 +   pool-def-source.auth.cephx.username);
 +goto cleanup;
 +}
 +
  base64_encode_alloc((char *)secret_value,
  secret_value_size, rados_key);
  memset(secret_value, 0, secret_value_size);
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 09/11] storage: Use the internal API to get the secret value instead

2013-05-28 Thread Osier Yang 
Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no
way to get the value of private secret. And error out if the
secret value is not found.
---
 src/storage/storage_backend_rbd.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 953a8ee..d66d3f9 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -23,6 +23,7 @@
 
 #include config.h
 
+#include datatypes.h
 #include virerror.h
 #include storage_backend_rbd.h
 #include storage_conf.h
@@ -88,7 +89,17 @@ static int 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr,
 goto cleanup;
 }
 
-secret_value = virSecretGetValue(secret, secret_value_size, 0);
+secret_value = conn-secretDriver-secretGetValue(secret, 
secret_value_size, 0,
+  
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+
+if (!secret_value) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(could not get the value of the secret 
+ for username %s),
+   pool-def-source.auth.cephx.username);
+goto cleanup;
+}
+
 base64_encode_alloc((char *)secret_value,
 secret_value_size, rados_key);
 memset(secret_value, 0, secret_value_size);
-- 
1.8.1.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list