[libvirt] [PATCH] phyp: Adding Storage Management driver (comments fixed)

[Eduardo Otubo]

>>       virBufferVSprintf(&buf, "-r prof --filter "
>>                         "profile_names=%s -F virtual_eth_adapters,"
>>                         "virtual_opti_pool_id,virtual_scsi_adapters,"
>>                         "virtual_serial_adapters|sed -e 's/\"//g' -e "
>>                         "'s/,/\\n/g'|sed -e 's/\\(^[0-9][0-9]\\*\\).*$/\\1/'"
>> -                      "|sort|tail -n 1` +1 ))", profile);
>> +                      "|sort|tail -n 1|sed -s 's/^[0]//'` +1 ))", profile);
>
> Here's the first real meat I found in the patch (70 lines in!), but it
> still has an issue - you are only stripping one, instead of all, leading
> zeroes.  I still think that doing the conversion to int, then the +1
> operation, in C, will be much better than trying to do it in shell with
> $(()).

Ok I'll fix that.

>
> My quick glance at spot checks in the rest of the file did see that you
> are making progress; you did incorporate what looks like quite a few of
> my comments.  However, I did not complete my review this time around
> because of the mix of multiple changes in one patch.
>
> On the other hand, I agree that this still seems like something worth
> getting in 0.8.2 if we can clean it up fast enough.  Do you need any
> help separating the patch into separate stages?

I'll separate the identation from the semantic code right away. Thanks 
for the suggestion :)

-- 
Eduardo Otubo
Software Engineer
Linux Technology Center
IBM Systems & Technology Group
Mobile: +55 19 8135 0885
eotubo at linux.vnet.ibm.com