Re: [PATCH 2/2] KVM: Use pci_store/load_saved_state() around VM device usage

Avi Kivity
2011-04-20T15:27:21Z

On 04/20/2011 06:13 PM, Alex Williamson wrote:
> >
> > > 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?
>
> Ok, I'm not sure what corrective action a user would take or what they
> should expect not to work, but I guess a KERN_DEBUG printk is
> reasonable.

"X didn't work" vs "X didn't work and I got this in the log"

> > Note avoiding the pointer would have removed the problem altogether.
>
> Returning a struct on store? We lose any kind of opacity that way since
> the caller needs to know about the struct then. I thought the pointer
> makes it clear the caller shouldn't be touching the contents, but if you
> think it's a better way to go, I can try it. Thanks,

Avoid the allocation altogether. Having the caller be responsible for
storage (in our case, embed the struct instead of the pointer).

You can encrypt the contents using the TPM, or maybe a comment
indicating that the contents should suffice.

--
error compiling committee.c: too many arguments to function