[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] migrate: Introduce a 'dc->vmsd' check to avo
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2] migrate: Introduce a 'dc->vmsd' check to avoid segfault for --only-migratable |
Date: |
Sat, 4 Feb 2017 01:21:23 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
On 04/02/2017 00:59, Ashijeet Acharya 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 <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> ---
> Changes in v2:
> - place dc->vmsd check in hw/usb/bus.c as well
> ---
> hw/usb/bus.c | 2 +-
> qdev-monitor.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index 1dcc35c..1e39b2c 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -731,7 +731,7 @@ USBDevice *usbdevice_create(const char *cmdline)
>
> dc = DEVICE_CLASS(klass);
>
> - if (only_migratable) {
> + if (only_migratable && dc->vmsd) {
> if (dc->vmsd->unmigratable) {
> error_report("Device %s is not migratable, but --only-migratable
> "
> "was specified", f->name);
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 549f45f..b72e5a4 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -579,7 +579,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);
>
Could you handle only_migratable in device_set_realized (inside "if
(value && !dev->realized)"), to avoid the code duplication? Separate
patch from this one, of course.
Thanks,
Paolo