Re: [PATCH v2 3/3] pci: ROM preallocation for incoming migration

[Juan Quintela]

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, May 02, 2023 at 12:11:38PM +0200, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> >> > CC pbonzini,dgilbert,quintela,armbru : guys, is poking at 
>> >> > runstate_check like
>> >> > this the right way to figure out we are not going to use the
>> >> > device locally before incoming migration will overwrite ROM contents?
>> >> 
>> >> RUN_STATE_INMIGRATE is set in the only one place in qemu_init() when
>> >> we parse cmdline option -incoming. VM is not running for sure. And
>> >> starting the VM comes with changing the state. So it's OK.
>> >> 
>> >> The possible problem, if we add netcard on target which we didn't
>> >> have on source. I now checked, this works.. But that doesn't seem
>> >> correct to add device that was not present on source - how would it
>> >> work - it's not guaranteed anyway.
>> >
>> > You can add it on source too while migration is in progress, no?
>> 
>> DeviceState *qdev_device_add_from_qdict(const QDict *opts,
>>                                         bool from_json, Error **errp)
>> {
>>     ....
>>     if (!migration_is_idle()) {
>>         error_setg(errp, "device_add not allowed while migrating");
>>         return NULL;
>>     }
>> 
>> It should be similar for unplug.
>> 
>> We only support hotplug for some devices during migration, and we
>> shouldn't need any.
>> 
>> What I think he means is that you can add a device on the command line
>> on destination that don't exist on the source machine, and that will
>> confuse things.
>> 
>> In that case, I would say that the problem is that you are doing
>> something not supported.  You are expected that when you run migration
>> you use the same command line that on source, module whatever
>> hot[un]plug operations you have done before migration.
>> 
>> Anything else is not supported.
>> And for instance, if you are using libvirt, it will do the right thing.
>> 
>> Later, Juan.
>
> OK, so you ack this patch?

Reviewed-by: Juan Quintela <quintela@redhat.com>

It is ok, or should I do it at toplevel?

Later, Juan.