On 04/18/2011 10:43 PM, Alex Williamson wrote:
On Sun, 2011-04-17 at 12:25 +0300, Avi Kivity wrote:
> On 04/15/2011 10:54 PM, Alex Williamson wrote:
> > Store the device saved state so that we can reload the device back
> > to the original state when it's unassigned. This has the benefit
> > that the state survives across pci_reset_function() calls via
> > the PCI sysfs reset interface while the VM is using the device.
>
> > @@ -516,7 +518,7 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> >
> > pci_reset_function(dev);
> > pci_save_state(dev);
> > -
> > + match->pci_saved_state = pci_store_saved_state(dev);
> > match->assigned_dev_id = assigned_dev->assigned_dev_id;
>
> Error check?
>
> It might be better to give up the opacity of the data structure and make
> pci_saved_state the full struct, not a pointer.
pci_store_saved_state() returns NULL on error, which is correctly
handled if we pass NULL to pci_load_saved_state() or a pointer to NULL
to pci_load_and_free_saved_state().
But we silently swallow an error, this isn't good.
This is also why I changed the
__pci_reset_function() back to a normal pci_reset_function(), so we're
never left with an uninitialized device like we are now.
We could be more verbose or return an error here, but we've gone for a
long time not even doing this save/restore across VM usage, so I don't
think it's worthy of preventing the device attachment if it fails.
At least a log?
Note avoiding the pointer would have removed the problem altogether.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html