[libvirt] [PATCHv4 3/4]vbox: Use vboxUniformedAPI to write common code
Michal Privoznik
mprivozn at redhat.com
Wed Jul 2 14:07:32 UTC 2014
On 02.07.2014 15:45, Taowei Luo wrote:
>
>
>
> 2014-07-02 17:12 GMT+08:00 Michal Privoznik <mprivozn at redhat.com
> <mailto:mprivozn at 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 at gmail.com
> <mailto:uaedante at 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)?
Looks good to me.
>
> 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?
Well, I don't think that's a good idea, since functions can have
different labels. If you want to do this, then I think:
NS_FAILED_GOTO(rc, cleanup);
is better idea.
Michal
More information about the libvir-list
mailing list