[libvirt] [PATCH v4 1/2] Add VIR_TYPED_PARAM_STRING
Daniel P. Berrange
berrange at redhat.com
Wed Oct 26 09:19:36 UTC 2011
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 :|
More information about the libvir-list
mailing list