On 30 January 2017 at 14:41, Ashijeet Acharya <ashijeetacha...@gmail.com> wrote:
> Commit a3a3d8c7 introduced a segfault bug while checking for
> 'dc->vmsd->unmigratable' which caused QEMU to crash when trying to add
> devices which do no set their 'dc->vmsd' yet while initialization.
> Place a 'dc->vmsd' check prior to it so that we do not segfault for
> such devices.
>
> NOTE: This doesn't compromise the functioning of --only-migratable
> option as all the unmigratable devices do set their 'dc->vmsd'.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacha...@gmail.com>
> ---
> qdev-monitor.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 81d01df..a1106fd 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -578,7 +578,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> return NULL;
> }
>
> - if (only_migratable) {
> + if (only_migratable && dc->vmsd) {
> if (dc->vmsd->unmigratable) {
> error_setg(errp, "Device %s is not migratable, but "
> "--only-migratable was specified", driver);
This seems like a good fix for the crash as a short term fix,
but longer term I think it would be better to make setting
dc->vmsd mandatory. I think devices which don't set it fall into
one of these categories:
* deliberately has no VMState struct as it has no state that
needs migrating -- we should have some kind of flag for
such devices to positively assert that they don't need to
deal with migration
* accidentally failed to provide a VMState struct -- this is
a bug and the device should be fixed to at minimum mark
itself as unmigratable (and ideally fixed to implement
migration!)
* didn't provide a VMState struct because they handle
migration manually using vmstate_register() -- we should
update these to use VMState, or failing that use the
flag to say "deliberately not providing VMState"
Then we can make QEMU assert if dc->vmsd is NULL, and
catch future occurrences of "oops I didn't think about
migration" bugs in new device models. We also have an
easy way to grep the codebase for devices that need to
have migration implemented.
thanks
-- PMM
