From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 09778CA9EB9 for ; Tue, 29 Oct 2019 05:53:11 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B683020659 for ; Tue, 29 Oct 2019 05:53:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=freimann-org.20150623.gappssmtp.com header.i=@freimann-org.20150623.gappssmtp.com header.b="yXY0Egw/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B683020659 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=freimann.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:51428 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iPKRJ-0006W3-MG for qemu-devel@archiver.kernel.org; Tue, 29 Oct 2019 01:53:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49880) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iPKQ8-0005Dk-Ru for qemu-devel@nongnu.org; Tue, 29 Oct 2019 01:51:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iPKQ6-00006O-FW for qemu-devel@nongnu.org; Tue, 29 Oct 2019 01:51:56 -0400 Received: from mail-qk1-x741.google.com ([2607:f8b0:4864:20::741]:35012) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iPKQ6-0008W7-5i for qemu-devel@nongnu.org; Tue, 29 Oct 2019 01:51:54 -0400 Received: by mail-qk1-x741.google.com with SMTP id h6so2974827qkf.2 for ; Mon, 28 Oct 2019 22:51:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freimann-org.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2mvBPqbc3ndXC2YNcaQjAkbvaVpnPT610EWMUsk3cE8=; b=yXY0Egw/gUcCHiyOweKKG6QICLXeSmDSA4hhuqWM3H7Dr/67C6trOSAj1kVbTXMGH2 v+v/KpORtUDjUUwjECNTS+wdWgWVvjZsEqGpamfebmedMuycK3LdbMaUqIeXmagZOm/2 S+o7DipzCA9S7MZzLGU5nNtWl5cF7SiRFUMLwobWj/yGQ+x8TyAl9Ab6ObWx+6vorN07 ciUleGaPa+qjraFKpArR/V4TTu/hdQ0NtyGrnv3sKX7yEaMWIdRXu+8Jh1cHtQ1HEJ38 FUPd+iV9PuJ9fVxiVsiT0J5E3OFRib2j2gYLrr0uOnPqeG8HE7MOziiaGPxLrJBMr78k /Jtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2mvBPqbc3ndXC2YNcaQjAkbvaVpnPT610EWMUsk3cE8=; b=k96RMRAhzv6CdJXMYl7vYKoC4gjvAPp5RTxvg7OQKv2qfPNbH/qwXYKi1p7h+hqxR9 IqVU60A+m27gz1evtSyi3Yaek34pG/8C/i57sPjqIFv7SZBt4Mu0VMk4Jxh+YI9Ha8aR d752I1zAq7CZvy0Zw8USF4VOc6mF3D/KG6J2I0EvQC6vSfa3ChsHJ+V3rNueuV+2z8FQ gz8pG0jXhR9Pek+hVDLeYlqJ0iGMrbBuNIUm3JCK+l02AqVrdpEcTonTedYPU+gV8Kcd 7TQr0GNwgZrroo9S/+fCm+069M431VyKTp+7PI6cjJj8kb04+YWzLM7BoEU8aZ/FPlYP yiiQ== X-Gm-Message-State: APjAAAWPPtzIrd4hrA8yksce+4wTALfmqiRp7iLwiT3Y+2+PJEwtcjfV JkkYZy+DCuaZx/KTRXdLdE6zmNVNeurPrhG2jzt9 X-Google-Smtp-Source: APXvYqyMnuTueUx96f3Qk+iEOVGkEBeRFC3WqeyOCQUTBKjs2M0SISWNcms5Dne1VC3erI6nZpnRZd1mSGk0LJVJb5A= X-Received: by 2002:a05:620a:852:: with SMTP id u18mr20138344qku.169.1572328311064; Mon, 28 Oct 2019 22:51:51 -0700 (PDT) MIME-Version: 1.0 References: <20191025121930.6855-1-jfreimann@redhat.com> <20191025121930.6855-9-jfreimann@redhat.com> <20191029025650.GE2508@work-vm> In-Reply-To: <20191029025650.GE2508@work-vm> From: Jens Freimann Date: Tue, 29 Oct 2019 06:51:37 +0100 Message-ID: Subject: Re: [PATCH v6 08/11] migration: add new migration state wait-unplug To: "Dr. David Alan Gilbert" Content-Type: multipart/alternative; boundary="000000000000739c910596063666" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::741 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pkrempa@redhat.com, berrange@redhat.com, ehabkost@redhat.com, mst@redhat.com, aadam@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, alex.williamson@redhat.com, laine@redhat.com, jfreimann@redhat.com, ailan@redhat.com, parav@mellanox.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000739c910596063666 Content-Type: text/plain; charset="UTF-8" Dr. David Alan Gilbert schrieb am Di., 29. Okt. 2019, 03:57: > * Jens Freimann (jfreimann@redhat.com) wrote: > > This patch adds a new migration state called wait-unplug. It is entered > > after the SETUP state if failover devices are present. It will transition > > into ACTIVE once all devices were succesfully unplugged from the guest. > > > > So if a guest doesn't respond or takes long to honor the unplug request > > the user will see the migration state 'wait-unplug'. > > > > In the migration thread we query failover devices if they're are still > > pending the guest unplug. When all are unplugged the migration > > continues. If one device won't unplug migration will stay in wait_unplug > > state. > > > > Signed-off-by: Jens Freimann > > Acked-by: Cornelia Huck > > I think this is OK, so > > > Reviewed-by: Dr. David Alan Gilbert > > but see question below > > > --- > > include/migration/vmstate.h | 2 ++ > > migration/migration.c | 21 +++++++++++++++++++++ > > migration/migration.h | 3 +++ > > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > > migration/savevm.h | 2 ++ > > qapi/migration.json | 5 ++++- > > 6 files changed, 68 insertions(+), 1 deletion(-) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index b9ee563aa4..ac4f46a67d 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -186,6 +186,8 @@ struct VMStateDescription { > > int (*pre_save)(void *opaque); > > int (*post_save)(void *opaque); > > bool (*needed)(void *opaque); > > + bool (*dev_unplug_pending)(void *opaque); > > + > > const VMStateField *fields; > > const VMStateDescription **subsections; > > }; > > diff --git a/migration/migration.c b/migration/migration.c > > index 3febd0f8f3..51764f2565 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -52,6 +52,7 @@ > > #include "hw/qdev-properties.h" > > #include "monitor/monitor.h" > > #include "net/announce.h" > > +#include "qemu/queue.h" > > > > #define MAX_THROTTLE (32 << 20) /* Migration transfer speed > throttling */ > > > > @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state) > > case MIGRATION_STATUS_SETUP: > > case MIGRATION_STATUS_PRE_SWITCHOVER: > > case MIGRATION_STATUS_DEVICE: > > + case MIGRATION_STATUS_WAIT_UNPLUG: > > return true; > > > > default: > > @@ -954,6 +956,9 @@ static void fill_source_migration_info(MigrationInfo > *info) > > case MIGRATION_STATUS_CANCELLED: > > info->has_status = true; > > break; > > + case MIGRATION_STATUS_WAIT_UNPLUG: > > + info->has_status = true; > > + break; > > } > > info->status = s->state; > > } > > @@ -1694,6 +1699,7 @@ bool migration_is_idle(void) > > case MIGRATION_STATUS_COLO: > > case MIGRATION_STATUS_PRE_SWITCHOVER: > > case MIGRATION_STATUS_DEVICE: > > + case MIGRATION_STATUS_WAIT_UNPLUG: > > return false; > > case MIGRATION_STATUS__MAX: > > g_assert_not_reached(); > > @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque) > > > > qemu_savevm_state_setup(s->to_dst_file); > > > > + if (qemu_savevm_nr_failover_devices()) { > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > + MIGRATION_STATUS_WAIT_UNPLUG); > > + > > + while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && > > + !qemu_savevm_state_guest_unplug_pending()) { > > + qemu_sem_timedwait(&s->wait_unplug_sem, 250); > > + } > > + > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > > + MIGRATION_STATUS_ACTIVE); > > + } > > + > > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > MIGRATION_STATUS_ACTIVE); > > @@ -3511,6 +3530,7 @@ static void migration_instance_finalize(Object > *obj) > > qemu_mutex_destroy(&ms->qemu_file_lock); > > g_free(params->tls_hostname); > > g_free(params->tls_creds); > > + qemu_sem_destroy(&ms->wait_unplug_sem); > > qemu_sem_destroy(&ms->rate_limit_sem); > > qemu_sem_destroy(&ms->pause_sem); > > qemu_sem_destroy(&ms->postcopy_pause_sem); > > @@ -3556,6 +3576,7 @@ static void migration_instance_init(Object *obj) > > qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); > > qemu_sem_init(&ms->rp_state.rp_sem, 0); > > qemu_sem_init(&ms->rate_limit_sem, 0); > > + qemu_sem_init(&ms->wait_unplug_sem, 0); > > qemu_mutex_init(&ms->qemu_file_lock); > > } > > > > diff --git a/migration/migration.h b/migration/migration.h > > index 4f2fe193dc..79b3dda146 100644 > > --- a/migration/migration.h > > +++ b/migration/migration.h > > @@ -206,6 +206,9 @@ struct MigrationState > > /* Flag set once the migration thread called bdrv_inactivate_all */ > > bool block_inactive; > > > > + /* Migration is waiting for guest to unplug device */ > > + QemuSemaphore wait_unplug_sem; > > + > > /* Migration is paused due to pause-before-switchover */ > > QemuSemaphore pause_sem; > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 8d95e261f6..0f18dea49e 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f) > > } > > } > > > > +int qemu_savevm_nr_failover_devices(void) > > +{ > > + SaveStateEntry *se; > > + int n = 0; > > + > > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > > + if (se->vmsd && se->vmsd->dev_unplug_pending) { > > + n++; > > + } > > + } > > + > > + return n; > > +} > > + > > +bool qemu_savevm_state_guest_unplug_pending(void) > > +{ > > + int nr_failover_devs; > > + SaveStateEntry *se; > > + bool ret = false; > > + int n = 0; > > + > > + nr_failover_devs = qemu_savevm_nr_failover_devices(); > > + > > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > > + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { > > + continue; > > + } > > + ret = se->vmsd->dev_unplug_pending(se->opaque); > > + if (!ret) { > > + n++; > > + } > > + } > > + > > + return n == nr_failover_devs; > > I was expecting != I think? If all the devices say > they've got one pending then doesn't n==nr_failover_devs and > it returns true? But then what happens if only one has one pending? > It's increased when unplug pending is false, which means the device is done. So it returns true when all devices are done with unplugging. It is correct but not obvious. I can reverse it in a follow up to make it more clear. regards, Jens > > Dave > > > +} > > + > > void qemu_savevm_state_setup(QEMUFile *f) > > { > > SaveStateEntry *se; > > diff --git a/migration/savevm.h b/migration/savevm.h > > index 51a4b9caa8..c42b9c80ee 100644 > > --- a/migration/savevm.h > > +++ b/migration/savevm.h > > @@ -31,6 +31,8 @@ > > > > bool qemu_savevm_state_blocked(Error **errp); > > void qemu_savevm_state_setup(QEMUFile *f); > > +int qemu_savevm_nr_failover_devices(void); > > +bool qemu_savevm_state_guest_unplug_pending(void); > > int qemu_savevm_state_resume_prepare(MigrationState *s); > > void qemu_savevm_state_header(QEMUFile *f); > > int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); > > diff --git a/qapi/migration.json b/qapi/migration.json > > index e9e7a97c03..b7348d0c8b 100644 > > --- a/qapi/migration.json > > +++ b/qapi/migration.json > > @@ -133,6 +133,9 @@ > > # @device: During device serialisation when pause-before-switchover is > enabled > > # (since 2.11) > > # > > +# @wait-unplug: wait for device unplug request by guest OS to be > completed. > > +# (since 4.2) > > +# > > # Since: 2.3 > > # > > ## > > @@ -140,7 +143,7 @@ > > 'data': [ 'none', 'setup', 'cancelling', 'cancelled', > > 'active', 'postcopy-active', 'postcopy-paused', > > 'postcopy-recover', 'completed', 'failed', 'colo', > > - 'pre-switchover', 'device' ] } > > + 'pre-switchover', 'device', 'wait-unplug' ] } > > > > ## > > # @MigrationInfo: > > -- > > 2.21.0 > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > --000000000000739c910596063666 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Dr. David Alan Gilbert <dgilbert@redhat.com> schrieb am Di., 29. Okt. 2019, 03:5= 7:
* Jens Freimann (jfreimann@red= hat.com) wrote:
> This patch adds a new migration state called wait-unplug.=C2=A0 It is = entered
> after the SETUP state if failover devices are present. It will transit= ion
> into ACTIVE once all devices were succesfully unplugged from the guest= .
>
> So if a guest doesn't respond or takes long to honor the unplug re= quest
> the user will see the migration state 'wait-unplug'.
>
> In the migration thread we query failover devices if they're are s= till
> pending the guest unplug. When all are unplugged the migration
> continues. If one device won't unplug migration will stay in wait_= unplug
> state.
>
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Acked-by: Cornelia Huck <cohuck@redhat.com>

I think this is OK, so


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

but see question below

> ---
>=C2=A0 include/migration/vmstate.h |=C2=A0 2 ++
>=C2=A0 migration/migration.c=C2=A0 =C2=A0 =C2=A0 =C2=A0| 21 +++++++++++= ++++++++++
>=C2=A0 migration/migration.h=C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 3 +++ >=C2=A0 migration/savevm.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 36 ++++++= ++++++++++++++++++++++++++++++
>=C2=A0 migration/savevm.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 2 += +
>=C2=A0 qapi/migration.json=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 5 += +++-
>=C2=A0 6 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h=
> index b9ee563aa4..ac4f46a67d 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -186,6 +186,8 @@ struct VMStateDescription {
>=C2=A0 =C2=A0 =C2=A0 int (*pre_save)(void *opaque);
>=C2=A0 =C2=A0 =C2=A0 int (*post_save)(void *opaque);
>=C2=A0 =C2=A0 =C2=A0 bool (*needed)(void *opaque);
> +=C2=A0 =C2=A0 bool (*dev_unplug_pending)(void *opaque);
> +
>=C2=A0 =C2=A0 =C2=A0 const VMStateField *fields;
>=C2=A0 =C2=A0 =C2=A0 const VMStateDescription **subsections;
>=C2=A0 };
> diff --git a/migration/migration.c b/migration/migration.c
> index 3febd0f8f3..51764f2565 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -52,6 +52,7 @@
>=C2=A0 #include "hw/qdev-properties.h"
>=C2=A0 #include "monitor/monitor.h"
>=C2=A0 #include "net/announce.h"
> +#include "qemu/queue.h"
>=C2=A0
>=C2=A0 #define MAX_THROTTLE=C2=A0 (32 << 20)=C2=A0 =C2=A0 =C2=A0 = /* Migration transfer speed throttling */
>=C2=A0
> @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state)
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_SETUP:
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_PRE_SWITCHOVER:
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_DEVICE:
> +=C2=A0 =C2=A0 case MIGRATION_STATUS_WAIT_UNPLUG:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return true;
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 default:
> @@ -954,6 +956,9 @@ static void fill_source_migration_info(MigrationIn= fo *info)
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_CANCELLED:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 info->has_status =3D true;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case MIGRATION_STATUS_WAIT_UNPLUG:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 info->has_status =3D true;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>=C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0 info->status =3D s->state;
>=C2=A0 }
> @@ -1694,6 +1699,7 @@ bool migration_is_idle(void)
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_COLO:
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_PRE_SWITCHOVER:
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS_DEVICE:
> +=C2=A0 =C2=A0 case MIGRATION_STATUS_WAIT_UNPLUG:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
>=C2=A0 =C2=A0 =C2=A0 case MIGRATION_STATUS__MAX:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 g_assert_not_reached();
> @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque)
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 qemu_savevm_state_setup(s->to_dst_file);
>=C2=A0
> +=C2=A0 =C2=A0 if (qemu_savevm_nr_failover_devices()) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 migrate_set_state(&s->state, MIGRA= TION_STATUS_SETUP,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 MIGRATION_STATUS_WAIT_UNPLUG);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 while (s->state =3D=3D MIGRATION_STATU= S_WAIT_UNPLUG &&
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !qemu_savevm_= state_guest_unplug_pending()) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_sem_timedwait(&s-&= gt;wait_unplug_sem, 250);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 migrate_set_state(&s->state, MIGRA= TION_STATUS_WAIT_UNPLUG,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 MIGRATION_STA= TUS_ACTIVE);
> +=C2=A0 =C2=A0 }
> +
>=C2=A0 =C2=A0 =C2=A0 s->setup_time =3D qemu_clock_get_ms(QEMU_CLOCK_= HOST) - setup_start;
>=C2=A0 =C2=A0 =C2=A0 migrate_set_state(&s->state, MIGRATION_STAT= US_SETUP,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 MIGRATION_STATUS_ACTIVE);
> @@ -3511,6 +3530,7 @@ static void migration_instance_finalize(Object *= obj)
>=C2=A0 =C2=A0 =C2=A0 qemu_mutex_destroy(&ms->qemu_file_lock); >=C2=A0 =C2=A0 =C2=A0 g_free(params->tls_hostname);
>=C2=A0 =C2=A0 =C2=A0 g_free(params->tls_creds);
> +=C2=A0 =C2=A0 qemu_sem_destroy(&ms->wait_unplug_sem);
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_destroy(&ms->rate_limit_sem);
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_destroy(&ms->pause_sem);
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_destroy(&ms->postcopy_pause_sem);<= br> > @@ -3556,6 +3576,7 @@ static void migration_instance_init(Object *obj)=
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_init(&ms->postcopy_pause_rp_sem, 0= );
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_init(&ms->rp_state.rp_sem, 0);
>=C2=A0 =C2=A0 =C2=A0 qemu_sem_init(&ms->rate_limit_sem, 0);
> +=C2=A0 =C2=A0 qemu_sem_init(&ms->wait_unplug_sem, 0);
>=C2=A0 =C2=A0 =C2=A0 qemu_mutex_init(&ms->qemu_file_lock);
>=C2=A0 }
>=C2=A0
> diff --git a/migration/migration.h b/migration/migration.h
> index 4f2fe193dc..79b3dda146 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -206,6 +206,9 @@ struct MigrationState
>=C2=A0 =C2=A0 =C2=A0 /* Flag set once the migration thread called bdrv_= inactivate_all */
>=C2=A0 =C2=A0 =C2=A0 bool block_inactive;
>=C2=A0
> +=C2=A0 =C2=A0 /* Migration is waiting for guest to unplug device */ > +=C2=A0 =C2=A0 QemuSemaphore wait_unplug_sem;
> +
>=C2=A0 =C2=A0 =C2=A0 /* Migration is paused due to pause-before-switcho= ver */
>=C2=A0 =C2=A0 =C2=A0 QemuSemaphore pause_sem;
>=C2=A0
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 8d95e261f6..0f18dea49e 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f)
>=C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 }
>=C2=A0
> +int qemu_savevm_nr_failover_devices(void)
> +{
> +=C2=A0 =C2=A0 SaveStateEntry *se;
> +=C2=A0 =C2=A0 int n =3D 0;
> +
> +=C2=A0 =C2=A0 QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (se->vmsd && se->vmsd-&g= t;dev_unplug_pending) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n++;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return n;
> +}
> +
> +bool qemu_savevm_state_guest_unplug_pending(void)
> +{
> +=C2=A0 =C2=A0 int nr_failover_devs;
> +=C2=A0 =C2=A0 SaveStateEntry *se;
> +=C2=A0 =C2=A0 bool ret =3D false;
> +=C2=A0 =C2=A0 int n =3D 0;
> +
> +=C2=A0 =C2=A0 nr_failover_devs =3D qemu_savevm_nr_failover_devices();=
> +
> +=C2=A0 =C2=A0 QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!se->vmsd || !se->vmsd->dev_= unplug_pending) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D se->vmsd->dev_unplug_pendin= g(se->opaque);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!ret) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n++;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return n =3D=3D nr_failover_devs;

I was expecting !=3D I think?=C2=A0 If all the devices say
they've got one pending then doesn't n=3D=3Dnr_failover_devs and it returns true? But then what happens if only one has one pending?

It's= increased when unplug pending is false, which means the device is done. So= it returns true when all devices are done with unplugging. It is correct b= ut not obvious. I can reverse it in a follow up to make it more clear.

regards,
Jens

Dave

> +}
> +
>=C2=A0 void qemu_savevm_state_setup(QEMUFile *f)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 SaveStateEntry *se;
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 51a4b9caa8..c42b9c80ee 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -31,6 +31,8 @@
>=C2=A0
>=C2=A0 bool qemu_savevm_state_blocked(Error **errp);
>=C2=A0 void qemu_savevm_state_setup(QEMUFile *f);
> +int qemu_savevm_nr_failover_devices(void);
> +bool qemu_savevm_state_guest_unplug_pending(void);
>=C2=A0 int qemu_savevm_state_resume_prepare(MigrationState *s);
>=C2=A0 void qemu_savevm_state_header(QEMUFile *f);
>=C2=A0 int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index e9e7a97c03..b7348d0c8b 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -133,6 +133,9 @@
>=C2=A0 # @device: During device serialisation when pause-before-switcho= ver is enabled
>=C2=A0 #=C2=A0 =C2=A0 =C2=A0 =C2=A0 (since 2.11)
>=C2=A0 #
> +# @wait-unplug: wait for device unplug request by guest OS to be comp= leted.
> +#=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(since 4.2) > +#
>=C2=A0 # Since: 2.3
>=C2=A0 #
>=C2=A0 ##
> @@ -140,7 +143,7 @@
>=C2=A0 =C2=A0 'data': [ 'none', 'setup', 'c= ancelling', 'cancelled',
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'active', '= ;postcopy-active', 'postcopy-paused',
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'postcopy-recover&= #39;, 'completed', 'failed', 'colo',
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'pre-switchover', &= #39;device' ] }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'pre-switchover', &= #39;device', 'wait-unplug' ] }
>=C2=A0
>=C2=A0 ##
>=C2=A0 # @MigrationInfo:
> --
> 2.21.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


--000000000000739c910596063666--