Re: [libvirt] [PATCHv4 3/4]vbox: Use vboxUniformedAPI to write common code

2014-07-02 17:12 GMT+08:00 Michal Privoznik <mprivozn(a)redhat.com <mailto:mprivozn@redhat.com>>: On 26.06.2014 15 <tel:26.06.2014%2015>:51, Taowei wrote: In vbox_common.c: vboxInitialize and vboxDomainSave are rewrited with vboxUniformedAPI. In vbox_common.h Some common definitions in vbox_CAPI_v*.h are directly extracted to this file. Some other incompatible defintions are simplified here. So we can write common code with it. --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/vbox/vbox_common.c | 150 ++++++++++++++++++++++++++++++__+++++++++++++++++ src/vbox/vbox_common.h | 151 ++++++++++++++++++++++++++++++__++++++++++++++++++ 4 files changed, 303 insertions(+) create mode 100644 src/vbox/vbox_common.c create mode 100644 src/vbox/vbox_common.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 31a8381..8c1b712 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -213,6 +213,7 @@ src/util/virxml.c src/vbox/vbox_MSCOMGlue.c src/vbox/vbox_XPCOMCGlue.c src/vbox/vbox_driver.c +src/vbox/vbox_common.c src/vbox/vbox_snapshot_conf.c src/vbox/vbox_tmpl.c src/vmware/vmware_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index c1e3f45..7a935e5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -674,6 +674,7 @@ VBOX_DRIVER_SOURCES = \ vbox/vbox_V4_2_20.c vbox/vbox_CAPI_v4_2_20.h \ vbox/vbox_V4_3.c vbox/vbox_CAPI_v4_3.h \ vbox/vbox_V4_3_4.c vbox/vbox_CAPI_v4_3_4.h \ + vbox/vbox_common.c vbox/vbox_common.h \ vbox/vbox_uniformed_api.h VBOX_DRIVER_EXTRA_DIST = \ diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c new file mode 100644 index 0000000..27211a0 --- /dev/null +++ b/src/vbox/vbox_common.c @@ -0,0 +1,150 @@ +/* + * Copyright 2014, Taowei Luo (uaedante(a)gmail.com <mailto:uaedante@gmail.com>) + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>__. + */ + +#include <config.h> + +#include <unistd.h> + +#include "internal.h" +#include "datatypes.h" +#include "domain_conf.h" +#include "domain_event.h" +#include "virlog.h" + +#include "vbox_common.h" +#include "vbox_uniformed_api.h" + +/* Common codes for vbox driver. With the definitions in vbox_common.h, + * it treats vbox structs as a void*. Though vboxUniformedAPI + * it call vbox functions. This file is a high level implement about + * the vbox driver. + */ + +#define VIR_FROM_THIS VIR_FROM_VBOX + +VIR_LOG_INIT("vbox.vbox___common"); + +#define RC_SUCCEEDED(rc) NS_SUCCEEDED(rc.resultCode) +#define RC_FAILED(rc) NS_FAILED(rc.resultCode) + +#define VBOX_RELEASE(arg) \ + do { \ + if (arg) { \ + pVBoxAPI->nsisupportsRelease((__void *)arg); \ + (arg) = NULL; \ + } \ + } while (0) + +#define VBOX_OBJECT_CHECK(conn, type, value) \ +vboxGlobalData *data = conn->privateData;\ +type ret = value;\ +if (!data->vboxObj) {\ + return ret;\ +} + +static vboxUniformedAPI *pVBoxAPI; + +void vboxRegisterUniformedAPI(__vboxUniformedAPI *vboxAPI) +{ + VIR_DEBUG("VirtualBox Uniformed API has been registered"); + pVBoxAPI = vboxAPI; +} + +int vboxInitialize(vboxGlobalData *data) +{ + if (pVBoxAPI->pfnInitialize(data) != 0) + goto cleanup; + + if (pVBoxAPI->__fWatchNeedInitialize && pVBoxAPI->initializeFWatch(__data) != 0) + goto cleanup; + + if (data->vboxObj == NULL) { + virReportError(VIR_ERR___INTERNAL_ERROR, "%s", + _("IVirtualBox object is null")); + goto cleanup; + } + + if (data->vboxSession == NULL) { + virReportError(VIR_ERR___INTERNAL_ERROR, "%s", + _("ISession object is null")); + goto cleanup; + } + + return 0; + + cleanup: + return -1; +} + +int vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED) +{ + VBOX_OBJECT_CHECK(dom->conn, int, -1); + IConsole *console = NULL; + vboxIIDUnion iid; + IMachine *machine = NULL; + nsresult rc; + + pVBoxAPI->initializeVboxIID(&__iid); + /* VirtualBox currently doesn't support saving to a file + * at a location other then the machine folder and thus + * setting path to ATTRIBUTE_UNUSED for now, will change + * this behaviour once get the VirtualBox API in right + * shape to do this + */ + This down here .. + /* Open a Session for the machine */ + pVBoxAPI->vboxIIDFromUUID(__data, &iid, dom->uuid); + if (pVBoxAPI->__getMachineForSession) { + /* Get machine for the call to VBOX_SESSION_OPEN_EXISTING */ + rc = pVBoxAPI->objectGetMachine(__data, &iid, &machine); + if (NS_FAILED(rc)) { + virReportError(VIR_ERR_NO___DOMAIN, "%s", + _("no domain with matching uuid")); + return -1; + } + } .. I guess is going to be used fairly frequently, so maybe it can be turned into a separate function. pVBoxAPI->vboxIIDFromUUID(__data, &iid, dom->uuid); rc = pVBoxAPI->objectGetMachine(__data, &iid, &machine); if (NS_FAILED(rc)) { virReportError(VIR_ERR_NO___DOMAIN, "%s", _("no domain with matching uuid")); return -1; } This part indeed be used frequently, but some places check the flag getMachineForSession while other places don't. So the prototype would be this: int openSessionForMachine(vboxGlobaldata *data, vboxIID *iid, unsigned char* dom_uuid, IMachine *machine, bool checkflag); Is this okay (or too complex)?

Meanwhile, When NS_FAILED(rc), some functions returned -1 (like this one), but some else goto the cleanup and unalloc the vboxIID, I perfer to make all NS_FAILED(rc) goto cleanup, what's your opinion?