Re: [RFC] virCommandRun return error number

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

 



On 04/11/2013 07:48 AM, harryxiyou wrote:
> On Thu, Apr 11, 2013 at 9:38 PM, Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:
> [...]
>> Maybe you've pasted wrong code snippet, but the one you've pasted is
>> correct. If virCommandRun() fails and returns -1; we return that value
>> to the caller. And if the command doesn't fail, we call
>> virStorageBackendSheepdogParseNodeInfo to parse output of the command.
>>
> 
> Sorry, i have asked for a stupid bug. Let us check following one.
> 
> [...]
> 114 static int
> 115 virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> 116                                      virStoragePoolObjPtr pool)
> 117 {
> 118     int ret;
> 119     char *output = NULL;
> 120     virCommandPtr cmd;
> 121
> 122     cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL);
> 123     virStorageBackendSheepdogAddHostArg(cmd, pool);
> [...]
> 
> virCommandNewArgList may return ‘NULL’, so i think we should check
> this condition, which we need not do following stuffs, right?

Not necessary.  virCommand is DESIGNED for streamlined usage, so that it
is much easier to read how the command is constructed without being
distracted by error checking in the caller every step of the way.  As a
virCommandPtr has no semantic impact until it is run, it is sufficient
to delay error checking until the caller is actually ready to run the
command.  Therefore, we wrote virCommandRun() to specifically check for
NULL, and report an error at that time, so that the caller need not
worry about virCommandNew* returning NULL.

No bug here.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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]