In D40108#913423, @rew wrote:

not sure that devices should define a snapshot handler when not supported by the device.

At least it saves pdi.pi_cfgdata. Do you think it is waste doing and .pi_cfgdata can not be changed by guest OS or be changed between bhyve versions ?

pci_snapshot() could be setup to silently ignore devices that dont implement snapshot functionality.

either way, I don't have a preference

Currently it is mandatory to have .pe_snapshot(). Some devices do not support save/restore, for example, virtio-scsi. And if allow to be .pe_snapshot() as optional, bhyve process will be snapshotted, but result snapshot can not be resumed properly.

In D40108#913426, @gusev.vitaliy_gmail.com wrote:

In D40108#913423, @rew wrote:

not sure that devices should define a snapshot handler when not supported by the device.

At least it saves pdi.pi_cfgdata.

I meant original code does not save PCI 'hostbridge' device at all.

In D40108#913426, @gusev.vitaliy_gmail.com wrote:

At least it saves pdi.pi_cfgdata. Do you think it is waste doing and .pi_cfgdata can not be changed by guest OS or be changed between bhyve versions ?

I don't know off the top of my head - haven't looked at it that close.

Currently it is mandatory to have .pe_snapshot(). Some devices do not support save/restore, for example, virtio-scsi. And if allow to be .pe_snapshot() as optional, bhyve process will be snapshotted, but result snapshot can not be resumed properly.

Alright.

Why don't we call pci_snapshot_pci_dev unconditionally even if pe_snapshot is NULL?

In D40108#913868, @corvink wrote:

Why don't we call pci_snapshot_pci_dev unconditionally even if pe_snapshot is NULL?

If .pe_snapshot() is NULL, save stage must fail. Otherwise it will succeed for PCI devices that still don't support suspend/resume, but resume will be Undefined behavior (UB).