On 10/12/2011 03:26 AM, Hu Tao wrote:
Apologies on the delayed review, but I'm finally getting to this one.
This makes string can be transported between client and server.
For compatibility,
o new server should not send strings to old client if it
doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY.
o new client that wants to be able to send/receive strings
should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY;
if it is rejected by a old server that doesn't understand
VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have
a second try with out flag VIR_DOMAIN_TYPED_STRING_OKAY
to cope with an old server.
Ideas for compatibility are coming from Eric, thanks.
---
daemon/remote.c | 32 +++++++++++++++++++++++++++++---
include/libvirt/libvirt.h.in | 25 ++++++++++++++++++++++++-
src/libvirt.c | 38 ++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 30 ++++++++++++++++++++++++++++--
src/remote/remote_protocol.x | 2 ++
src/remote_protocol-structs | 2 ++
src/rpc/gendispatch.pl | 2 +-
src/util/util.c | 15 +++++++++++++++
src/util/util.h | 2 ++
9 files changed, 141 insertions(+), 7 deletions(-)
This patch did not compile for out of the box:
libvirt.c: In function 'virDomainSetMemory':
libvirt.c:3482:5: error: implicit declaration of function
'virDomainCheckTypedStringFlag' [-Wimplicit-function-declaration]
but I think that is how git (mis-)applied the patch based on all the
changes that have occurred in the meantime, and the fact that the hunks
were contextually ambiguous. Git tried to patch
virDomainSetMemoryFlags, which does not use typed parameters, instead of
virDomainSetBlkioParameters ;( That just goes to show that a lot has
happened in libvirt.c since this patch was proposed.
+++ b/include/libvirt/libvirt.h.in
@@ -208,6 +208,27 @@ typedef enum {
} virDomainModificationImpact;
/**
+ * virDomainFlags:
Given that this enum only affects the use of typed parameters, I think
it fits slightly better if listed near typed parameters, with a slightly
different name.
+ *
+ * Flags that can be used with some libvirt APIs.
+ *
+ * These enums should not confilict with those of virDomainModificationImpact.
s/confilict/conflict/
+ */
+typedef enum {
+ VIR_DOMAIN_TYPED_STRING_OKAY = 1<< 2, /* Usage of this flag:
virTypedParameters are not domain-specific - it is feasible that we will
reuse the type even on the node or network level in the future. I'm
dropping _DOMAIN out of the name (and yes, that means I have to rebase a
lot of the patch, accordingly - but better to get a good name out of
things).
+ * o new server should not send strings to old client if it
mention 0.9.7 as what constitutes new (a year from now, 0.9.7 will feel
old :)
+ * doesn't see the flag VIR_DOMAIN_TYPED_STRING_OKAY.
+ * o new client that wants to be able to send/receive strings
+ * should always set the flag VIR_DOMAIN_TYPED_STRING_OKAY;
+ * if it is rejected by a old server that doesn't understand
s/a old/an old/
+ * VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have
+ * a second try without flag VIR_DOMAIN_TYPED_STRING_OKAY to
+ * cope with an old server.
+ */
I reformatted this, for a better 80 column fit.
+++ b/src/libvirt.c
@@ -3447,6 +3447,23 @@ error:
return -1;
}
+static int virDomainCheckTypedStringFlag(virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
Again, I went with a more generic name: virTypedParameterStringCheck
+++ b/src/util/util.c
@@ -2509,3 +2509,18 @@ or other application using the libvirt API.\n\
return 0;
}
+
+void virTypedParameterFree(virTypedParameterPtr params, int nparams)
+{
Needs an export in libvirt_private.syms. And unfortunately, it violates
the typical vir*Free convention of taking a single parameter, so it's
not free-like, but I don't know how to fix that.
Here's what I'm squashing in:
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 26a3306..38d1dfb 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -200,6 +200,8 @@ typedef virDomainControlInfo *virDomainControlInfoPtr;
* current domain state. VIR_DOMAIN_AFFECT_LIVE requires a running
* domain, and VIR_DOMAIN_AFFECT_CONFIG requires a persistent domain
* (whether or not it is running).
+ *
+ * These enums should not conflict with those of virTypedParameterFlags.
*/
typedef enum {
VIR_DOMAIN_AFFECT_CURRENT = 0, /* Affect current domain
state. */
@@ -208,27 +210,6 @@ typedef enum {
} virDomainModificationImpact;
/**
- * virDomainFlags:
- *
- * Flags that can be used with some libvirt APIs.
- *
- * These enums should not confilict with those of
virDomainModificationImpact.
- */
-typedef enum {
- VIR_DOMAIN_TYPED_STRING_OKAY = 1 << 2, /* Usage of this flag:
- * o new server should
not send strings to old client if it
- * doesn't see the flag
VIR_DOMAIN_TYPED_STRING_OKAY.
- * o new client that
wants to be able to send/receive strings
- * should always set
the flag VIR_DOMAIN_TYPED_STRING_OKAY;
- * if it is rejected by
a old server that doesn't understand
- *
VIR_DOMAIN_TYPED_STRING_OKAY, then the client should have
- * a second try without
flag VIR_DOMAIN_TYPED_STRING_OKAY to
- * cope with an old server.
- */
-
-} virDomainFlags;
-
-/**
* virDomainInfoPtr:
*
* a virDomainInfo is a structure filled by virDomainGetInfo() and
extracting
@@ -511,10 +492,31 @@ typedef enum {
VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */
VIR_TYPED_PARAM_DOUBLE = 5, /* double case */
VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */
- VIR_TYPED_PARAM_STRING = 7, /* string case */
+ VIR_TYPED_PARAM_STRING = 7, /* string case; see
virTypedParameterFlags */
} virTypedParameterType;
/**
+ * virTypedParameterFlags:
+ *
+ * Flags related to libvirt APIs that use virTypedParameter.
+ *
+ * These enums should not conflict with those of
virDomainModificationImpact.
+ */
+typedef enum {
+ /* Usage of this flag:
+ * - servers before 0.9.7 do not understand typed strings.
+ * - servers 0.9.7 and newer will not send or receive strings
+ * without the flag VIR_TYPED_PARAM_STRING_OKAY.
+ * - clients that want to be able to send/receive strings should
+ * first try to set the flag VIR_TYPED_PARAM_STRING_OKAY; if
+ * it is rejected by an old server, then the client can try
+ * again without the flag, to get only non-string parameters.
+ */
+ VIR_TYPED_PARAM_STRING_OKAY = 1 << 2,
+
+} virTypedParameterFlags;
+
+/**
* VIR_TYPED_PARAM_FIELD_LENGTH:
*
* Macro providing the field length of virTypedParameter name
diff --git i/src/libvirt.c w/src/libvirt.c
index 6e524c3..379bb20 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -3479,9 +3479,6 @@ virDomainSetMemory(virDomainPtr domain, unsigned
long memory)
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0)
- return -1;
-
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -3549,9 +3546,6 @@ virDomainSetMemoryFlags(virDomainPtr domain,
unsigned long memory,
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0)
- return -1;
-
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -3585,11 +3579,12 @@ error:
return -1;
}
-static int virDomainCheckTypedStringFlag(virTypedParameterPtr params,
- int nparams,
- unsigned int flags)
+static int
+virTypedParameterStringCheck(virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
{
- if (!(flags & VIR_DOMAIN_TYPED_STRING_OKAY)) {
+ if (!(flags & VIR_TYPED_PARAM_STRING_OKAY)) {
int i;
for (i = 0; i < nparams; i++) {
if (params[i].type == VIR_TYPED_PARAM_STRING) {
@@ -3608,7 +3603,7 @@ static int
virDomainCheckTypedStringFlag(virTypedParameterPtr params,
* @params: pointer to memory parameter objects
* @nparams: number of memory parameter (this value can be the same or
* less than the number of parameters supported)
- * @flags: bitwise-OR of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Change all or a subset of the memory tunables.
* This function may require privileged access to the hypervisor.
@@ -3627,7 +3622,7 @@ virDomainSetMemoryParameters(virDomainPtr domain,
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0)
+ if (virTypedParameterStringCheck(params, nparams, flags) < 0)
return -1;
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
@@ -3666,7 +3661,7 @@ error:
* @params: pointer to memory parameter object
* (return value, allocated by the caller)
* @nparams: pointer to number of memory parameters
- * @flags: one of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Get all memory parameters, the @params array will be filled with
the values
* equal to the number of parameters suggested by @nparams
@@ -3705,7 +3700,7 @@ virDomainGetMemoryParameters(virDomainPtr domain,
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0)
+ if (virTypedParameterStringCheck(params, *nparams, flags) < 0)
return -1;
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
@@ -3740,7 +3735,7 @@ error:
* @params: pointer to blkio parameter objects
* @nparams: number of blkio parameters (this value can be the same or
* less than the number of parameters supported)
- * @flags: an OR'ed set of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Change all or a subset of the blkio tunables.
* This function may require privileged access to the hypervisor.
@@ -3759,6 +3754,9 @@ virDomainSetBlkioParameters(virDomainPtr domain,
virResetLastError();
+ if (virTypedParameterStringCheck(params, nparams, flags) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -3795,7 +3793,7 @@ error:
* @params: pointer to blkio parameter object
* (return value, allocated by the caller)
* @nparams: pointer to number of blkio parameters
- * @flags: an OR'ed set of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Get all blkio parameters, the @params array will be filled with the
values
* equal to the number of parameters suggested by @nparams.
@@ -3818,6 +3816,9 @@ virDomainGetBlkioParameters(virDomainPtr domain,
virResetLastError();
+ if (virTypedParameterStringCheck(params, *nparams, flags) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6336,9 +6337,6 @@ virDomainGetSchedulerType(virDomainPtr domain, int
*nparams)
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0)
- return -1;
-
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6388,6 +6386,9 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
virResetLastError();
+ if (virTypedParameterStringCheck(params, *nparams, 0) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6424,7 +6425,7 @@ error:
* @nparams: pointer to number of scheduler parameter
* (this value should be same than the returned value
* nparams of virDomainGetSchedulerType)
- * @flags: one of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Get the scheduler parameters, the @params array will be filled with the
* values.
@@ -6446,7 +6447,7 @@ virDomainGetSchedulerParametersFlags(virDomainPtr
domain,
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0)
+ if (virTypedParameterStringCheck(params, *nparams, flags) < 0)
return -1;
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
@@ -6503,6 +6504,9 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
virResetLastError();
+ if (virTypedParameterStringCheck(params, nparams, 0) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6543,7 +6547,7 @@ error:
* @nparams: number of scheduler parameter objects
* (this value can be the same or less than the returned value
* nparams of virDomainGetSchedulerType)
- * @flags: bitwise-OR of virDomainModificationImpact
+ * @flags: bitwise-OR of virDomainModificationImpact and
virTypedParameterFlags
*
* Change a subset or all scheduler parameters. The value of @flags
* should be either VIR_DOMAIN_AFFECT_CURRENT, or a bitwise-or of
@@ -6566,6 +6570,9 @@ virDomainSetSchedulerParametersFlags(virDomainPtr
domain,
virResetLastError();
+ if (virTypedParameterStringCheck(params, nparams, flags) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6636,9 +6643,6 @@ virDomainBlockStats (virDomainPtr dom, const char
*path,
virResetLastError();
- if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0)
- return -1;
-
if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
@@ -6672,7 +6676,7 @@ error:
* @params: pointer to block stats parameter object
* (return value)
* @nparams: pointer to number of block stats
- * @flags: unused, always passes 0
+ * @flags: bitwise-OR of virTypedParameterFlags
*
* This function is to get block stats parameters for block
* devices attached to the domain.
@@ -6710,6 +6714,9 @@ int virDomainBlockStatsFlags (virDomainPtr dom,
virResetLastError();
+ if (virTypedParameterStringCheck(params, *nparams, flags) < 0)
+ return -1;
+
if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
diff --git i/src/util/util.c w/src/util/util.c
index 895e6b1..506d355 100644
--- i/src/util/util.c
+++ w/src/util/util.c
@@ -2571,7 +2571,8 @@ or other application using the libvirt API.\n\
return 0;
}
-void virTypedParameterFree(virTypedParameterPtr params, int nparams)
+void
+virTypedParameterFree(virTypedParameterPtr params, int nparams)
{
int i;
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list