bhyve: snapshot fix for ahci-cd

[gusev.vitaliy_gmail.com]

Summary

Snapshot is interrupted with message:
    "pci_ahci_snapshot: invalid address: ioreq->cfis"

'ioreq->cfis' is NULL when is not initialized and can point
to not relevant address of the finished IO requests. In both
cases snapshot just needs to save NULL.

Test Plan

Suspend finishes without error and then resumed VM keeps working.

[jhb]

I feel like the better fix for some of this is that the ioreq state should be saved in one place (e.g. in pci_ahci_snapshot_save_queues and pci_ahci_snapshot_restore_queues). This would allow the right logic to force cfis of free I/O requests to be NULL and only save the cfis for busy requests without requiring the true hack.

[jhb]

Doing the pause/resume for ahci-cd is probably the right change though, and I can commit that now.

[gusev.vitaliy_gmail.com]

In D26266#612113, @jhb wrote:

I feel like the better fix for some of this is that the ioreq state should be saved in one place (e.g. in pci_ahci_snapshot_save_queues and pci_ahci_snapshot_restore_queues). This would allow the right logic to force cfis of free I/O requests to be NULL and only save the cfis for busy requests without requiring the true hack.

Fail occurs before pci_ahci_snapshot_save_queues() is called. Current patch also can be as sanity check to prevent access data after request is done. There is no reason to keep wild pointer in free-list requests.

[gusev.vitaliy_gmail.com]

up?

[markj]

Let's please at least add subroutines for allocating request structures from the freelist and freeing request structures to the freelist, so the assignment aior->cfis = NULL isn't duplicated everywhere. There should also be a comment explaining why that assignment is there. I'd be willing to commit the diff then.

[markj]

usr.sbin/bhyve/snapshot.c

157

This piece is already committed, the diff should be rebased.

[gusev.vitaliy_gmail.com]

Close that review in favour of D26267