[PATCH v3 2/7] migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Oct 27 19:14:29 UTC 2020



On 10/27/20 9:47 AM, Hao Wang wrote:
> Introduce DomainGetDirtyRateInfo API for domain's memory dirty rate
> calculation and query.
> 
> Signed-off-by: Hao Wang <wanghao232 at huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan at huawei.com>
> ---


This patch does not compile/test successfully in my env. There are
at least 2 things going wrong here:


- the stub you added in qemu_driver.c is generating compile warnings:

../src/qemu/qemu_driver.c: In function ‘qemuDomainGetDirtyRateInfo’:
../src/qemu/qemu_driver.c:20127:41: error: unused parameter ‘dom’ [-Werror=unused-parameter]
20127 | qemuDomainGetDirtyRateInfo(virDomainPtr dom,
       |                            ~~~~~~~~~~~~~^~~
../src/qemu/qemu_driver.c:20128:54: error: unused parameter ‘info’ [-Werror=unused-parameter]
20128 |                            virDomainDirtyRateInfoPtr info,
       |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
../src/qemu/qemu_driver.c:20129:38: error: unused parameter ‘sec’ [-Werror=unused-parameter]
20129 |                            long long sec,
       |                            ~~~~~~~~~~^~~
../src/qemu/qemu_driver.c:20130:32: error: unused parameter ‘flags’ [-Werror=unused-parameter]
20130 |


This is the code:


static int
qemuDomainGetDirtyRateInfo(virDomainPtr dom,
                            virDomainDirtyRateInfoPtr info,
                            long long sec,
                            int flags)
{
     /* TODO */
     return 0;
}



I don't think you need this placeholder here. You're not using the domainGetDirtyRateInfo
API anywhere in this patch, so just move this code to patch 03 where the
function is implemented.


After fixing it myself and compiling it again I found another issue:



--- command ---
17:56:04 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3 /home/danielhb/kvm-project/libvirt/scripts/check-remote-protocol.py remote_protocol virt_remote_driver /home/danielhb/kvm-project/libvirt/build/src/remote/libvirt_remote_driver.a /usr/bin/pdwtags /home/danielhb/kvm-project/libvirt/build/../src/remote_protocol-structs
--- stdout ---
--- /home/danielhb/kvm-project/libvirt/build/../src/remote_protocol-structs	2020-10-23 08:48:53.871557535 -0300
+++ -	2020-10-27 14:56:04.587438336 -0300
@@ -3142,6 +3142,17 @@
  struct remote_domain_backup_get_xml_desc_ret {
          remote_nonnull_string      xml;
  };
+struct remote_domain_get_dirty_rate_info_args {
+        remote_nonnull_domain      dom;
+        int64_t                    sec;
+        int                        flags;
+};
+struct remote_domain_get_dirty_rate_info_ret {
+        int                        status;
+        int64_t                    dirtyRate;
+        int64_t                    startTime;
+        int64_t                    calcTime;
+};
  enum remote_procedure {
          REMOTE_PROC_CONNECT_OPEN = 1,
          REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3566,4 +3577,5 @@
          REMOTE_PROC_DOMAIN_BACKUP_BEGIN = 421,
          REMOTE_PROC_DOMAIN_BACKUP_GET_XML_DESC = 422,
          REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423,
+        REMOTE_PROC_DOMAIN_GET_DIRTY_RATE_INFO = 424,
  };
-------

152/307 libvirt:syntax-check / sc_flags_usage                                    FAIL           2.75s (exit status 2)

--- command ---
17:56:11 /usr/bin/make -C /home/danielhb/kvm-project/libvirt/build/build-aux sc_flags_usage
--- stdout ---
make: Entering directory '/home/danielhb/kvm-project/libvirt/build/build-aux'
flags_usage
/home/danielhb/kvm-project/libvirt/include/libvirt/libvirt-domain.h:5126:                              int flags);
/home/danielhb/kvm-project/libvirt/src/driver-hypervisor.h:1394:                                int flags);
/home/danielhb/kvm-project/libvirt/src/libvirt-domain.c:12778:                          int flags)
/home/danielhb/kvm-project/libvirt/src/remote/remote_protocol.x:3785:    int flags;
make: Leaving directory '/home/danielhb/kvm-project/libvirt/build/build-aux'
--- stderr ---
build-aux/syntax-check.mk: flags should be unsigned
make: *** [/home/danielhb/kvm-project/libvirt/build-aux/syntax-check.mk:342: sc_flags_usage] Error 1
-------


 From the error message I believe it has to do with 'flags' being an int
instead of unsigned int. This changes goes back to patch 1, where you introduced
the virDomainDirtyRateInfo. You'll need to change the flag type to unsigned
int in both patches 1 and 2, regenerated the protocol files and see if you
get the tests passed.



And for the record, what I'm doing to compile/test the patches is

$ ninja -C build test



Thanks,


DHB



>   include/libvirt/libvirt-domain.h |  5 ++++
>   src/driver-hypervisor.h          |  7 +++++
>   src/libvirt-domain.c             | 46 ++++++++++++++++++++++++++++++++
>   src/libvirt_public.syms          |  5 ++++
>   src/qemu/qemu_driver.c           | 12 +++++++++
>   src/remote/remote_driver.c       |  1 +
>   src/remote/remote_protocol.x     | 21 ++++++++++++++-
>   7 files changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 145f517068..1c63191baa 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5120,4 +5120,9 @@ struct _virDomainDirtyRateInfo {
>   
>   typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr;
>   
> +int virDomainGetDirtyRateInfo(virDomainPtr domain,
> +                              virDomainDirtyRateInfoPtr info,
> +                              long long sec,
> +                              int flags);
> +
>   #endif /* LIBVIRT_DOMAIN_H */
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index bce023017d..dc2aefa910 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1387,6 +1387,12 @@ typedef char *
>   (*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain,
>                                   unsigned int flags);
>   
> +typedef int
> +(*virDrvDomainGetDirtyRateInfo)(virDomainPtr domain,
> +                                virDomainDirtyRateInfoPtr info,
> +                                long long sec,
> +                                int flags);
> +
>   typedef struct _virHypervisorDriver virHypervisorDriver;
>   typedef virHypervisorDriver *virHypervisorDriverPtr;
>   
> @@ -1650,4 +1656,5 @@ struct _virHypervisorDriver {
>       virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout;
>       virDrvDomainBackupBegin domainBackupBegin;
>       virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
> +    virDrvDomainGetDirtyRateInfo domainGetDirtyRateInfo;
>   };
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 3c5f55176a..8714c1ca93 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12758,3 +12758,49 @@ virDomainBackupGetXMLDesc(virDomainPtr domain,
>       virDispatchError(conn);
>       return NULL;
>   }
> +
> +
> +/**
> + * virDomainGetDirtyRateInfo:
> + * @domain: a domain object.
> + * @info: return value of current domain's memory dirty rate info.
> + * @sec: show dirty rate within specified seconds.
> + * @flags: the flags of getdirtyrate action -- calculate and/or query.
> + *
> + * Get the current domain's memory dirty rate (in MB/s).
> + *
> + * Returns 0 in case of success, -1 otherwise.
> + */
> +int
> +virDomainGetDirtyRateInfo(virDomainPtr domain,
> +                          virDomainDirtyRateInfoPtr info,
> +                          long long sec,
> +                          int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(domain, -1);
> +    conn = domain->conn;
> +
> +    virCheckNonNullArgGoto(info, error);
> +    virCheckReadOnlyGoto(conn->flags, error);
> +
> +    if (info)
> +        memset(info, 0, sizeof(*info));
> +
> +    if (conn->driver->domainGetDirtyRateInfo) {
> +        if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags) < 0)
> +            goto error;
> +        VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
> +        return 0;
> +    }
> +
> +    virReportUnsupportedError();
> + error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 539d2e3943..11864f48b1 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -873,4 +873,9 @@ LIBVIRT_6.0.0 {
>           virDomainBackupGetXMLDesc;
>   } LIBVIRT_5.10.0;
>   
> +LIBVIRT_6.9.0 {
> +    global:
> +        virDomainGetDirtyRateInfo;
> +} LIBVIRT_6.0.0;
> +
>   # .... define new API here using predicted next version number ....
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f1191c1210..513290c934 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20123,6 +20123,17 @@ qemuDomainAgentSetResponseTimeout(virDomainPtr dom,
>   }
>   
>   
> +static int
> +qemuDomainGetDirtyRateInfo(virDomainPtr dom,
> +                           virDomainDirtyRateInfoPtr info,
> +                           long long sec,
> +                           int flags)
> +{
> +    /* TODO */
> +    return 0;
> +}
> +
> +
>   static virHypervisorDriver qemuHypervisorDriver = {
>       .name = QEMU_DRIVER_NAME,
>       .connectURIProbe = qemuConnectURIProbe,
> @@ -20362,6 +20373,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
>       .domainAgentSetResponseTimeout = qemuDomainAgentSetResponseTimeout, /* 5.10.0 */
>       .domainBackupBegin = qemuDomainBackupBegin, /* 6.0.0 */
>       .domainBackupGetXMLDesc = qemuDomainBackupGetXMLDesc, /* 6.0.0 */
> +    .domainGetDirtyRateInfo = qemuDomainGetDirtyRateInfo, /* 6.9.0 */
>   };
>   
>   
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 9cd2fd36ae..325968a22b 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -8458,6 +8458,7 @@ static virHypervisorDriver hypervisor_driver = {
>       .domainAgentSetResponseTimeout = remoteDomainAgentSetResponseTimeout, /* 5.10.0 */
>       .domainBackupBegin = remoteDomainBackupBegin, /* 6.0.0 */
>       .domainBackupGetXMLDesc = remoteDomainBackupGetXMLDesc, /* 6.0.0 */
> +    .domainGetDirtyRateInfo = remoteDomainGetDirtyRateInfo, /* 6.9.0 */
>   };
>   
>   static virNetworkDriver network_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 5e5e781e76..3838383bc9 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -3779,6 +3779,19 @@ struct remote_domain_backup_get_xml_desc_ret {
>       remote_nonnull_string xml;
>   };
>   
> +struct remote_domain_get_dirty_rate_info_args {
> +    remote_nonnull_domain dom;
> +    hyper sec;
> +    int flags;
> +};
> +
> +struct remote_domain_get_dirty_rate_info_ret { /* insert at 1 */
> +    int status;
> +    hyper dirtyRate;
> +    hyper startTime;
> +    hyper calcTime;
> +};
> +
>   /*----- Protocol. -----*/
>   
>   /* Define the program number, protocol version and procedure numbers here. */
> @@ -6682,5 +6695,11 @@ enum remote_procedure {
>        * @generate: both
>        * @acl: none
>        */
> -    REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423
> +    REMOTE_PROC_DOMAIN_EVENT_MEMORY_FAILURE = 423,
> +
> +    /**
> +     * @generate: both
> +     * @acl: domain:read
> +     */
> +    REMOTE_PROC_DOMAIN_GET_DIRTY_RATE_INFO = 424
>   };
> 




More information about the libvir-list mailing list