Chris Lalancette <clalance@xxxxxxxxxx> 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);
...
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list