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

Eric Blake eblake at redhat.com
Fri May 3 16:39:01 UTC 2013


On 05/02/2013 06:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> 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 at redhat.com>
> ---
>  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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130503/986f84d3/attachment-0001.sig>


More information about the libvir-list mailing list