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

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




      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 linux vnet ibm com


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