From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v2 2/3] migration: fix memory leak when updating tls-creds and tls-hostname Date: Mon, 18 Feb 2019 16:26:45 +0800 Message-ID: References: <20190111063732.10484-1-xiaoguangrong@tencent.com> <20190111063732.10484-3-xiaoguangrong@tencent.com> <20190115075154.GI24343@xz-x1> <20190115102404.GA2135@work-vm> <2caebc3a-ccd0-d8c6-0368-4f339f326cdf@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, mst@redhat.com, mtosatti@redhat.com, Xiao Guangrong , qemu-devel@nongnu.org, Markus Armbruster , quintela@redhat.com, wei.w.wang@intel.com, cota@braap.org, pbonzini@redhat.com To: Eric Blake , "Dr. David Alan Gilbert" , Peter Xu Return-path: In-Reply-To: <2caebc3a-ccd0-d8c6-0368-4f339f326cdf@redhat.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel2=m.gmane.org@nongnu.org Sender: "Qemu-devel" List-Id: kvm.vger.kernel.org On 1/16/19 12:03 AM, Eric Blake wrote: > On 1/15/19 4:24 AM, Dr. David Alan Gilbert wrote: > >> I think the problem is that >> migrate_params_check checks a MigrationParameters >> >> while the QMP command gives us a MigrateSetParameters; but we also use >> migrate_params_check for the global check you added (8b0b29dc) which is >> against migrationParameters; so that's why migrate_params_check takes >> a MigrationParameters. >> >> It's horrible we've got stuff duped so much. > > Indeed. > >> >> However, I don't like this fix because if someone later was to add >> a test for tls parameters to migrate_params_check, then they would be >> confused why the hostname/creds weren't checked. >> So while we have migrate_params_test_apply, it should cover all >> parameters. >> >> I think a cleaner check would be to write a MigrateParameters_free >> that free'd any strings, and call that in qmp_migrate_set_parameters >> on both exit paths. > > We already have it; it's named qapi_free_MigrationParameters(), > generated in qapi-types-migration.h. > Thank you all (and sorry for the delay reply due to Chinese New Year :)), i will use this interface instead in the next version.