[libvirt] [PATCH 7/7] list: Use virConnectListAllNodeDevices in virsh

Eric Blake eblake at redhat.com
Thu Sep 6 22:22:31 UTC 2012


On 09/06/2012 04:05 PM, Peter Krempa wrote:

>> +    if (cap_str) {
>> +        ncaps = vshStringToArray((char *)cap_str, &caps);
> 
> You shouldn't modify const strings.

Indeed; that argues that your earlier patch for vshStringToArray should
be modified to take 'const char *' as the string to split, and that
internally, it should strdup() if it is going to modify in-place, rather
than requiring the caller to pass in a modifiable string.  It would also
be possible to write vshStringToArray without modifying the input string
in place, but probably with a bit more effort (you'd have to use
strndup() of the individual array elements, and if you teach it how to
do ',,' escaped comma parsing, it gets even trickier).


>> virNodeDeviceLookupByName(ctl->conn, devices[i]);
>> -            if (dev && STRNEQ(devices[i], "computer")) {
>> +        for (i = 0; i < list->ndevices; i++) {
>> +            virNodeDevicePtr dev = list->devices[i];
>> +            if (STRNEQ(names[i], "computer")) {
> 
> You need to modify this condition to fix the bug with missing output
> when you specify filters. You will have to abuse
> virNodeDeviceGetParent() that returns NULL if it has no parent. This
> will probably induce a situation where multiple parents will be printed
> but that's right in this situation.

Or, if you take my hint from 6/7 and merely reject --tree and --caps as
incompatible options, then you don't have to worry about this part of
the code changing.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120906/3eacd35e/attachment-0001.sig>


More information about the libvir-list mailing list