On Fri, Apr 9, 2010 at 5:47 PM, Kevin Wolf <kw...@redhat.com> 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