Re: [libvirt] PATCH: 17/25: Concurrent client dispatch in libvirtd
On Mon, Jan 19, 2009 at 10:56:18PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Tue, Jan 13, 2009 at 05:45:43PM +0000, Daniel P. Berrange wrote:
>> Historically libvirtd was single threaded, serializing all
>> requests across clients. An recent patch allowed multiple
>> threads, so multiple clients could run in parallel. A single
>> client was still serialized.
...
I haven't finished this one, but here's partial feedback:
> +void
> +qemudClientMessageQueuePush(struct qemud_client_message **queue,
> + struct qemud_client_message *msg)
> +{
> + struct qemud_client_message *tmp = *queue;
> +
> + if (tmp) {
> + while (tmp->next)
> + tmp = tmp->next;
> + tmp->next = msg;
> + } else {
> + *queue = msg;
> + }
> +}
> +
> +static struct qemud_client_message *
> +qemudClientMessageQueuePop(struct qemud_client_message **queue)
> +{
> + struct qemud_client_message *tmp = *queue;
> +
> + if (tmp)
> + *queue = tmp->next;
> + else
> + *queue = NULL;
If tmp really can be NULL (tested for above),
then you can't dereference it below.
Also, since ...QueuePush appends,
I would have expected ...QueuePop to remove from the end.
The daemon needs a FIFO queue for fairness, so removing from head
is correct. To avoid confusion with Perl's push/pop/shift/unshift
semantics I should rename this Pop method to Shift instead.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|