qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd member @arg
Date: Tue, 8 Nov 2022 17:21:21 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2

On 11/4/22 19:06, Markus Armbruster wrote:
block-export-add argument @name defaults to the value of argument
@node-name.

nbd_export_create() implements this by copying @node_name to @name.
It leaves @has_node_name false, violating the "has_node_name ==
!!node_name" invariant.  Unclean.  Falls apart when we elide
@has_node_name (next commit): then QAPI frees the same value twice,
once for @node_name and once @name.  iotest 307 duly explodes.

Goes back to commit c62d24e906 "blockdev-nbd: Boxed argument type for
nbd-server-add" (v5.0.0).  Got moved from qmp_nbd_server_add() to
nbd_export_create() (commit 56ee86261e), then copied back (commit
b6076afcab).  Commit 8675cbd68b "nbd: Utilize QAPI_CLONE for type
conversion" (v5.2.0) cleaned up the copy in qmp_nbd_server_add()
noting

     Second, our assignment to arg->name is fishy: the generated QAPI code
     for qapi_free_NbdServerAddOptions does not visit arg->name if
     arg->has_name is false, but if it DID visit it, we would have
     introduced a double-free situation when arg is finally freed.

Exactly.  However, the copy in nbd_export_create() remained dirty.

Clean it up.  Since the value stored in member @name is not actually
used outside this function, use a local variable instead of modifying
the QAPI object.

Signed-off-by: Markus Armbruster<armbru@redhat.com>
Cc: Eric Blake<eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
Cc:qemu-block@nongnu.org


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]