Am 15.11.2010 23:25, Alex Williamson wrote:
> On Mon, 2010-11-15 at 23:04 +0100, Jan Kiszka wrote:
>> Am 15.11.2010 21:25, Alex Williamson wrote:
>>> On Mon, 2010-11-15 at 21:05 +0100, Jan Kiszka wrote:
>>>> Am 15.11.2010 20:41, Alex Williamson wrote:
>>>>> Use register_device_unmigratable() to declare ourselves as
>>>>> non-migratable.
>>>>>
>>>>> Signed-off-by: Alex Williamson <alex.william...@redhat.com>
>>>>> ---
>>>>>
>>>>> hw/device-assignment.c | 15 +++++++++++++++
>>>>> 1 files changed, 15 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>>>>> index bde231d..cd93941 100644
>>>>> --- a/hw/device-assignment.c
>>>>> +++ b/hw/device-assignment.c
>>>>> @@ -1434,6 +1434,13 @@ static void
>>>>> assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
>>>>> dev->msix_table_page = NULL;
>>>>> }
>>>>>
>>>>> +/* This should never get called, but we're required to create a
>>>>> save_state
>>>>> + * handler or else the no_migrate flag will never be checked. */
>>>>> +static void assigned_save(QEMUFile* f, void *opaque)
>>>>> +{
>>>>> + abort();
>>>>> +}
>>>>> +
>>>>> static int assigned_initfn(struct PCIDevice *pci_dev)
>>>>> {
>>>>> AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>>>> @@ -1490,6 +1497,13 @@ static int assigned_initfn(struct PCIDevice
>>>>> *pci_dev)
>>>>>
>>>>> assigned_dev_load_option_rom(dev);
>>>>> QLIST_INSERT_HEAD(&devs, dev, next);
>>>>> +
>>>>> + /* Assigned devices are not migratable, register a save
>>>>> + * state entry so that we can mark it unmigratable. */
>>>>> + register_savevm(&dev->dev.qdev, "pci-assign", 0, 0,
>>>>> + assigned_save, NULL, dev);
>>>>> + register_device_unmigratable(&dev->dev.qdev, "pci-assign", dev);
>>>>> +
>>>>
>>>> Isn't this expressible via some VMStateDescription? If not, that should
>>>> be changed first.
>>>
>>> Nope, save state handlers aren't allowed to fail. I tried to fix it:
>>>
>>> http://lists.nongnu.org/archive/html/qemu-devel/2010-11/msg00417.html
>>>
>>> (you can find more discussion in other branches of that subject) I've
>>> succumbed to not getting that series in, so now I'm just trying to use
>>> the code as it exists. Thanks,
>>
>> Hmm, didn't get why you need that series for the purpose of no_migration
>> declaration.
>> My point is:
>
> I was hoping you were going down the path I started, that we don't need
> to special case non-migratable devices if we just allow save to return
> an error.
Ah, of course. Still, vmstate is the way to go IMO.
>
>> My point is:
>>
>> struct VMStateDescription {
>> const char *name;
>> int version_id;
>> int minimum_version_id;
>> int minimum_version_id_old;
>> int no_migrate; /* or 'flags' */
>> ...
>>
>> so that you can specify an empty vmstate with that flag set, and you do
>> not need to register/unregister things via to-be-deprecated service
>> calls. Or am I missing some subtle detail?
>
> We don't seem to be enforcing that new drivers should use vmsd vs the
> old style handling
Don't we? We are enforcing qdev but not vmstate? Sounds silly. I think
new devices should all be expressible this way, no?
> and there are some drivers that are currently too
> complicated for vmsd to handle, which means no_migrate has to be
> registered on the save state entry, not the vmsd. So I could create a
> dummy vmsd, then call register_device_unmigratable, to achieve roughly
> the same effect. Six of one, half dozen of the other... I can switch to
> a vmsd dummy save if it's preferred. Thanks,
IMHO, new designs should not use register_savevm anymore.
Jan
signature.asc
Description: OpenPGP digital signature
