[libvirt] [PATCH] qemu_blockjob: Don't leak old job when registering a new one

Peter Krempa pkrempa at redhat.com
Sat Sep 14 07:33:32 UTC 2019


On Fri, Sep 13, 2019 at 22:26:35 +0200, Michal Privoznik wrote:
> If a disk that a block job is registered with already has a job
> (stored in private data) it needs to be unrefed to keep the
> refcounter in balance (and properly free the object):
> 
> 2,577 (104 direct, 2,473 indirect) bytes in 1 blocks are definitely lost in loss record 168 of 174
>    at 0x4838B86: calloc (vg_replace_malloc.c:752)
>    by 0x4A0AD42: virAllocVar (viralloc.c:346)
>    by 0x4A84D12: virObjectNew (virobject.c:243)
>    by 0x21EE13: qemuBlockJobDataNew (qemu_blockjob.c:111)
>    by 0x184EBC: qemuDomainObjPrivateXMLParseBlockjobData (qemu_domain.c:3120)
>    by 0x1854B5: qemuDomainObjPrivateXMLParseBlockjobs (qemu_domain.c:3191)
>    by 0x186D7D: qemuDomainObjPrivateXMLParse (qemu_domain.c:3622)
>    by 0x4B198CC: virDomainObjParseXML (domain_conf.c:21490)
>    by 0x4B19DDF: virDomainObjParseNode (domain_conf.c:21630)
>    by 0x4B19E7D: virDomainObjParseFile (domain_conf.c:21649)
>    by 0x13FDD5: testCompareStatusXMLToXMLFiles (qemuxml2xmltest.c:61)
>    by 0x16F7D2: virTestRun (testutils.c:115)

Is there some wrong test data perhaps? I suspect I made a mistake and
there's two jobs referencing the same disk in the (fake) test data.

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_blockjob.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index a991309ee7..67b1f677f8 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -151,6 +151,7 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job,
>      if (disk) {
>          job->disk = disk;
>          job->chain = virObjectRef(disk->src);
> +        virObjectUnref(QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob);

This is not the right fix. Our block job code only ever allows one job
per disk. The only thing we can do here is repeat the logic to refuse
to register a job if there is one already registered prior to doing
other things into the function but we should never just unregister
random jobs from disks.

>          QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job);
>      }
>  
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190914/66e72d2d/attachment-0001.sig>


More information about the libvir-list mailing list