Chris Lalancette <clalance@redhat.com> wrote:

Attached is a relatively simple patch to the ruby-libvirt bindings with some bugfixes for the migrate call. The first problem was that there was no way to pass a "nil" through to the underlying virDomainMigrate() call. This is important because generally the "dname" and "uri" parameters end up being NULL. This patch also makes all the parameters beyond the first 2 optional. I've tested this out by live migrating a KVM guest between two hosts, and this now does what I expect.

Hi Chris, I'm no ruby integration guru, but took a look anyhow. Looks fine modulo a couple nits.

diff -r c6a3e36cdf54 ext/libvirt/_libvirt.c --- a/ext/libvirt/_libvirt.c Thu Jul 17 15:24:26 2008 -0700 +++ b/ext/libvirt/_libvirt.c Fri Aug 08 06:04:56 2008 -0400 @@ -637,16 +637,51 @@ VALUE libvirt_conn_num_of_defined_storag } #endif

+static char *get_string_or_nil(VALUE arg) +{ + if (TYPE(arg) == T_NIL) + return NULL; + else if (TYPE(arg) == T_STRING) + return STR2CSTR(arg);

STR2CSTR is marked as obsolete in ruby.h, where it says to use StringValuePtr instead: /* obsolete API - use StringValuePtr() */ #define STR2CSTR(x) rb_str2cstr((VALUE)(x),0)

+ else + rb_raise(rb_eTypeError, "wrong argument type (expected String or nil)"); return NULL; +} + /* * Class Libvirt::Domain */ -VALUE libvirt_dom_migrate(VALUE s, VALUE dconn, VALUE flags, - VALUE dname, VALUE uri, VALUE bandwidth) { +VALUE libvirt_dom_migrate(int argc, VALUE *argv, VALUE s) { virDomainPtr ddom = NULL; + VALUE dconn; + unsigned long flags; + char *dname; + char *uri;

Both pointers can be "const". Marking them as const also helps to emphasize that they must not be freed.

+ unsigned long bandwidth;

- ddom = virDomainMigrate(domain_get(s), conn(dconn), NUM2UINT(flags), - StringValueCStr(dname), StringValueCStr(uri), - NUM2UINT(bandwidth)); + if (argc < 2 || argc > 5) { + rb_raise(rb_eArgError, "Must provide between 2 and 5 arguments"); + } + + dconn = argv[0]; + flags = NUM2UINT(argv[1]); + dname = NULL; + uri = NULL; + bandwidth = 0; + + switch(argc) { + case 5: + Check_Type(argv[4], T_FIXNUM); + bandwidth = NUM2UINT(argv[4]); + /* fallthrough */ + case 4: + uri = get_string_or_nil(argv[3]); + /* fallthrough */ + case 3: + dname = get_string_or_nil(argv[2]); + } + + ddom = virDomainMigrate(domain_get(s), conn(dconn), flags, + dname, uri, bandwidth); ...