Re: [libvirt] PATCH: 17/25: Concurrent client dispatch in libvirtd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 19, 2009 at 10:56:18PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]