[libvirt] [PATCH v4 0/7] admin: Introduce server listing API
Since v3: - refactors to virHashForEach and hash iterators - servers are now stored in a hash table instead of an array - added forgotten virObjectLock in remoteAdminConnectListServers Erik Skultety (7): util: Add a return value to void hash iterators util: Refactor virHashForEach so it returns as soon as an iterator fails virnetdaemon: Store servers in a hash table admin: Move admin_server.{h,c} to admin.{h,c} admin: Introduce virAdmServer structure admin: Introduce adminDaemonConnectListServers API virt-admin: Introduce cmdSrvList daemon/Makefile.am | 5 +- daemon/admin.c | 181 +++++++++++++++ daemon/admin.h | 36 +++ daemon/admin_server.c | 111 +++------- daemon/admin_server.h | 26 +-- daemon/libvirtd.c | 6 +- include/libvirt/libvirt-admin.h | 25 ++- po/POTFILES.in | 3 +- src/admin/admin_protocol.x | 26 ++- src/admin/admin_remote.c | 71 ++++++ src/admin_protocol-structs | 15 ++ src/conf/network_conf.c | 15 +- src/conf/nwfilter_params.c | 8 +- src/conf/snapshot_conf.c | 31 +-- src/conf/virchrdev.c | 7 +- src/conf/virdomainobjlist.c | 17 +- src/datatypes.c | 35 +++ src/datatypes.h | 38 ++++ src/libvirt-admin.c | 79 +++++++ src/libvirt_admin_private.syms | 5 + src/libvirt_admin_public.syms | 3 + src/locking/lock_daemon.c | 10 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_controller.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 3 +- src/nwfilter/nwfilter_gentech_driver.c | 5 +- src/qemu/qemu_domain.c | 7 +- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 13 +- src/rpc/virnetdaemon.c | 225 +++++++++++++------ src/rpc/virnetdaemon.h | 8 +- src/test/test_driver.c | 14 +- src/uml/uml_driver.c | 13 +- src/util/virclosecallbacks.c | 9 +- src/util/virhash.c | 28 ++- src/util/virhash.h | 6 +- src/xen/xm_internal.c | 7 +- tests/virhashtest.c | 54 +++-- .../input-data-admin-server-names.json | 129 +++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 8 +- .../output-data-admin-server-names.json | 126 +++++++++++ .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 242 +++++++++++---------- tests/virnetdaemontest.c | 63 +++--- tools/virt-admin.c | 61 ++++++ 47 files changed, 1361 insertions(+), 442 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h create mode 100644 tests/virnetdaemondata/input-data-admin-server-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-server-names.json -- 2.4.3
Our existing virHashForEach method iterates through all items disregarding the fact, that some of the iterators might have actually failed. Errors are usually dispatched through an error element in opaque data which then causes the original caller of virHashForEach to return -1. In that case, virHashForEach could return as soon as one of the iterators fail. This patch changes the iterator return type and adjusts all of its instances accordingly, so the actual refactor of virHashForEach method can be dealt with later. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/network_conf.c | 15 +++++++++------ src/conf/nwfilter_params.c | 8 +++++--- src/conf/snapshot_conf.c | 31 +++++++++++++++++-------------- src/conf/virchrdev.c | 7 ++++--- src/conf/virdomainobjlist.c | 17 +++++++++++------ src/locking/lock_daemon.c | 3 ++- src/nwfilter/nwfilter_dhcpsnoop.c | 3 ++- src/nwfilter/nwfilter_gentech_driver.c | 5 +++-- src/qemu/qemu_domain.c | 7 ++++--- src/qemu/qemu_domain.h | 6 +++--- src/qemu/qemu_driver.c | 13 ++++++++----- src/test/test_driver.c | 14 ++++++++------ src/uml/uml_driver.c | 13 +++++++------ src/util/virclosecallbacks.c | 9 +++++---- src/util/virhash.c | 5 +++-- src/util/virhash.h | 4 +++- src/xen/xm_internal.c | 7 ++++--- tests/virhashtest.c | 19 ++++++++++++------- 18 files changed, 110 insertions(+), 76 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 0ffb325..6300178 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -4399,7 +4399,7 @@ struct virNetworkObjListData { bool error; }; -static void +static int virNetworkObjListPopulate(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -4409,7 +4409,7 @@ virNetworkObjListPopulate(void *payload, virNetworkPtr net = NULL; if (data->error) - return; + return 0; virObjectLock(obj); @@ -4434,6 +4434,7 @@ virNetworkObjListPopulate(void *payload, cleanup: virObjectUnlock(obj); + return 0; } int @@ -4478,7 +4479,7 @@ struct virNetworkObjListForEachHelperData { int ret; }; -static void +static int virNetworkObjListForEachHelper(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -4487,6 +4488,7 @@ virNetworkObjListForEachHelper(void *payload, if (data->callback(payload, data->opaque) < 0) data->ret = -1; + return 0; } /** @@ -4524,7 +4526,7 @@ struct virNetworkObjListGetHelperData { bool error; }; -static void +static int virNetworkObjListGetHelper(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -4533,11 +4535,11 @@ virNetworkObjListGetHelper(void *payload, virNetworkObjPtr obj = payload; if (data->error) - return; + return 0; if (data->nnames >= 0 && data->got == data->nnames) - return; + return 0; virObjectLock(obj); @@ -4557,6 +4559,7 @@ virNetworkObjListGetHelper(void *payload, cleanup: virObjectUnlock(obj); + return 0; } int diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index b6c6e78..496ffda 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -709,19 +709,19 @@ struct addToTableStruct { }; -static void +static int addToTable(void *payload, const void *name, void *data) { struct addToTableStruct *atts = (struct addToTableStruct *)data; virNWFilterVarValuePtr val; if (atts->errOccurred) - return; + return 0; val = virNWFilterVarValueCopy((virNWFilterVarValuePtr)payload); if (!val) { atts->errOccurred = 1; - return; + return 0; } if (virNWFilterHashTablePut(atts->target, (const char *)name, val) < 0) { @@ -731,6 +731,8 @@ addToTable(void *payload, const void *name, void *data) atts->errOccurred = 1; virNWFilterVarValueFree(val); } + + return 0; } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 58646bd..c7794e9 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -844,48 +844,49 @@ struct virDomainSnapshotNameData { bool error; }; -static void virDomainSnapshotObjListCopyNames(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *opaque) +static int virDomainSnapshotObjListCopyNames(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) { virDomainSnapshotObjPtr obj = payload; struct virDomainSnapshotNameData *data = opaque; if (data->error) - return; + return 0; /* Caller already sanitized flags. Filtering on DESCENDANTS was * done by choice of iteration in the caller. */ if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren) - return; + return 0; if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren) - return; + return 0; if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) { if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) && obj->def->state == VIR_DOMAIN_SHUTOFF) - return; + return 0; if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && obj->def->state == VIR_DOMAIN_DISK_SNAPSHOT) - return; + return 0; if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) && obj->def->state != VIR_DOMAIN_SHUTOFF && obj->def->state != VIR_DOMAIN_DISK_SNAPSHOT) - return; + return 0; } if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL) && virDomainSnapshotIsExternal(obj)) - return; + return 0; if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) && !virDomainSnapshotIsExternal(obj)) - return; + return 0; if (data->names && data->count < data->maxnames && VIR_STRDUP(data->names[data->count], obj->def->name) < 0) { data->error = true; - return; + return 0; } data->count++; + return 0; } int @@ -1012,7 +1013,7 @@ struct snapshot_act_on_descendant { void *data; }; -static void +static int virDomainSnapshotActOnDescendant(void *payload, const void *name, void *data) @@ -1024,6 +1025,7 @@ virDomainSnapshotActOnDescendant(void *payload, curr->iter, curr->data); (curr->iter)(payload, name, curr->data); + return 0; } /* Run iter(data) on all descendants of snapshot, while ignoring all @@ -1055,7 +1057,7 @@ struct snapshot_set_relation { virDomainSnapshotObjListPtr snapshots; int err; }; -static void +static int virDomainSnapshotSetRelations(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) @@ -1085,6 +1087,7 @@ virDomainSnapshotSetRelations(void *payload, obj->parent->nchildren++; obj->sibling = obj->parent->first_child; obj->parent->first_child = obj; + return 0; } /* Populate parent link and child count of all snapshots, with all diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 701b326..bca9c37 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -292,13 +292,14 @@ virChrdevsPtr virChrdevAlloc(void) /** * Helper to clear stream callbacks when freeing the hash */ -static void virChrdevFreeClearCallbacks(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) +static int virChrdevFreeClearCallbacks(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) { virStreamPtr st = payload; virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL); + return 0; } /** diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 61fe468..05c532a 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -607,7 +607,7 @@ struct virDomainObjListData { }; -static void +static int virDomainObjListCount(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -627,6 +627,7 @@ virDomainObjListCount(void *payload, } cleanup: virObjectUnlock(obj); + return 0; } @@ -653,7 +654,7 @@ struct virDomainIDData { }; -static void +static int virDomainObjListCopyActiveIDs(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -668,6 +669,7 @@ virDomainObjListCopyActiveIDs(void *payload, data->ids[data->numids++] = obj->def->id; cleanup: virObjectUnlock(obj); + return 0; } @@ -697,7 +699,7 @@ struct virDomainNameData { }; -static void +static int virDomainObjListCopyInactiveNames(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -706,7 +708,7 @@ virDomainObjListCopyInactiveNames(void *payload, struct virDomainNameData *data = opaque; if (data->oom) - return; + return 0; virObjectLock(obj); if (data->filter && @@ -720,6 +722,7 @@ virDomainObjListCopyInactiveNames(void *payload, } cleanup: virObjectUnlock(obj); + return 0; } @@ -753,7 +756,7 @@ struct virDomainListIterData { }; -static void +static int virDomainObjListHelper(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -762,6 +765,7 @@ virDomainObjListHelper(void *payload, if (data->callback(payload, data->opaque) < 0) data->ret = -1; + return 0; } @@ -850,7 +854,7 @@ struct virDomainListData { }; -static void +static int virDomainObjListCollectIterator(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -858,6 +862,7 @@ virDomainObjListCollectIterator(void *payload, struct virDomainListData *data = opaque; data->vms[data->nvms++] = virObjectRef(payload); + return 0; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index a5a40fe..23f52b5 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -649,7 +649,7 @@ struct virLockDaemonClientReleaseData { bool gotError; }; -static void +static int virLockDaemonClientReleaseLockspace(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -664,6 +664,7 @@ virLockDaemonClientReleaseLockspace(void *payload, data->hadSomeLeases = true; else if (rc < 0) data->gotError = true; + return 0; } diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 7dbf467..c671cd8 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1864,7 +1864,7 @@ virNWFilterSnoopPruneIter(const void *payload, * Iterator to write all leases of a single request to a file. * Call this function with the SnoopLock held. */ -static void +static int virNWFilterSnoopSaveIter(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) @@ -1880,6 +1880,7 @@ virNWFilterSnoopSaveIter(void *payload, ignore_value(virNWFilterSnoopLeaseFileWrite(tfd, req->ifkey, ipl)); virNWFilterSnoopReqUnlock(req); + return 0; } /* diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 5a4cff1..761a01b 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -219,19 +219,20 @@ struct printString }; -static void +static int printString(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { struct printString *ps = data; if ((STREQ((char *)name, NWFILTER_STD_VAR_IP) && !ps->reportIP) || (STREQ((char *)name, NWFILTER_STD_VAR_MAC) && !ps->reportMAC)) - return; + return 0; if (virBufferUse(&ps->buf) && ps->separator) virBufferAdd(&ps->buf, ps->separator, -1); virBufferAdd(&ps->buf, name, -1); + return 0; } /** diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 495c76b..3c84886 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2802,9 +2802,9 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, } /* Hash iterator callback to discard multiple snapshots. */ -void qemuDomainSnapshotDiscardAll(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) +int qemuDomainSnapshotDiscardAll(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) { virDomainSnapshotObjPtr snap = payload; virQEMUSnapRemovePtr curr = data; @@ -2816,6 +2816,7 @@ void qemuDomainSnapshotDiscardAll(void *payload, curr->metadata_only); if (err && !curr->err) curr->err = err; + return 0; } int diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 285af8c..704bcdb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -401,9 +401,9 @@ struct _virQEMUSnapRemove { bool current; }; -void qemuDomainSnapshotDiscardAll(void *payload, - const void *name, - void *data); +int qemuDomainSnapshotDiscardAll(void *payload, + const void *name, + void *data); int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2bbc724..52111a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2239,7 +2239,7 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) /* Count how many snapshots in a set are external snapshots or checkpoints. */ -static void +static int qemuDomainSnapshotCountExternal(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) @@ -2249,6 +2249,7 @@ qemuDomainSnapshotCountExternal(void *payload, if (virDomainSnapshotIsExternal(snap)) (*count)++; + return 0; } static int @@ -10771,7 +10772,7 @@ qemuDomainBlockResize(virDomainPtr dom, } -static void +static int qemuDomainBlockStatsGatherTotals(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -10792,6 +10793,7 @@ qemuDomainBlockStatsGatherTotals(void *payload, QEMU_BLOCK_STAT_TOTAL(rd_total_times); QEMU_BLOCK_STAT_TOTAL(flush_total_times); #undef QEMU_BLOCK_STAT_TOTAL + return 0; } @@ -15549,7 +15551,7 @@ struct _virQEMUSnapReparent { }; -static void +static int qemuDomainSnapshotReparentChildren(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) @@ -15558,7 +15560,7 @@ qemuDomainSnapshotReparentChildren(void *payload, virQEMUSnapReparentPtr rep = data; if (rep->err < 0) - return; + return 0; VIR_FREE(snap->def->parent); snap->parent = rep->parent; @@ -15566,7 +15568,7 @@ qemuDomainSnapshotReparentChildren(void *payload, if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { rep->err = -1; - return; + return 0; } if (!snap->sibling) @@ -15574,6 +15576,7 @@ qemuDomainSnapshotReparentChildren(void *payload, rep->err = qemuDomainSnapshotWriteMetadata(rep->vm, snap, rep->caps, rep->cfg->snapshotDir); + return 0; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 727382e..21c66db 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6295,10 +6295,10 @@ struct _testSnapRemoveData { bool current; }; -static void +static int testDomainSnapshotDiscardAll(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) + const void *name ATTRIBUTE_UNUSED, + void *data) { virDomainSnapshotObjPtr snap = payload; testSnapRemoveDataPtr curr = data; @@ -6306,6 +6306,7 @@ testDomainSnapshotDiscardAll(void *payload, if (snap->def->current) curr->current = true; virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); + return 0; } typedef struct _testSnapReparentData testSnapReparentData; @@ -6317,7 +6318,7 @@ struct _testSnapReparentData { virDomainSnapshotObjPtr last; }; -static void +static int testDomainSnapshotReparentChildren(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) @@ -6326,7 +6327,7 @@ testDomainSnapshotReparentChildren(void *payload, testSnapReparentDataPtr rep = data; if (rep->err < 0) - return; + return 0; VIR_FREE(snap->def->parent); snap->parent = rep->parent; @@ -6334,11 +6335,12 @@ testDomainSnapshotReparentChildren(void *payload, if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { rep->err = -1; - return; + return 0; } if (!snap->sibling) rep->last = snap; + return 0; } static int diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index b0dba5c..d656704 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -724,9 +724,9 @@ struct umlProcessAutoDestroyData { virConnectPtr conn; }; -static void umlProcessAutoDestroyDom(void *payload, - const void *name, - void *opaque) +static int umlProcessAutoDestroyDom(void *payload, + const void *name, + void *opaque) { struct umlProcessAutoDestroyData *data = opaque; virConnectPtr conn = payload; @@ -738,17 +738,17 @@ static void umlProcessAutoDestroyDom(void *payload, VIR_DEBUG("conn=%p uuidstr=%s thisconn=%p", conn, uuidstr, data->conn); if (data->conn != conn) - return; + return 0; if (virUUIDParse(uuidstr, uuid) < 0) { VIR_WARN("Failed to parse %s", uuidstr); - return; + return 0; } if (!(dom = virDomainObjListFindByUUID(data->driver->domains, uuid))) { VIR_DEBUG("No domain object to kill"); - return; + return 0; } VIR_DEBUG("Killing domain"); @@ -766,6 +766,7 @@ static void umlProcessAutoDestroyDom(void *payload, if (event) umlDomainEventQueue(data->driver, event); virHashRemoveEntry(data->driver->autodestroy, uuidstr); + return 0; } /* diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c index 9006d36..82d4071 100644 --- a/src/util/virclosecallbacks.c +++ b/src/util/virclosecallbacks.c @@ -252,7 +252,7 @@ struct virCloseCallbacksData { bool oom; }; -static void +static int virCloseCallbacksGetOne(void *payload, const void *key, void *opaque) @@ -263,23 +263,24 @@ virCloseCallbacksGetOne(void *payload, unsigned char uuid[VIR_UUID_BUFLEN]; if (virUUIDParse(uuidstr, uuid) < 0) - return; + return 0; VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p", closeDef->conn, data->conn, uuidstr, closeDef->cb); if (data->conn != closeDef->conn || !closeDef->cb) - return; + return 0; if (VIR_EXPAND_N(data->list->entries, data->list->nentries, 1) < 0) { data->oom = true; - return; + return 0; } memcpy(data->list->entries[data->list->nentries - 1].uuid, uuid, VIR_UUID_BUFLEN); data->list->entries[data->list->nentries - 1].callback = closeDef->cb; + return 0; } static virCloseCallbacksListPtr diff --git a/src/util/virhash.c b/src/util/virhash.c index fab621b..d6f208e 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -738,8 +738,8 @@ struct getKeysIter size_t arrayIdx; }; -static void virHashGetKeysIterator(void *payload, - const void *key, void *data) +static int virHashGetKeysIterator(void *payload, + const void *key, void *data) { struct getKeysIter *iter = data; @@ -747,6 +747,7 @@ static void virHashGetKeysIterator(void *payload, iter->sortArray[iter->arrayIdx].value = payload; iter->arrayIdx++; + return 0; } typedef int (*qsort_comp)(const void *, const void *); diff --git a/src/util/virhash.h b/src/util/virhash.h index 50b9a04..ed6bba3 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -43,8 +43,10 @@ typedef void (*virHashDataFree) (void *payload, const void *name); * @data: user supplied data blob * * Callback to process a hash entry during iteration + * + * Returns -1 to stop the iteration, e.g. in case of an error */ -typedef void (*virHashIterator) (void *payload, const void *name, void *data); +typedef int (*virHashIterator) (void *payload, const void *name, void *data); /** * virHashSearcher: * @payload: the data in the hash diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index a0d6f74..ef1a460 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1118,7 +1118,7 @@ struct xenXMListIteratorContext { char ** names; }; -static void +static int xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) @@ -1127,10 +1127,10 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, virDomainDefPtr def = NULL; if (ctx->oom) - return; + return 0; if (ctx->count == ctx->max) - return; + return 0; def = xenDaemonLookupByName(ctx->conn, name); if (!def) { @@ -1141,6 +1141,7 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, } else { virDomainDefFree(def); } + return 0; } diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 41f0e76..323c68e 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -56,11 +56,12 @@ testHashInit(int size) return hash; } -static void +static int testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { + return 0; } static int @@ -183,7 +184,7 @@ testHashRemove(const void *data ATTRIBUTE_UNUSED) const int testHashCountRemoveForEachSome = ARRAY_CARDINALITY(uuids) - ARRAY_CARDINALITY(uuids_subset); -static void +static int testHashRemoveForEachSome(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) @@ -200,12 +201,13 @@ testHashRemoveForEachSome(void *payload ATTRIBUTE_UNUSED, break; } } + return 0; } const int testHashCountRemoveForEachAll = 0; -static void +static int testHashRemoveForEachAll(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) @@ -213,12 +215,13 @@ testHashRemoveForEachAll(void *payload ATTRIBUTE_UNUSED, virHashTablePtr hash = data; virHashRemoveEntry(hash, name); + return 0; } const int testHashCountRemoveForEachForbidden = ARRAY_CARDINALITY(uuids); -static void +static int testHashRemoveForEachForbidden(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) @@ -238,6 +241,7 @@ testHashRemoveForEachForbidden(void *payload ATTRIBUTE_UNUSED, break; } } + return 0; } @@ -302,15 +306,15 @@ testHashSteal(const void *data ATTRIBUTE_UNUSED) } -static void +static int testHashIter(void *payload ATTRIBUTE_UNUSED, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { - return; + return 0; } -static void +static int testHashForEachIter(void *payload ATTRIBUTE_UNUSED, const void *name ATTRIBUTE_UNUSED, void *data) @@ -332,6 +336,7 @@ testHashForEachIter(void *payload ATTRIBUTE_UNUSED, if (virHashForEach(hash, testHashIter, NULL) >= 0) VIR_TEST_VERBOSE("\niterating through hash in ForEach" " should be forbidden"); + return 0; } static int -- 2.4.3
The method will now return 0 on success and -1 on error, rather than number of items which it iterated over before it returned back to the caller. Since the only place where we actually check the number of elements iterated is in virhashtest, return value of 0 and -1 can be a pretty accurate hint that it iterated over all the items. However, if we really want to know the number of items iterated over (like virhashtest does), a counter has to be provided through opaque data to each iterator call. This patch adjusts return value of virHashForEach, refactors the body, so it returns as soon as one of the iterators fail and adjusts virhashtest to reflect these changes. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virhash.c | 23 +++++++++++++++-------- src/util/virhash.h | 2 +- tests/virhashtest.c | 35 ++++++++++++++--------------------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index d6f208e..a8e0edf 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -579,13 +579,16 @@ virHashRemoveEntry(virHashTablePtr table, const void *name) * Iterates over every element in the hash table, invoking the * 'iter' callback. The callback is allowed to remove the current element * using virHashRemoveEntry but calling other virHash* functions is prohibited. + * If @iter fails and returns a negative value, the evaluation is stopped and -1 + * is returned. * - * Returns number of items iterated over upon completion, -1 on failure + * Returns 0 on success or -1 on failure. */ -ssize_t +int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data) { - size_t i, count = 0; + size_t i; + int ret = -1; if (table == NULL || iter == NULL) return -1; @@ -599,20 +602,24 @@ virHashForEach(virHashTablePtr table, virHashIterator iter, void *data) virHashEntryPtr entry = table->table[i]; while (entry) { virHashEntryPtr next = entry->next; - table->current = entry; - iter(entry->payload, entry->name, data); + ret = iter(entry->payload, entry->name, data); table->current = NULL; - count++; + if (ret < 0) + goto cleanup; + entry = next; } } - table->iterating = false; - return count; + ret = 0; + cleanup: + table->iterating = false; + return ret; } + /** * virHashRemoveSet * @table: the hash table to process diff --git a/src/util/virhash.h b/src/util/virhash.h index ed6bba3..61c172b 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -191,7 +191,7 @@ bool virHashEqual(const virHashTable *table1, /* * Iterators */ -ssize_t virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); +int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data); void *virHashSearch(const virHashTable *table, virHashSearcher iter, const void *data); diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 323c68e..e538ea4 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -61,24 +61,27 @@ testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { + ssize_t *count = data; + *count += 1; return 0; } static int -testHashCheckCount(virHashTablePtr hash, size_t count) +testHashCheckCount(virHashTablePtr hash, int count) { - ssize_t iter_count = 0; + int iter_count = 0; if (virHashSize(hash) != count) { VIR_TEST_VERBOSE("\nhash contains %zu instead of %zu elements\n", - (size_t)virHashSize(hash), count); + (size_t)virHashSize(hash), (size_t)count); return -1; } - iter_count = virHashForEach(hash, testHashCheckForEachCount, NULL); + virHashForEach(hash, testHashCheckForEachCount, &iter_count); if (count != iter_count) { - VIR_TEST_VERBOSE("\nhash claims to have %zu elements but iteration finds %zu\n", - count, (size_t)iter_count); + VIR_TEST_VERBOSE("\nhash claims to have %zu elements but iteration" + "finds %zu\n", + (size_t)count, (size_t)iter_count); return -1; } @@ -88,7 +91,7 @@ testHashCheckCount(virHashTablePtr hash, size_t count) struct testInfo { void *data; - size_t count; + int count; }; @@ -250,18 +253,13 @@ testHashRemoveForEach(const void *data) { const struct testInfo *info = data; virHashTablePtr hash; - int count; int ret = -1; if (!(hash = testHashInit(0))) return -1; - count = virHashForEach(hash, (virHashIterator) info->data, hash); - - if (count != ARRAY_CARDINALITY(uuids)) { - VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries," - " %d != %zu\n", - count, ARRAY_CARDINALITY(uuids)); + if (virHashForEach(hash, (virHashIterator) info->data, hash)) { + VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries"); goto cleanup; } @@ -343,18 +341,13 @@ static int testHashForEach(const void *data ATTRIBUTE_UNUSED) { virHashTablePtr hash; - int count; int ret = -1; if (!(hash = testHashInit(0))) return -1; - count = virHashForEach(hash, testHashForEachIter, hash); - - if (count != ARRAY_CARDINALITY(uuids)) { - VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries," - " %d != %zu\n", - count, ARRAY_CARDINALITY(uuids)); + if (virHashForEach(hash, testHashForEachIter, hash)) { + VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries"); goto cleanup; } -- 2.4.3
On 12.02.2016 11:08, Erik Skultety wrote:
The method will now return 0 on success and -1 on error, rather than number of items which it iterated over before it returned back to the caller. Since the only place where we actually check the number of elements iterated is in virhashtest, return value of 0 and -1 can be a pretty accurate hint that it iterated over all the items. However, if we really want to know the number of items iterated over (like virhashtest does), a counter has to be provided through opaque data to each iterator call. This patch adjusts return value of virHashForEach, refactors the body, so it returns as soon as one of the iterators fail and adjusts virhashtest to reflect these changes.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virhash.c | 23 +++++++++++++++-------- src/util/virhash.h | 2 +- tests/virhashtest.c | 35 ++++++++++++++--------------------- 3 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/util/virhash.c b/src/util/virhash.c index d6f208e..a8e0edf 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -579,13 +579,16 @@ virHashRemoveEntry(virHashTablePtr table, const void *name) * Iterates over every element in the hash table, invoking the * 'iter' callback. The callback is allowed to remove the current element * using virHashRemoveEntry but calling other virHash* functions is prohibited. + * If @iter fails and returns a negative value, the evaluation is stopped and -1 + * is returned. * - * Returns number of items iterated over upon completion, -1 on failure + * Returns 0 on success or -1 on failure. */ -ssize_t +int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data) { - size_t i, count = 0; + size_t i; + int ret = -1;
if (table == NULL || iter == NULL) return -1; @@ -599,20 +602,24 @@ virHashForEach(virHashTablePtr table, virHashIterator iter, void *data) virHashEntryPtr entry = table->table[i]; while (entry) { virHashEntryPtr next = entry->next; - table->current = entry; - iter(entry->payload, entry->name, data); + ret = iter(entry->payload, entry->name, data); table->current = NULL;
- count++; + if (ret < 0) + goto cleanup; + entry = next; } } - table->iterating = false;
- return count; + ret = 0; + cleanup: + table->iterating = false; + return ret; }
+ /** * virHashRemoveSet * @table: the hash table to process diff --git a/src/util/virhash.h b/src/util/virhash.h index ed6bba3..61c172b 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -191,7 +191,7 @@ bool virHashEqual(const virHashTable *table1, /* * Iterators */ -ssize_t virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); +int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data); void *virHashSearch(const virHashTable *table, virHashSearcher iter, const void *data); diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 323c68e..e538ea4 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -61,24 +61,27 @@ testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED, const void *name ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { + ssize_t *count = data; + *count += 1; return 0; }
static int -testHashCheckCount(virHashTablePtr hash, size_t count) +testHashCheckCount(virHashTablePtr hash, int count) { - ssize_t iter_count = 0; + int iter_count = 0;
Why are you changing the type if the callback expects ssize_t anyway?
if (virHashSize(hash) != count) { VIR_TEST_VERBOSE("\nhash contains %zu instead of %zu elements\n", - (size_t)virHashSize(hash), count); + (size_t)virHashSize(hash), (size_t)count);
This makes not much sense. We are in the control of formatting string, so why the typecasting?
return -1; }
- iter_count = virHashForEach(hash, testHashCheckForEachCount, NULL); + virHashForEach(hash, testHashCheckForEachCount, &iter_count); if (count != iter_count) { - VIR_TEST_VERBOSE("\nhash claims to have %zu elements but iteration finds %zu\n", - count, (size_t)iter_count); + VIR_TEST_VERBOSE("\nhash claims to have %zu elements but iteration" + "finds %zu\n", + (size_t)count, (size_t)iter_count); return -1; }
ACK with this squashed in: diff --git a/tests/virhashtest.c b/tests/virhashtest.c index e538ea4..ed9ad46 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -67,21 +67,20 @@ testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED, } static int -testHashCheckCount(virHashTablePtr hash, int count) +testHashCheckCount(virHashTablePtr hash, size_t count) { - int iter_count = 0; + ssize_t iter_count = 0; if (virHashSize(hash) != count) { - VIR_TEST_VERBOSE("\nhash contains %zu instead of %zu elements\n", - (size_t)virHashSize(hash), (size_t)count); + VIR_TEST_VERBOSE("\nhash contains %zd instead of %zu elements\n", + virHashSize(hash), count); return -1; } virHashForEach(hash, testHashCheckForEachCount, &iter_count); if (count != iter_count) { - VIR_TEST_VERBOSE("\nhash claims to have %zu elements but iteration" - "finds %zu\n", - (size_t)count, (size_t)iter_count); + VIR_TEST_VERBOSE("\nhash claims to have %zd elements but iteration" + "finds %zd\n", count, iter_count); return -1; } Michal
Since the daemon can manage and add (at fresh start) multiple servers, we also should be able to add them from a JSON state file in case of a daemon restart, so post exec restart support for multiple servers is also provided. Patch also updates virnetdaemontest accordingly. Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/libvirtd.c | 4 +- src/locking/lock_daemon.c | 7 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 186 ++++++++++------ src/rpc/virnetdaemon.h | 7 +- .../input-data-admin-server-names.json | 129 +++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 8 +- .../output-data-admin-server-names.json | 126 +++++++++++ .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 242 +++++++++++---------- tests/virnetdaemontest.c | 63 +++--- 14 files changed, 561 insertions(+), 234 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-admin-server-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-server-names.json diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 250094b..a747553 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1399,7 +1399,7 @@ int main(int argc, char **argv) { } if (!(dmn = virNetDaemonNew()) || - virNetDaemonAddServer(dmn, srv) < 0) { + virNetDaemonAddServer(dmn, "libvirtd", srv) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1472,7 +1472,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (virNetDaemonAddServer(dmn, srvAdm) < 0) { + if (virNetDaemonAddServer(dmn, "admin", srvAdm) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 23f52b5..cd70aa9 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -170,7 +170,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) goto error; if (!(lockd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(lockd->dmn, srv) < 0) + virNetDaemonAddServer(lockd->dmn, "virtlockd", srv) < 0) goto error; if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, @@ -267,6 +267,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) goto error; if (!(srv = virNetDaemonAddServerPostExec(lockd->dmn, + "virtlockd", virLockDaemonClientNew, virLockDaemonClientNewPostExecRestart, virLockDaemonClientPreExecRestart, @@ -1369,7 +1370,7 @@ int main(int argc, char **argv) { goto cleanup; } - srv = virNetDaemonGetServer(lockDaemon->dmn, 0); + srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); if ((rv = virLockDaemonSetupNetworkingSystemD(srv) < 0)) { ret = VIR_LOCK_DAEMON_ERR_NETWORK; goto cleanup; @@ -1382,7 +1383,7 @@ int main(int argc, char **argv) { goto cleanup; } } else if (rv == 1) { - srv = virNetDaemonGetServer(lockDaemon->dmn, 0); + srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); } if (timeout != -1) { diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 6da5876..07a03c2 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -160,7 +160,7 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) goto error; if (!(logd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(logd->dmn, logd->srv) < 0) + virNetDaemonAddServer(logd->dmn, "virtlogd", logd->srv) < 0) goto error; if (!(logd->handler = virLogHandlerNew(privileged, @@ -209,6 +209,7 @@ virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) goto error; if (!(logd->srv = virNetDaemonAddServerPostExec(logd->dmn, + "virtlogd", virLogDaemonClientNew, virLogDaemonClientNewPostExecRestart, virLogDaemonClientPreExecRestart, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 438103a..76bef82 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -967,7 +967,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) goto error; if (!(ctrl->daemon = virNetDaemonNew()) || - virNetDaemonAddServer(ctrl->daemon, srv) < 0) + virNetDaemonAddServer(ctrl->daemon, "LXC", srv) < 0) goto error; virNetDaemonUpdateServices(ctrl->daemon, true); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 18c962c..9210de4 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -37,6 +37,7 @@ #include "virnetserver.h" #include "virnetservermdns.h" #include "virdbus.h" +#include "virhash.h" #include "virstring.h" #include "virsystemd.h" @@ -69,8 +70,7 @@ struct _virNetDaemon { int sigwrite; int sigwatch; - size_t nservers; - virNetServerPtr *servers; + virHashTablePtr servers; virJSONValuePtr srvObject; bool quit; @@ -102,9 +102,7 @@ virNetDaemonDispose(void *obj) if (dmn->sigwatch > 0) virEventRemoveHandle(dmn->sigwatch); - for (i = 0; i < dmn->nservers; i++) - virObjectUnref(dmn->servers[i]); - VIR_FREE(dmn->servers); + virHashFree(dmn->servers); virJSONValueFree(dmn->srvObject); } @@ -136,6 +134,9 @@ virNetDaemonNew(void) if (!(dmn = virObjectLockableNew(virNetDaemonClass))) return NULL; + if (!(dmn->servers = virHashCreate(5, virObjectFreeHashData))) + goto error; + dmn->sigwrite = dmn->sigread = -1; dmn->privileged = geteuid() == 0; dmn->autoShutdownInhibitFd = -1; @@ -156,49 +157,34 @@ virNetDaemonNew(void) int -virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr srv) +virNetDaemonAddServer(virNetDaemonPtr dmn, + const char *serverName, + virNetServerPtr srv) { int ret = -1; virObjectLock(dmn); - if (VIR_APPEND_ELEMENT_COPY(dmn->servers, dmn->nservers, srv) < 0) + if (virHashAddEntry(dmn->servers, serverName, srv) < 0) goto cleanup; virObjectRef(srv); - ret = dmn->nservers - 1; + + ret = 0; cleanup: virObjectUnlock(dmn); return ret; } -/* - * Separate function merely for the purpose of unified error - * reporting. - */ -static virNetServerPtr -virNetDaemonGetServerInternal(virNetDaemonPtr dmn, - int subServerID) -{ - if (subServerID < 0 || subServerID >= dmn->nservers) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid server ID: %d"), - subServerID); - return NULL; - } - - return virObjectRef(dmn->servers[subServerID]); -} - virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, - int subServerID) + const char *serverName) { virNetServerPtr srv = NULL; virObjectLock(dmn); - srv = virNetDaemonGetServerInternal(dmn, subServerID); + srv = virObjectRef(virHashLookup(dmn->servers, serverName)); virObjectUnlock(dmn); return srv; @@ -206,6 +192,7 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -230,9 +217,23 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, virJSONValueFree(dmn->srvObject); dmn->srvObject = NULL; } - } else { + } else if (virJSONValueObjectGetByType(dmn->srvObject, + "min_workers", + VIR_JSON_TYPE_NUMBER)) { object = dmn->srvObject; dmn->srvObject = NULL; + } else { + int ret = virJSONValueObjectRemoveKey(dmn->srvObject, + serverName, + &object); + if (ret != 1) { + if (ret == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Server '%s' not found in JSON"), serverName); + } + goto error; + } + } srv = virNetServerNewPostExecRestart(object, @@ -242,7 +243,10 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, clientPrivFree, clientPrivOpaque); - if (!srv || VIR_APPEND_ELEMENT_COPY(dmn->servers, dmn->nservers, srv) < 0) + if (!srv) + goto error; + + if (virHashAddEntry(dmn->servers, serverName, srv) < 0) goto error; virJSONValueFree(object); @@ -283,29 +287,51 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object) } +static int +daemonServerCompare(const virHashKeyValuePair *a, const virHashKeyValuePair *b) +{ + const char *as = a->key; + const char *bs = b->key; + + return strcmp(as, bs); +} + virJSONValuePtr virNetDaemonPreExecRestart(virNetDaemonPtr dmn) { - virJSONValuePtr object, srvArray = NULL; - size_t i; + size_t i = 0; + virJSONValuePtr object = NULL; + virJSONValuePtr srvObj = NULL; + virHashKeyValuePairPtr srvArray = NULL; virObjectLock(dmn); if (!(object = virJSONValueNewObject())) goto error; - if (!(srvArray = virJSONValueNewArray()) || - virJSONValueObjectAppend(object, "servers", srvArray) < 0) + if (!(srvObj = virJSONValueNewObject())) goto error; - for (i = 0; i < dmn->nservers; i++) { - virJSONValuePtr srvJSON = NULL; - srvJSON = virNetServerPreExecRestart(dmn->servers[i]); + if (virJSONValueObjectAppend(object, "servers", srvObj) < 0) { + virJSONValueFree(srvObj); + goto error; + } + + if (!(srvArray = virHashGetItems(dmn->servers, daemonServerCompare))) + goto error; + + for (i = 0; srvArray[i].key; i++) { + virNetServerPtr server = virHashLookup(dmn->servers, srvArray[i].key); + virJSONValuePtr srvJSON; + if (!server) + goto error; + + srvJSON = virNetServerPreExecRestart(server); if (!srvJSON) goto error; - if (virJSONValueArrayAppend(srvArray, srvJSON) < 0) { + if (virJSONValueObjectAppend(srvObj, srvArray[i].key, srvJSON) < 0) { virJSONValueFree(srvJSON); goto error; } @@ -316,8 +342,8 @@ virNetDaemonPreExecRestart(virNetDaemonPtr dmn) return object; error: + VIR_FREE(srvArray); virJSONValueFree(object); - virJSONValueFree(srvArray); virObjectUnlock(dmn); return NULL; } @@ -627,24 +653,53 @@ virNetDaemonAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED, virObjectUnlock(dmn); } +static int +daemonServerUpdateServices(void *payload, + const void *key ATTRIBUTE_UNUSED, + void *opaque) +{ + bool *enable = opaque; + virNetServerPtr srv = payload; + + virNetServerUpdateServices(srv, *enable); + return 0; +} + void virNetDaemonUpdateServices(virNetDaemonPtr dmn, bool enabled) { - size_t i; - virObjectLock(dmn); - for (i = 0; i < dmn->nservers; i++) - virNetServerUpdateServices(dmn->servers[i], enabled); + virHashForEach(dmn->servers, daemonServerUpdateServices, &enabled); virObjectUnlock(dmn); } +static int +daemonServerRun(void *payload, + const void *key ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virNetServerPtr srv = payload; + + return virNetServerStart(srv); +}; + +static int +daemonServerProcessClients(void *payload, + const void *key ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virNetServerPtr srv = payload; + + virNetServerProcessClients(srv); + return 0; +} + void virNetDaemonRun(virNetDaemonPtr dmn) { int timerid = -1; bool timerActive = false; - size_t i; virObjectLock(dmn); @@ -654,10 +709,8 @@ virNetDaemonRun(virNetDaemonPtr dmn) goto cleanup; } - for (i = 0; i < dmn->nservers; i++) { - if (virNetServerStart(dmn->servers[i]) < 0) - goto cleanup; - } + if (virHashForEach(dmn->servers, daemonServerRun, NULL) < 0) + goto cleanup; dmn->quit = false; @@ -705,8 +758,7 @@ virNetDaemonRun(virNetDaemonPtr dmn) } virObjectLock(dmn); - for (i = 0; i < dmn->nservers; i++) - virNetServerProcessClients(dmn->servers[i]); + virHashForEach(dmn->servers, daemonServerProcessClients, NULL); } cleanup: @@ -725,32 +777,42 @@ virNetDaemonQuit(virNetDaemonPtr dmn) virObjectUnlock(dmn); } +static int +daemonServerClose(void *payload, + const void *key ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virNetServerPtr srv = payload; + + virNetServerClose(srv); + return 0; +} void virNetDaemonClose(virNetDaemonPtr dmn) { - size_t i; - if (!dmn) return; virObjectLock(dmn); - for (i = 0; i < dmn->nservers; i++) - virNetServerClose(dmn->servers[i]); + virHashForEach(dmn->servers, daemonServerClose, NULL); virObjectUnlock(dmn); } -bool -virNetDaemonHasClients(virNetDaemonPtr dmn) +static int +daemonServerHasClients(void *payload, + const void *key ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { - size_t i = 0; + virNetServerPtr srv = payload; - for (i = 0; i < dmn->nservers; i++) { - if (virNetServerHasClients(dmn->servers[i])) - return true; - } + return virNetServerHasClients(srv); +} - return false; +bool +virNetDaemonHasClients(virNetDaemonPtr dmn) +{ + return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; } diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index bb32053..968708b 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -37,9 +37,12 @@ virNetDaemonPtr virNetDaemonNew(void); -int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr); +int virNetDaemonAddServer(virNetDaemonPtr dmn, + const char *serverName, + virNetServerPtr srv); virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -79,6 +82,6 @@ void virNetDaemonClose(virNetDaemonPtr dmn); bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, - int subServerID); + const char *serverName); #endif /* __VIR_NET_DAEMON_H__ */ diff --git a/tests/virnetdaemondata/input-data-admin-server-names.json b/tests/virnetdaemondata/input-data-admin-server-names.json new file mode 100644 index 0000000..94fba61 --- /dev/null +++ b/tests/virnetdaemondata/input-data-admin-server-names.json @@ -0,0 +1,129 @@ +{ + "servers": + { + "testServer0": + { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + }, + "testServer1": + { + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-admin-nomdns.json b/tests/virnetdaemondata/output-data-admin-nomdns.json index a814aeb..8827c04 100644 --- a/tests/virnetdaemondata/output-data-admin-nomdns.json +++ b/tests/virnetdaemondata/output-data-admin-nomdns.json @@ -1,6 +1,6 @@ { - "servers": [ - { + "servers": { + "testServer0": { "min_workers": 10, "max_workers": 50, "priority_workers": 5, @@ -61,7 +61,7 @@ } ] }, - { + "testServer1": { "min_workers": 2, "max_workers": 50, "priority_workers": 5, @@ -122,5 +122,5 @@ } ] } - ] + } } diff --git a/tests/virnetdaemondata/output-data-admin-server-names.json b/tests/virnetdaemondata/output-data-admin-server-names.json new file mode 100644 index 0000000..8827c04 --- /dev/null +++ b/tests/virnetdaemondata/output-data-admin-server-names.json @@ -0,0 +1,126 @@ +{ + "servers": { + "testServer0": { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + }, + "testServer1": { + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-anon-clients.json b/tests/virnetdaemondata/output-data-anon-clients.json index 05fc0ae..df93e3b 100644 --- a/tests/virnetdaemondata/output-data-anon-clients.json +++ b/tests/virnetdaemondata/output-data-anon-clients.json @@ -1,6 +1,6 @@ { - "servers": [ - { + "servers": { + "testServer0": { "min_workers": 10, "max_workers": 50, "priority_workers": 5, @@ -61,5 +61,5 @@ } ] } - ] + } } diff --git a/tests/virnetdaemondata/output-data-initial-nomdns.json b/tests/virnetdaemondata/output-data-initial-nomdns.json index 400e47b..154962b 100644 --- a/tests/virnetdaemondata/output-data-initial-nomdns.json +++ b/tests/virnetdaemondata/output-data-initial-nomdns.json @@ -1,6 +1,6 @@ { - "servers": [ - { + "servers": { + "testServer0": { "min_workers": 10, "max_workers": 50, "priority_workers": 5, @@ -61,5 +61,5 @@ } ] } - ] + } } diff --git a/tests/virnetdaemondata/output-data-initial.json b/tests/virnetdaemondata/output-data-initial.json index e875cff..b7c27df 100644 --- a/tests/virnetdaemondata/output-data-initial.json +++ b/tests/virnetdaemondata/output-data-initial.json @@ -1,6 +1,6 @@ { - "servers": [ - { + "servers": { + "testServer0": { "min_workers": 10, "max_workers": 50, "priority_workers": 5, @@ -62,5 +62,5 @@ } ] } - ] + } } diff --git a/tests/virnetdaemondata/output-data-no-keepalive-required.json b/tests/virnetdaemondata/output-data-no-keepalive-required.json index df282ed..a8ba828 100644 --- a/tests/virnetdaemondata/output-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/output-data-no-keepalive-required.json @@ -1,124 +1,126 @@ { - "servers": [ - { - "min_workers": 10, - "max_workers": 50, - "priority_workers": 5, - "max_clients": 100, - "keepaliveInterval": 120, - "keepaliveCount": 5, - "services": [ - { - "auth": 0, - "readonly": true, - "nrequests_client_max": 2, - "socks": [ - { - "fd": 100, - "errfd": -1, - "pid": 0, - "isClient": false - } + "servers": { + "testServer0": { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } ] - }, - { - "auth": 2, - "readonly": false, - "nrequests_client_max": 5, - "socks": [ - { - "fd": 101, - "errfd": -1, - "pid": 0, - "isClient": false - } - ] - } - ], - "clients": [ - { - "auth": 1, - "readonly": true, - "nrequests_max": 15, - "sock": { - "fd": 102, - "errfd": -1, - "pid": -1, - "isClient": true - } - }, - { - "auth": 2, - "readonly": true, - "nrequests_max": 66, - "sock": { - "fd": 103, - "errfd": -1, - "pid": -1, - "isClient": true - } - } - ] - }, - { - "min_workers": 2, - "max_workers": 50, - "priority_workers": 5, - "max_clients": 100, - "keepaliveInterval": 120, - "keepaliveCount": 5, - "services": [ - { - "auth": 0, - "readonly": true, - "nrequests_client_max": 2, - "socks": [ - { - "fd": 100, - "errfd": -1, - "pid": 0, - "isClient": false - } - ] - }, - { - "auth": 2, - "readonly": false, - "nrequests_client_max": 5, - "socks": [ - { - "fd": 101, - "errfd": -1, - "pid": 0, - "isClient": false - } - ] - } - ], - "clients": [ - { - "auth": 1, - "readonly": true, - "nrequests_max": 15, - "sock": { - "fd": 102, - "errfd": -1, - "pid": -1, - "isClient": true - } - }, - { - "auth": 2, - "readonly": true, - "nrequests_max": 66, - "sock": { - "fd": 103, - "errfd": -1, - "pid": -1, - "isClient": true - } - } - ] + }, + "testServer1": { + { + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } } - ] } diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index e5d09a3..2f855fd 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -135,7 +135,7 @@ testCreateServer(const char *host, int family) goto cleanup; } -static char *testGenerateJSON(void) +static char *testGenerateJSON(const char *server_name) { virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; @@ -155,15 +155,14 @@ static char *testGenerateJSON(void) if (!has_ipv4 && !has_ipv6) return NULL; - if (!(srv = testCreateServer( - has_ipv4 ? "127.0.0.1" : "::1", - has_ipv4 ? AF_INET : AF_INET6))) + if (!(srv = testCreateServer(has_ipv4 ? "127.0.0.1" : "::1", + has_ipv4 ? AF_INET : AF_INET6))) goto cleanup; if (!(dmn = virNetDaemonNew())) goto cleanup; - if (virNetDaemonAddServer(dmn, srv) < 0) + if (virNetDaemonAddServer(dmn, server_name, srv) < 0) goto cleanup; if (!(json = virNetDaemonPreExecRestart(dmn))) @@ -186,6 +185,7 @@ static char *testGenerateJSON(void) struct testExecRestartData { const char *jsonfile; + const char **serverNames; int nservers; bool pass; }; @@ -241,7 +241,7 @@ static int testExecRestart(const void *opaque) goto cleanup; for (i = 0; i < data->nservers; i++) { - if (!(srv = virNetDaemonAddServerPostExec(dmn, + if (!(srv = virNetDaemonAddServerPostExec(dmn, data->serverNames[i], NULL, NULL, NULL, NULL, NULL))) goto cleanup; @@ -257,18 +257,18 @@ static int testExecRestart(const void *opaque) if (virtTestCompareToFile(outjsonstr, outfile) < 0) goto cleanup; - if (!data->pass) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Test should've failed"); - goto cleanup; - } - ret = 0; cleanup: if (ret < 0) { - if (!data->pass) + if (!data->pass) { + VIR_TEST_DEBUG("Got expected error: %s\n", + virGetLastErrorMessage()); + virResetLastError(); ret = 0; - else - virDispatchError(NULL); + } + } else if (!data->pass) { + VIR_TEST_DEBUG("Test should have failed\n"); + ret = -1; } VIR_FREE(infile); VIR_FREE(outfile); @@ -289,6 +289,7 @@ static int mymain(void) { int ret = 0; + const char *server_names[] = { "testServer0", "testServer1" }; if (virInitialize() < 0 || virEventRegisterDefaultImpl() < 0) { @@ -302,7 +303,7 @@ mymain(void) * numbers with 100, 101, 102, 103. */ if (getenv("VIR_GENERATE_JSON")) { - char *json = testGenerateJSON(); + char *json = testGenerateJSON(server_names[0]); if (!json) return EXIT_FAILURE; @@ -311,26 +312,28 @@ mymain(void) return ret; } -# define EXEC_RESTART_TEST_FULL(file, servers, pass) \ - do { \ - struct testExecRestartData data = { \ - file, servers, pass \ - }; \ - if (virtTestRun("ExecRestart " file, \ - testExecRestart, &data) < 0) \ - ret = -1; \ +# define EXEC_RESTART_TEST_FULL(file, nservers, pass) \ + do { \ + struct testExecRestartData data = { \ + file, server_names, nservers, pass \ + }; \ + if (virtTestRun("ExecRestart " file, \ + testExecRestart, &data) < 0) \ + ret = -1; \ } while (0) -# define EXEC_RESTART_TEST(file) EXEC_RESTART_TEST_FULL(file, 1, true) +# define EXEC_RESTART_TEST(file, N) EXEC_RESTART_TEST_FULL(file, N, true) +# define EXEC_RESTART_TEST_FAIL(file, N) EXEC_RESTART_TEST_FULL(file, N, false) + # ifdef WITH_AVAHI - EXEC_RESTART_TEST("initial"); + EXEC_RESTART_TEST("initial", 1); # endif - EXEC_RESTART_TEST("initial-nomdns"); - EXEC_RESTART_TEST("anon-clients"); - - EXEC_RESTART_TEST_FULL("anon-clients", 2, false); - EXEC_RESTART_TEST_FULL("admin-nomdns", 2, true); + EXEC_RESTART_TEST("initial-nomdns", 1); + EXEC_RESTART_TEST("anon-clients", 1); + EXEC_RESTART_TEST("admin-nomdns", 2); + EXEC_RESTART_TEST("admin-server-names", 2); + EXEC_RESTART_TEST_FAIL("anon-clients", 2); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3
This change is merely because admin_server would contain all the code from dispatchers and helpers to the actual APIs. Admin should have similar structure to the daemon-side remote driver - dispatchers and helpers in a separate module, APIs in a separate module. Best viewed with -M. Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/Makefile.am | 5 +- daemon/admin.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin.h | 36 +++++++++++++++ daemon/admin_server.c | 126 -------------------------------------------------- daemon/admin_server.h | 36 --------------- daemon/libvirtd.c | 2 +- po/POTFILES.in | 2 +- 7 files changed, 167 insertions(+), 166 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h delete mode 100644 daemon/admin_server.c delete mode 100644 daemon/admin_server.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index be1b5a9..9edaf5f 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -128,7 +128,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirtd_admin.la libvirtd_admin_la_SOURCES = \ - admin_server.c admin_server.h + admin.c admin.h libvirtd_admin_la_CFLAGS = \ $(AM_CFLAGS) \ @@ -319,7 +319,8 @@ endif ! WITH_POLKIT remote.c: $(DAEMON_GENERATED) remote.h: $(DAEMON_GENERATED) -admin_server.c: $(DAEMON_GENERATED) +admin.c: $(DAEMON_GENERATED) +admin.h: $(DAEMON_GENERATED) LOGROTATE_CONFS = libvirtd.qemu.logrotate libvirtd.lxc.logrotate \ libvirtd.libxl.logrotate libvirtd.uml.logrotate \ diff --git a/daemon/admin.c b/daemon/admin.c new file mode 100644 index 0000000..fa6caf3 --- /dev/null +++ b/daemon/admin.c @@ -0,0 +1,126 @@ +/* + * admin.c: handlers for admin RPC method calls + * + * Copyright (C) 2014-2016 Red Hat, Inc. + * + * 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/>. + * + * Author: Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "libvirtd.h" +#include "libvirt_internal.h" + +#include "admin_protocol.h" +#include "admin.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin"); + + +void +remoteAdmClientFreeFunc(void *data) +{ + struct daemonAdmClientPrivate *priv = data; + + virMutexDestroy(&priv->lock); + virObjectUnref(priv->dmn); + VIR_FREE(priv); +} + +void * +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + struct daemonAdmClientPrivate *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportSystemError(errno, "%s", _("unable to init mutex")); + return NULL; + } + + /* + * We don't necessarily need to ref this object right now as there + * must be one ref being held throughout the life of the daemon, + * but let's just be safe for future. + */ + priv->dmn = virObjectRef(opaque); + + return priv; +} + +/* Functions */ +static int +adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_connect_open_args *args) +{ + unsigned int flags; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + int ret = -1; + + VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); + virMutexLock(&priv->lock); + + flags = args->flags; + virCheckFlagsGoto(0, cleanup); + + ret = 0; + cleanup: + if (ret < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return ret; +} + +static int +adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) +{ + virNetServerClientDelayedClose(client); + return 0; +} + +static int +adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + unsigned long long *libVer) +{ + if (libVer) + *libVer = LIBVIR_VERSION_NUMBER; + return 0; +} + +#include "admin_dispatch.h" diff --git a/daemon/admin.h b/daemon/admin.h new file mode 100644 index 0000000..c869cc7 --- /dev/null +++ b/daemon/admin.h @@ -0,0 +1,36 @@ +/* + * admin.h: handlers for admin RPC method calls + * + * Copyright (C) 2014-2016 Red Hat, Inc. + * + * 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/>. + * + * Author: Martin Kletzander <mkletzan@redhat.com> + */ + +#ifndef __LIBVIRTD_ADMIN_H__ +# define __LIBVIRTD_ADMIN_H__ + +# include "rpc/virnetserverprogram.h" +# include "rpc/virnetserverclient.h" + + +extern virNetServerProgramProc adminProcs[]; +extern size_t adminNProcs; + +void remoteAdmClientFreeFunc(void *data); +void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); + +#endif /* __ADMIN_REMOTE_H__ */ diff --git a/daemon/admin_server.c b/daemon/admin_server.c deleted file mode 100644 index 189091e..0000000 --- a/daemon/admin_server.c +++ /dev/null @@ -1,126 +0,0 @@ -/* - * admin_server.c: - * - * Copyright (C) 2014-2015 Red Hat, Inc. - * - * 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/>. - * - * Author: Martin Kletzander <mkletzan@redhat.com> - */ - -#include <config.h> - -#include "internal.h" -#include "libvirtd.h" -#include "libvirt_internal.h" - -#include "admin_protocol.h" -#include "admin_server.h" -#include "datatypes.h" -#include "viralloc.h" -#include "virerror.h" -#include "virlog.h" -#include "virnetdaemon.h" -#include "virnetserver.h" -#include "virstring.h" -#include "virthreadjob.h" - -#define VIR_FROM_THIS VIR_FROM_ADMIN - -VIR_LOG_INIT("daemon.admin"); - - -void -remoteAdmClientFreeFunc(void *data) -{ - struct daemonAdmClientPrivate *priv = data; - - virMutexDestroy(&priv->lock); - virObjectUnref(priv->dmn); - VIR_FREE(priv); -} - -void * -remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, - void *opaque) -{ - struct daemonAdmClientPrivate *priv; - - if (VIR_ALLOC(priv) < 0) - return NULL; - - if (virMutexInit(&priv->lock) < 0) { - VIR_FREE(priv); - virReportSystemError(errno, "%s", _("unable to init mutex")); - return NULL; - } - - /* - * We don't necessarily need to ref this object right now as there - * must be one ref being held throughout the life of the daemon, - * but let's just be safe for future. - */ - priv->dmn = virObjectRef(opaque); - - return priv; -} - -/* Functions */ -static int -adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - struct admin_connect_open_args *args) -{ - unsigned int flags; - struct daemonAdmClientPrivate *priv = - virNetServerClientGetPrivateData(client); - int ret = -1; - - VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); - virMutexLock(&priv->lock); - - flags = args->flags; - virCheckFlagsGoto(0, cleanup); - - ret = 0; - cleanup: - if (ret < 0) - virNetMessageSaveError(rerr); - virMutexUnlock(&priv->lock); - return ret; -} - -static int -adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) -{ - virNetServerClientDelayedClose(client); - return 0; -} - -static int -adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, - unsigned long long *libVer) -{ - if (libVer) - *libVer = LIBVIR_VERSION_NUMBER; - return 0; -} - -#include "admin_dispatch.h" diff --git a/daemon/admin_server.h b/daemon/admin_server.h deleted file mode 100644 index 26721a6..0000000 --- a/daemon/admin_server.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * admin_server.h - * - * Copyright (C) 2014 Red Hat, Inc. - * - * 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/>. - * - * Author: Martin Kletzander <mkletzan@redhat.com> - */ - -#ifndef __LIBVIRTD_ADMIN_H__ -# define __LIBVIRTD_ADMIN_H__ - -# include "rpc/virnetserverprogram.h" -# include "rpc/virnetserverclient.h" - - -extern virNetServerProgramProc adminProcs[]; -extern size_t adminNProcs; - -void remoteAdmClientFreeFunc(void *data); -void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); - -#endif /* __ADMIN_REMOTE_H__ */ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index a747553..d1b98c9 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -44,7 +44,7 @@ #include "libvirtd.h" #include "libvirtd-config.h" -#include "admin_server.h" +#include "admin.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" diff --git a/po/POTFILES.in b/po/POTFILES.in index 4d82a8f..5fd6402 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,5 +1,5 @@ daemon/admin_dispatch.h -daemon/admin_server.c +daemon/admin.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.3
This is the key structure of all management operations performed on the daemon/clients. An admin client needs to be able to identify another client (either admin or non-privileged client) to perform an action on it. This identification includes a server the client is connected to, thus a client-side representation of a server is needed. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- include/libvirt/libvirt-admin.h | 17 +++++++++++++++++ src/admin/admin_protocol.x | 8 ++++++++ src/admin_protocol-structs | 3 +++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 38 ++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 3 +++ 6 files changed, 104 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index ab9df96..b342510 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -43,6 +43,14 @@ extern "C" { typedef struct _virAdmConnect virAdmConnect; /** + * virAdmServer: + * + * a virAdmServer is a private structure and client-side representation of + * a remote server object + */ +typedef struct _virAdmServer virAdmServer; + +/** * virAdmConnectPtr: * * a virAdmConnectPtr is pointer to a virAdmConnect private structure, @@ -51,6 +59,15 @@ typedef struct _virAdmConnect virAdmConnect; */ typedef virAdmConnect *virAdmConnectPtr; +/** + * virAdmServerPtr: + * + * a virAdmServerPtr is a pointer to a virAdmServer structure, + * this is the type used to reference client-side representation of a + * remote server object throughout all the APIs. + */ +typedef virAdmServer *virAdmServerPtr; + virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 711201a..549fdf3 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -32,12 +32,20 @@ */ const ADMIN_STRING_MAX = 4194304; +/* Upper limit on list of servers */ +const ADMIN_SERVER_LIST_MAX = 16384; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string<ADMIN_STRING_MAX>; /* A long string, which may be NULL. */ typedef admin_nonnull_string *admin_string; +/* A server which may NOT be NULL */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 809379b..95ea4e4 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,4 +1,7 @@ /* -*- c -*- */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; struct admin_connect_open_args { u_int flags; }; diff --git a/src/datatypes.c b/src/datatypes.c index c832d80..da6ec37 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -65,6 +65,9 @@ virClassPtr virAdmConnectCloseCallbackDataClass; static void virAdmConnectDispose(void *obj); static void virAdmConnectCloseCallbackDataDispose(void *obj); +virClassPtr virAdmServerClass; +static void virAdmServerDispose(void *obj); + static int virDataTypesOnceInit(void) { @@ -94,6 +97,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS_LOCKABLE(virAdmConnect); DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData); + DECLARE_CLASS(virAdmServer); #undef DECLARE_CLASS_COMMON #undef DECLARE_CLASS_LOCKABLE @@ -859,3 +863,34 @@ virAdmConnectCloseCallbackDataDispose(void *obj) virObjectUnlock(cb_data); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name) +{ + virAdmServerPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + goto error; + + if (!(ret = virObjectNew(virAdmServerClass))) + goto error; + if (VIR_STRDUP(ret->name, name) < 0) + goto error; + + ret->conn = virObjectRef(conn); + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server srv=%p name=%s", srv, srv->name); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index 1b1777d..31c636c 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -42,6 +42,7 @@ extern virClassPtr virStorageVolClass; extern virClassPtr virStoragePoolClass; extern virClassPtr virAdmConnectClass; +extern virClassPtr virAdmServerClass; # define virCheckConnectReturn(obj, retval) \ do { \ @@ -317,6 +318,30 @@ extern virClassPtr virAdmConnectClass; } \ } while (0) +# define virCheckAdmServerReturn(obj, retval) \ + do { \ + virAdmServerPtr _srv = (obj); \ + if (!virObjectIsClass(_srv, virAdmServerClass) || \ + !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) +# define virCheckAdmServerGoto(obj, label) \ + do { \ + virAdmServerPtr _srv = (obj); \ + if (!virObjectIsClass(_srv, virAdmServerClass) || \ + !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0); + /** * VIR_DOMAIN_DEBUG: * @dom: domain @@ -417,6 +442,17 @@ struct _virAdmConnect { virAdmConnectCloseCallbackDataPtr closeCallback; }; +/** + * _virAdmServer: + * + * Internal structure associated to a daemon server + */ +struct _virAdmServer { + virObject object; + virAdmConnectPtr conn; /* pointer back to the admin connection */ + char *name; /* the server external name */ +}; + /** * _virDomain: @@ -601,4 +637,6 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, virAdmConnectPtr virAdmConnectNew(void); +virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, + const char *name); #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 85380dc..31f1f8d 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,9 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_open_args; +# datatypes.h +virAdmGetServer; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t -- 2.4.3
On 12.02.2016 11:08, Erik Skultety wrote:
This is the key structure of all management operations performed on the daemon/clients. An admin client needs to be able to identify another client (either admin or non-privileged client) to perform an action on it. This identification includes a server the client is connected to, thus a client-side representation of a server is needed.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- include/libvirt/libvirt-admin.h | 17 +++++++++++++++++ src/admin/admin_protocol.x | 8 ++++++++ src/admin_protocol-structs | 3 +++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 38 ++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 3 +++ 6 files changed, 104 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index ab9df96..b342510 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -43,6 +43,14 @@ extern "C" { typedef struct _virAdmConnect virAdmConnect;
/** + * virAdmServer: + * + * a virAdmServer is a private structure and client-side representation of + * a remote server object + */ +typedef struct _virAdmServer virAdmServer; + +/** * virAdmConnectPtr: * * a virAdmConnectPtr is pointer to a virAdmConnect private structure, @@ -51,6 +59,15 @@ typedef struct _virAdmConnect virAdmConnect; */ typedef virAdmConnect *virAdmConnectPtr;
+/** + * virAdmServerPtr: + * + * a virAdmServerPtr is a pointer to a virAdmServer structure, + * this is the type used to reference client-side representation of a + * remote server object throughout all the APIs. + */ +typedef virAdmServer *virAdmServerPtr; + virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn);
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 711201a..549fdf3 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -32,12 +32,20 @@ */ const ADMIN_STRING_MAX = 4194304;
+/* Upper limit on list of servers */ +const ADMIN_SERVER_LIST_MAX = 16384; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string<ADMIN_STRING_MAX>;
/* A long string, which may be NULL. */ typedef admin_nonnull_string *admin_string;
+/* A server which may NOT be NULL */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 809379b..95ea4e4 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,4 +1,7 @@ /* -*- c -*- */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; struct admin_connect_open_args { u_int flags; }; diff --git a/src/datatypes.c b/src/datatypes.c index c832d80..da6ec37 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -65,6 +65,9 @@ virClassPtr virAdmConnectCloseCallbackDataClass; static void virAdmConnectDispose(void *obj); static void virAdmConnectCloseCallbackDataDispose(void *obj);
+virClassPtr virAdmServerClass; +static void virAdmServerDispose(void *obj); + static int virDataTypesOnceInit(void) { @@ -94,6 +97,7 @@ virDataTypesOnceInit(void)
DECLARE_CLASS_LOCKABLE(virAdmConnect); DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData); + DECLARE_CLASS(virAdmServer);
#undef DECLARE_CLASS_COMMON #undef DECLARE_CLASS_LOCKABLE @@ -859,3 +863,34 @@ virAdmConnectCloseCallbackDataDispose(void *obj)
virObjectUnlock(cb_data); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name) +{ + virAdmServerPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + goto error; + + if (!(ret = virObjectNew(virAdmServerClass))) + goto error; + if (VIR_STRDUP(ret->name, name) < 0) + goto error; + + ret->conn = virObjectRef(conn); + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server srv=%p name=%s", srv, srv->name); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index 1b1777d..31c636c 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -42,6 +42,7 @@ extern virClassPtr virStorageVolClass; extern virClassPtr virStoragePoolClass;
extern virClassPtr virAdmConnectClass; +extern virClassPtr virAdmServerClass;
# define virCheckConnectReturn(obj, retval) \ do { \ @@ -317,6 +318,30 @@ extern virClassPtr virAdmConnectClass; } \ } while (0)
+# define virCheckAdmServerReturn(obj, retval) \ + do { \ + virAdmServerPtr _srv = (obj); \ + if (!virObjectIsClass(_srv, virAdmServerClass) || \ + !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) +# define virCheckAdmServerGoto(obj, label) \ + do { \ + virAdmServerPtr _srv = (obj); \ + if (!virObjectIsClass(_srv, virAdmServerClass) || \ + !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0); + /** * VIR_DOMAIN_DEBUG: * @dom: domain @@ -417,6 +442,17 @@ struct _virAdmConnect { virAdmConnectCloseCallbackDataPtr closeCallback; };
+/** + * _virAdmServer: + * + * Internal structure associated to a daemon server + */ +struct _virAdmServer { + virObject object; + virAdmConnectPtr conn; /* pointer back to the admin connection */ + char *name; /* the server external name */ +}; +
/** * _virDomain: @@ -601,4 +637,6 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
virAdmConnectPtr virAdmConnectNew(void);
+virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, + const char *name); #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 85380dc..31f1f8d 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,9 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_open_args;
+# datatypes.h +virAdmGetServer;
I guess you'll need to expose virAdmServerClass too. Otherwise you may get into linking troubles since in virCheckAdmServer*() macros defined above the symbol is accessed directly. ACK with that changed. Michal
This API is merely a convenience API, i.e. when managing clients connected to daemon's servers, we should know (convenience) which server the specific client is connected to. This implies a client-side representation of a server along with a basic API to let the administrating client know what servers are actually available on the daemon. Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/Makefile.am | 2 +- daemon/admin.c | 55 ++++++++++++++++++++++++++++ daemon/admin_server.c | 73 +++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 34 ++++++++++++++++++ include/libvirt/libvirt-admin.h | 8 ++++- po/POTFILES.in | 1 + src/admin/admin_protocol.x | 18 +++++++++- src/admin/admin_remote.c | 71 ++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 12 +++++++ src/libvirt-admin.c | 79 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 3 ++ src/rpc/virnetdaemon.c | 39 ++++++++++++++++++++ src/rpc/virnetdaemon.h | 1 + 14 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 9edaf5f..2dbe81b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -128,7 +128,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirtd_admin.la libvirtd_admin_la_SOURCES = \ - admin.c admin.h + admin.c admin.h admin_server.c admin_server.h libvirtd_admin_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/daemon/admin.c b/daemon/admin.c index fa6caf3..0c1ddc0 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -28,6 +28,7 @@ #include "admin_protocol.h" #include "admin.h" +#include "admin_server.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" @@ -77,6 +78,15 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; } +/* Helpers */ + +static void +make_nonnull_server(admin_nonnull_server *srv_dst, + virAdmServerPtr srv_src) +{ + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -123,4 +133,49 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; } +static int +adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + admin_connect_list_servers_args *args, + admin_connect_list_servers_ret *ret) +{ + virAdmServerPtr *servers = NULL; + int nservers = 0; + int rv = -1; + size_t i; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if ((nservers = + adminDaemonListServers(priv->dmn, + args->need_results ? &servers : NULL, + args->flags)) < 0) + goto cleanup; + + if (servers && nservers) { + if (VIR_ALLOC_N(ret->servers.servers_val, nservers) < 0) + goto cleanup; + + ret->servers.servers_len = nservers; + for (i = 0; i < nservers; i++) + make_nonnull_server(ret->servers.servers_val + i, servers[i]); + } else { + ret->servers.servers_len = 0; + ret->servers.servers_val = NULL; + } + + ret->ret = nservers; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (servers && nservers > 0) + for (i = 0; i < nservers; i++) + virObjectUnref(servers[i]); + VIR_FREE(servers); + return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..a054421 --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,73 @@ +/* + * admin_server.c: admin methods to manage daemons and clients + * + * Copyright (C) 2016 Red Hat, Inc. + * + * 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/>. + * + * Authors: Erik Skultety <eskultet@redhat.com> + * Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin_server"); + +int +adminDaemonListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + const char **srv_names = NULL; + virAdmServerPtr *srvs = NULL; + size_t i; + ssize_t nsrvs = 0; + + virCheckFlags(0, -1); + + if ((nsrvs = virNetDaemonGetServerNames(dmn, &srv_names)) < 0) + goto cleanup; + + if (servers) { + if (VIR_ALLOC_N(srvs, nsrvs + 1) < 0) + goto cleanup; + + for (i = 0; i < nsrvs; i++) { + if (!(srvs[i] = virAdmGetServer(NULL, srv_names[i]))) + goto cleanup; + } + + *servers = srvs; + srvs = NULL; + } + + ret = nsrvs; + + cleanup: + virObjectListFree(srvs); + return ret; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h new file mode 100644 index 0000000..2a5aa16 --- /dev/null +++ b/daemon/admin_server.h @@ -0,0 +1,34 @@ +/* + * admin_server.h: admin methods to manage daemons and clients + * + * Copyright (C) 2016 Red Hat, Inc. + * + * 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/>. + * + * Authors: Erik Skultety <eskultet@redhat.com> + * Martin Kletzander <mkletzan@redhat.com> + */ + +#ifndef __LIBVIRTD_ADMIN_SERVER_H__ +# define __LIBVIRTD_ADMIN_SERVER_H__ + +# include "rpc/virnetdaemon.h" + +int +adminDaemonListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags); + +#endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index b342510..e9ec394 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -70,9 +70,13 @@ typedef virAdmServer *virAdmServerPtr; virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); - int virAdmConnectRef(virAdmConnectPtr conn); int virAdmConnectIsAlive(virAdmConnectPtr conn); +int virAdmServerFree(virAdmServerPtr srv); + +int virAdmConnectListServers(virAdmConnectPtr dmn, + virAdmServerPtr **servers, + unsigned int flags); int virAdmGetVersion(unsigned long long *libVer); @@ -100,6 +104,8 @@ int virAdmConnectRegisterCloseCallback(virAdmConnectPtr conn, int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn, virAdmConnectCloseFunc cb); +const char *virAdmServerGetName(virAdmServerPtr srv); + # ifdef __cplusplus } # endif diff --git a/po/POTFILES.in b/po/POTFILES.in index 5fd6402..8f87a33 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -10,6 +10,7 @@ gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c +src/admin/admin_remote.c src/bhyve/bhyve_command.c src/bhyve/bhyve_device.c src/bhyve/bhyve_driver.c diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 549fdf3..089ce57 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -55,6 +55,16 @@ struct admin_connect_get_lib_version_ret { unsigned hyper libVer; }; +struct admin_connect_list_servers_args { + unsigned int need_results; + unsigned int flags; +}; + +struct admin_connect_list_servers_ret { + admin_nonnull_server servers<ADMIN_SERVER_LIST_MAX>; + unsigned int ret; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -90,5 +100,11 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3 + ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, + + /** + * @generate: none + * @priority: high + */ + ADMIN_PROC_CONNECT_LIST_SERVERS = 4 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 7b40ea1..e634fa7 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -47,6 +47,14 @@ remoteAdminPrivDispose(void *opaque) } +/* Helpers */ +static virAdmServerPtr +get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server) +{ + return virAdmGetServer(conn, server.name); +} + + static int callFull(virAdmConnectPtr conn ATTRIBUTE_UNUSED, remoteAdminPrivPtr priv, @@ -214,3 +222,66 @@ remoteAdminPrivNew(const char *sock_path) virObjectUnref(priv); return NULL; } + +static int +remoteAdminConnectListServers(virAdmConnectPtr conn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int rv = -1; + size_t i; + virAdmServerPtr *tmp_srvs = NULL; + remoteAdminPrivPtr priv = conn->privateData; + admin_connect_list_servers_args args; + admin_connect_list_servers_ret ret; + + args.need_results = !!servers; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + virObjectLock(priv); + + if (call(conn, + 0, + ADMIN_PROC_CONNECT_LIST_SERVERS, + (xdrproc_t) xdr_admin_connect_list_servers_args, + (char *) &args, + (xdrproc_t) xdr_admin_connect_list_servers_ret, + (char *) &ret) == -1) + goto done; + + if (ret.servers.servers_len > ADMIN_SERVER_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many servers '%d' for limit '%d'"), + ret.servers.servers_len, ADMIN_SERVER_LIST_MAX); + goto cleanup; + } + + if (servers) { + if (VIR_ALLOC_N(tmp_srvs, ret.servers.servers_len + 1) < 0) + goto cleanup; + + for (i = 0; i < ret.servers.servers_len; i++) { + tmp_srvs[i] = get_nonnull_server(conn, ret.servers.servers_val[i]); + if (!tmp_srvs[i]) + goto cleanup; + } + *servers = tmp_srvs; + tmp_srvs = NULL; + } + + rv = ret.ret; + + cleanup: + if (tmp_srvs) { + for (i = 0; i < ret.servers.servers_len; i++) + virObjectUnref(tmp_srvs[i]); + VIR_FREE(tmp_srvs); + } + + xdr_free((xdrproc_t) xdr_admin_connect_list_servers_ret, (char *) &ret); + + done: + virObjectUnlock(priv); + return rv; +} diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 95ea4e4..8f2633a 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -8,8 +8,20 @@ struct admin_connect_open_args { struct admin_connect_get_lib_version_ret { uint64_t libVer; }; +struct admin_connect_list_servers_args { + u_int need_results; + u_int flags; +}; +struct admin_connect_list_servers_ret { + struct { + u_int servers_len; + admin_nonnull_server * servers_val; + } servers; + u_int ret; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, + ADMIN_PROC_CONNECT_LIST_SERVERS = 4, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 6e6da81..3667444 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -549,7 +549,86 @@ int virAdmConnectGetLibVersion(virAdmConnectPtr conn, goto error; return 0; + error: + virDispatchError(NULL); + return -1; +} + +/** + * virAdmServerGetName: + * @srv: a server object + * + * Get the public name for specified server + * + * Returns a pointer to the name or NULL. The string doesn't need to be + * deallocated since its lifetime will be the same as the server object. + */ +const char * +virAdmServerGetName(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + + virResetLastError(); + virCheckAdmServerReturn(srv, NULL); + + return srv->name; +} + +/** + * virAdmServerFree: + * @srv: server object + * + * Release the server object. The running instance is kept alive. + * The data structure is freed and should not be used thereafter. + * + * Returns 0 on success, -1 on failure. + */ +int virAdmServerFree(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + virResetLastError(); + virCheckAdmServerReturn(srv, -1); + + virObjectUnref(srv); + return 0; +} + +/** + * virAdmConnectListServers: + * @conn: daemon connection reference + * @servers: Pointer to a list to store an array containing objects or NULL + * if the list is not required (number of servers only) + * @flags: bitwise-OR of virAdmConnectListServersFlags + * + * Collect list of all servers provided by daemon the client is connected to. + * + * Returns the number of servers available on daemon side or -1 in case of a + * failure, setting @servers to NULL. There is a guaranteed extra element set + * to NULL in the @servers list returned to make the iteration easier, excluding + * this extra element from the final count. + * Caller is responsible to call virAdmServerFree() on each list element, + * followed by freeing @servers. + */ +int +virAdmConnectListServers(virAdmConnectPtr conn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags); + + virResetLastError(); + + if (servers) + *servers = NULL; + + virCheckAdmConnectReturn(conn, -1); + if ((ret = remoteAdminConnectListServers(conn, servers, flags)) < 0) + goto error; + + return ret; error: virDispatchError(NULL); return -1; diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 31f1f8d..ae6b9dd 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -7,6 +7,8 @@ # admin/admin_protocol.x xdr_admin_connect_get_lib_version_ret; +xdr_admin_connect_list_servers_args; +xdr_admin_connect_list_servers_ret; xdr_admin_connect_open_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 33b1db4..52ff2dc 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -21,4 +21,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectGetLibVersion; virAdmConnectRegisterCloseCallback; virAdmConnectUnregisterCloseCallback; + virAdmConnectListServers; + virAdmServerGetName; + virAdmServerFree; }; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 9210de4..298fbf4 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -190,6 +190,45 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, return srv; } + +/* + * Returns number of names allocated in *servers, on error sets + * *servers to NULL and returns -1. List of *servers must be free()d, + * but not the items in it (similarly to virHashGetItems). + */ +ssize_t +virNetDaemonGetServerNames(virNetDaemonPtr dmn, + const char ***servers) +{ + virHashKeyValuePairPtr items = NULL; + size_t nservers = 0; + ssize_t ret = -1; + size_t i; + + *servers = NULL; + + virObjectLock(dmn); + + items = virHashGetItems(dmn->servers, NULL); + if (!items) + goto cleanup; + + for (i = 0; items[i].key; i++) { + if (VIR_APPEND_ELEMENT(*servers, nservers, items[i].key) < 0) + goto cleanup; + } + + ret = nservers; + + cleanup: + if (ret < 0) + VIR_FREE(*servers); + VIR_FREE(items); + virObjectUnlock(dmn); + return ret; +} + + virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, const char *serverName, diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 968708b..9a3404f 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -83,5 +83,6 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, const char *serverName); +ssize_t virNetDaemonGetServerNames(virNetDaemonPtr dmn, const char ***servers); #endif /* __VIR_NET_DAEMON_H__ */ -- 2.4.3
On 12.02.2016 11:08, Erik Skultety wrote:
This API is merely a convenience API, i.e. when managing clients connected to daemon's servers, we should know (convenience) which server the specific client is connected to. This implies a client-side representation of a server along with a basic API to let the administrating client know what servers are actually available on the daemon.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- daemon/Makefile.am | 2 +- daemon/admin.c | 55 ++++++++++++++++++++++++++++ daemon/admin_server.c | 73 +++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 34 ++++++++++++++++++ include/libvirt/libvirt-admin.h | 8 ++++- po/POTFILES.in | 1 + src/admin/admin_protocol.x | 18 +++++++++- src/admin/admin_remote.c | 71 ++++++++++++++++++++++++++++++++++++ src/admin_protocol-structs | 12 +++++++ src/libvirt-admin.c | 79 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 3 ++ src/rpc/virnetdaemon.c | 39 ++++++++++++++++++++ src/rpc/virnetdaemon.h | 1 + 14 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 9edaf5f..2dbe81b 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -128,7 +128,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS)
noinst_LTLIBRARIES += libvirtd_admin.la libvirtd_admin_la_SOURCES = \ - admin.c admin.h + admin.c admin.h admin_server.c admin_server.h
libvirtd_admin_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/daemon/admin.c b/daemon/admin.c index fa6caf3..0c1ddc0 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -28,6 +28,7 @@
#include "admin_protocol.h" #include "admin.h" +#include "admin_server.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" @@ -77,6 +78,15 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; }
+/* Helpers */ + +static void +make_nonnull_server(admin_nonnull_server *srv_dst, + virAdmServerPtr srv_src) +{ + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -123,4 +133,49 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; }
+static int +adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + admin_connect_list_servers_args *args, + admin_connect_list_servers_ret *ret) +{ + virAdmServerPtr *servers = NULL; + int nservers = 0; + int rv = -1; + size_t i; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if ((nservers = + adminDaemonListServers(priv->dmn, + args->need_results ? &servers : NULL, + args->flags)) < 0) + goto cleanup; + + if (servers && nservers) { + if (VIR_ALLOC_N(ret->servers.servers_val, nservers) < 0) + goto cleanup; + + ret->servers.servers_len = nservers; + for (i = 0; i < nservers; i++) + make_nonnull_server(ret->servers.servers_val + i, servers[i]); + } else { + ret->servers.servers_len = 0; + ret->servers.servers_val = NULL; + } + + ret->ret = nservers; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (servers && nservers > 0) + for (i = 0; i < nservers; i++) + virObjectUnref(servers[i]); + VIR_FREE(servers); + return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..a054421 --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,73 @@ +/* + * admin_server.c: admin methods to manage daemons and clients + * + * Copyright (C) 2016 Red Hat, Inc. + * + * 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/>. + * + * Authors: Erik Skultety <eskultet@redhat.com> + * Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin_server"); + +int +adminDaemonListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + const char **srv_names = NULL; + virAdmServerPtr *srvs = NULL; + size_t i; + ssize_t nsrvs = 0; + + virCheckFlags(0, -1); + + if ((nsrvs = virNetDaemonGetServerNames(dmn, &srv_names)) < 0) + goto cleanup; + + if (servers) { + if (VIR_ALLOC_N(srvs, nsrvs + 1) < 0)
Here ^^ and below you are allocating just one more over what's needed. So while you return the array size, you want it to be NULL terminated too? Is that to be super safe?
+ goto cleanup; + + for (i = 0; i < nsrvs; i++) { + if (!(srvs[i] = virAdmGetServer(NULL, srv_names[i]))) + goto cleanup; + } + + *servers = srvs; + srvs = NULL; + } + + ret = nsrvs; + + cleanup: + virObjectListFree(srvs); + return ret; +}
Tentative ACK meanwhile. Michal
Since we introduced listing API earlier in these series, it's time to wire up the API to the virt-admin client. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 1372963..23af274 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -300,6 +300,56 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) return !!priv->conn; } + +/* --------------- + * Command Srv-list + * --------------- + */ + +static const vshCmdInfo info_srv_list[] = { + {.name = "help", + .data = N_("list available servers on a daemon") + }, + {.name = "desc", + .data = N_("List all manageable servers on a daemon.") + }, + {.name = NULL} +}; + +static bool +cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int nsrvs = 0; + size_t i; + bool ret = false; + const char *uri = NULL; + virAdmServerPtr *srvs = NULL; + vshAdmControlPtr priv = ctl->privData; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(priv->conn, &srvs, 0)) < 0) { + uri = virAdmConnectGetURI(priv->conn); + vshError(ctl, _("failed to obtain list of available servers from %s"), + NULLSTR(uri)); + goto cleanup; + } + + printf(" %-5s %-15s\n", "Id", "Name"); + printf("---------------\n"); + for (i = 0; i < nsrvs; i++) + vshPrint(ctl, " %-5zu %-15s\n", i, virAdmServerGetName(srvs[i])); + + ret = true; + cleanup: + if (srvs) { + for (i = 0; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + VIR_FREE(srvs); + } + + return ret; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -592,8 +642,19 @@ static const vshCmdDef vshAdmCmds[] = { {.name = NULL} }; +static const vshCmdDef monitoringCmds[] = { + {.name = "srv-list", + .handler = cmdSrvList, + .opts = NULL, + .info = info_srv_list, + .flags = 0 + }, + {.name = NULL} +}; + static const vshCmdGrp cmdGroups[] = { {"Virt-admin itself", "virt-admin", vshAdmCmds}, + {"Monitoring commands", "monitor", monitoringCmds}, {NULL, NULL, NULL} }; -- 2.4.3
On 12.02.2016 11:08, Erik Skultety wrote:
Since we introduced listing API earlier in these series, it's time to wire up the API to the virt-admin client.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 1372963..23af274 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -300,6 +300,56 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) return !!priv->conn; }
+ +/* --------------- + * Command Srv-list + * --------------- + */ + +static const vshCmdInfo info_srv_list[] = { + {.name = "help", + .data = N_("list available servers on a daemon") + }, + {.name = "desc", + .data = N_("List all manageable servers on a daemon.") + }, + {.name = NULL} +}; + +static bool +cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int nsrvs = 0; + size_t i; + bool ret = false; + const char *uri = NULL; + virAdmServerPtr *srvs = NULL; + vshAdmControlPtr priv = ctl->privData; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(priv->conn, &srvs, 0)) < 0) { + uri = virAdmConnectGetURI(priv->conn); + vshError(ctl, _("failed to obtain list of available servers from %s"), + NULLSTR(uri)); + goto cleanup; + } + + printf(" %-5s %-15s\n", "Id", "Name"); + printf("---------------\n"); + for (i = 0; i < nsrvs; i++) + vshPrint(ctl, " %-5zu %-15s\n", i, virAdmServerGetName(srvs[i])); + + ret = true; + cleanup: + if (srvs) { + for (i = 0; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + VIR_FREE(srvs); + }
Or: for (i = 0; i < nsrvs && srvs; i++); I've noticed this could be changed in previous patch too. The similar pattern appears there. But it's your call what you prefer more. ACK Michal
On 12.02.2016 11:08, Erik Skultety wrote:
Since v3: - refactors to virHashForEach and hash iterators - servers are now stored in a hash table instead of an array - added forgotten virObjectLock in remoteAdminConnectListServers
Erik Skultety (7): util: Add a return value to void hash iterators util: Refactor virHashForEach so it returns as soon as an iterator fails virnetdaemon: Store servers in a hash table admin: Move admin_server.{h,c} to admin.{h,c} admin: Introduce virAdmServer structure admin: Introduce adminDaemonConnectListServers API virt-admin: Introduce cmdSrvList
daemon/Makefile.am | 5 +- daemon/admin.c | 181 +++++++++++++++ daemon/admin.h | 36 +++ daemon/admin_server.c | 111 +++------- daemon/admin_server.h | 26 +-- daemon/libvirtd.c | 6 +- include/libvirt/libvirt-admin.h | 25 ++- po/POTFILES.in | 3 +- src/admin/admin_protocol.x | 26 ++- src/admin/admin_remote.c | 71 ++++++ src/admin_protocol-structs | 15 ++ src/conf/network_conf.c | 15 +- src/conf/nwfilter_params.c | 8 +- src/conf/snapshot_conf.c | 31 +-- src/conf/virchrdev.c | 7 +- src/conf/virdomainobjlist.c | 17 +- src/datatypes.c | 35 +++ src/datatypes.h | 38 ++++ src/libvirt-admin.c | 79 +++++++ src/libvirt_admin_private.syms | 5 + src/libvirt_admin_public.syms | 3 + src/locking/lock_daemon.c | 10 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_controller.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 3 +- src/nwfilter/nwfilter_gentech_driver.c | 5 +- src/qemu/qemu_domain.c | 7 +- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 13 +- src/rpc/virnetdaemon.c | 225 +++++++++++++------ src/rpc/virnetdaemon.h | 8 +- src/test/test_driver.c | 14 +- src/uml/uml_driver.c | 13 +- src/util/virclosecallbacks.c | 9 +- src/util/virhash.c | 28 ++- src/util/virhash.h | 6 +- src/xen/xm_internal.c | 7 +- tests/virhashtest.c | 54 +++-- .../input-data-admin-server-names.json | 129 +++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 8 +- .../output-data-admin-server-names.json | 126 +++++++++++ .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 242 +++++++++++---------- tests/virnetdaemontest.c | 63 +++--- tools/virt-admin.c | 61 ++++++ 47 files changed, 1361 insertions(+), 442 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h create mode 100644 tests/virnetdaemondata/input-data-admin-server-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-server-names.json
ACK series modulo 6/7 as I have some questions there. Michal
participants (2)
-
Erik Skultety -
Michal Privoznik