[libvirt] [PATCH 00/66] vbox: Rewrite vbox domain driver

Daniel P. Berrange berrange at redhat.com
Thu Aug 14 15:54:40 UTC 2014


On Thu, Aug 14, 2014 at 05:50:18PM +0200, Yohan Belleguic wrote:
> Le Wednesday 13 August 2014 17:18:44, Michal Privoznik a écrit :
> > [CC-ing Yohan BELLEGUIC and Manuel VIVES]
> > 
> > On 12.08.2014 17:31, Michal Privoznik wrote:
> > > On 11.08.2014 12:06, Taowei wrote:
> > >> This series of patches rewrite the vbox's domain
> > >> driver. The driver is separated into two parts: the version
> > >> specified and the common part. The common driver use
> > >> vboxUniformedAPI to build a general driver for all vbox
> > >> versions. The vboxUniformedAPI take the responsiblity to
> > >> communicate with virtualbox. Since there are some incompatible
> > >> changes in virtualbox, vboxUniformedAPI should be aware of
> > >> these changes and provide a uniformed api for the upper layer.
> > >> 
> > >> The significant result of this patch is that we replace all
> > >> vir${vbox_version}Driver into one virCommonDriver. So, we will
> > >> have only one vbox driver implementation for all vbox versions
> > >> in libvirt.
> > >> 
> > >> PS: I have send part of my patches before:
> > >> https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
> > >> But I have to resend it beacuse I did some improvement on previous
> > >> patches:
> > >> *Remove the test case for vboxUniformedAPI, because it would raise
> > >> 
> > >>   "break strict-aliasing rules" warning in some distibutions
> > >> 
> > >> *Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
> > >> 
> > >>   So, we use one flag to indicate whether vbox support callbacks
> > >>   as well as we need to initialize variables for it.
> > >> 
> > >> Taowei (66):
> > >>    vbox: Begin to rewrite, vboxConnectOpen
> > >>    vbox: Rewrite vboxConnectClose
> > >>    vbox: Rewrite vboxDomainSave
> > >>    vbox: Rewrite vboxConnectGetVersion
> > >>    vbox: Rewrite vboxConnectGetHostname
> > >>    vbox: Rewrite vboxConnectIsSecure
> > >>    vbox: Rewrite vboxConnectIsEncrypted
> > >>    vbox: Rewrite vboxConnectIsAlive
> > >>    vbox: Rewrite vboxConnectGetMaxVcpus
> > >>    vbox: Rewrite vboxConnectGetCapabilities
> > >>    vbox: Rewrite vboxConnectListDomains
> > >>    vbox: Rewrite vboxConnectNumOfDomains
> > >>    vbox: Rewrite vboxDomainLookupById
> > >>    vbox: Rewrite vboxDomainLookupByUUID
> > >>    vbox: Rewrite vboxDomainUndefineFlags
> > >>    vbox: Rewrite vboxDomainDefineXML
> > >>    vbox: Rewrite vboxDomainCreateWithFlags
> > >>    vbox: Rewrite vboxDomainCreate
> > >>    vbox: Rewrite vboxDomainCreateXML
> > >>    vbox: Rewrite vboxDomainLookupByName
> > >>    vbox: Rewrite vboxDomainIsActive
> > >>    vbox: Rewrite vboxDomainIsPersistent
> > >>    vbox: Rewrite vboxDomainIsUpdated
> > >>    vbox: Rewrite vboxDomainSuspend
> > >>    vbox: Rewrite vboxDomainResume
> > >>    vbox: Rewrite vboxDomainShutdownFlags
> > >>    vbox: Rewrite vboxDomainShutdown
> > >>    vbox: Rewrite vboxDomainReboot
> > >>    vbox: Rewrite vboxDomainDestroyFlags
> > >>    vbox: Rewrite vboxDomainDestroy
> > >>    vbox: Rewrite vboxDomainGetOSType
> > >>    vbox: Rewrite vboxDomainSetMemory
> > >>    vbox: Rewrite vboxDomainGetInfo
> > >>    vbox: Rewrite vboxDomainGetState
> > >>    vbox: Rewrite vboxDomainSetVcpusFlags
> > >>    vbox: Rewrite vboxDomainSetVcpus
> > >>    vbox: Rewrite vboxDomainGetVcpusFlags
> > >>    vbox: Rewrite vboxDomainGetMaxVcpus
> > >>    vbox: Add API for vboxDomainGetXMLDesc
> > >>    vbox: Rewrite vboxDomainGetXMLDesc
> > >>    vbox: Rewrite vboxConnectListDefinedDomains
> > >>    vbox: Rewrite vboxConnectNumOfDefinedDomains
> > >>    vbox: Rewrite vboxDomainUndefine
> > >>    vbox: Rewrite vboxDomainAttachDevice
> > >>    vbox: Rewrite vboxDomainAttachDeviceFlags
> > >>    vbox: Rewrite vboxDomainUpdateDeviceFlags
> > >>    vbox: Rewrite vboxDomainDetachDevice
> > >>    vbox: Rewrite vboxDomainDetachDeviceFlags
> > >>    vbox: Add API for vboxDomainSnapshotCreateXML
> > >>    vbox: Rewrite vboxDomainSnapshotCreateXML
> > >>    vbox: Rewrite vboxDomainSnapshotGetXMLDesc
> > >>    vbox: Rewrite vboxDomainSnapshotNum
> > >>    vbox: Rewrite vboxDomainSnapshotListNames
> > >>    vbox: Rewrite vboxSnapshotLookupByName
> > >>    vbox: Rewrite vboxDomainHasCurrentSnapshot
> > >>    vbox: Rewrite vboxDomainSnapshotGetParent
> > >>    vbox: Rewrite vboxDomainSnapshotCurrent
> > >>    vbox: Rewrite vboxDomainSnapshotIsCurrent
> > >>    vbox: Rewrite vboxDomainSnapshotHasMetadata
> > >>    vbox: Rewrite vboxDomainRevertToSnapshot
> > >>    vbox: Rewrite vboxDomainSnapshotDelete
> > >>    vbox: Rewrite vboxDomainScreenshot
> > >>    vbox: Rewrite vboxConnectListAllDomains
> > >>    vbox: Rewrite vboxNode functions
> > >>    vbox: Add registerDomainEvent
> > >>    vbox: Introducing vboxCommonDriver
> > >>   
> > >>   po/POTFILES.in                |    1 +
> > >>   src/Makefile.am               |    5 +-
> > >>   src/vbox/README               |    7 +-
> > >>   src/vbox/vbox_common.c        | 7550 +++++++++++++++++++++
> > >>   src/vbox/vbox_common.h        |  306 +
> > >>   src/vbox/vbox_driver.c        |   40 +-
> > >>   src/vbox/vbox_install_api.h   |   26 +
> > >>   src/vbox/vbox_tmpl.c          |14557
> > >> 
> > >> +++++++++++++----------------------------
> > >> 
> > >>   src/vbox/vbox_uniformed_api.h |  551 ++
> > >>   9 files changed, 13186 insertions(+), 9857 deletions(-)
> > >>   create mode 100644 src/vbox/vbox_common.c
> > >>   create mode 100644 src/vbox/vbox_common.h
> > >>   create mode 100644 src/vbox/vbox_install_api.h
> > >>   create mode 100644 src/vbox/vbox_uniformed_api.h
> > > 
> > > ACK to all the patches. I've fixed all the small nits I found. I'm
> > > keeping the patches on my private branch for some time to give others
> > > time to share their opinions. Nevertheless, incredible work in making
> > > the vbox driver look more sane what you've done!
> > 
> > Yohan and Manuel,
> > 
> > can you guys please give this patchset a test? You seem to be interested
> > in our virtual box driver (esp. snapshots) and we won't be happy if we
> > break something by merging the patches. I haven't spotted anything wrong
> > in the patches, so I gave my ACK but I'll postpone the push for a while
> > to give you chance to try them out.
> 
> Everything seems good to me, I didn't see any apparent problem.
> We will not update immediately libvirt, so we won't be able to thoroughly test 
> the vbox driver.

Ok, thanks for the feedback. We just wanted to make sure your team don't
have any significant objections to this patch series before we went ahead
and merged it :-)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|




More information about the libvir-list mailing list