> On Nov 23, 2017, at 9:29 AM, Kevin Wolf <kw...@redhat.com> wrote: > > Am 23.11.2017 um 18:05 hat Deepa Srinivasan geschrieben: >> blk_aio_prwv() now takes a void pointer and the coroutine functions >> have been modified to cast it into QEMUIOVector if needed. It does not >> use an union in BlkRwCo since this leads to code - blk_aio_prwv() >> would have to write to the void pointer member, but coroutines would >> sometimes read the QEMUIOVector member. Paolo also suggested not using >> a union. > > I don't particularly like void pointers, but I guess it's fair enough.
Agreed, but if a union were to hold QEMUIOVector* and void* in BlkRwCo, blk_aio_prwv() would always write to void* but some coroutine functions would read from the QEMUIOVector* member. Keeping it as a void pointer is a safer option. > >> Note that a similar issue exists in >> blk_ioctl()/blk_ioctl_entry()/blk_prw() where blk_prw() always creates >> the QEMUIOVector even if blk_ioctl()/blk_ioctl_entry() does not need a >> QEMUIOVector. This will need to be fixed separately to keep it >> consistent with the AIO path. > > I don't think there is an actual problem in the blk_ioctl() path because > the iov on the stack stays valid as long as the coroutine runs. AIO is > different because it returns before the coroutine has terminated. > The problem in blk_ioctl() is not a crash, because blk_prwv() waits for the coroutine completion, as you say. The issue is that it unnecessarily creates a QEMUIOVector for the ioctl case. I was saying, if this is to be kept consistent with the AIO patch, then it could be done in a separate patch. > Kevin >
