Re: [PATCH v4 0/3] ESX: Add routines to interface driver

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

 



2012/7/29 Ata E Husain Bohra <ata.husain@xxxxxxxxxxx>:
> Updated the patch against review comments from Laine and Matthias.
>
> Ata E Husain Bohra (3):
>   ESX: Add routines to interface driver
>   ESX: Add routines to interface driver
>   ESX: Add routines to interface driver
>
>  src/esx/esx_interface_driver.c |  499 +++++++++++++++++++++++++++++++++++++++-
>  src/esx/esx_vi.c               |  126 ++++++++++
>  src/esx/esx_vi.h               |   10 +
>  src/esx/esx_vi_generator.input |  227 ++++++++++++++++++
>  src/esx/esx_vi_generator.py    |   31 ++-
>  src/esx/esx_vi_types.c         |   18 +-
>  6 files changed, 906 insertions(+), 5 deletions(-)

Okay, a first quick review:

You should really merge your two incremental patches into the first
one and propose a single updated patch.

You still have your
ESX_VI__TEMPLATE__PROPERTY__DESERIALIZE_STRING_LIST workaround in
there. The problem is already fixed in git, so please remove this
part.

There are several unchecked strdup calls. All strdup calls need to be
checked for NULL return and report an OOM error is necessary.

All esxVI_* functions already report errors. But on some calls to such
functions (for example esxVI_RemoveVirtualNic and
esxVI_LookupPhysicalNicFromPortGroup) you're reporting an extra error
overwriting the original. probably more detailed error. You should
avoid this.

I still need to review this patch on the conceptual side, I'll
probably have time for this later this week.

-- 
Matthias Bolte
http://photron.blogspot.com

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