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.

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;
 

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,
+static virDomainObjPtr
-                                      const char *filename,
+virDomainObjParseFile(virCapsPtr caps,
-                                      unsigned int expectedVirtTypes,
+                      const char *filename,
-                                      unsigned int flags)
+                      unsigned int expectedVirtTypes,
+                      unsigned int flags)
 {
     xmlDocPtr xml;
     virDomainObjPtr obj = NULL;

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);
 

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 ? */

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

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);

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;
 

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);

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)

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;
 

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"));