[PATCH 01/12] migration/migration-pin: get migration pid for migration pin

Jiang Jiacheng posted 12 patches 3 years, 1 month ago
There is a newer version of this series
[PATCH 01/12] migration/migration-pin: get migration pid for migration pin
Posted by Jiang Jiacheng 3 years, 1 month ago
From: zhengchuan <zhengchuan@huawei.com>

Firstly, we need to get migration pids, add
virDomainMigrateGetMigrationPids() for migration pin.

Signed-off-by:zhengchuan<zhengchuan@huawei.com>
---
 include/libvirt/libvirt-domain.h |  3 +++
 src/driver-hypervisor.h          |  5 ++++
 src/libvirt-domain.c             | 39 ++++++++++++++++++++++++++++++++
 src/libvirt_public.syms          |  5 ++++
 src/qemu/qemu_domain.c           |  2 ++
 src/qemu/qemu_domain.h           |  2 ++
 src/qemu/qemu_driver.c           | 31 +++++++++++++++++++++++++
 src/remote/remote_driver.c       |  1 +
 src/remote/remote_protocol.x     | 17 +++++++++++++-
 src/remote_protocol-structs      |  8 +++++++
 10 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 295fd30c93..e11f2795f1 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -6457,4 +6457,7 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
                                 int seconds,
                                 unsigned int flags);
 
+char *virDomainMigrateGetMigrationPids(virDomainPtr domain,
+                                       unsigned int flags);
+
 #endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 016d5cec7c..618f116012 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1441,6 +1441,10 @@ typedef int
                                   int seconds,
                                   unsigned int flags);
 
+typedef char *
+(*virDrvDomainMigrateGetMigrationPids)(virDomainPtr domain,
+                                       unsigned int flags);
+
 typedef struct _virHypervisorDriver virHypervisorDriver;
 
 /**
@@ -1712,4 +1716,5 @@ struct _virHypervisorDriver {
     virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet;
     virDrvDomainGetMessages domainGetMessages;
     virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc;
+    virDrvDomainMigrateGetMigrationPids domainMigrateGetMigrationPids;
 };
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 78c26b2219..fabfb2dd7f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13972,3 +13972,42 @@ virDomainStartDirtyRateCalc(virDomainPtr domain,
     virDispatchError(conn);
     return -1;
 }
+
+/**
+ * virDomainMigrateGetMigrationPids:
+ * @domain: pointer to domain object
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Get migration thread pid.
+ *
+ * Returns the migration pids which must be freed by the caller, or
+ * NULL if there was an error.
+ *
+ * Since: 9.1.0
+ */
+char *
+virDomainMigrateGetMigrationPids(virDomainPtr domain,
+                                 unsigned int flags)
+{
+    virConnectPtr conn;
+    VIR_DOMAIN_DEBUG(domain, "migration pids flags=0x%x", flags);
+    virResetLastError();
+    virCheckDomainReturn(domain, NULL);
+    conn = domain->conn;
+
+    virCheckReadOnlyGoto(domain->conn->flags, error);
+
+    if (conn->driver->domainMigrateGetMigrationPids) {
+        char *ret;
+        ret = conn->driver->domainMigrateGetMigrationPids(domain, flags);
+        if (!ret)
+            goto error;
+        return ret;
+    }
+
+    virReportUnsupportedError();
+
+ error:
+    virDispatchError(domain->conn);
+    return NULL;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 297a2c436a..f11fe1b26b 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -927,4 +927,9 @@ LIBVIRT_8.5.0 {
         virDomainAbortJobFlags;
 } LIBVIRT_8.4.0;
 
+LIBVIRT_9.1.0 {
+    global:
+        virDomainMigrateGetMigrationPids;
+} LIBVIRT_8.5.0;
+
 # .... define new API here using predicted next version number ....
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5c05032ce3..0bff24dc47 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1780,6 +1780,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivate *priv)
     /* remove automatic pinning data */
     g_clear_pointer(&priv->autoNodeset, virBitmapFree);
     g_clear_pointer(&priv->autoCpuset, virBitmapFree);
+    g_clear_pointer(&priv->pcpumap, virBitmapFree);
     g_clear_pointer(&priv->pciaddrs, virDomainPCIAddressSetFree);
     g_clear_pointer(&priv->usbaddrs, virDomainUSBAddressSetFree);
     g_clear_pointer(&priv->origCPU, virCPUDefFree);
@@ -1827,6 +1828,7 @@ qemuDomainObjPrivateFree(void *data)
     virObjectUnref(priv->monConfig);
     g_free(priv->lockState);
     g_free(priv->origname);
+    g_free(priv->migrationPids);
 
     virChrdevFree(priv->devs);
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2f027fad87..a804a1b46e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -139,6 +139,7 @@ struct _qemuDomainObjPrivate {
     char *origname;
     int nbdPort; /* Port used for migration with NBD */
     unsigned short migrationPort;
+    char *migrationPids;
     int preMigrationState;
     unsigned long long preMigrationMemlock; /* Original RLIMIT_MEMLOCK in case
                                                it was changed for the current
@@ -163,6 +164,7 @@ struct _qemuDomainObjPrivate {
     /* Bitmaps below hold data from the auto NUMA feature */
     virBitmap *autoNodeset;
     virBitmap *autoCpuset;
+    virBitmap *pcpumap;
 
     bool signalIOError; /* true if the domain condition should be signalled on
                            I/O error */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d509582719..b9dc5f29f5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20648,6 +20648,36 @@ qemuDomainStartDirtyRateCalc(virDomainPtr dom,
     return ret;
 }
 
+static char *
+qemuDomainMigrateGetMigrationPids(virDomainPtr dom,
+                                  unsigned int flags)
+{
+    char *ret = NULL;
+    virDomainObj *vm = NULL;
+    qemuDomainObjPrivate *priv = NULL;
+
+    virCheckFlags(0, NULL);
+
+    vm = qemuDomainObjFromDomain(dom);
+    if (!vm)
+        goto cleanup;
+
+    if (virDomainMigrateGetMigrationPidsEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    priv = vm->privateData;
+
+    if (priv->migrationPids) {
+        ret = g_strdup(priv->migrationPids);
+        if (!ret)
+            VIR_ERROR(_("failed to strdup migrationPids"));
+    }
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
 
 static virHypervisorDriver qemuHypervisorDriver = {
     .name = QEMU_DRIVER_NAME,
@@ -20897,6 +20927,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
     .domainGetMessages = qemuDomainGetMessages, /* 7.1.0 */
     .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */
     .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */
+    .domainMigrateGetMigrationPids = qemuDomainMigrateGetMigrationPids, /* 9.1.0 */
 };
 
 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b0dba9057b..9d8d7142b7 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8651,6 +8651,7 @@ static virHypervisorDriver hypervisor_driver = {
     .domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */
     .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */
     .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */
+    .domainMigrateGetMigrationPids = remoteDomainMigrateGetMigrationPids, /* 9.1.0 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 7dfb4548f4..d1b799db13 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2528,6 +2528,15 @@ struct remote_domain_migrate_set_compression_cache_args {
     unsigned int flags;
 };
 
+struct remote_domain_migrate_get_migration_pids_args {
+    remote_nonnull_domain dom;
+    unsigned int flags;
+};
+
+struct remote_domain_migrate_get_migration_pids_ret {
+    remote_nonnull_string migrationPids;
+};
+
 struct remote_domain_migrate_set_max_speed_args {
     remote_nonnull_domain dom;
     unsigned hyper bandwidth;
@@ -6961,5 +6970,11 @@ enum remote_procedure {
      * @generate: both
      * @acl: domain:write
      */
-    REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442
+    REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442,
+
+    /**
+     * @generate: both
+     * @acl: domain:migrate
+     */
+    REMOTE_PROC_DOMAIN_MIGRATE_GET_MIGRATION_PIDS = 443
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index ca5222439d..da4d799f42 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -3268,6 +3268,13 @@ struct remote_domain_event_memory_device_size_change_msg {
         remote_nonnull_string      alias;
         uint64_t                   size;
 };
+struct remote_domain_migrate_get_migration_pids_args {
+        remote_nonnull_domain      dom;
+        u_int                      flags;
+};
+struct remote_domain_migrate_get_migration_pids_ret {
+        remote_nonnull_string      migrationPids;
+};
 enum remote_procedure {
         REMOTE_PROC_CONNECT_OPEN = 1,
         REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3711,4 +3718,5 @@ enum remote_procedure {
         REMOTE_PROC_DOMAIN_SAVE_PARAMS = 440,
         REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441,
         REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442,
+        REMOTE_PROC_DOMAIN_MIGRATE_GET_MIGRATION_PIDS = 443,
 };
-- 
2.33.0
Re: [PATCH 01/12] migration/migration-pin: get migration pid for migration pin
Posted by Daniel P. Berrangé 3 years, 1 month ago
On Tue, Jan 03, 2023 at 09:08:20PM +0800, Jiang Jiacheng wrote:
> From: zhengchuan <zhengchuan@huawei.com>
> 
> Firstly, we need to get migration pids, add
> virDomainMigrateGetMigrationPids() for migration pin.
> 
> Signed-off-by:zhengchuan<zhengchuan@huawei.com>
> ---
>  include/libvirt/libvirt-domain.h |  3 +++
>  src/driver-hypervisor.h          |  5 ++++
>  src/libvirt-domain.c             | 39 ++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  5 ++++
>  src/qemu/qemu_domain.c           |  2 ++
>  src/qemu/qemu_domain.h           |  2 ++
>  src/qemu/qemu_driver.c           | 31 +++++++++++++++++++++++++
>  src/remote/remote_driver.c       |  1 +
>  src/remote/remote_protocol.x     | 17 +++++++++++++-
>  src/remote_protocol-structs      |  8 +++++++
>  10 files changed, 112 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 295fd30c93..e11f2795f1 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -6457,4 +6457,7 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
>                                  int seconds,
>                                  unsigned int flags);
>  
> +char *virDomainMigrateGetMigrationPids(virDomainPtr domain,
> +                                       unsigned int flags);

Exposing QEMU thread PIDs in the public libvirt API is not something
we should be doing. We've explicitly aimed to avoid exposing the notion
of PIDs in our API in general.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 01/12] migration/migration-pin: get migration pid for migration pin
Posted by Jiang Jiacheng 3 years, 1 month ago

On 2023/1/9 22:46, Daniel P. Berrangé wrote:
> On Tue, Jan 03, 2023 at 09:08:20PM +0800, Jiang Jiacheng wrote:
>> From: zhengchuan <zhengchuan@huawei.com>
>>
>> Firstly, we need to get migration pids, add
>> virDomainMigrateGetMigrationPids() for migration pin.
>>
>> Signed-off-by:zhengchuan<zhengchuan@huawei.com>
>> ---
>>  include/libvirt/libvirt-domain.h |  3 +++
>>  src/driver-hypervisor.h          |  5 ++++
>>  src/libvirt-domain.c             | 39 ++++++++++++++++++++++++++++++++
>>  src/libvirt_public.syms          |  5 ++++
>>  src/qemu/qemu_domain.c           |  2 ++
>>  src/qemu/qemu_domain.h           |  2 ++
>>  src/qemu/qemu_driver.c           | 31 +++++++++++++++++++++++++
>>  src/remote/remote_driver.c       |  1 +
>>  src/remote/remote_protocol.x     | 17 +++++++++++++-
>>  src/remote_protocol-structs      |  8 +++++++
>>  10 files changed, 112 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index 295fd30c93..e11f2795f1 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -6457,4 +6457,7 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
>>                                  int seconds,
>>                                  unsigned int flags);
>>  
>> +char *virDomainMigrateGetMigrationPids(virDomainPtr domain,
>> +                                       unsigned int flags);
> 
> Exposing QEMU thread PIDs in the public libvirt API is not something
> we should be doing. We've explicitly aimed to avoid exposing the notion
> of PIDs in our API in general.
> 

Thanks for your reply!

As we should not exposing QEMU thread PIDs, is it a better way to
proactively detect the PIDs of the qemu migration thread?
For example, using QMP command during migration to detect the QEMU
migration thread PIDs like iothreadpin does. Compared to QEMU event,
detecting proactively may spend more time to complete migration pin,
but i think its still makes sense for migration.


Thanks,
Jiang Jiacheng

> 
> With regards,
> Daniel
Re: [PATCH 01/12] migration/migration-pin: get migration pid for migration pin
Posted by Daniel P. Berrangé 3 years, 1 month ago
On Tue, Jan 10, 2023 at 07:30:08PM +0800, Jiang Jiacheng wrote:
> 
> 
> On 2023/1/9 22:46, Daniel P. Berrangé wrote:
> > On Tue, Jan 03, 2023 at 09:08:20PM +0800, Jiang Jiacheng wrote:
> >> From: zhengchuan <zhengchuan@huawei.com>
> >>
> >> Firstly, we need to get migration pids, add
> >> virDomainMigrateGetMigrationPids() for migration pin.
> >>
> >> Signed-off-by:zhengchuan<zhengchuan@huawei.com>
> >> ---
> >>  include/libvirt/libvirt-domain.h |  3 +++
> >>  src/driver-hypervisor.h          |  5 ++++
> >>  src/libvirt-domain.c             | 39 ++++++++++++++++++++++++++++++++
> >>  src/libvirt_public.syms          |  5 ++++
> >>  src/qemu/qemu_domain.c           |  2 ++
> >>  src/qemu/qemu_domain.h           |  2 ++
> >>  src/qemu/qemu_driver.c           | 31 +++++++++++++++++++++++++
> >>  src/remote/remote_driver.c       |  1 +
> >>  src/remote/remote_protocol.x     | 17 +++++++++++++-
> >>  src/remote_protocol-structs      |  8 +++++++
> >>  10 files changed, 112 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> >> index 295fd30c93..e11f2795f1 100644
> >> --- a/include/libvirt/libvirt-domain.h
> >> +++ b/include/libvirt/libvirt-domain.h
> >> @@ -6457,4 +6457,7 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
> >>                                  int seconds,
> >>                                  unsigned int flags);
> >>  
> >> +char *virDomainMigrateGetMigrationPids(virDomainPtr domain,
> >> +                                       unsigned int flags);
> > 
> > Exposing QEMU thread PIDs in the public libvirt API is not something
> > we should be doing. We've explicitly aimed to avoid exposing the notion
> > of PIDs in our API in general.
> > 
> 
> Thanks for your reply!
> 
> As we should not exposing QEMU thread PIDs, is it a better way to
> proactively detect the PIDs of the qemu migration thread?
> For example, using QMP command during migration to detect the QEMU
> migration thread PIDs like iothreadpin does. Compared to QEMU event,
> detecting proactively may spend more time to complete migration pin,
> but i think its still makes sense for migration.

iothreadpin doesn't take thread PIDs, it takes integer
identifiers associated with the threads.

Why does the mgmt app need to know the migration PIDs ? The
API for setting pinning in the next patch doesn't accept a
PID, and we don't need to set different pinning for each
migration thread. Only libvirt QEMU driver code needs to
know the PIDs, not the libvirt public API/mgmt app.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 01/12] migration/migration-pin: get migration pid for migration pin
Posted by Jiang Jiacheng 3 years, 1 month ago

On 2023/1/10 19:44, Daniel P. Berrangé wrote:
> On Tue, Jan 10, 2023 at 07:30:08PM +0800, Jiang Jiacheng wrote:
>>
>>
>> On 2023/1/9 22:46, Daniel P. Berrangé wrote:
>>> On Tue, Jan 03, 2023 at 09:08:20PM +0800, Jiang Jiacheng wrote:
>>>> From: zhengchuan <zhengchuan@huawei.com>
>>>>
>>>> Firstly, we need to get migration pids, add
>>>> virDomainMigrateGetMigrationPids() for migration pin.
>>>>
>>>> Signed-off-by:zhengchuan<zhengchuan@huawei.com>
>>>> ---
>>>>  include/libvirt/libvirt-domain.h |  3 +++
>>>>  src/driver-hypervisor.h          |  5 ++++
>>>>  src/libvirt-domain.c             | 39 ++++++++++++++++++++++++++++++++
>>>>  src/libvirt_public.syms          |  5 ++++
>>>>  src/qemu/qemu_domain.c           |  2 ++
>>>>  src/qemu/qemu_domain.h           |  2 ++
>>>>  src/qemu/qemu_driver.c           | 31 +++++++++++++++++++++++++
>>>>  src/remote/remote_driver.c       |  1 +
>>>>  src/remote/remote_protocol.x     | 17 +++++++++++++-
>>>>  src/remote_protocol-structs      |  8 +++++++
>>>>  10 files changed, 112 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>>>> index 295fd30c93..e11f2795f1 100644
>>>> --- a/include/libvirt/libvirt-domain.h
>>>> +++ b/include/libvirt/libvirt-domain.h
>>>> @@ -6457,4 +6457,7 @@ int virDomainStartDirtyRateCalc(virDomainPtr domain,
>>>>                                  int seconds,
>>>>                                  unsigned int flags);
>>>>  
>>>> +char *virDomainMigrateGetMigrationPids(virDomainPtr domain,
>>>> +                                       unsigned int flags);
>>>
>>> Exposing QEMU thread PIDs in the public libvirt API is not something
>>> we should be doing. We've explicitly aimed to avoid exposing the notion
>>> of PIDs in our API in general.
>>>
>>
>> Thanks for your reply!
>>
>> As we should not exposing QEMU thread PIDs, is it a better way to
>> proactively detect the PIDs of the qemu migration thread?
>> For example, using QMP command during migration to detect the QEMU
>> migration thread PIDs like iothreadpin does. Compared to QEMU event,
>> detecting proactively may spend more time to complete migration pin,
>> but i think its still makes sense for migration.
> 
> iothreadpin doesn't take thread PIDs, it takes integer
> identifiers associated with the threads.
> 
> Why does the mgmt app need to know the migration PIDs ? The
> API for setting pinning in the next patch doesn't accept a
> PID, and we don't need to set different pinning for each
> migration thread. Only libvirt QEMU driver code needs to
> know the PIDs, not the libvirt public API/mgmt app.
> 
>

I have got that the pids shouldn't show up in libvirt public API/mgmt
app, so I want to drop those changes related with PIDs in mgmt app and
try to get the migration pid using qmp command like the way getting
iothreads' pid in 'qemuProcessDetectIOThreadPIDs' in qemu_process.c.

Thanks
Jiang Jiacheng
> With regards,
> Daniel