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

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

 



On 14.08.2014 17:50, 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.

Michal


Hello,

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.


Regards


Perfect! So I've merged this. Thanks to Taowei for taking the effort and turning spaghetti code into something more readable.

Michal

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