On Wed, Oct 12, 2011 at 05:26:34PM +0800, Hu Tao wrote:
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(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c991dfc..fa53147 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -208,6 +208,27 @@ 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;
I'm not really a fan of this - it is exposing details of the RPC implementation to the applications. IMHO we should be using the internal RPC features capability, and the VIR_DRV_SUPPORTS_FEATURE to detect support internally, and then return 'VIR_ERR_NO_SUPPORT' if string parameters are not supported.
diff --git a/src/libvirt.c b/src/libvirt.c index f07c720..59d6d26 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3472,6 +3489,9 @@ virDomainSetMemoryParameters(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3547,6 +3567,9 @@ virDomainGetMemoryParameters(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3598,6 +3621,9 @@ virDomainSetBlkioParameters(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -3657,6 +3683,9 @@ virDomainGetBlkioParameters(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6279,6 +6308,9 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6396,6 +6428,9 @@ virDomainSetSchedulerParametersFlags(virDomainPtr domain,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); @@ -6537,6 +6572,9 @@ int virDomainBlockStatsFlags (virDomainPtr dom,
virResetLastError();
+ if (virDomainCheckTypedStringFlag(params, *nparams, flags) < 0) + return -1; + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL);
So IMHO all these should be VIR_DRV_SUPPORTS_FEATURE(VIR_DRV_FEATURE_TYPED_STRING) and if the app gets back an error, it can re-try without the string parameter.
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f95253e..994c339 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -317,6 +317,8 @@ union remote_typed_param_value switch (int type) { double d; case VIR_TYPED_PARAM_BOOLEAN: int b; + case VIR_TYPED_PARAM_STRING: + remote_nonnull_string s; };
struct remote_typed_param {
I'm kind of suprised this actually works, but I presume you've tested old client with new server, and vica-verca so I guess its OK. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|