Re: [Libguestfs] [PATCH libnbd v3 1/2] lib: Implement closure lifetimes.

On 7/25/19 10:43 AM, Eric Blake wrote: >> +++ b/generator/states-reply-structured.c >> @@ -298,7 +298,7 @@ >> * current error rather than any earlier one. If the callback fails >> * without setting errno, then use the server's error below. >> */ >> - if (cmd->cb.fn.read (cmd->cb.fn_user_data, >> + if (cmd->cb.fn.read (LIBNBD_CALLBACK_VALID, cmd->cb.fn_user_data, >> cmd->data + (offset - cmd->offset), >> 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) >> if (cmd->error == 0) > > We could still optimize this file based on NBD_REPLY_FLAG_DONE, but that > can be a followup. > >> @@ -499,7 +499,7 @@ >> /* Call the caller's extent function. */ >> int error = cmd->error; >> >> - if (cmd->cb.fn.extent (cmd->cb.fn_user_data, >> + if (cmd->cb.fn.extent (LIBNBD_CALLBACK_VALID, cmd->cb.fn_user_data, >> meta_context->name, cmd->offset, >> &h->bs_entries[1], (length-4) / 4, &error) == -1) >> if (cmd->error == 0) > > Hmm - no change to the FINISH state, which means you are relying on > command retirement to free chunk/extent instead. As long as that > happens, we should be okay, though. Another thought: if a user calls nbd_aio_pread_structured_callback (..., chunk, &data, callback, &data, ...), where chunk ignores FREE but callback(FREE) calls free(data), is there any problem with the fact that we may end up with a call order of callback(VALID|FREE) before chunk(FREE)? I think we're okay - as long as chunk doesn't dereference data except when VALID is set, then the fact that chunk(FREE) is called with a stale pointer should not be a problem.