Re: [PATCH 10/11] Unmerge attach/update/modify device APIs in drivers

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

 



On 05/02/2013 06:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> The LXC, QEMU, and LibXL drivers have all merged their handling of
> the attach/update/modify device APIs into one large
> 
>   'xxxxDomainModifyDeviceFlags'
> 
> which then does a 'switch()' based on the actual API being invoked.
> While this saves some lines of code, it is not really all that
> significant in the context of the driver API impls as a whole.
> 
> This merger of the handling of different APIs creates pain when
> wanting to automated analysis of the code and do things which
> are specific to individual APIs. The slight duplication of code
> from unmerged the API impls, is preferrable to allow for easier
> automated analysis.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/libxl/libxl_driver.c | 241 +++++++++++++++++++++++++++++-------
>  src/lxc/lxc_driver.c     | 278 ++++++++++++++++++++++++++++++++---------
>  src/qemu/qemu_driver.c   | 316 ++++++++++++++++++++++++++++++++++++++---------
>  3 files changed, 671 insertions(+), 164 deletions(-)

ACK.  Looks huge, but it is mostly copy and paste of the existing code,
followed by flattening out the switch back into the per-API call.  And I
agree that not multiplexing the code will make it easier to add ACL that
allow device updates differently than device unplug.

I still think you could reduce the overall code size by having helper
functions for common tasks (such as flag sanitization of LIVE vs.
CONFIG), instead of verbatim repetition of large preambles, but that can
be separate patches.

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

Attachment: signature.asc
Description: OpenPGP digital signature

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