On Mon, Oct 12, 2015 at 12:16:13PM +0300, Alberto Garcia wrote:
> The 'snapshot-node-name' parameter of blockdev-snapshot-sync allows
> setting the node name of the image that is going to be created.
>
> Before creating the image, external_snapshot_prepare() checks that the
> name is not already being used. The check is however incomplete since
> it only considers existing node names, but node names must not clash
> with device IDs either because they share the same namespace.
>
> If the user attempts to create a snapshot using the name of an
> existing device for the 'snapshot-node-name' parameter the operation
> will eventually fail, but only after the new image has been created.
>
> This patch replaces bdrv_find_node() with bdrv_lookup_bs() to extend
> the check to existing device IDs, and thus detect possible name
> clashes before the new image is created.
>
> Signed-off-by: Alberto Garcia <be...@igalia.com>
> ---
> blockdev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 4731843..0898d1f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1570,8 +1570,9 @@ static void
> external_snapshot_prepare(BlkTransactionState *common,
> return;
> }
>
> - if (has_snapshot_node_name && bdrv_find_node(snapshot_node_name)) {
> - error_setg(errp, "New snapshot node name already existing");
> + if (has_snapshot_node_name &&
> + bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
> + error_setg(errp, "New snapshot node name already in use");
> return;
> }
>
> --
> 2.6.1
>
>
A downfall is we don't communicate back if the collision is with a
device name or a node name, but I guess that is relatively minor (and
probably not worth splitting this into two different calls to
bdrv_lookup_bs() to differentiate).
Reviewed-by: Jeff Cody <jc...@redhat.com>
