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

[Michael S. Tsirkin]

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?

-- 
MST