On Thu, May 1, 2014 at 5:03 PM, Richard W.M. Jones <rjo...@redhat.com> wrote:
> On Thu, May 01, 2014 at 04:54:41PM +0200, Stefan Hajnoczi wrote:
>> Drop the assumption that we're using the main AioContext. Use
>> bdrv_get_aio_context() to register fd handlers in the right AioContext
>> for this BlockDriverState.
>>
>> The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces
>> are not needed since no fd handlers, timers, or BHs stay registered when
>> requests have been drained.
>>
>> For now this doesn't make much difference but will allow ssh to work in
>> IOThread instances in the future.
>>
>> Cc: Richard W.M. Jones <rjo...@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
>> ---
>> block/ssh.c | 36 +++++++++++++++++++-----------------
>> 1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/block/ssh.c b/block/ssh.c
>> index aa63c9d..3f4a9fb 100644
>> --- a/block/ssh.c
>> +++ b/block/ssh.c
>> @@ -742,7 +742,7 @@ static void restart_coroutine(void *opaque)
>> qemu_coroutine_enter(co, NULL);
>> }
>>
>> -static coroutine_fn void set_fd_handler(BDRVSSHState *s)
>> +static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState
>> *bs)
>> {
>> int r;
>> IOHandler *rd_handler = NULL, *wr_handler = NULL;
>> @@ -760,24 +760,26 @@ static coroutine_fn void set_fd_handler(BDRVSSHState
>> *s)
>> DPRINTF("s->sock=%d rd_handler=%p wr_handler=%p", s->sock,
>> rd_handler, wr_handler);
>>
>> - qemu_aio_set_fd_handler(s->sock, rd_handler, wr_handler, co);
>> + aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,
>> + rd_handler, wr_handler, co);
>> }
>>
>> -static coroutine_fn void clear_fd_handler(BDRVSSHState *s)
>> +static coroutine_fn void clear_fd_handler(BDRVSSHState *s,
>> + BlockDriverState *bs)
>> {
>> DPRINTF("s->sock=%d", s->sock);
>> - qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL);
>> + aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock, NULL, NULL, NULL);
>> }
>>
>> /* A non-blocking call returned EAGAIN, so yield, ensuring the
>> * handlers are set up so that we'll be rescheduled when there is an
>> * interesting event on the socket.
>> */
>> -static coroutine_fn void co_yield(BDRVSSHState *s)
>> +static coroutine_fn void co_yield(BDRVSSHState *s, BlockDriverState *bs)
>> {
>> - set_fd_handler(s);
>> + set_fd_handler(s, bs);
>> qemu_coroutine_yield();
>> - clear_fd_handler(s);
>> + clear_fd_handler(s, bs);
>> }
>>
>> /* SFTP has a function `libssh2_sftp_seek64' which seeks to a position
>> @@ -807,7 +809,7 @@ static void ssh_seek(BDRVSSHState *s, int64_t offset,
>> int flags)
>> }
>> }
>>
>> -static coroutine_fn int ssh_read(BDRVSSHState *s,
>> +static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs,
>> int64_t offset, size_t size,
>> QEMUIOVector *qiov)
>> {
>> @@ -840,7 +842,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s,
>> DPRINTF("sftp_read returned %zd", r);
>>
>> if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {
>> - co_yield(s);
>> + co_yield(s, bs);
>> goto again;
>> }
>> if (r < 0) {
>> @@ -875,14 +877,14 @@ static coroutine_fn int ssh_co_readv(BlockDriverState
>> *bs,
>> int ret;
>>
>> qemu_co_mutex_lock(&s->lock);
>> - ret = ssh_read(s, sector_num * BDRV_SECTOR_SIZE,
>> + ret = ssh_read(s, bs, sector_num * BDRV_SECTOR_SIZE,
>> nb_sectors * BDRV_SECTOR_SIZE, qiov);
>> qemu_co_mutex_unlock(&s->lock);
>>
>> return ret;
>> }
>>
>> -static int ssh_write(BDRVSSHState *s,
>> +static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
>> int64_t offset, size_t size,
>> QEMUIOVector *qiov)
>> {
>> @@ -910,7 +912,7 @@ static int ssh_write(BDRVSSHState *s,
>> DPRINTF("sftp_write returned %zd", r);
>>
>> if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {
>> - co_yield(s);
>> + co_yield(s, bs);
>> goto again;
>> }
>> if (r < 0) {
>> @@ -929,7 +931,7 @@ static int ssh_write(BDRVSSHState *s,
>> */
>> if (r == 0) {
>> ssh_seek(s, offset + written, SSH_SEEK_WRITE|SSH_SEEK_FORCE);
>> - co_yield(s);
>> + co_yield(s, bs);
>> goto again;
>> }
>>
>> @@ -957,7 +959,7 @@ static coroutine_fn int ssh_co_writev(BlockDriverState
>> *bs,
>> int ret;
>>
>> qemu_co_mutex_lock(&s->lock);
>> - ret = ssh_write(s, sector_num * BDRV_SECTOR_SIZE,
>> + ret = ssh_write(s, bs, sector_num * BDRV_SECTOR_SIZE,
>> nb_sectors * BDRV_SECTOR_SIZE, qiov);
>> qemu_co_mutex_unlock(&s->lock);
>>
>> @@ -978,7 +980,7 @@ static void unsafe_flush_warning(BDRVSSHState *s, const
>> char *what)
>>
>> #ifdef HAS_LIBSSH2_SFTP_FSYNC
>>
>> -static coroutine_fn int ssh_flush(BDRVSSHState *s)
>> +static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs)
>> {
>> int r;
>>
>> @@ -986,7 +988,7 @@ static coroutine_fn int ssh_flush(BDRVSSHState *s)
>> again:
>> r = libssh2_sftp_fsync(s->sftp_handle);
>> if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {
>> - co_yield(s);
>> + co_yield(s, bs);
>> goto again;
>> }
>> if (r == LIBSSH2_ERROR_SFTP_PROTOCOL &&
>> @@ -1008,7 +1010,7 @@ static coroutine_fn int ssh_co_flush(BlockDriverState
>> *bs)
>> int ret;
>>
>> qemu_co_mutex_lock(&s->lock);
>> - ret = ssh_flush(s);
>> + ret = ssh_flush(s, bs);
>> qemu_co_mutex_unlock(&s->lock);
>>
>> return ret;
>> --
>> 1.9.0
>
> As this appears to simply be about adding a context pointer to several
> calls, it seems to be a simple, mechanical change, so ACK.
Yes. I wrote about the reason for the changes and what to look out
for in the cover letter of this series if you want to know more.
Stefan
