Re: [Qemu-devel] [PATCH 2/3] Test for full Backup

[Stefan Hajnoczi]

On Wed, Aug 23, 2017 at 06:34:39PM +0530, Ishani Chugh wrote:
> +CONFIG_FILE=$TEST_DIR/backup-config
> +SOCKET=unix:$TEST_DIR/backup_socket
> +size=128M
> +
> +_make_test_img $size
> +export QEMU_BACKUP_CONFIG=$CONFIG_FILE
> +qemu_comm_method="monitor"
> +echo
> +_launch_qemu -drive if=virtio,file=$TEST_IMG -qmp $SOCKET,server,nowait

I tend to put doublequotes around any variable expansion unless the
variable is trivially guaranteed to contain no spaces.  Imagine
TEST_DIR="a b", then your command expands to:

  '-qmp' 'unix:a' 'b/backup_socket,server,nowait'

QEMU will think the option is -qmp unix:a only.

If you use doublequotes then the space will not split the argument into
multiple arguments:

  -qmp "$SOCKET",server,nowait

expands to:

  '-qmp' 'unix:a b/backup_socket,server,nowait'

Please use doublequotes around all variable expansion.

> +$PYTHON ../../contrib/backup/qemu-backup.py guest add --guest adad --qmp 
> $SOCKET
> +$PYTHON ../../contrib/backup/qemu-backup.py drive add --id virtio0 --guest 
> adad --target $TEST_DIR/virtio0
> +echo
> +echo "== Creating backup =="
> +$PYTHON ../../contrib/backup/qemu-backup.py backup --guest adad
> +_send_qemu_cmd $QEMU_HANDLE 'quit' ''
> +wait=1 _cleanup_qemu
> +echo
> +echo "== Comparing images =="
> +$QEMU_IMG compare $TEST_DIR/virtio0 $TEST_IMG
> +rm $TEST_DIR/virtio0
> +
> +_launch_qemu -drive if=virtio,id=virtio0,file=$TEST_IMG -qmp 
> $SOCKET,server,nowait
> +echo
> +echo "== Writing Pattern =="
> +_send_qemu_cmd $QEMU_HANDLE 'qemu-io virtio0 "write -P 0x22 0 1M"' "(qemu)" 
> | _filter_qemu_io
> +echo
> +echo "== Creating backup =="
> +$PYTHON ../../contrib/backup/qemu-backup.py backup --guest adad
> +_send_qemu_cmd $QEMU_HANDLE 'quit' ''
> +wait=1 _cleanup_qemu
> +echo
> +echo "== Comparing images =="
> +$QEMU_IMG compare $TEST_DIR/virtio0 $TEST_IMG
> +rm $TEST_DIR/virtio0
> +rm $CONFIG_FILE

All cleanup should be done from a trap handler function.  This way the
cleanup happens even if the process terminates early:

_cleanup()
{
        rm -f "$TEST_DIR"/virtio0
        rm -f "$CONFIG_FILE"
        _cleanup_test_img
}
trap "_cleanup; exit \$status" 0 1 2 3 15