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

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

 




      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@xxxxxxxxxxxxxxxxxx

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