Re: [PATCHv5 1/2] virsh: add new --details option to pool-list

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

 



On 06/28/2010 10:47 AM, Justin Clift wrote:
> This patch adds a new --details option to the virsh pool-list
> command, making its output more useful to people who use virsh
> for significant lengths of time.
> 
> Addresses BZ # 605543
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=605543
> 
> ---

> plus the sizeof() and array
> type/allocations fixed that Eric pointed out. Also renamed the
> "function_ret" variable to functionReturn for consistency, plus other
> minor tweaks.

ACK to these cleanups.

> 
> Updated version of the patch. This one has the original "return FALSE"
> style behaviour if no pools were found,

Nope, that's not the original behavior.

$ virsh -c test:///default pool-list --inactive; echo $?
Name                 State      Autostart
-----------------------------------------
0
$ tools/virsh -c test:///default pool-list --inactive; echo $?
Failed to list any pools
1

We really DO want to give 0 exit status, after listing a header but no
clients.

However, the rest of your patch is in good shape, so I squashed this in:

diff --git i/tools/virsh.c w/tools/virsh.c
index 7973c0b..de4876f 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -4947,11 +4947,6 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)

     /* Determine the total number of pools to list */
     numAllPools = numActivePools + numInactivePools;
-    if (!numAllPools) {
-        /* No pools to list, so cleanup and return */
-        vshPrint(ctl, "%s", _("Failed to list any pools"));
-        return FALSE;
-    }

     /* Allocate memory for arrays of storage pool names and info */
     poolNames = vshCalloc(ctl, numAllPools, sizeof(*poolNames));



then pushed the result.

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