On Fri, Nov 11, 2011 at 16:21:59 +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Directly messing around with the linked list is potentially
> dangerous. Introduce some helper APIs to deal with list
> manipulating the list
>
> * src/rpc/virnetclient.c: Create linked list handlers
> ---
> src/rpc/virnetclient.c | 207 +++++++++++++++++++++++++++++++++---------------
> 1 files changed, 144 insertions(+), 63 deletions(-)
>
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 4b7d4a9..463dc5a 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -110,6 +110,97 @@ static void virNetClientIncomingEvent(virNetSocketPtr sock,
> int events,
> void *opaque);
>
> +/* Append a call to the end of the list */
> +static void virNetClientCallQueue(virNetClientCallPtr *head,
> + virNetClientCallPtr call)
> +{
> + virNetClientCallPtr tmp = *head;
> + while (tmp && tmp->next) {
> + tmp = tmp->next;
> + }
> + if (tmp)
> + tmp->next = call;
> + else
> + *head = call;
> + call->next = NULL;
> +}
> +
> +#if 0
> +/* Obtain a call from the head of the list */
> +static virNetClientCallPtr virNetClientCallServe(virNetClientCallPtr *head)
> +{
> + virNetClientCallPtr tmp = *head;
> + if (tmp)
> + *head = tmp->next;
> + else
> + *head = NULL;
> + tmp->next = NULL;
> + return tmp;
> +}
> +#endif
> +
> +/* Remove a call from anywhere in the list */
> +static void virNetClientCallRemove(virNetClientCallPtr *head,
> + virNetClientCallPtr call)
> +{
> + virNetClientCallPtr tmp = *head;
> + virNetClientCallPtr prev = NULL;
> + while (tmp) {
> + if (tmp == call) {
> + if (prev)
> + prev->next = tmp->next;
> + else
> + *head = tmp->next;
> + tmp->next = NULL;
> + return;
> + }
> + prev = tmp;
> + tmp = tmp->next;
> + }
> +}
> +
> +/* Predicate returns true if matches */
> +typedef bool (*virNetClientCallPredicate)(virNetClientCallPtr call, void *opaque);
> +
> +/* Remove a list of calls from the list based on a predicate */
> +static void virNetClientCallRemovePredicate(virNetClientCallPtr *head,
> + virNetClientCallPredicate pred,
> + void *opaque)
> +{
> + virNetClientCallPtr tmp = *head;
> + virNetClientCallPtr prev = NULL;
> + while (tmp) {
> + virNetClientCallPtr next = tmp->next;
> + tmp->next = NULL; /* Temp unlink */
> + if (pred(tmp, opaque)) {
> + if (prev)
> + prev->next = next;
> + else
> + *head = next;
> + } else {
> + tmp->next = next; /* Reverse temp unlink */
> + prev = tmp;
> + }
> + tmp = next;
> + }
> +}
> +
> +/* Returns true if the predicate matches at least one call in the list */
> +static bool virNetClientCallMatchPredicate(virNetClientCallPtr head,
> + virNetClientCallPredicate pred,
> + void *opaque)
> +{
> + virNetClientCallPtr tmp = head;
> + while (tmp) {
> + if (pred(tmp, opaque)) {
> + return true;
> + }
> + tmp = tmp->next;
> + }
> + return false;
> +}
> +
> +
> static void virNetClientEventFree(void *opaque)
> {
> virNetClientPtr client = opaque;
> @@ -896,6 +987,42 @@ virNetClientIOHandleInput(virNetClientPtr client)
> }
>
>
> +static bool virNetClientIOEventLoopPollEvents(virNetClientCallPtr call,
> + void *opaque)
> +{
> + struct pollfd *fd = opaque;
> +
> + if (call->mode == VIR_NET_CLIENT_MODE_WAIT_RX)
> + fd->events |= POLLIN;
> + if (call->mode == VIR_NET_CLIENT_MODE_WAIT_TX)
> + fd->events |= POLLOUT;
> +
> + return true;
> +}
This should return false otherwise we only set fd->events according to the
first call in the queue. We need to go through all calls.
> +
> +
> +static bool virNetClientIOEventLoopRemoveDone(virNetClientCallPtr call,
> + void *opaque)
> +{
> + virNetClientCallPtr thiscall = opaque;
> +
> + if (call == thiscall)
> + return false;
> +
> + if (call->mode != VIR_NET_CLIENT_MODE_COMPLETE)
> + return false;
> +
> + /*
> + * ...they won't actually wakeup until
> + * we release our mutex a short while
> + * later...
> + */
> + VIR_DEBUG("Waking up sleeping call %p", call);
> + virCondSignal(&call->cond);
> +
> + return true;
> +}
> +
> /*
> * Process all calls pending dispatch/receive until we
> * get a reply to our own call. Then quit and pass the buck
> @@ -911,8 +1038,6 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
> fds[1].fd = client->wakeupReadFD;
>
> for (;;) {
> - virNetClientCallPtr tmp = client->waitDispatch;
> - virNetClientCallPtr prev;
> char ignore;
> sigset_t oldmask, blockedsigs;
> int timeout = -1;
> @@ -928,14 +1053,11 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
> fds[1].events = fds[1].revents = 0;
>
> fds[1].events = POLLIN;
> - while (tmp) {
> - if (tmp->mode == VIR_NET_CLIENT_MODE_WAIT_RX)
> - fds[0].events |= POLLIN;
> - if (tmp->mode == VIR_NET_CLIENT_MODE_WAIT_TX)
> - fds[0].events |= POLLOUT;
>
> - tmp = tmp->next;
> - }
> + /* Calculate poll events for calls */
> + virNetClientCallMatchPredicate(client->waitDispatch,
> + virNetClientIOEventLoopPollEvents,
> + &fds[0]);
>
> /* We have to be prepared to receive stream data
> * regardless of whether any of the calls waiting
...
And we should avoid lines longer than 80 character, which is mostly not a
problem of this patch but some of the other patches in this series don't
follow this rule (mainly in function prototypes).
ACK with the small issue fixed.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list