libvirt: do not mix internal flags into public API

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.  In the case of virDomainGetXMLDesc, it turns out that
no one was ever mixing VIR_DOMAIN_XML_INTERNAL_STATUS with
the dumpxml path in the first place; that internal flag was
only used in saving and restoring state files, which happened
to be in functions internal to a single file, so there is no
mixing of the internal flag with a public flags argument.
Additionally, virDomainMemoryStats passed a flags argument
over RPC, but not to the driver.

* src/driver.h (VIR_DOMAIN_XML_FLAGS_MASK)
(VIR_SECRET_GET_VALUE_FLAGS_MASK): Delete.
(virDrvSecretGetValue): Separate out internal flags.
(virDrvDomainMemoryStats): Provide missing flags argument.
* src/driver.c (verify): Drop unused check.
* src/conf/domain_conf.h (virDomainObjParseFile): Delete
declaration.
(virDomainXMLInternalFlags): Move...
* src/conf/domain_conf.c: ...here.  Delete redundant include.
(virDomainObjParseFile): Make static.
* src/libvirt.c (virDomainGetXMLDesc, virSecretGetValue): Update
clients.
(virDomainMemoryPeek, virInterfaceGetXMLDesc)
(virDomainMemoryStats, virDomainBlockPeek, virNetworkGetXMLDesc)
(virStoragePoolGetXMLDesc, virStorageVolGetXMLDesc)
(virNodeNumOfDevices, virNodeListDevices, virNWFilterGetXMLDesc):
Don't mask unknown flags.
* src/interface/netcf_driver.c (interfaceGetXMLDesc): Reject
unknown flags.
* src/secret/secret_driver.c (secretGetValue): Update clients.
* src/remote/remote_driver.c (remoteSecretGetValue)
(remoteDomainMemoryStats): Likewise.
* src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainMemoryStats): Likewise.
* daemon/remote.c (remoteDispatchDomainMemoryStats): Likewise.

diff --git a/daemon/remote.c b/daemon/remote.c
index e93a0dcba0ce3c3217f0b73e91c2590fffbcc143..daad39d3a52bf679284d6ba515024ab131c38eda 100644 (file)
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -810,7 +810,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    nr_stats = virDomainMemoryStats(dom, stats, args->maxStats, 0);
+    nr_stats = virDomainMemoryStats(dom, stats, args->maxStats, args->flags);
     if (nr_stats < 0)
         goto cleanup;
 

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c3e44ed8810cf0ed5b7de7985a676315be7ac2f..3c3ab39699e12308baf7d92c131cc90e0752e4a1 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -48,7 +48,6 @@
 #include "storage_file.h"
 #include "files.h"
 #include "bitmap.h"
-#include "verify.h"
 #include "count-one-bits.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -57,6 +56,12 @@
  * verify that it doesn't overflow an unsigned int when shifting */
 verify(VIR_DOMAIN_VIRT_LAST <= 32);
 
+/* Private flag used internally by virDomainSaveStatus and
+ * virDomainObjParseFile.  */
+typedef enum {
+   VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */
+} virDomainXMLInternalFlags;
+
 VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
               "custom-argv",
               "custom-monitor",
@@ -6995,10 +7000,11 @@ cleanup:
 }
 
 
-virDomainObjPtr virDomainObjParseFile(virCapsPtr caps,
-                                      const char *filename,
-                                      unsigned int expectedVirtTypes,
-                                      unsigned int flags)
+static virDomainObjPtr
+virDomainObjParseFile(virCapsPtr caps,
+                      const char *filename,
+                      unsigned int expectedVirtTypes,
+                      unsigned int flags)
 {
     xmlDocPtr xml;
     virDomainObjPtr obj = NULL;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 172d3c28780a73e3ce7f129aa77b4d41d11219a5..7a3d72b9e800f412ec430027c9bcbc6a5a61e41b 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -41,11 +41,6 @@
 # include "macvtap.h"
 # include "sysinfo.h"
 
-/* Private component of virDomainXMLFlags */
-typedef enum {
-   VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */
-} virDomainXMLInternalFlags;
-
 /* Different types of hypervisor */
 /* NB: Keep in sync with virDomainVirtTypeToString impl */
 enum virDomainVirtType {
@@ -1417,11 +1412,6 @@ virDomainDefPtr virDomainDefParseNode(virCapsPtr caps,
                                       unsigned int expectedVirtTypes,
                                       unsigned int flags);
 
-virDomainObjPtr virDomainObjParseFile(virCapsPtr caps,
-                                      const char *filename,
-                                      unsigned int expectedVirtTypes,
-                                      unsigned int flags);
-
 bool virDomainDefCheckABIStability(virDomainDefPtr src,
                                    virDomainDefPtr dst);
 

diff --git a/src/driver.c b/src/driver.c
index 579c2b34853876889b5a20c8618891391ccc0b3e..50342778389100c525b143a241cbcb80a27fa2ee 100644 (file)
--- a/src/driver.c
+++ b/src/driver.c
@@ -32,10 +32,6 @@
 
 #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
 
-/* Make sure ... INTERNAL_CALL cannot be set by the caller */
-verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL &
-        VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0);
-
 #ifdef WITH_DRIVER_MODULES
 
 /* XXX re-implment this for other OS, or use libtools helper lib ? */

diff --git a/src/driver.h b/src/driver.h
index 70ea4c235fd63bfbd60a9f5489e6d44604ae381c..9d0d3de9c46470782120afc33792c65a92f4bb13 100644 (file)
--- a/src/driver.h
+++ b/src/driver.h
@@ -191,7 +191,7 @@ typedef char *
                                          unsigned int flags);
 typedef char *
         (*virDrvDomainGetXMLDesc)              (virDomainPtr dom,
-                                         unsigned int flags);
+                                                 unsigned int flags);
 typedef char *
         (*virDrvConnectDomainXMLFromNative) (virConnectPtr conn,
                                              const char *nativeFormat,
@@ -331,7 +331,8 @@ typedef int
     (*virDrvDomainMemoryStats)
                     (virDomainPtr domain,
                      struct _virDomainMemoryStat *stats,
-                     unsigned int nr_stats);
+                     unsigned int nr_stats,
+                     unsigned int flags);
 
 typedef int
     (*virDrvDomainBlockPeek)
@@ -1229,16 +1230,10 @@ struct _virDeviceMonitor {
     virDrvNodeDeviceDestroy deviceDestroy;
 };
 
-/* bits 16 and above of virDomainXMLFlags are for internal use */
-# define VIR_DOMAIN_XML_FLAGS_MASK 0xffff
-
-/* Bits 16 and above of virSecretGetValue flags are for internal use */
-# define VIR_SECRET_GET_VALUE_FLAGS_MASK 0xffff
-
 enum {
     /* This getValue call is inside libvirt, override the "private" flag.
        This flag cannot be set by outside callers. */
-    VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 16
+    VIR_SECRET_GET_VALUE_INTERNAL_CALL = 1 << 0,
 };
 
 typedef virSecretPtr
@@ -1263,7 +1258,8 @@ typedef int
 typedef unsigned char *
     (*virDrvSecretGetValue)                  (virSecretPtr secret,
                                               size_t *value_size,
-                                              unsigned int flags);
+                                              unsigned int flags,
+                                              unsigned int internalFlags);
 typedef int
     (*virDrvSecretUndefine)                  (virSecretPtr secret);
 typedef int

diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index 855b5a3ed740633f2f9132a688d5ea88bbebb8d5..2f322b4db9428f67bbab5f9aa84d2075a2b4098b 100644 (file)
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -344,6 +344,8 @@ static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
     virInterfaceDefPtr ifacedef = NULL;
     char *ret = NULL;
 
+    virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
+
     interfaceDriverLock(driver);
 
     iface = interfaceDriverGetNetcfIF(driver->netcf, ifinfo);

diff --git a/src/libvirt.c b/src/libvirt.c
index 001c38dd75679b38274bdbd3d4d71271de7bcc40..39e2041e40947af32c301dfd2e421c6bd2153b80 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3381,8 +3381,6 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
         goto error;
     }
 
-    flags &= VIR_DOMAIN_XML_FLAGS_MASK;
-
     if (conn->driver->domainGetXMLDesc) {
         char *ret;
         ret = conn->driver->domainGetXMLDesc(domain, flags);
@@ -6040,11 +6038,6 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats,
         virDispatchError(NULL);
         return -1;
     }
-    if (flags != 0) {
-        virLibDomainError(VIR_ERR_INVALID_ARG,
-                           _("flags must be zero"));
-        goto error;
-    }
 
     if (!stats || nr_stats == 0)
         return 0;
@@ -6054,7 +6047,8 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats,
 
     conn = dom->conn;
     if (conn->driver->domainMemoryStats) {
-        nr_stats_ret = conn->driver->domainMemoryStats (dom, stats, nr_stats);
+        nr_stats_ret = conn->driver->domainMemoryStats (dom, stats, nr_stats,
+                                                        flags);
         if (nr_stats_ret == -1)
             goto error;
         return nr_stats_ret;
@@ -6139,12 +6133,6 @@ virDomainBlockPeek (virDomainPtr dom,
         goto error;
     }
 
-    if (flags != 0) {
-        virLibDomainError(VIR_ERR_INVALID_ARG,
-                           _("flags must be zero"));
-        goto error;
-    }
-
     /* Allow size == 0 as an access test. */
     if (size > 0 && !buffer) {
         virLibDomainError(VIR_ERR_INVALID_ARG,
@@ -6248,9 +6236,10 @@ virDomainMemoryPeek (virDomainPtr dom,
      * because of incompatible licensing.
      */
 
-    if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) {
+    /* Exactly one of these two flags must be set.  */
+    if (!(flags & VIR_MEMORY_VIRTUAL) == !(flags & VIR_MEMORY_PHYSICAL)) {
         virLibDomainError(VIR_ERR_INVALID_ARG,
-                     _("flags parameter must be VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
+                     _("flags parameter must include VIR_MEMORY_VIRTUAL or VIR_MEMORY_PHYSICAL"));
         goto error;
     }
 
@@ -8474,10 +8463,6 @@ virNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags)
         virDispatchError(NULL);
         return NULL;
     }
-    if (flags != 0) {
-        virLibNetworkError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     conn = network->conn;
 
@@ -8987,10 +8972,6 @@ virInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags)
         virDispatchError(NULL);
         return NULL;
     }
-    if ((flags & ~VIR_INTERFACE_XML_INACTIVE) != 0) {
-        virLibInterfaceError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     conn = iface->conn;
 
@@ -10460,10 +10441,6 @@ virStoragePoolGetXMLDesc(virStoragePoolPtr pool,
         virDispatchError(NULL);
         return NULL;
     }
-    if (flags != 0) {
-        virLibStoragePoolError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     conn = pool->conn;
 
@@ -11358,10 +11335,6 @@ virStorageVolGetXMLDesc(virStorageVolPtr vol,
         virDispatchError(NULL);
         return NULL;
     }
-    if (flags != 0) {
-        virLibStorageVolError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     conn = vol->conn;
 
@@ -11450,10 +11423,6 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags)
         virDispatchError(NULL);
         return -1;
     }
-    if (flags != 0) {
-        virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     if (conn->deviceMonitor && conn->deviceMonitor->numOfDevices) {
         int ret;
@@ -11502,7 +11471,7 @@ virNodeListDevices(virConnectPtr conn,
         virDispatchError(NULL);
         return -1;
     }
-    if ((flags != 0) || (names == NULL) || (maxnames < 0)) {
+    if ((names == NULL) || (maxnames < 0)) {
         virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         goto error;
     }
@@ -12708,12 +12677,10 @@ virSecretGetValue(virSecretPtr secret, size_t *value_size, unsigned int flags)
         goto error;
     }
 
-    flags &= VIR_SECRET_GET_VALUE_FLAGS_MASK;
-
     if (conn->secretDriver != NULL && conn->secretDriver->getValue != NULL) {
         unsigned char *ret;
 
-        ret = conn->secretDriver->getValue(secret, value_size, flags);
+        ret = conn->secretDriver->getValue(secret, value_size, flags, 0);
         if (ret == NULL)
             goto error;
         return ret;
@@ -14242,10 +14209,6 @@ virNWFilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int flags)
         virDispatchError(NULL);
         return NULL;
     }
-    if (flags != 0) {
-        virLibNWFilterError(VIR_ERR_INVALID_ARG, __FUNCTION__);
-        goto error;
-    }
 
     conn = nwfilter->conn;
 

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8d54e58a9b6914cfd811f5f9a7ee2cfd705b0040..8870e33eebaa9473a3bcb1beb3c1d584c9d163d6 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6166,12 +6166,15 @@ qemudDomainInterfaceStats (virDomainPtr dom,
 static int
 qemudDomainMemoryStats (virDomainPtr dom,
                         struct _virDomainMemoryStat *stats,
-                        unsigned int nr_stats)
+                        unsigned int nr_stats,
+                        unsigned int flags)
 {
     struct qemud_driver *driver = dom->conn->privateData;
     virDomainObjPtr vm;
     unsigned int ret = -1;
 
+    virCheckFlags(0, -1);
+
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d0085e0eef496abcd9185faaf31523ea60c24643..448b06e0615376c19acda4348af0e6cc25f49c3e 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -276,7 +276,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
                                               enc->secrets[0]->uuid);
     if (secret == NULL)
         goto cleanup;
-    data = conn->secretDriver->getValue(secret, &size,
+    data = conn->secretDriver->getValue(secret, &size, 0,
                                         VIR_SECRET_GET_VALUE_INTERNAL_CALL);
     virUnrefSecret(secret);
     if (data == NULL)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 2d5dc15171f06e4c15c22b994bc0785563aaba1a..c2f8bbde06d6d4268e719e469ad911016d8dc569 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1850,7 +1850,8 @@ done:
 static int
 remoteDomainMemoryStats (virDomainPtr domain,
                          struct _virDomainMemoryStat *stats,
-                         unsigned int nr_stats)
+                         unsigned int nr_stats,
+                         unsigned int flags)
 {
     int rv = -1;
     remote_domain_memory_stats_args args;
@@ -1868,7 +1869,7 @@ remoteDomainMemoryStats (virDomainPtr domain,
         goto done;
     }
     args.maxStats = nr_stats;
-    args.flags = 0;
+    args.flags = flags;
     memset (&ret, 0, sizeof ret);
 
     if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_MEMORY_STATS,
@@ -3173,7 +3174,7 @@ remoteSecretClose (virConnectPtr conn)
 
 static unsigned char *
 remoteSecretGetValue (virSecretPtr secret, size_t *value_size,
-                      unsigned int flags)
+                      unsigned int flags, unsigned int internalFlags)
 {
     unsigned char *rv = NULL;
     remote_secret_get_value_args args;
@@ -3182,6 +3183,12 @@ 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 a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index c45ba51147abe739033959398cd7b01abc5df203..02cdbb912286467399a8a250986f7a96145b7f5b 100644 (file)
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -873,12 +873,15 @@ cleanup:
 }
 
 static unsigned char *
-secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags)
+secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags,
+               unsigned int internalFlags)
 {
     virSecretDriverStatePtr driver = obj->conn->secretPrivateData;
     unsigned char *ret = NULL;
     virSecretEntryPtr secret;
 
+    virCheckFlags(0, NULL);
+
     secretDriverLock(driver);
 
     secret = secretFindByUUID(driver, obj->uuid);
@@ -898,7 +901,7 @@ secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags)
         goto cleanup;
     }
 
-    if ((flags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
+    if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
         secret->def->private) {
         virSecretReportError(VIR_ERR_OPERATION_DENIED, "%s",
                              _("secret is private"));