Re: [PATCH] fix virsh's regression

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

 



On 04/25/2011 09:03 PM, Wen Congyang wrote:
> This patch does the following things:
> 1. The return value of cmdSchedInfoUpdate() can be -1, 0 and 1. So the
>    type of return value should be int not bool.(This function is not a
>    entry of a virsh command, but the name of this function likes cmdXXX)

Phooey - mass replacement makes for a patch that's too hard to review,
and indeed snuck in this regression.

> 
> 2. The type of cmdSchedinfo()'s, cmdFreecell()'s, cmdPoolList()'s and
>    cmdVolList()'s return value is bool not int, so change the type of
>    variable ret_val, func_ret and functionReturn.

Not quite as serious (int holds bool), but still worth fixing.

> 
> 3. Add a variable functionReturn for cmdMigrate(), cmdAttachInterface(),
>    cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to save the
>    return value.
> 
> 4. Change the type of variable ret in the function cmdAttachDevice(),
>    cmdDetachDevice(), cmdUpdateDevice(), cmdAttachInterface(),
>    cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to int, as
>    we use it to save the return value of virXXX() and the type of virXXX()'s
>    return value is int not bool.

Also a regression.  Thanks for catching these.

> 
> 5. Do some cleanup when virBuff.error is 1.
> 
> The bug 1-4 were introduced by commit b56fa5bb.
> 
> ---
>  tools/virsh.c |   63 +++++++++++++++++++++++++++++---------------------------
>  1 files changed, 33 insertions(+), 30 deletions(-)

ACK.  And I'm glad I pushed my patch as early as I did in the release
cycle, to let us catch these sort of issues pre-release.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]