[libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap

Jiri Denemark posted 5 patches 7 years, 6 months ago
[libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
Posted by Jiri Denemark 7 years, 6 months ago
Each time we need to check whether a given migration capability is
supported by QEMU, we call query-migrate-capabilities QMP command and
lookup the capability in the returned list. Asking for the list of
supported capabilities once when we connect to QEMU and storing the
result in a bitmap is much better and we don't need to enter a monitor
just to check whether a migration capability is supported.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_domain.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h  |  9 +++++++
 src/qemu/qemu_process.c | 13 +---------
 3 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05e8b96aa4..a8cabc5727 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1767,6 +1767,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
     priv->namespaces = NULL;
 
     priv->reconnectBlockjobs = VIR_TRISTATE_BOOL_ABSENT;
+
+    virBitmapFree(priv->migrationCaps);
+    priv->migrationCaps = NULL;
 }
 
 
@@ -10122,3 +10125,68 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
 
     return ret;
 }
+
+
+int
+qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver,
+                                     virDomainObjPtr vm,
+                                     qemuDomainAsyncJob asyncJob)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    char **caps = NULL;
+    char **capStr;
+    int ret = -1;
+    int rc;
+
+    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+        return -1;
+
+    rc = qemuMonitorGetMigrationCapabilities(priv->mon, &caps);
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+        goto cleanup;
+
+    if (!caps) {
+        ret = 0;
+        goto cleanup;
+    }
+
+    priv->migrationCaps = virBitmapNew(QEMU_MONITOR_MIGRATION_CAPS_LAST);
+    if (!priv->migrationCaps)
+        goto cleanup;
+
+    for (capStr = caps; *capStr; capStr++) {
+        int cap = qemuMonitorMigrationCapsTypeFromString(*capStr);
+
+        if (cap < 0) {
+            VIR_DEBUG("Unknown migration capability: '%s'", *capStr);
+        } else {
+            ignore_value(virBitmapSetBit(priv->migrationCaps, cap));
+            VIR_DEBUG("Found migration capability: '%s'", *capStr);
+        }
+    }
+
+    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
+        if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+            goto cleanup;
+
+        rc = qemuMonitorSetMigrationCapability(priv->mon,
+                                               QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
+                                               true);
+
+        if (qemuDomainObjExitMonitor(driver, vm) < 0)
+            goto cleanup;
+
+        if (rc < 0) {
+            virResetLastError();
+            VIR_DEBUG("Cannot enable migration events; clearing capability");
+            virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
+        }
+    }
+
+    ret = 0;
+
+ cleanup:
+    virStringListFree(caps);
+    return ret;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 5201c6a0ac..fb20d8ea63 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
 
     /* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */
     virTristateBool reconnectBlockjobs;
+
+    /* Migration capabilities. Rechecked on reconnect, not to be saved in
+     * private XML. */
+    virBitmapPtr migrationCaps;
 };
 
 # define QEMU_DOMAIN_PRIVATE(vm)	\
@@ -978,4 +982,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm,
 char *
 qemuDomainGetMachineName(virDomainObjPtr vm);
 
+int
+qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver,
+                                     virDomainObjPtr vm,
+                                     qemuDomainAsyncJob asyncJob);
+
 #endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 24675498a2..cea2f90ce1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1816,18 +1816,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
     if (qemuProcessInitMonitor(driver, vm, asyncJob) < 0)
         return -1;
 
-    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
-        return -1;
-
-    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) &&
-        qemuMonitorSetMigrationCapability(priv->mon,
-                                          QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
-                                          true) < 0) {
-        VIR_DEBUG("Cannot enable migration events; clearing capability");
-        virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
-    }
-
-    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+    if (qemuDomainCheckMigrationCapabilities(driver, vm, asyncJob) < 0)
         return -1;
 
     return 0;
-- 
2.14.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
Posted by John Ferlan 7 years, 6 months ago

On 10/18/2017 07:29 AM, Jiri Denemark wrote:
> Each time we need to check whether a given migration capability is
> supported by QEMU, we call query-migrate-capabilities QMP command and
> lookup the capability in the returned list. Asking for the list of
> supported capabilities once when we connect to QEMU and storing the
> result in a bitmap is much better and we don't need to enter a monitor
> just to check whether a migration capability is supported.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/qemu/qemu_domain.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  9 +++++++
>  src/qemu/qemu_process.c | 13 +---------
>  3 files changed, 78 insertions(+), 12 deletions(-)
> 

There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and
qemuDomainObjPrivateXMLParse in order to handle the restart scenario.

The rest of this looks OK, but do you need the Format/Parse logic for
the bitmap?  It was late in my day and I was too lazy to go chase seeing
as I already did the merging ;-)

John

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 05e8b96aa4..a8cabc5727 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1767,6 +1767,9 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
>      priv->namespaces = NULL;
>  
>      priv->reconnectBlockjobs = VIR_TRISTATE_BOOL_ABSENT;
> +
> +    virBitmapFree(priv->migrationCaps);
> +    priv->migrationCaps = NULL;
>  }
>  
>  
> @@ -10122,3 +10125,68 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
>  
>      return ret;
>  }
> +
> +
> +int
> +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver,
> +                                     virDomainObjPtr vm,
> +                                     qemuDomainAsyncJob asyncJob)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char **caps = NULL;
> +    char **capStr;
> +    int ret = -1;
> +    int rc;
> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +        return -1;
> +
> +    rc = qemuMonitorGetMigrationCapabilities(priv->mon, &caps);
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
> +        goto cleanup;
> +
> +    if (!caps) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    priv->migrationCaps = virBitmapNew(QEMU_MONITOR_MIGRATION_CAPS_LAST);
> +    if (!priv->migrationCaps)
> +        goto cleanup;
> +
> +    for (capStr = caps; *capStr; capStr++) {
> +        int cap = qemuMonitorMigrationCapsTypeFromString(*capStr);
> +
> +        if (cap < 0) {
> +            VIR_DEBUG("Unknown migration capability: '%s'", *capStr);
> +        } else {
> +            ignore_value(virBitmapSetBit(priv->migrationCaps, cap));
> +            VIR_DEBUG("Found migration capability: '%s'", *capStr);
> +        }
> +    }
> +
> +    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) {
> +        if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +            goto cleanup;
> +
> +        rc = qemuMonitorSetMigrationCapability(priv->mon,
> +                                               QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
> +                                               true);
> +
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            goto cleanup;
> +
> +        if (rc < 0) {
> +            virResetLastError();
> +            VIR_DEBUG("Cannot enable migration events; clearing capability");
> +            virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
> +        }
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    virStringListFree(caps);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 5201c6a0ac..fb20d8ea63 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
>  
>      /* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */
>      virTristateBool reconnectBlockjobs;
> +
> +    /* Migration capabilities. Rechecked on reconnect, not to be saved in
> +     * private XML. */
> +    virBitmapPtr migrationCaps;
>  };
>  
>  # define QEMU_DOMAIN_PRIVATE(vm)	\
> @@ -978,4 +982,9 @@ qemuDomainFixupCPUs(virDomainObjPtr vm,
>  char *
>  qemuDomainGetMachineName(virDomainObjPtr vm);
>  
> +int
> +qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver,
> +                                     virDomainObjPtr vm,
> +                                     qemuDomainAsyncJob asyncJob);
> +
>  #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 24675498a2..cea2f90ce1 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1816,18 +1816,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
>      if (qemuProcessInitMonitor(driver, vm, asyncJob) < 0)
>          return -1;
>  
> -    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> -        return -1;
> -
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) &&
> -        qemuMonitorSetMigrationCapability(priv->mon,
> -                                          QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
> -                                          true) < 0) {
> -        VIR_DEBUG("Cannot enable migration events; clearing capability");
> -        virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
> -    }
> -
> -    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +    if (qemuDomainCheckMigrationCapabilities(driver, vm, asyncJob) < 0)
>          return -1;
>  
>      return 0;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
Posted by Jiri Denemark 7 years, 6 months ago
On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
> 
> 
> On 10/18/2017 07:29 AM, Jiri Denemark wrote:
> > Each time we need to check whether a given migration capability is
> > supported by QEMU, we call query-migrate-capabilities QMP command and
> > lookup the capability in the returned list. Asking for the list of
> > supported capabilities once when we connect to QEMU and storing the
> > result in a bitmap is much better and we don't need to enter a monitor
> > just to check whether a migration capability is supported.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/qemu/qemu_domain.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_domain.h  |  9 +++++++
> >  src/qemu/qemu_process.c | 13 +---------
> >  3 files changed, 78 insertions(+), 12 deletions(-)
> > 
> 
> There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and
> qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
> 
> The rest of this looks OK, but do you need the Format/Parse logic for
> the bitmap?

No. The migration capabilities are rechecked every time libvirt connects
to QEMU as said in the commit message and in qemu_domain.h:

> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> > index 5201c6a0ac..fb20d8ea63 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
> >  
> >      /* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */
> >      virTristateBool reconnectBlockjobs;
> > +
> > +    /* Migration capabilities. Rechecked on reconnect, not to be saved in
> > +     * private XML. */
> > +    virBitmapPtr migrationCaps;
> >  };

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
Posted by John Ferlan 7 years, 6 months ago

On 10/20/2017 03:03 AM, Jiri Denemark wrote:
> On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
>>
>>
>> On 10/18/2017 07:29 AM, Jiri Denemark wrote:
>>> Each time we need to check whether a given migration capability is
>>> supported by QEMU, we call query-migrate-capabilities QMP command and
>>> lookup the capability in the returned list. Asking for the list of
>>> supported capabilities once when we connect to QEMU and storing the
>>> result in a bitmap is much better and we don't need to enter a monitor
>>> just to check whether a migration capability is supported.
>>>
>>> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
>>> ---
>>>  src/qemu/qemu_domain.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  src/qemu/qemu_domain.h  |  9 +++++++
>>>  src/qemu/qemu_process.c | 13 +---------
>>>  3 files changed, 78 insertions(+), 12 deletions(-)
>>>
>>
>> There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and
>> qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
>>
>> The rest of this looks OK, but do you need the Format/Parse logic for
>> the bitmap?
> 
> No. The migration capabilities are rechecked every time libvirt connects
> to QEMU as said in the commit message and in qemu_domain.h:
> 

OK, so to be official...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 5201c6a0ac..fb20d8ea63 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
>>>  
>>>      /* Tracks blockjob state for vm. Valid only while reconnecting to qemu. */
>>>      virTristateBool reconnectBlockjobs;
>>> +
>>> +    /* Migration capabilities. Rechecked on reconnect, not to be saved in
>>> +     * private XML. */
>>> +    virBitmapPtr migrationCaps;
>>>  };
> 
> Jirka
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
Posted by Jiri Denemark 7 years, 6 months ago
On Fri, Oct 20, 2017 at 07:08:37 -0400, John Ferlan wrote:
> 
> 
> On 10/20/2017 03:03 AM, Jiri Denemark wrote:
> > On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
> >>
> >>
> >> On 10/18/2017 07:29 AM, Jiri Denemark wrote:
> >>> Each time we need to check whether a given migration capability is
> >>> supported by QEMU, we call query-migrate-capabilities QMP command and
> >>> lookup the capability in the returned list. Asking for the list of
> >>> supported capabilities once when we connect to QEMU and storing the
> >>> result in a bitmap is much better and we don't need to enter a monitor
> >>> just to check whether a migration capability is supported.
> >>>
> >>> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> >>> ---
> >>>  src/qemu/qemu_domain.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  src/qemu/qemu_domain.h  |  9 +++++++
> >>>  src/qemu/qemu_process.c | 13 +---------
> >>>  3 files changed, 78 insertions(+), 12 deletions(-)
> >>>
> >>
> >> There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and
> >> qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
> >>
> >> The rest of this looks OK, but do you need the Format/Parse logic for
> >> the bitmap?
> > 
> > No. The migration capabilities are rechecked every time libvirt connects
> > to QEMU as said in the commit message and in qemu_domain.h:
> > 
> 
> OK, so to be official...
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>

I pushed this series. Thanks for the review.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list