On Wed, Jul 2, 2014 at 8:46 AM, Ming Lei <[email protected]> wrote: > On Wed, Jul 2, 2014 at 12:56 AM, Paolo Bonzini <[email protected]> wrote: >> Il 01/07/2014 17:21, Kevin Wolf ha scritto: >> >>>>>> Does this bs->file forwarding work for more than the raw driver? For >>>>>> example, if drv is an image format driver that needs to read some >>>>>> metadata from the image before it can submit the payload, does this >>>>>> still do what you were intending? >>>> >>>> >>>> Sorry for not understanding the problem, and you are right, these >>>> patches can't support other formats, and for solving the dependency, >>>> changes to image format driver should be needed. >>> >>> >>> Then let's drop the bs->file recursion here and add an explicit >>> .bdrv_io_plug/unplug callback to the raw driver. >> >> >> Actually I thought about this in my review, and there's no reason for this >> not to work for image formats. >> >> While bs->file is plugged, image formats will start executing their >> bdrv_co_readv/bdrv_co_writev callbacks, and issue reads or writes as >> necessary. The reads and writes will accumulate in bs->file until it is >> unplugged, which is exactly the effect we want. > > For some image formats, meta data need to be read first before > the payload can be read since how/what to read payload might > depend on content of meta data. > >> >> The change in bdrv_drain_all is ugly though. I don't have a better idea, >> but I would like to understand better why it is needed. Ming Lei, did you >> see a deadlock without it? > > Not yet, just for safe reason to make sure all queued data has chance > to be flushed. If you think it isn't necessary, I can remove it.
Another interface like bdrv_io_flush() may be needed for such purpose, and the accumulated IOs is really required to be flushed before doing flush. I will add the interface in v4. Thanks, -- Ming Lei
