On Tue, Sep 06, 2011 at 07:09:53AM -0600, Eric Blake wrote:
> On 09/06/2011 04:44 AM, Daniel Veillard wrote:
> >On Tue, Sep 06, 2011 at 06:18:40PM +0800, Hu Tao wrote:
> >>---
> >> daemon/remote.c | 15 +++++++++++++++
> >> include/libvirt/libvirt.h.in | 4 +++-
> >> src/remote/remote_driver.c | 15 +++++++++++++++
> >> src/remote/remote_protocol.x | 2 ++
> >> 4 files changed, 35 insertions(+), 1 deletions(-)
> >>
> >>+++ b/include/libvirt/libvirt.h.in
> >>@@ -481,7 +481,8 @@ typedef enum {
> >> VIR_TYPED_PARAM_LLONG = 3, /* long long case */
> >> 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_BOOLEAN = 6, /* boolean(character) case */
> >>+ VIR_TYPED_PARAM_STRING = 7 /* string case */
>
> New enum value is probably okay,
>
> >> } virTypedParameterType;
> >>
> >> /**
> >>@@ -512,6 +513,7 @@ struct _virTypedParameter {
> >> unsigned long long int ul; /* type is ULLONG */
> >> double d; /* type is DOUBLE */
> >> char b; /* type is BOOLEAN */
> >>+ char *s; /* type is STRING */
>
> and new char * field to a union doesn't change the struct size,
>
> >>+++ b/src/remote/remote_protocol.x
> >>@@ -314,6 +314,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;
>
> but I'm quite worried about the on-the-wire compatibility aspect of
> this change. If a new server sends the new enum value and a string,
> will the old server that does not know that enum value properly
> reject the rpc call, or will it cause a crash or other bad things to
> happen?
If a new server sends a string parameter to an old client, you
will get a fatal error on the client decoding the XDR data. This
will cause libvirt to report an XDR decoding error. It (probably)
isn't fatal, since we've read the entire packet off the wire
the next RPC call should still work. It is still not too pleasant
though since old virsh will not work with new libvirtd IIUC,
so I don't think we can do this without getting a better compat
story here which does not require changing existing apps like
virsh.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list