[libvirt] [PATCHv4] libvirt: do not mix internal flags into public API

[Eric Blake]

On 07/18/2011 01:27 PM, Eric Blake wrote:
> On 07/15/2011 05:12 PM, Eric Blake wrote:
>> There were two API in driver.c that were silently masking flags
>> bits prior to calling out to the drivers, and several others
>> that were explicitly masking flags bits. This is not
>> forward-compatible - if we ever have that many flags in the
>> future, then talking to an old server that masks out the
>> flags would be indistinguishable from talking to a new server
>> that can honor the flag. In general, libvirt.c should forward
>> _all_ flags on to drivers, and only the drivers should reject
>> unknown flags.
>>
>> In the case of virDrvSecretGetValue, the solution is to separate
>> the internal driver callback function to have two parameters
>> instead of one, with only one parameter affected by the public
>> API.
>
> On second thought, I don't like this. It's nicer to have all the driver
> callbacks match the public API as much as possible, and to instead
> create a new entry point in libvirt_private.syms that qemu can use for
> the case where it needs to call the internal API instead of the public
> virSecretGetValue. v5 coming up.

Here's the interdiff for v5:

diff --git i/src/driver.h w/src/driver.h
index 9d0d3de..759150d 100644
--- i/src/driver.h
+++ w/src/driver.h
@@ -1258,6 +1258,10 @@ typedef int
  typedef unsigned char *
      (*virDrvSecretGetValue)                  (virSecretPtr secret,
                                                size_t *value_size,
+                                              unsigned int flags);
+typedef unsigned char *
+    (*virDrvSecretGetValueInternal)          (virSecretPtr secret,
+                                              size_t *value_size,
                                                unsigned int flags,
                                                unsigned int internalFlags);
  typedef int
@@ -1295,6 +1299,7 @@ struct _virSecretDriver {
      virDrvSecretGetXMLDesc getXMLDesc;
      virDrvSecretSetValue setValue;
      virDrvSecretGetValue getValue;
+    virDrvSecretGetValueInternal getValueInternal;
      virDrvSecretUndefine undefine;
  };

diff --git i/src/libvirt.c w/src/libvirt.c
index 39e2041..34acede 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -12680,7 +12680,7 @@ virSecretGetValue(virSecretPtr secret, size_t 
*value_size, unsigned int flags)
      if (conn->secretDriver != NULL && conn->secretDriver->getValue != 
NULL) {
          unsigned char *ret;

-        ret = conn->secretDriver->getValue(secret, value_size, flags, 0);
+        ret = conn->secretDriver->getValue(secret, value_size, flags);
          if (ret == NULL)
              goto error;
          return ret;
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 448b06e..cb7575f 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -257,7 +257,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,

      if (conn->secretDriver == NULL ||
          conn->secretDriver->lookupByUUID == NULL ||
-        conn->secretDriver->getValue == NULL) {
+        conn->secretDriver->getValueInternal == NULL) {
          qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
                          _("secret storage not supported"));
          goto cleanup;
@@ -276,8 +276,8 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
                                                enc->secrets[0]->uuid);
      if (secret == NULL)
          goto cleanup;
-    data = conn->secretDriver->getValue(secret, &size, 0,
- 
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+    data = conn->secretDriver->getValueInternal(secret, &size, 0,
+ 
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
      virUnrefSecret(secret);
      if (data == NULL)
          goto cleanup;
diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c
index c2f8bbd..d3b5df9 100644
--- i/src/remote/remote_driver.c
+++ w/src/remote/remote_driver.c
@@ -3174,7 +3174,7 @@ remoteSecretClose (virConnectPtr conn)

  static unsigned char *
  remoteSecretGetValue (virSecretPtr secret, size_t *value_size,
-                      unsigned int flags, unsigned int internalFlags)
+                      unsigned int flags)
  {
      unsigned char *rv = NULL;
      remote_secret_get_value_args args;
@@ -3183,12 +3183,6 @@ remoteSecretGetValue (virSecretPtr secret, size_t 
*value_size,

      remoteDriverLock (priv);

-    /* internalFlags intentionally do not go over the wire */
-    if (internalFlags) {
-        remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no internalFlags 
support"));
-        goto done;
-    }
-
      make_nonnull_secret (&args.secret, secret);
      args.flags = flags;

diff --git i/src/secret/secret_driver.c w/src/secret/secret_driver.c
index 02cdbb9..87e2b83 100644
--- i/src/secret/secret_driver.c
+++ w/src/secret/secret_driver.c
@@ -873,8 +873,9 @@ cleanup:
  }

  static unsigned char *
-secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags,
-               unsigned int internalFlags)
+secretGetValueInternal(virSecretPtr obj, size_t *value_size,
+                       unsigned int flags,
+                       unsigned int internalFlags)
  {
      virSecretDriverStatePtr driver = obj->conn->secretPrivateData;
      unsigned char *ret = NULL;
@@ -921,6 +922,12 @@ cleanup:
      return ret;
  }

+static unsigned char *
+secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags)
+{
+    return secretGetValueInternal(obj, value_size, flags, 0);
+}
+
  static int
  secretUndefine(virSecretPtr obj)
  {
@@ -1078,6 +1085,7 @@ static virSecretDriver secretDriver = {
      .getXMLDesc = secretGetXMLDesc, /* 0.7.1 */
      .setValue = secretSetValue, /* 0.7.1 */
      .getValue = secretGetValue, /* 0.7.1 */
+    .getValueInternal = secretGetValueInternal, /* 0.9.4 */
      .undefine = secretUndefine, /* 0.7.1 */
  };



-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org