Re: [PATCH 1/1] complete netlink event integration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/29/2012 08:43 AM, D. Herrendoerfer wrote:
>
> On Feb 29, 2012, at 9:51 AM, Laine Stump wrote:
>
>> Since I found a couple other problems, but have made you suffer through
>> enough back and forth already, I've made some final suggested changes
>> myself, and am sending a diff patch as a response to this message
>> (differences from your latest version to mine), as well as doing a
>> repost of the original two patches with your latest changes and mine
>> squashed in.
>>
>> Please 1) take a look at the changes in my diff patches , then 2) test
>> the updated versions of the full patches (I'm unable to test beyond
>> compiling), and send your ACK if they're okay. Once I have your ACK
>> back, I'll push (I promise, I won't find any new issues *this* time :-)
>>
>
> ACK.
>
> The code tests out fine. One nit I found in the logs was that the removal
> was indicated by the wrong method:
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index fd6f751..614f8da 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -509,7 +509,7 @@ virNetlinkEventRemoveClient(int watch, const
> unsigned char *
>
>              virNetlinkEventRemoveClientPrimitive(i);
>              VIR_DEBUG("removed client: %d by %s.",
> srv->handles[i].watch,
> -                      srv->handles[i].watch ? "index" : "mac");
> +                      srv->handles[i].watch ? "mac" : "index");


Aha! Actually it's not that it's switched, it's that I stupidly changed
the check to use srv->handles[i].watch (what's stored in the object)
instead of using watch (which came in the call args). I'll switch that
back and push.

Thanks for the fast test turnaround!


>              ret = 0;
>              goto cleanup;
>          }
> -- 
>
> Thanks for the effort !
>
> Dirk H
>
>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]