Re: [PATCH v6 6/8] hw/nvme: add basic live migration support

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

On Sun, Apr 19, 2026 at 03:01:37PM +0200, Alexander Mikhalitsyn wrote:
> @@ -4903,6 +4916,25 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr,
>      __nvme_init_sq(sq);
>  }
>  
> +static void nvme_restore_sq(NvmeSQueue *sq_from)
> +{
> +    NvmeCtrl *n = sq_from->ctrl;
> +    NvmeSQueue *sq = sq_from;
> +
> +    if (sq_from->sqid == 0) {
> +        sq = &n->admin_sq;

docs/devel/migration/main.rst says:

  - The destination should treat an incoming migration stream as hostile
    (which we do to varying degrees in the existing code).  Check that offsets
    into buffers and the like can't cause overruns.  Fail the incoming migration
    in the case of a corrupted stream like this.

Can a corrupt/malicious device state reach this point multiple times
(i.e. several sqid 0 queues are stored in the device state)? If yes,
then input validation would be good here to avoid undefined behavior
later in the sq code.

The same issue may apply to duplicate sqids in general. It seems safest
to reject them during restore.

> +        sq->ctrl = n;
> +        sq->dma_addr = sq_from->dma_addr;
> +        sq->sqid = sq_from->sqid;
> +        sq->size = sq_from->size;
> +        sq->cqid = sq_from->cqid;
> +        sq->head = sq_from->head;
> +        sq->tail = sq_from->tail;
> +    }
> +
> +    __nvme_init_sq(sq);
> +}
> +
>  static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest *req)
>  {
>      NvmeSQueue *sq;
> @@ -5605,6 +5637,39 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
>      __nvme_init_cq(cq);
>  }
>  
> +static void copy_cq_req_list(NvmeCQueue *cq_to, NvmeCQueue *cq_from)

This moves the NvmeRequests rather than copying them. Rename to
move_cq_req_list()?

> +{
> +    NvmeRequest *req, *next;
> +
> +    QTAILQ_FOREACH_SAFE(req, &cq_from->req_list, entry, next) {
> +        QTAILQ_REMOVE(&cq_from->req_list, req, entry);
> +        QTAILQ_INSERT_TAIL(&cq_to->req_list, req, entry);
> +    }
> +}
> +
> +static void nvme_restore_cq(NvmeCQueue *cq_from)
> +{
> +    NvmeCtrl *n = cq_from->ctrl;
> +    NvmeCQueue *cq = cq_from;
> +
> +    if (cq_from->cqid == 0) {
> +        cq = &n->admin_cq;

Same question about duplicate CQs in corrupt/malicious device states.

I reviewed the new draining (busy wait removal). I'll leave the rest for
Klaus because I'm not that familiar with the NVMe emulation code.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]