[libvirt] [PATCH 00/11] vz: change vz driver to be stateful driver and other enhancements
There is no benefit in providing two ways of connecting to vz driver: by connecting via daemon and directly from client. Both ways finally come to a host where vz daemon sits. Always connecting via daemon allows us to have a single list of domains and share it among all connections. Maxim Nestratov (11): virsh: report when vz driver is compiled vz: change the order of capabilities reported vz: remove drivername field from vzConn structure vz: build driver as module and don't register it on client's side vz: pass vzConnPtr to prlsdkXxx functions instead of virConnectPtr vz: make vzConn structure be a new lockable object and use it vz: implement connectGetSysinfo hypervisor callback vz: remove close callback implementations vz: remove vzDriverLock/Unlock function vz: minor cleanup vz: change vzConnectIsAlive behavior daemon/Makefile.am | 4 + daemon/libvirtd.c | 9 + src/Makefile.am | 21 ++- src/libvirt.c | 7 - src/vz/vz_driver.c | 479 ++++++++++++++++++++++++++++++----------------------- src/vz/vz_sdk.c | 36 ++-- src/vz/vz_sdk.h | 6 +- src/vz/vz_utils.c | 18 +- src/vz/vz_utils.h | 15 +- tools/virsh.c | 3 + 10 files changed, 341 insertions(+), 257 deletions(-) -- 2.4.3
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- tools/virsh.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 57b4ff3..89a2113 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -580,6 +580,9 @@ virshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_OPENVZ vshPrint(ctl, " OpenVZ"); #endif +#ifdef WITH_VZ + vshPrint(ctl, " Virtuozzo"); +#endif #ifdef WITH_VMWARE vshPrint(ctl, " VMware"); #endif -- 2.4.3
'vz' goes first now to make clients like virt-manager choose 'vz' instead of 'parallels' Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index e12a95a..47a5060 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -109,10 +109,10 @@ vzBuildCapabilities(void) VIR_DOMAIN_OSTYPE_EXE }; virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 }; - const char *const emulators[] = { "parallels", "vz" }; + const char *const emulators[] = { "vz", "parallels"}; virDomainVirtType virt_types[] = { - VIR_DOMAIN_VIRT_PARALLELS, - VIR_DOMAIN_VIRT_VZ + VIR_DOMAIN_VIRT_VZ, + VIR_DOMAIN_VIRT_PARALLELS }; size_t i, j, k; -- 2.4.3
On 29.03.2016 15:45, Maxim Nestratov wrote:
'vz' goes first now to make clients like virt-manager choose 'vz' instead of 'parallels'
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
ACK
No need to remember connection name and have corresponding domain type to keep backward compatibility with former 'parallels' driver. It is enough to be able to accept 'parallels' uri and domain types. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 2 -- src/vz/vz_utils.c | 5 +---- src/vz/vz_utils.h | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 47a5060..9de88cd 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -228,8 +228,6 @@ vzOpenDefault(virConnectPtr conn) goto err_free; } - privconn->drivername = conn->driver->name; - if (prlsdkInit()) { VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); goto err_free; diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index fed48a5..64e469c 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -178,10 +178,7 @@ vzNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) pdom->cache.stats = PRL_INVALID_HANDLE; pdom->cache.count = -1; - if (STREQ(privconn->drivername, "vz")) - def->virtType = VIR_DOMAIN_VIRT_VZ; - else - def->virtType = VIR_DOMAIN_VIRT_PARALLELS; + def->virtType = VIR_DOMAIN_VIRT_VZ; if (!(dom = virDomainObjListAdd(privconn->domains, def, privconn->xmlopt, diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index f373850..b415b0f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -70,7 +70,6 @@ struct _vzConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; - const char *drivername; /* Immutable pointer, self-locking APIs */ virConnectCloseCallbackDataPtr closeCallback; unsigned long vzVersion; -- 2.4.3
Make it possible to build vz driver as a module and don't link it with libvirt.so statically. Remove registering it on client's side as far as we start relying on daemon Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- daemon/Makefile.am | 4 ++++ daemon/libvirtd.c | 9 +++++++++ src/Makefile.am | 21 ++++++++++++++++----- src/libvirt.c | 7 ------- src/vz/vz_driver.c | 27 +++++++++++++++++++++++++-- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 2dbe81b..78d7d21 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -233,6 +233,10 @@ if WITH_VBOX libvirtd_LDADD += ../src/libvirt_driver_vbox.la endif WITH_VBOX +if WITH_VZ + libvirtd_LDADD += ../src/libvirt_driver_vz.la +endif WITH_VZ + if WITH_STORAGE libvirtd_LDADD += ../src/libvirt_driver_storage.la endif WITH_STORAGE diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..92b4080 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -102,6 +102,9 @@ # include "nwfilter/nwfilter_driver.h" # endif #endif +#ifdef WITH_VZ +# include "vz/vz_driver.h" +#endif #include "configmake.h" @@ -390,6 +393,9 @@ static void daemonInitialize(void) # ifdef WITH_BHYVE virDriverLoadModule("bhyve"); # endif +# ifdef WITH_VZ + virDriverLoadModule("vz"); +# endif #else # ifdef WITH_NETWORK networkRegister(); @@ -430,6 +436,9 @@ static void daemonInitialize(void) # ifdef WITH_BHYVE bhyveRegister(); # endif +# ifdef WITH_VZ + vzRegister(); +# endif #endif } diff --git a/src/Makefile.am b/src/Makefile.am index dad7bab..b26be1b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -629,6 +629,7 @@ DRIVER_SOURCE_FILES = \ $(NULL) STATEFUL_DRIVER_SOURCE_FILES = \ + $(VZ_DRIVER_SOURCES) \ $(BHYVE_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(LIBXL_DRIVER_SOURCES) \ @@ -885,7 +886,9 @@ HYPERV_DRIVER_EXTRA_DIST = \ hyperv/hyperv_wmi_generator.py \ $(HYPERV_DRIVER_GENERATED) -VZ_DRIVER_SOURCES = \ +VZ_DRIVER_SOURCES = \ + datatypes.c \ + util/vircommand.c \ vz/vz_driver.h \ vz/vz_driver.c \ vz/vz_utils.c \ @@ -1478,13 +1481,21 @@ libvirt_driver_hyperv_la_SOURCES = $(HYPERV_DRIVER_SOURCES) endif WITH_HYPERV if WITH_VZ +noinst_LTLIBRARIES += libvirt_driver_vz_impl.la +libvirt_driver_vz_la_SOURCES = +libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_vz.la +libvirt_driver_vz_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_driver_vz.la -libvirt_la_BUILT_LIBADD += libvirt_driver_vz.la -libvirt_driver_vz_la_CFLAGS = \ +endif ! WITH_DRIVER_MODULES +libvirt_driver_vz_impl_la_CFLAGS = \ -I$(srcdir)/conf $(AM_CFLAGS) \ $(PARALLELS_SDK_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) -libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES) +libvirt_driver_vz_impl_la_SOURCES = $(VZ_DRIVER_SOURCES) +libvirt_driver_vz_impl_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) endif WITH_VZ if WITH_BHYVE diff --git a/src/libvirt.c b/src/libvirt.c index dd58e9c..a21d00e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -92,9 +92,6 @@ #ifdef WITH_XENAPI # include "xenapi/xenapi_driver.h" #endif -#ifdef WITH_VZ -# include "vz/vz_driver.h" -#endif #ifdef WITH_BHYVE # include "bhyve/bhyve_driver.h" #endif @@ -433,10 +430,6 @@ virGlobalInit(void) if (xenapiRegister() == -1) goto error; # endif -# ifdef WITH_VZ - if (vzRegister() == -1) - goto error; -# endif #endif #ifdef WITH_REMOTE if (remoteRegister() == -1) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 9de88cd..d3dcf3d 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1561,6 +1561,26 @@ static virConnectDriver vzConnectDriver = { .hypervisorDriver = &vzDriver, }; +static int +vzStateCleanup(void) +{ + return 0; +} + +static int +vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + return 0; +} + +static virStateDriver vzStateDriver = { + .name = "vz", + .stateInitialize = vzStateInitialize, + .stateCleanup = vzStateCleanup, +}; + /* Parallels domain type backward compatibility*/ static virHypervisorDriver parallelsDriver; static virConnectDriver parallelsConnectDriver; @@ -1588,10 +1608,13 @@ vzRegister(void) parallelsDriver.name = "Parallels"; parallelsConnectDriver = vzConnectDriver; parallelsConnectDriver.hypervisorDriver = ¶llelsDriver; - if (virRegisterConnectDriver(¶llelsConnectDriver, false) < 0) + if (virRegisterConnectDriver(¶llelsConnectDriver, true) < 0) + return -1; + + if (virRegisterConnectDriver(&vzConnectDriver, true) < 0) return -1; - if (virRegisterConnectDriver(&vzConnectDriver, false) < 0) + if (virRegisterStateDriver(&vzStateDriver) < 0) return -1; return 0; -- 2.4.3
On 29.03.2016 15:45, Maxim Nestratov wrote:
Make it possible to build vz driver as a module and don't link it with libvirt.so statically. Remove registering it on client's side as far as we start relying on daemon
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- daemon/Makefile.am | 4 ++++ daemon/libvirtd.c | 9 +++++++++ src/Makefile.am | 21 ++++++++++++++++----- src/libvirt.c | 7 ------- src/vz/vz_driver.c | 27 +++++++++++++++++++++++++-- 5 files changed, 54 insertions(+), 14 deletions(-)
...
diff --git a/src/Makefile.am b/src/Makefile.am index dad7bab..b26be1b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -629,6 +629,7 @@ DRIVER_SOURCE_FILES = \ $(NULL)
STATEFUL_DRIVER_SOURCE_FILES = \ + $(VZ_DRIVER_SOURCES) \ $(BHYVE_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(LIBXL_DRIVER_SOURCES) \ @@ -885,7 +886,9 @@ HYPERV_DRIVER_EXTRA_DIST = \ hyperv/hyperv_wmi_generator.py \ $(HYPERV_DRIVER_GENERATED)
-VZ_DRIVER_SOURCES = \ +VZ_DRIVER_SOURCES = \ + datatypes.c \ + util/vircommand.c \
looks like should not be here ...
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 9de88cd..d3dcf3d 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1561,6 +1561,26 @@ static virConnectDriver vzConnectDriver = { .hypervisorDriver = &vzDriver, };
+static int +vzStateCleanup(void) +{ + return 0; +} + +static int +vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + return 0; +} + +static virStateDriver vzStateDriver = { + .name = "vz", + .stateInitialize = vzStateInitialize, + .stateCleanup = vzStateCleanup, +}; + /* Parallels domain type backward compatibility*/ static virHypervisorDriver parallelsDriver; static virConnectDriver parallelsConnectDriver; @@ -1588,10 +1608,13 @@ vzRegister(void) parallelsDriver.name = "Parallels"; parallelsConnectDriver = vzConnectDriver; parallelsConnectDriver.hypervisorDriver = ¶llelsDriver; - if (virRegisterConnectDriver(¶llelsConnectDriver, false) < 0) + if (virRegisterConnectDriver(¶llelsConnectDriver, true) < 0) + return -1; + + if (virRegisterConnectDriver(&vzConnectDriver, true) < 0) return -1;
- if (virRegisterConnectDriver(&vzConnectDriver, false) < 0) + if (virRegisterStateDriver(&vzStateDriver) < 0) return -1;
return 0;
i would move this hunk to the patch that adds cleanup and init the essense. Nikolay
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 6 +++--- src/vz/vz_sdk.c | 25 +++++++++++-------------- src/vz/vz_sdk.h | 6 +++--- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index d3dcf3d..7de21d8 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -679,10 +679,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) if (!newdom) goto cleanup; if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - if (prlsdkCreateVm(conn, def)) + if (prlsdkCreateVm(privconn, def)) goto cleanup; } else if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { - if (prlsdkCreateCt(conn, def)) + if (prlsdkCreateCt(privconn, def)) goto cleanup; } else { virReportError(VIR_ERR_INVALID_ARG, @@ -717,7 +717,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } } else { - if (prlsdkApplyConfig(conn, olddom, def)) + if (prlsdkApplyConfig(privconn, olddom, def)) goto cleanup; if (prlsdkUpdateDomain(privconn, olddom)) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 629906e..541060a 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3600,7 +3600,7 @@ prlsdkSetBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def) } static int -prlsdkDoApplyConfig(virConnectPtr conn, +prlsdkDoApplyConfig(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDefPtr def, virDomainDefPtr olddef) @@ -3666,11 +3666,11 @@ prlsdkDoApplyConfig(virConnectPtr conn, if (olddef) { for (i = 0; i < olddef->nnets; i++) - prlsdkCleanupBridgedNet(conn->privateData, olddef->nets[i]); + prlsdkCleanupBridgedNet(privconn, olddef->nets[i]); } for (i = 0; i < def->nnets; i++) { - if (prlsdkAddNet(conn->privateData, sdkdom, def->nets[i], IS_CT(def)) < 0) + if (prlsdkAddNet(privconn, sdkdom, def->nets[i], IS_CT(def)) < 0) goto error; } @@ -3691,7 +3691,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, } for (i = 0; i < def->ndisks; i++) { - if (prlsdkAddDisk(conn->privateData, sdkdom, def->disks[i]) < 0) + if (prlsdkAddDisk(privconn, sdkdom, def->disks[i]) < 0) goto error; } @@ -3709,17 +3709,16 @@ prlsdkDoApplyConfig(virConnectPtr conn, VIR_FREE(mask); for (i = 0; i < def->nnets; i++) - prlsdkCleanupBridgedNet(conn->privateData, def->nets[i]); + prlsdkCleanupBridgedNet(privconn, def->nets[i]); return -1; } int -prlsdkApplyConfig(virConnectPtr conn, +prlsdkApplyConfig(vzConnPtr privconn, virDomainObjPtr dom, virDomainDefPtr new) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; int ret; @@ -3732,7 +3731,7 @@ prlsdkApplyConfig(virConnectPtr conn, if (PRL_FAILED(waitJob(job))) return -1; - ret = prlsdkDoApplyConfig(conn, sdkdom, new, dom->def); + ret = prlsdkDoApplyConfig(privconn, sdkdom, new, dom->def); if (ret == 0) { job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); @@ -3746,9 +3745,8 @@ prlsdkApplyConfig(virConnectPtr conn, } int -prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateVm(vzConnPtr privconn, virDomainDefPtr def) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; @@ -3772,7 +3770,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); prlsdkCheckRetGoto(pret, cleanup); - ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); + ret = prlsdkDoApplyConfig(privconn, sdkdom, def, NULL); if (ret) goto cleanup; @@ -3786,9 +3784,8 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) } int -prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateCt(vzConnPtr privconn, virDomainDefPtr def) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_GET_VM_CONFIG_PARAM_DATA confParam; PRL_HANDLE job = PRL_INVALID_HANDLE; @@ -3834,7 +3831,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) } - ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); + ret = prlsdkDoApplyConfig(privconn, sdkdom, def, NULL); if (ret) goto cleanup; diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 2f11d4f..a1b2e52 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -53,11 +53,11 @@ prlsdkDomainChangeStateLocked(vzConnPtr privconn, virDomainObjPtr dom, prlsdkChangeStateFunc chstate); int -prlsdkApplyConfig(virConnectPtr conn, +prlsdkApplyConfig(vzConnPtr privconn, virDomainObjPtr dom, virDomainDefPtr new); -int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def); -int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def); +int prlsdkCreateVm(vzConnPtr privconn, virDomainDefPtr def); +int prlsdkCreateCt(vzConnPtr privconn, virDomainDefPtr def); int prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags); int -- 2.4.3
On 29.03.2016 15:45, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 6 +++--- src/vz/vz_sdk.c | 25 +++++++++++-------------- src/vz/vz_sdk.h | 6 +++--- 3 files changed, 17 insertions(+), 20 deletions(-)
ACK
This patch introduces a new 'vzConn' lockable object and provides helper functions to allocate/destroy it. Now we store domain related objects such as domain list, capabitilies etc. within a single vz_driver vzConn structure, which is shared by all driver connections. It is allocated in a lazy manner in any function that is trying to acces it. When a connection to vz daemon drops, vzDestroyConnection is called, which prevents further usage of vz_driver until a new connection to it is established. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 329 ++++++++++++++++++++++++++++++++++------------------- src/vz/vz_sdk.c | 9 +- src/vz/vz_utils.c | 13 ++- src/vz/vz_utils.h | 9 +- 4 files changed, 234 insertions(+), 126 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 7de21d8..36030a7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -63,18 +63,24 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl" static int vzConnectClose(virConnectPtr conn); +static virClassPtr vzConnClass; void vzDriverLock(vzConnPtr driver) { - virMutexLock(&driver->lock); + virObjectLock(driver); } void vzDriverUnlock(vzConnPtr driver) { - virMutexUnlock(&driver->lock); + virObjectUnlock(driver); } +static virMutex vz_driver_lock; +static vzConnPtr vz_driver; + +static vzConnPtr +vzConnObjNew(void); static int vzCapsAddGuestDomain(virCapsPtr caps, @@ -158,15 +164,67 @@ vzBuildCapabilities(void) goto cleanup; } +static void vzConnDispose(void * obj) +{ + vzConnPtr conn = obj; + + if (conn->server) { + prlsdkUnsubscribeFromPCSEvents(conn); + prlsdkDisconnect(conn); + } + + virObjectUnref(conn->domains); + virObjectUnref(conn->caps); + virObjectUnref(conn->xmlopt); + virObjectEventStateFree(conn->domainEventState); +} + +static int vzConnOnceInit(void) +{ + if (!(vzConnClass = virClassNew(virClassForObjectLockable(), + "vzConn", + sizeof(vzConn), + vzConnDispose))) + return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(vzConn) + +vzConnPtr +vzGetConnection(void) +{ + virMutexLock(&vz_driver_lock); + if (!vz_driver) + vz_driver = vzConnObjNew(); + virObjectRef(vz_driver); + virMutexUnlock(&vz_driver_lock); + return vz_driver; +} + +void +vzDestroyConnection(void) +{ + vzConnPtr privconn; + virMutexLock(&vz_driver_lock); + privconn = vz_driver; + vz_driver = NULL; + virMutexUnlock(&vz_driver_lock); + virObjectUnref(privconn); +} + static char * -vzConnectGetCapabilities(virConnectPtr conn) +vzConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; char *xml; vzDriverLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); vzDriverUnlock(privconn); + virObjectUnref(privconn); return xml; } @@ -214,78 +272,42 @@ virDomainDefParserConfig vzDomainDefParserConfig = { .domainPostParseCallback = vzDomainDefPostParse, }; - -static int -vzOpenDefault(virConnectPtr conn) +static vzConnPtr +vzConnObjNew(void) { - vzConnPtr privconn; - - if (VIR_ALLOC(privconn) < 0) - return VIR_DRV_OPEN_ERROR; - if (virMutexInit(&privconn->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - goto err_free; - } - - if (prlsdkInit()) { - VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); - goto err_free; - } - - if (prlsdkConnect(privconn) < 0) - goto err_free; - - if (vzInitVersion(privconn) < 0) - goto error; - - if (!(privconn->caps = vzBuildCapabilities())) - goto error; + vzConnPtr conn; - vzDomainDefParserConfig.priv = &privconn->vzCaps; - if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, - NULL, NULL))) - goto error; - - if (!(privconn->domains = virDomainObjListNew())) - goto error; - - if (!(privconn->domainEventState = virObjectEventStateNew())) - goto error; - - if (prlsdkSubscribeToPCSEvents(privconn)) - goto error; - - if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) - goto error; - - conn->privateData = privconn; + if (vzConnInitialize() < 0) + return NULL; - if (prlsdkLoadDomains(privconn)) - goto error; + if (!(conn = virObjectLockableNew(vzConnClass))) + return NULL; - return VIR_DRV_OPEN_SUCCESS; + vzDomainDefParserConfig.priv = &conn->vzCaps; + + if (!(conn->caps = vzBuildCapabilities()) || + !(conn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, + NULL, NULL)) || + !(conn->domainEventState = virObjectEventStateNew()) || + !(conn->domains = virDomainObjListNew()) || + (vzInitVersion(conn) < 0) || + (prlsdkConnect(conn) < 0) || + (prlsdkSubscribeToPCSEvents(conn) < 0) || + (prlsdkLoadDomains(conn) < 0) + ) { + virObjectUnref(conn); + return NULL; + } - error: - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectUnref(privconn->domains); - virObjectUnref(privconn->caps); - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - prlsdkDeinit(); - err_free: - conn->privateData = NULL; - VIR_FREE(privconn); - return VIR_DRV_OPEN_ERROR; + return conn; } static virDrvOpenStatus -vzConnectOpen(virConnectPtr conn, +vzConnectOpen(virConnectPtr conn ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { - int ret; + vzConnPtr privconn = NULL; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -317,44 +339,33 @@ vzConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; } - if ((ret = vzOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS) - return ret; + if (!(privconn = vzGetConnection())) + return VIR_DRV_OPEN_ERROR; + virObjectUnref(privconn); return VIR_DRV_OPEN_SUCCESS; } static int -vzConnectClose(virConnectPtr conn) +vzConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; - + vzConnPtr privconn = vzGetConnection(); if (!privconn) return 0; - vzDriverLock(privconn); - prlsdkUnsubscribeFromPCSEvents(privconn); - virObjectUnref(privconn->caps); - virObjectUnref(privconn->xmlopt); - virObjectUnref(privconn->domains); - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - conn->privateData = NULL; - prlsdkDeinit(); - vzDriverUnlock(privconn); - virMutexDestroy(&privconn->lock); - - VIR_FREE(privconn); + virObjectUnref(privconn); return 0; } static int -vzConnectGetVersion(virConnectPtr conn, unsigned long *hvVer) +vzConnectGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) { - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; *hvVer = privconn->vzVersion; + virObjectUnref(privconn); return 0; } @@ -366,38 +377,48 @@ static char *vzConnectGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) static int -vzConnectListDomains(virConnectPtr conn, int *ids, int maxids) +vzConnectListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int *ids, int maxids) { - vzConnPtr privconn = conn->privateData; int n; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; vzDriverLock(privconn); n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, NULL, NULL); vzDriverUnlock(privconn); + virObjectUnref(privconn); return n; } static int -vzConnectNumOfDomains(virConnectPtr conn) +vzConnectNumOfDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; int count; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL); vzDriverUnlock(privconn); + virObjectUnref(privconn); return count; } static int -vzConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) +vzConnectListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, + char **const names, + int maxnames) { - vzConnPtr privconn = conn->privateData; int n; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; vzDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); @@ -405,20 +426,23 @@ vzConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxnames maxnames, NULL, NULL); vzDriverUnlock(privconn); + virObjectUnref(privconn); return n; } static int -vzConnectNumOfDefinedDomains(virConnectPtr conn) +vzConnectNumOfDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; int count; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, false, NULL, NULL); vzDriverUnlock(privconn); - + virObjectUnref(privconn); return count; } @@ -427,8 +451,10 @@ vzConnectListAllDomains(virConnectPtr conn, virDomainPtr **domains, unsigned int flags) { - vzConnPtr privconn = conn->privateData; int ret = -1; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); vzDriverLock(privconn); @@ -436,17 +462,21 @@ vzConnectListAllDomains(virConnectPtr conn, NULL, flags); vzDriverUnlock(privconn); + virObjectUnref(privconn); return ret; } static virDomainPtr vzDomainLookupByID(virConnectPtr conn, int id) { - vzConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; virDomainObjPtr dom; vzDriverLock(privconn); + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; + dom = virDomainObjListFindByID(privconn->domains, id); vzDriverUnlock(privconn); @@ -462,17 +492,21 @@ vzDomainLookupByID(virConnectPtr conn, int id) cleanup: if (dom) virObjectUnlock(dom); + virObjectUnref(privconn); return ret; } static virDomainPtr vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - vzConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; virDomainObjPtr dom; vzDriverLock(privconn); + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; + dom = virDomainObjListFindByUUID(privconn->domains, uuid); vzDriverUnlock(privconn); @@ -491,13 +525,16 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) cleanup: if (dom) virObjectUnlock(dom); + virObjectUnref(privconn); return ret; } static virDomainPtr vzDomainLookupByName(virConnectPtr conn, const char *name) { - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; virDomainPtr ret = NULL; virDomainObjPtr dom; @@ -517,6 +554,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name) cleanup: virDomainObjEndAPI(&dom); + virObjectUnref(privconn); return ret; } @@ -613,10 +651,12 @@ vzDomainGetState(virDomainPtr domain, static char * vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) { - vzConnPtr privconn = domain->conn->privateData; virDomainDefPtr def; virDomainObjPtr privdom; char *ret = NULL; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; /* Flags checked by virDomainDefFormat */ @@ -631,6 +671,7 @@ vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) cleanup: if (privdom) virObjectUnlock(privdom); + virObjectUnref(privconn); return ret; } @@ -655,13 +696,16 @@ vzDomainGetAutostart(virDomainPtr domain, int *autostart) static virDomainPtr vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) { - vzConnPtr privconn = conn->privateData; virDomainPtr retdom = NULL; virDomainDefPtr def; virDomainObjPtr olddom = NULL; virDomainObjPtr newdom = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) @@ -740,6 +784,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) } virDomainDefFree(def); vzDriverUnlock(privconn); + virObjectUnref(privconn); return retdom; } @@ -845,7 +890,7 @@ vzNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED, } static int -vzConnectDomainEventRegisterAny(virConnectPtr conn, +vzConnectDomainEventRegisterAny(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr domain, int eventID, virConnectDomainEventGenericCallback callback, @@ -853,21 +898,27 @@ vzConnectDomainEventRegisterAny(virConnectPtr conn, virFreeCallback freecb) { int ret = -1; - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; if (virDomainEventStateRegisterID(conn, privconn->domainEventState, domain, eventID, callback, opaque, freecb, &ret) < 0) ret = -1; + + virObjectUnref(privconn); return ret; } static int -vzConnectDomainEventDeregisterAny(virConnectPtr conn, +vzConnectDomainEventDeregisterAny(virConnectPtr conn ATTRIBUTE_UNUSED, int callbackID) { - vzConnPtr privconn = conn->privateData; int ret = -1; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; if (virObjectEventStateDeregisterID(conn, privconn->domainEventState, @@ -877,6 +928,7 @@ vzConnectDomainEventDeregisterAny(virConnectPtr conn, ret = 0; cleanup: + virObjectUnref(privconn); return ret; } @@ -939,20 +991,24 @@ static int vzDomainUndefineFlags(virDomainPtr domain, unsigned int flags) { - vzConnPtr privconn = domain->conn->privateData; virDomainObjPtr dom = NULL; - int ret; + int ret = -1; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); if (!(dom = vzDomObjFromDomain(domain))) - return -1; + goto cleanup; ret = prlsdkUnregisterDomain(privconn, dom, flags); if (ret) virObjectUnlock(dom); + cleanup: + virObjectUnref(privconn); return ret; } @@ -985,16 +1041,19 @@ vzDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) static int vzDomainManagedSave(virDomainPtr domain, unsigned int flags) { - vzConnPtr privconn = domain->conn->privateData; virDomainObjPtr dom = NULL; int state, reason; int ret = -1; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; + virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); if (!(dom = vzDomObjFromDomain(domain))) - return -1; + goto cleanup; state = virDomainObjGetState(dom, &reason); @@ -1007,7 +1066,9 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags) ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkSuspend); cleanup: - virObjectUnlock(dom); + if (dom) + virObjectUnlock(dom); + virObjectUnref(privconn); return ret; } @@ -1039,16 +1100,19 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { int ret = -1; - vzConnPtr privconn = dom->conn->privateData; virDomainDeviceDefPtr dev = NULL; virDomainObjPtr privdom = NULL; bool domactive = false; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(privdom = vzDomObjFromDomain(dom))) - return -1; + goto cleanup; if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1100,8 +1164,11 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, } ret = 0; + cleanup: - virObjectUnlock(privdom); + if (privdom) + virObjectUnlock(privdom); + virObjectUnref(privconn); return ret; } @@ -1115,17 +1182,19 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { int ret = -1; - vzConnPtr privconn = dom->conn->privateData; virDomainDeviceDefPtr dev = NULL; virDomainObjPtr privdom = NULL; bool domactive = false; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); privdom = vzDomObjFromDomain(dom); if (privdom == NULL) - return -1; + goto cleanup; if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1178,7 +1247,9 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, ret = 0; cleanup: - virObjectUnlock(privdom); + if (privdom) + virObjectUnlock(privdom); + virObjectUnref(privconn); return ret; } @@ -1564,6 +1635,10 @@ static virConnectDriver vzConnectDriver = { static int vzStateCleanup(void) { + prlsdkDeinit(); + virObjectUnref(vz_driver); + vz_driver = NULL; + virMutexDestroy(&vz_driver_lock); return 0; } @@ -1572,7 +1647,25 @@ vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + if (!privileged) { + VIR_INFO("Not running privileged, disabling driver"); + return 0; + } + + if (prlsdkInit() < 0) { + VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); + return -1; + } + + if (virMutexInit(&vz_driver_lock) < 0) + goto error; + + vz_driver = vzConnObjNew(); return 0; + + error: + vzStateCleanup(); + return -1; } static virStateDriver vzStateDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 541060a..e6f6129 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1958,6 +1958,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) case PET_DSP_EVT_DISP_CONNECTION_CLOSED: virConnectCloseCallbackDataCall(privconn->closeCallback, VIR_CONNECT_CLOSE_REASON_EOF); + vzDestroyConnection(); break; default: VIR_DEBUG("Skipping event of type %d", prlEventType); @@ -2084,15 +2085,19 @@ int prlsdkDomainChangeState(virDomainPtr domain, prlsdkChangeStateFunc chstate) { - vzConnPtr privconn = domain->conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; virDomainObjPtr dom; int ret = -1; if (!(dom = vzDomObjFromDomain(domain))) - return -1; + goto cleanup; ret = prlsdkDomainChangeStateLocked(privconn, dom, chstate); virObjectUnlock(dom); + cleanup: + virObjectUnref(privconn); return ret; } diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 64e469c..f9bcbd2 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -71,8 +71,10 @@ virDomainObjPtr vzDomObjFromDomain(virDomainPtr domain) { virDomainObjPtr vm; - vzConnPtr privconn = domain->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid); if (!vm) { @@ -80,11 +82,10 @@ vzDomObjFromDomain(virDomainPtr domain) virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s' (%s)"), uuidstr, domain->name); - return NULL; } + virObjectUnref(privconn); return vm; - } /** @@ -101,8 +102,10 @@ virDomainObjPtr vzDomObjFromDomainRef(virDomainPtr domain) { virDomainObjPtr vm; - vzConnPtr privconn = domain->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid); if (!vm) { @@ -110,9 +113,9 @@ vzDomObjFromDomainRef(virDomainPtr domain) virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s' (%s)"), uuidstr, domain->name); - return NULL; } + virObjectUnref(privconn); return vm; } diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index b415b0f..00e795f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -61,7 +61,7 @@ typedef struct _vzCapabilities vzCapabilities; typedef struct _vzCapabilities *vzCapabilitiesPtr; struct _vzConn { - virMutex lock; + virObjectLockable parent; /* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; @@ -105,6 +105,13 @@ char * vzGetOutput(const char *binary, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; void vzDriverLock(vzConnPtr driver); void vzDriverUnlock(vzConnPtr driver); + +vzConnPtr +vzGetConnection(void); + +void +vzDestroyConnection(void); + virDomainObjPtr vzNewDomain(vzConnPtr privconn, char *name, -- 2.4.3
On 29.03.2016 15:45, Maxim Nestratov wrote:
This patch introduces a new 'vzConn' lockable object and provides helper functions to allocate/destroy it. Now we store domain related objects such as domain list, capabitilies etc. within a single vz_driver vzConn structure, which is shared by all driver connections. It is allocated in a lazy manner in any function that is trying to acces it. When a connection to vz daemon drops, vzDestroyConnection is called, which prevents further usage of vz_driver until a new connection to it is established.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 329 ++++++++++++++++++++++++++++++++++------------------- src/vz/vz_sdk.c | 9 +- src/vz/vz_utils.c | 13 ++- src/vz/vz_utils.h | 9 +- 4 files changed, 234 insertions(+), 126 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 7de21d8..36030a7 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -63,18 +63,24 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl"
static int vzConnectClose(virConnectPtr conn); +static virClassPtr vzConnClass;
void vzDriverLock(vzConnPtr driver) { - virMutexLock(&driver->lock); + virObjectLock(driver); }
void vzDriverUnlock(vzConnPtr driver) { - virMutexUnlock(&driver->lock); + virObjectUnlock(driver); } +static virMutex vz_driver_lock; +static vzConnPtr vz_driver; + +static vzConnPtr +vzConnObjNew(void);
static int vzCapsAddGuestDomain(virCapsPtr caps, @@ -158,15 +164,67 @@ vzBuildCapabilities(void) goto cleanup; }
+static void vzConnDispose(void * obj) +{ + vzConnPtr conn = obj; + + if (conn->server) { + prlsdkUnsubscribeFromPCSEvents(conn); + prlsdkDisconnect(conn); + } + + virObjectUnref(conn->domains); + virObjectUnref(conn->caps); + virObjectUnref(conn->xmlopt); + virObjectEventStateFree(conn->domainEventState); +}
i've noticed that you removed support for close callback later but here it looks like you miss it. I suggest you move drop close callback patch before this one or keep it here nicely. Also let's stick with privconn if we deal with vzConnPtr and we are in vz_driver.c
+ +static int vzConnOnceInit(void) +{ + if (!(vzConnClass = virClassNew(virClassForObjectLockable(), + "vzConn", + sizeof(vzConn), + vzConnDispose))) + return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(vzConn) + +vzConnPtr +vzGetConnection(void) +{ + virMutexLock(&vz_driver_lock); + if (!vz_driver) + vz_driver = vzConnObjNew(); + virObjectRef(vz_driver); + virMutexUnlock(&vz_driver_lock); + return vz_driver; +} + +void +vzDestroyConnection(void) +{ + vzConnPtr privconn; + virMutexLock(&vz_driver_lock); + privconn = vz_driver; + vz_driver = NULL; + virMutexUnlock(&vz_driver_lock); + virObjectUnref(privconn); +} + static char * -vzConnectGetCapabilities(virConnectPtr conn) +vzConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; char *xml;
vzDriverLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); vzDriverUnlock(privconn); + virObjectUnref(privconn); return xml; }
@@ -214,78 +272,42 @@ virDomainDefParserConfig vzDomainDefParserConfig = { .domainPostParseCallback = vzDomainDefPostParse, };
- -static int -vzOpenDefault(virConnectPtr conn) +static vzConnPtr +vzConnObjNew(void) { - vzConnPtr privconn; - - if (VIR_ALLOC(privconn) < 0) - return VIR_DRV_OPEN_ERROR; - if (virMutexInit(&privconn->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - goto err_free; - } - - if (prlsdkInit()) { - VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); - goto err_free; - } - - if (prlsdkConnect(privconn) < 0) - goto err_free; - - if (vzInitVersion(privconn) < 0) - goto error; - - if (!(privconn->caps = vzBuildCapabilities())) - goto error; + vzConnPtr conn;
- vzDomainDefParserConfig.priv = &privconn->vzCaps; - if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, - NULL, NULL))) - goto error; - - if (!(privconn->domains = virDomainObjListNew())) - goto error; - - if (!(privconn->domainEventState = virObjectEventStateNew())) - goto error; - - if (prlsdkSubscribeToPCSEvents(privconn)) - goto error; - - if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) - goto error; - - conn->privateData = privconn; + if (vzConnInitialize() < 0) + return NULL;
- if (prlsdkLoadDomains(privconn)) - goto error; + if (!(conn = virObjectLockableNew(vzConnClass))) + return NULL;
- return VIR_DRV_OPEN_SUCCESS; + vzDomainDefParserConfig.priv = &conn->vzCaps; + + if (!(conn->caps = vzBuildCapabilities()) || + !(conn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, + NULL, NULL)) || + !(conn->domainEventState = virObjectEventStateNew()) || + !(conn->domains = virDomainObjListNew()) || + (vzInitVersion(conn) < 0) || + (prlsdkConnect(conn) < 0) || + (prlsdkSubscribeToPCSEvents(conn) < 0) || + (prlsdkLoadDomains(conn) < 0) + ) {
I'd better move vzInitVersion, prlsdkConnect etc out of new. Looks like it is typical for constructors in libvirt not to have side effects.
+ virObjectUnref(conn); + return NULL; + }
Let's leave previous code structure alone here. This will make this patch more comprehensible. We can move from bunch of 'if's to long || sequence in different patch if we want to.
- error: - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectUnref(privconn->domains); - virObjectUnref(privconn->caps); - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - prlsdkDeinit(); - err_free: - conn->privateData = NULL; - VIR_FREE(privconn); - return VIR_DRV_OPEN_ERROR; + return conn; }
static virDrvOpenStatus -vzConnectOpen(virConnectPtr conn, +vzConnectOpen(virConnectPtr conn ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { - int ret; + vzConnPtr privconn = NULL;
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
@@ -317,44 +339,33 @@ vzConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; }
- if ((ret = vzOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS) - return ret; + if (!(privconn = vzGetConnection())) + return VIR_DRV_OPEN_ERROR;
+ virObjectUnref(privconn); return VIR_DRV_OPEN_SUCCESS; }
static int -vzConnectClose(virConnectPtr conn) +vzConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED) { - vzConnPtr privconn = conn->privateData; - + vzConnPtr privconn = vzGetConnection(); if (!privconn) return 0;
- vzDriverLock(privconn); - prlsdkUnsubscribeFromPCSEvents(privconn); - virObjectUnref(privconn->caps); - virObjectUnref(privconn->xmlopt); - virObjectUnref(privconn->domains); - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - conn->privateData = NULL; - prlsdkDeinit();
- vzDriverUnlock(privconn); - virMutexDestroy(&privconn->lock); - - VIR_FREE(privconn); + virObjectUnref(privconn); return 0; }
static int -vzConnectGetVersion(virConnectPtr conn, unsigned long *hvVer) +vzConnectGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer) { - vzConnPtr privconn = conn->privateData; + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return -1; *hvVer = privconn->vzVersion; + virObjectUnref(privconn); return 0; }
I find it too strange that privcon can change from call to call while we stay with same conn. Moreover it just won't work. privconn holds state like domainEventState for example and if connection to VZ is broken we loose all subscriptions without any notice from client. (taking into account that later you remove close callback implementation). ...
@@ -1564,6 +1635,10 @@ static virConnectDriver vzConnectDriver = { static int vzStateCleanup(void) { + prlsdkDeinit(); + virObjectUnref(vz_driver); + vz_driver = NULL; + virMutexDestroy(&vz_driver_lock); return 0; }
@@ -1572,7 +1647,25 @@ vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + if (!privileged) { + VIR_INFO("Not running privileged, disabling driver"); + return 0; + }
Is it correct? Driver init result is ok so someone could call it's methods... ...
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index b415b0f..00e795f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -61,7 +61,7 @@ typedef struct _vzCapabilities vzCapabilities; typedef struct _vzCapabilities *vzCapabilitiesPtr;
struct _vzConn { - virMutex lock; + virObjectLockable parent;
/* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; @@ -105,6 +105,13 @@ char * vzGetOutput(const char *binary, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; void vzDriverLock(vzConnPtr driver); void vzDriverUnlock(vzConnPtr driver); + +vzConnPtr +vzGetConnection(void); + +void +vzDestroyConnection(void); +
odd code
virDomainObjPtr vzNewDomain(vzConnPtr privconn, char *name,
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 26 ++++++++++++++++++++++++++ src/vz/vz_utils.h | 1 + 2 files changed, 27 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 36030a7..4c30305 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -176,6 +176,7 @@ static void vzConnDispose(void * obj) virObjectUnref(conn->domains); virObjectUnref(conn->caps); virObjectUnref(conn->xmlopt); + virSysinfoDefFree(conn->hostsysinfo); virObjectEventStateFree(conn->domainEventState); } @@ -299,6 +300,7 @@ vzConnObjNew(void) return NULL; } + conn->hostsysinfo = virSysinfoRead(); return conn; } @@ -375,6 +377,29 @@ static char *vzConnectGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) return virGetHostname(); } +static char * +vzConnectGetSysinfo(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) +{ + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virCheckFlags(0, NULL); + + if (!privconn->hostsysinfo) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Host SMBIOS information is not available")); + return NULL; + } + + if (virSysinfoFormat(&buf, privconn->hostsysinfo) < 0) + return NULL; + if (virBufferCheckError(&buf) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} static int vzConnectListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int *ids, int maxids) @@ -1568,6 +1593,7 @@ static virHypervisorDriver vzDriver = { .connectClose = vzConnectClose, /* 0.10.0 */ .connectGetVersion = vzConnectGetVersion, /* 0.10.0 */ .connectGetHostname = vzConnectGetHostname, /* 0.10.0 */ + .connectGetSysinfo = vzConnectGetSysinfo, /* 1.3.3 */ .connectGetMaxVcpus = vzConnectGetMaxVcpus, /* 1.2.21 */ .nodeGetInfo = vzNodeGetInfo, /* 0.10.0 */ .nodeGetCPUStats = vzNodeGetCPUStats, /* 1.2.21 */ diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 00e795f..a2b86c7 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -72,6 +72,7 @@ struct _vzConn { virObjectEventStatePtr domainEventState; /* Immutable pointer, self-locking APIs */ virConnectCloseCallbackDataPtr closeCallback; + virSysinfoDefPtr hostsysinfo; unsigned long vzVersion; vzCapabilities vzCaps; }; -- 2.4.3
These methods are not necessary because vz becomes a stateful driver and clients always work via daemon Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 52 ---------------------------------------------------- src/vz/vz_sdk.c | 2 -- src/vz/vz_utils.h | 2 -- 3 files changed, 56 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 4c30305..42b0c02 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1537,56 +1537,6 @@ vzNodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) return freeMem; } -static int -vzConnectRegisterCloseCallback(virConnectPtr conn, - virConnectCloseFunc cb, - void *opaque, - virFreeCallback freecb) -{ - vzConnPtr privconn = conn->privateData; - int ret = -1; - - vzDriverLock(privconn); - - if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A close callback is already registered")); - goto cleanup; - } - - virConnectCloseCallbackDataRegister(privconn->closeCallback, conn, cb, - opaque, freecb); - ret = 0; - - cleanup: - vzDriverUnlock(privconn); - - return ret; -} - -static int -vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) -{ - vzConnPtr privconn = conn->privateData; - int ret = -1; - - vzDriverLock(privconn); - - if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A different callback was requested")); - goto cleanup; - } - - virConnectCloseCallbackDataUnregister(privconn->closeCallback, cb); - ret = 0; - - cleanup: - vzDriverUnlock(privconn); - - return ret; -} - static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1650,8 +1600,6 @@ static virHypervisorDriver vzDriver = { .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ - .connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.2 */ - .connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.2 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index e6f6129..59bd89a 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1956,8 +1956,6 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) prlEvent = PRL_INVALID_HANDLE; break; case PET_DSP_EVT_DISP_CONNECTION_CLOSED: - virConnectCloseCallbackDataCall(privconn->closeCallback, - VIR_CONNECT_CLOSE_REASON_EOF); vzDestroyConnection(); break; default: diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index a2b86c7..406ddd9 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -70,8 +70,6 @@ struct _vzConn { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; - /* Immutable pointer, self-locking APIs */ - virConnectCloseCallbackDataPtr closeCallback; virSysinfoDefPtr hostsysinfo; unsigned long vzVersion; vzCapabilities vzCaps; -- 2.4.3
We don't need them anymore as all pointers within vzConn structure are not changed during the time it exists. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 31 ------------------------------- src/vz/vz_utils.h | 2 -- 2 files changed, 33 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 42b0c02..b069671 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -65,17 +65,6 @@ VIR_LOG_INIT("parallels.parallels_driver"); static int vzConnectClose(virConnectPtr conn); static virClassPtr vzConnClass; -void -vzDriverLock(vzConnPtr driver) -{ - virObjectLock(driver); -} - -void -vzDriverUnlock(vzConnPtr driver) -{ - virObjectUnlock(driver); -} static virMutex vz_driver_lock; static vzConnPtr vz_driver; @@ -222,9 +211,7 @@ vzConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) return NULL; char *xml; - vzDriverLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); - vzDriverUnlock(privconn); virObjectUnref(privconn); return xml; } @@ -409,10 +396,8 @@ vzConnectListDomains(virConnectPtr conn ATTRIBUTE_UNUSED, int *ids, int maxids) if (!privconn) return -1; - vzDriverLock(privconn); n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, NULL, NULL); - vzDriverUnlock(privconn); virObjectUnref(privconn); return n; @@ -426,10 +411,8 @@ vzConnectNumOfDomains(virConnectPtr conn ATTRIBUTE_UNUSED) if (!privconn) return -1; - vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL); - vzDriverUnlock(privconn); virObjectUnref(privconn); return count; @@ -445,11 +428,9 @@ vzConnectListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, if (!privconn) return -1; - vzDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); n = virDomainObjListGetInactiveNames(privconn->domains, names, maxnames, NULL, NULL); - vzDriverUnlock(privconn); virObjectUnref(privconn); return n; @@ -463,10 +444,8 @@ vzConnectNumOfDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED) if (!privconn) return -1; - vzDriverLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, false, NULL, NULL); - vzDriverUnlock(privconn); virObjectUnref(privconn); return count; } @@ -482,10 +461,8 @@ vzConnectListAllDomains(virConnectPtr conn, return -1; virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); - vzDriverLock(privconn); ret = virDomainObjListExport(privconn->domains, conn, domains, NULL, flags); - vzDriverUnlock(privconn); virObjectUnref(privconn); return ret; @@ -497,13 +474,11 @@ vzDomainLookupByID(virConnectPtr conn, int id) virDomainPtr ret = NULL; virDomainObjPtr dom; - vzDriverLock(privconn); vzConnPtr privconn = vzGetConnection(); if (!privconn) return NULL; dom = virDomainObjListFindByID(privconn->domains, id); - vzDriverUnlock(privconn); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, NULL); @@ -527,13 +502,11 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainPtr ret = NULL; virDomainObjPtr dom; - vzDriverLock(privconn); vzConnPtr privconn = vzGetConnection(); if (!privconn) return NULL; dom = virDomainObjListFindByUUID(privconn->domains, uuid); - vzDriverUnlock(privconn); if (dom == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -563,9 +536,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name) virDomainPtr ret = NULL; virDomainObjPtr dom; - vzDriverLock(privconn); dom = virDomainObjListFindByName(privconn->domains, name); - vzDriverUnlock(privconn); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, @@ -736,7 +707,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) if (flags & VIR_DOMAIN_DEFINE_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; - vzDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, parse_flags)) == NULL) goto cleanup; @@ -808,7 +778,6 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virObjectUnlock(newdom); } virDomainDefFree(def); - vzDriverUnlock(privconn); virObjectUnref(privconn); return retdom; } diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 406ddd9..73b493f 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -102,8 +102,6 @@ virDomainObjPtr vzDomObjFromDomainRef(virDomainPtr domain); char * vzGetOutput(const char *binary, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; -void vzDriverLock(vzConnPtr driver); -void vzDriverUnlock(vzConnPtr driver); vzConnPtr vzGetConnection(void); -- 2.4.3
remove unnecessary vzConnectClose prototype and make local structure vzDomainDefParserConfig be static Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b069671..83bdabe 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -62,7 +62,6 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl" -static int vzConnectClose(virConnectPtr conn); static virClassPtr vzConnClass; static virMutex vz_driver_lock; @@ -254,7 +253,7 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } -virDomainDefParserConfig vzDomainDefParserConfig = { +static virDomainDefParserConfig vzDomainDefParserConfig = { .macPrefix = {0x42, 0x1C, 0x00}, .devicesPostParseCallback = vzDomainDeviceDefPostParse, .domainPostParseCallback = vzDomainDefPostParse, -- 2.4.3
Now it detects if a connection to vz dispatcher is up or not Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 83bdabe..8abd49f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -809,6 +809,11 @@ static int vzConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) static int vzConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) { + vzConnPtr privconn = vzGetConnection(); + if (!privconn) + return 0; + + virObjectUnref(privconn); return 1; } -- 2.4.3
participants (2)
-
Maxim Nestratov -
Nikolay Shirokovskiy