[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] block: Convert bdrv_first to QTAILQ
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] block: Convert bdrv_first to QTAILQ |
Date: |
Fri, 9 Apr 2010 19:17:45 +0100 |
On Fri, Apr 9, 2010 at 5:47 PM, Kevin Wolf <address@hidden> wrote:
>> @@ -545,13 +542,15 @@ void bdrv_close(BlockDriverState *bs)
>>
>> void bdrv_delete(BlockDriverState *bs)
>> {
>> - BlockDriverState **pbs;
>> + BlockDriverState *bs1;
>>
>> - pbs = &bdrv_first;
>> - while (*pbs != bs && *pbs != NULL)
>> - pbs = &(*pbs)->next;
>> - if (*pbs == bs)
>> - *pbs = bs->next;
>> + /* remove from list, if necessary */
>> + QTAILQ_FOREACH(bs1, &bdrv_states, list) {
>> + if (bs1 == bs) {
>> + QTAILQ_REMOVE(&bdrv_states, bs, list);
>> + break;
>> + }
>> + }
>
> This loop looks strange, what is it used for? We had only a next pointer
> previously, so we needed to search the element. A QTAILQ has both prev
> and next pointers though, so you should be able to directly use
> QTAILQ_REMOVE.
Only named BlockDriverStates are added to the tail queue. Those with
an empty string as their name will not be on the tail queue.
The QTAILQ_REMOVE macro will not work if the element isn't on the tail
queue. I didn't see a clean way to check if an element is on a tail
queue using qemu-queue.h, so I kept the search behavior. The check I
want is tge_prev != NULL, I think.
I see three options:
1. Leave the search.
2. Modify qemu-queue.h to add a QTAILQ_ON_LIST(elm) macro.
3. Break the QTAILQ abstraction and test tge_prev directly.
What do you think?
Stefan