On 8/21/20 11:44 AM, Stefano Garzarella wrote: > On Thu, Aug 20, 2020 at 06:58:49PM +0200, Philippe Mathieu-Daudé wrote: >> As nvme_create_queue_pair() is allowed to fail, replace the >> alloc() calls by try_alloc() to avoid aborting QEMU. >> >> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> block/nvme.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/block/nvme.c b/block/nvme.c >> index 8c30a5fee28..e1893b4e792 100644 >> --- a/block/nvme.c >> +++ b/block/nvme.c >> @@ -213,14 +213,22 @@ static NVMeQueuePair >> *nvme_create_queue_pair(BlockDriverState *bs, >> int i, r; >> BDRVNVMeState *s = bs->opaque; >> Error *local_err = NULL; >> - NVMeQueuePair *q = g_new0(NVMeQueuePair, 1); >> + NVMeQueuePair *q; >> uint64_t prp_list_iova; >> >> + q = g_try_new0(NVMeQueuePair, 1); >> + if (!q) { >> + return NULL; >> + } >> + q->prp_list_pages = qemu_try_blockalign0(bs, >> + s->page_size * NVME_QUEUE_SIZE); > > Here you use NVME_QUEUE_SIZE instead of NVME_NUM_REQS, is that an > intentional change?
No... Thanks for spotting this, I missed it because git didn't emit any warning while rebasing on top of "block/nvme: support nested aio_poll". This value has been changed in 1086e95da17 ("block/nvme: switch to a NVMeRequest freelist"). Good catch! I'll respin (after reviewing the 'nested aio_poll' changes). > > Maybe is not an issue, sice NVME_QUEUE_SIZE is bigger than > NVME_NUM_REQS, but we should mention in the commit message. > > Thanks, > Stefano > >> + if (!q->prp_list_pages) { >> + goto fail; >> + } >> qemu_mutex_init(&q->lock); >> q->s = s; >> q->index = idx; >> qemu_co_queue_init(&q->free_req_queue); >> - q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS); >> q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs), >> nvme_process_completion_bh, q); >> r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, >> -- >> 2.26.2 >> >> >