5:42 p.m.
There were two API in driver.c that were silently masking flags
bits prior to calling out to the drivers. 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.
* src/driver.h (VIR_DOMAIN_XML_FLAGS_MASK)
(VIR_SECRET_GET_VALUE_FLAGS_MASK): Delete.
(virDrvSecretGetValue): Separate out internal flags.
* src/driver.c (verify): Drop unused check.
* src/conf/domain_conf.h (virDomainObjParseFile): Delete
declaration.
(virDomainXMLInternalFlags): Move...
* src/conf/domain_conf.c: ...here.
(virDomainDefFormat): Tighten validations on internal flags.
(virDomainObjParseFile): Make static.
* src/libvirt.c (virDomainGetXMLDesc, virSecretGetValue): Update
clients.
* src/secret/secret_driver.c (secretGetValue): Likewise.
* src/remote/remote_driver.c (remoteSecretGetValue): Likewise.
* src/qemu/qemu_process.c (qemuProcessGetVolumeQcowPassphrase):
Likewise.
---
v3: new patch
src/conf/domain_conf.c | 27 +++++++++++++++++++--------
src/conf/domain_conf.h | 10 ----------
src/driver.c | 4 ----
src/driver.h | 13 ++++---------
src/libvirt.c | 6 +-----
src/qemu/qemu_process.c | 2 +-
src/remote/remote_driver.c | 8 +++++++-
src/secret/secret_driver.c | 7 +++++--
8 files changed, 37 insertions(+), 40 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 788981f..1667688 100644
--- 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;
@@ -9615,6 +9621,13 @@ virDomainHostdevDefFormat(virBufferPtr buf,
}
+/* The set of public flags we support for domainGetXMLDesc */
+#define DOMAIN_GET_XML_DESC_FLAGS \
+ (VIR_DOMAIN_XML_SECURE | \
+ VIR_DOMAIN_XML_INACTIVE | \
+ VIR_DOMAIN_XML_UPDATE_CPU)
+verify((VIR_DOMAIN_XML_INTERNAL_STATUS & DOMAIN_GET_XML_DESC_FLAGS) == 0);
+
char *virDomainDefFormat(virDomainDefPtr def,
unsigned int flags)
{
@@ -9624,9 +9637,7 @@ char *virDomainDefFormat(virDomainDefPtr def,
const char *type = NULL;
int n, allones = 1;
- virCheckFlags(VIR_DOMAIN_XML_SECURE |
- VIR_DOMAIN_XML_INACTIVE |
- VIR_DOMAIN_XML_UPDATE_CPU, NULL);
+ virCheckFlags(DOMAIN_GET_XML_DESC_FLAGS, NULL);
if (!(type = virDomainVirtTypeToString(def->virtType))) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 172d3c2..7a3d72b 100644
--- 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 579c2b3..5034277 100644
--- 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 70ea4c2..0e0b808 100644
--- 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,
@@ -1229,16 +1229,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 +1257,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/libvirt.c b/src/libvirt.c
index 7e70caa..bffb9e3 100644
--- 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);
@@ -12695,12 +12693,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;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d0085e0..448b06e 100644
--- 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 2d5dc15..538c6aa 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3173,7 +3173,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 +3182,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 c45ba51..02cdbb9 100644
--- 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"));
--
1.7.4.4