From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Increase corruptions_fixed only after successful fix.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190227131433.197063-2-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-refcount.c | 8 ++++----

1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

for (i = 0; i < s->l1_size; i++) {

uint64_t l1_entry = s->l1_table[i];

uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK;

- bool l2_dirty = false;

+ int l2_dirty = 0;

if (!l2_offset) {

continue;

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

l2_table[j] = cpu_to_be64(refcount == 1

? l2_entry | QCOW_OFLAG_COPIED

: l2_entry & ~QCOW_OFLAG_COPIED);

- l2_dirty = true;

- res->corruptions_fixed++;

+ l2_dirty++;

} else {

res->corruptions++;

}

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

}

}

- if (l2_dirty) {

+ if (l2_dirty > 0) {

ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2,

l2_offset, s->cluster_size,

false);

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

res->check_errors++;

goto fail;

}

+ res->corruptions_fixed += l2_dirty;

}

}

--

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

qcow2_inc_refcounts_imrt() (through realloc_refcount_array()) can eat

an unpredictable amount of memory on corrupted table entries, which are

referencing regions far beyond the end of file.

Prevent this, by skipping such regions from further processing.

Interesting that iotest 138 checks exactly the behavior which we fix

here. So, change the test appropriately.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190227131433.197063-3-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-refcount.c | 19 +++++++++++++++++++

tests/qemu-iotests/138 | 12 +++++-------

tests/qemu-iotests/138.out | 5 ++++-

3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ int qcow2_inc_refcounts_imrt(BlockDriverState *bs, BdrvCheckResult *res,

{

BDRVQcow2State *s = bs->opaque;

uint64_t start, last, cluster_offset, k, refcount;

+ int64_t file_len;

int ret;

if (size <= 0) {

return 0;

}

+ file_len = bdrv_getlength(bs->file->bs);

+ if (file_len < 0) {

+ return file_len;

+ }

+

+ /*

+ * Last cluster of qcow2 image may be semi-allocated, so it may be OK to

+ * reference some space after file end but it should be less than one

+ * cluster.

+ */

+ if (offset + size - file_len >= s->cluster_size) {

+ fprintf(stderr, "ERROR: counting reference for region exceeding the "

+ "end of the file by one cluster or more: offset 0x%" PRIx64

+ " size 0x%" PRIx64 "\n", offset, size);

+ res->corruptions++;

+ return 0;

+ }

+

start = start_of_cluster(s, offset);

last = start_of_cluster(s, offset + size - 1);

for(cluster_offset = start; cluster_offset <= last;

diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/138

+++ b/tests/qemu-iotests/138

@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io

# Put the data cluster at a multiple of 2 TB, resulting in the image apparently

# having a multiple of 2^32 clusters

# (To be more specific: It is at 32 PB)

-poke_file "$TEST_IMG" 2048 "\x80\x80\x00\x00\x00\x00\x00\x00"

+poke_file "$TEST_IMG" $((2048 + 8)) "\x00\x80\x00\x00\x00\x00\x00\x00"

# An offset of 32 PB results in qemu-img check having to allocate an in-memory

-# refcount table of 128 TB (16 bit refcounts, 512 byte clusters).

-# This should be generally too much for any system and thus fail.

-# What this test is checking is that the qcow2 driver actually tries to allocate

-# such a large amount of memory (and is consequently aborting) instead of having

-# truncated the cluster count somewhere (which would result in much less memory

-# being allocated and then a segfault occurring).

+# refcount table of 128 TB (16 bit refcounts, 512 byte clusters), if qemu-img

+# don't check that referenced data cluster is far beyond the end of file.

+# But starting from 4.0, qemu-img does this check, and instead of "Cannot

+# allocate memory", we have an error showing that l2 entry is invalid.

_check_test_img

# success, all done

diff --git a/tests/qemu-iotests/138.out b/tests/qemu-iotests/138.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/138.out

+++ b/tests/qemu-iotests/138.out

@@ -XXX,XX +XXX,XX @@ QA output created by 138

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=512

wrote 512/512 bytes at offset 0

512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

-qemu-img: Check failed: Cannot allocate memory

+ERROR: counting reference for region exceeding the end of the file by one cluster or more: offset 0x80000000000000 size 0x200

+

+1 errors were found on the image.

+Data may be corrupted, or further writes to the image may corrupt it.

*** done

--

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reduce number of structures ignored in overlap check: when checking

active table ignore active tables, when checking inactive table ignore

inactive ones.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190227131433.197063-4-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-refcount.c | 16 +++++++++-------

1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ enum {

static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

void **refcount_table,

int64_t *refcount_table_size, int64_t l2_offset,

- int flags, BdrvCheckMode fix)

+ int flags, BdrvCheckMode fix, bool active)

{

BDRVQcow2State *s = bs->opaque;

uint64_t *l2_table, l2_entry;

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

if (fix & BDRV_FIX_ERRORS) {

uint64_t l2e_offset =

l2_offset + (uint64_t)i * sizeof(uint64_t);

+ int ign = active ? QCOW2_OL_ACTIVE_L2 :

+ QCOW2_OL_INACTIVE_L2;

l2_entry = QCOW_OFLAG_ZERO;

l2_table[i] = cpu_to_be64(l2_entry);

- ret = qcow2_pre_write_overlap_check(bs,

- QCOW2_OL_ACTIVE_L2 | QCOW2_OL_INACTIVE_L2,

+ ret = qcow2_pre_write_overlap_check(bs, ign,

l2e_offset, sizeof(uint64_t), false);

if (ret < 0) {

fprintf(stderr, "ERROR: Overlap check failed\n");

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l1(BlockDriverState *bs,

void **refcount_table,

int64_t *refcount_table_size,

int64_t l1_table_offset, int l1_size,

- int flags, BdrvCheckMode fix)

+ int flags, BdrvCheckMode fix, bool active)

{

BDRVQcow2State *s = bs->opaque;

uint64_t *l1_table = NULL, l2_offset, l1_size2;

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l1(BlockDriverState *bs,

/* Process and check L2 entries */

ret = check_refcounts_l2(bs, res, refcount_table,

refcount_table_size, l2_offset, flags,

- fix);

+ fix, active);

if (ret < 0) {

goto fail;

}

@@ -XXX,XX +XXX,XX @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,

/* current L1 table */

ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,

s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO,

- fix);

+ fix, true);

if (ret < 0) {

return ret;

}

@@ -XXX,XX +XXX,XX @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,

continue;

}

ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters,

- sn->l1_table_offset, sn->l1_size, 0, fix);

+ sn->l1_table_offset, sn->l1_size, 0, fix,

+ false);

if (ret < 0) {

return ret;

}

--

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Do not count a cluster which is fixed to be ZERO as allocated.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190227131433.197063-5-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-refcount.c | 18 +++++++++---------

1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

{

uint64_t offset = l2_entry & L2E_OFFSET_MASK;

- if (flags & CHECK_FRAG_INFO) {

- res->bfi.allocated_clusters++;

- if (next_contiguous_offset &&

- offset != next_contiguous_offset) {

- res->bfi.fragmented_clusters++;

- }

- next_contiguous_offset = offset + s->cluster_size;

- }

-

/* Correct offsets are cluster aligned */

if (offset_into_cluster(s, offset)) {

if (qcow2_get_cluster_type(bs, l2_entry) ==

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

}

}

+ if (flags & CHECK_FRAG_INFO) {

+ res->bfi.allocated_clusters++;

+ if (next_contiguous_offset &&

+ offset != next_contiguous_offset) {

+ res->bfi.fragmented_clusters++;

+ }

+ next_contiguous_offset = offset + s->cluster_size;

+ }

+

/* Mark cluster as used */

if (!has_data_file(bs)) {

ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table,

--

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

No reasons for not reporting found corruptions as corruptions in case

of some internal errors, especially in case of just failed to fix l2

entry (and in this case, missed corruptions may influence comparing

logic, when we calculate difference between corruptions fields of two

results)

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-id: 20190227131433.197063-6-vsementsov@virtuozzo.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-refcount.c | 19 +++++++++----------

1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

/* Correct offsets are cluster aligned */

if (offset_into_cluster(s, offset)) {

+ res->corruptions++;

+

if (qcow2_get_cluster_type(bs, l2_entry) ==

QCOW2_CLUSTER_ZERO_ALLOC)

{

@@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,

/* Do not abort, continue checking the rest of this

* L2 table's entries */

} else {

+ res->corruptions--;

res->corruptions_fixed++;

/* Skip marking the cluster as used

* (it is unused now) */

continue;

}

- } else {

- res->corruptions++;

}

} else {

fprintf(stderr, "ERROR offset=%" PRIx64 ": Data cluster is "

"not properly aligned; L2 entry corrupted.\n", offset);

- res->corruptions++;

}

}

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

continue;

}

if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) {

+ res->corruptions++;

fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d "

"l1_entry=%" PRIx64 " refcount=%" PRIu64 "\n",

repair ? "Repairing" : "ERROR", i, l1_entry, refcount);

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

res->check_errors++;

goto fail;

}

+ res->corruptions--;

res->corruptions_fixed++;

- } else {

- res->corruptions++;

}

}

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

}

}

if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) {

+ res->corruptions++;

fprintf(stderr, "%s OFLAG_COPIED data cluster: "

"l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n",

repair ? "Repairing" : "ERROR", l2_entry, refcount);

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

? l2_entry | QCOW_OFLAG_COPIED

: l2_entry & ~QCOW_OFLAG_COPIED);

l2_dirty++;

- } else {

- res->corruptions++;

}

}

}

@@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,

res->check_errors++;

goto fail;

}

+ res->corruptions -= l2_dirty;

res->corruptions_fixed += l2_dirty;

}

}

@@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,

}

if (cluster >= *nb_clusters) {

+ res->corruptions++;

fprintf(stderr, "%s refcount block %" PRId64 " is outside image\n",

fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);

@@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,

goto resize_fail;

}

+ res->corruptions--;

res->corruptions_fixed++;

ret = qcow2_inc_refcounts_imrt(bs, res,

refcount_table, nb_clusters,

@@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,

continue;

resize_fail:

- res->corruptions++;

*rebuild = true;

fprintf(stderr, "ERROR could not resize image: %s\n",

strerror(-ret));

- } else {

- res->corruptions++;

}

continue;

}

--

2.20.1

From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Bitmap data may take a lot of disk space, so it's better to discard it

always.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Message-id: 1551346019-293202-1-git-send-email-andrey.shinkevich@virtuozzo.com

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

[mreitz: Use the commit message proposed by Vladimir]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2-bitmap.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-bitmap.c

+++ b/block/qcow2-bitmap.c

@@ -XXX,XX +XXX,XX @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table,

continue;

}

- qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER);

+ qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS);

bitmap_table[i] = 0;

}

}

--

2.20.1

This requires some changes to keep iotests 104 and 207 working.

qemu-img info in 104 will now return a filename including the user name

and the port, which need to be filtered by adjusting REMOTE_TEST_DIR in

common.rc. This additional information has to be marked optional,

however (which is simple as REMOTE_TEST_DIR is a regex), because

otherwise 197 and 215 would fail: They use it (indirectly) to filter

qemu-img create output which contains a backing filename they have

passed to it -- which probably does not contain a user name or port

number.

The problem in 207 is a nice one to have: qemu-img info used to return

json:{} filenames, but with this patch it returns nice plain ones. We

now need to adjust the filtering to hide the user name (and port number

while we are at it). The simplest way to do this is to include both in

iotests.remote_filename() so that bdrv_refresh_filename() will not

change it, and then iotests.img_info_log() will filter it correctly

automatically.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Message-id: 20190225190828.17726-2-mreitz@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/ssh.c | 52 +++++++++++++++++++++++++++++++----

tests/qemu-iotests/207 | 10 +++----

tests/qemu-iotests/207.out | 10 +++----

tests/qemu-iotests/common.rc | 2 +-

tests/qemu-iotests/iotests.py | 2 +-

5 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c

index XXXXXXX..XXXXXXX 100644

--- a/block/ssh.c

+++ b/block/ssh.c

@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSSHState {

/* Used to warn if 'flush' is not supported. */

bool unsafe_flush_warning;

+

+ /*

+ * Store the user name for ssh_refresh_filename() because the

+ * default depends on the system you are on -- therefore, when we

+ * generate a filename, it should always contain the user name we

+ * are actually using.

+ */

+ char *user;

} BDRVSSHState;

static void ssh_state_init(BDRVSSHState *s)

@@ -XXX,XX +XXX,XX @@ static void ssh_state_init(BDRVSSHState *s)

static void ssh_state_free(BDRVSSHState *s)

{

+ g_free(s->user);

+

if (s->sftp_handle) {

libssh2_sftp_close(s->sftp_handle);

}

@@ -XXX,XX +XXX,XX @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts,

int ssh_flags, int creat_mode, Error **errp)

{

int r, ret;

- const char *user;

long port = 0;

if (opts->has_user) {

- user = opts->user;

+ s->user = g_strdup(opts->user);

} else {

- user = g_get_user_name();

- if (!user) {

+ s->user = g_strdup(g_get_user_name());

+ if (!s->user) {

error_setg_errno(errp, errno, "Can't get user name");

ret = -errno;

goto err;

@@ -XXX,XX +XXX,XX @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts,

}

/* Authenticate. */

- ret = authenticate(s, user, errp);

+ ret = authenticate(s, s->user, errp);

if (ret < 0) {

goto err;

}

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset,

return ssh_grow_file(s, offset, errp);

}

+static void ssh_refresh_filename(BlockDriverState *bs)

+{

+ BDRVSSHState *s = bs->opaque;

+ const char *path, *host_key_check;

+ int ret;

+

+ /*

+ * None of these options can be represented in a plain "host:port"

+ * format, so if any was given, we have to abort.

+ */

+ if (s->inet->has_ipv4 || s->inet->has_ipv6 || s->inet->has_to ||

+ s->inet->has_numeric)

+ {

+ return;

+ }

+

+ path = qdict_get_try_str(bs->full_open_options, "path");

+ assert(path); /* mandatory option */

+

+ host_key_check = qdict_get_try_str(bs->full_open_options, "host_key_check");

+

+ ret = snprintf(bs->exact_filename, sizeof(bs->exact_filename),

+ "ssh://%s@%s:%s%s%s%s",

+ s->user, s->inet->host, s->inet->port, path,

+ host_key_check ? "?host_key_check=" : "",

+ host_key_check ?: "");

+ if (ret >= sizeof(bs->exact_filename)) {

+ /* An overflow makes the filename unusable, so do not report any */

+ bs->exact_filename[0] = '\0';

+ }

+}

+

static const char *const ssh_strong_runtime_opts[] = {

"host",

"port",

@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_ssh = {

.bdrv_getlength = ssh_getlength,

.bdrv_co_truncate = ssh_co_truncate,

.bdrv_co_flush_to_disk = ssh_co_flush,

+ .bdrv_refresh_filename = ssh_refresh_filename,

.create_opts = &ssh_create_opts,

.strong_runtime_opts = ssh_strong_runtime_opts,

};

diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/207

+++ b/tests/qemu-iotests/207

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \

'size': 4194304 })

vm.shutdown()

- iotests.img_info_log(remote_path, filter_path=disk_path)

+ iotests.img_info_log(remote_path)

iotests.log("")

iotests.img_info_log(disk_path)

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \

'size': 8388608 })

vm.shutdown()

- iotests.img_info_log(remote_path, filter_path=disk_path)

+ iotests.img_info_log(remote_path)

vm.launch()

blockdev_create(vm, { 'driver': 'ssh',

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \

'size': 4194304 })

vm.shutdown()

- iotests.img_info_log(remote_path, filter_path=disk_path)

+ iotests.img_info_log(remote_path)

md5_key = subprocess.check_output(

'ssh-keyscan -t rsa 127.0.0.1 2>/dev/null | grep -v "\\^#" | ' +

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \

'size': 8388608 })

vm.shutdown()

- iotests.img_info_log(remote_path, filter_path=disk_path)

+ iotests.img_info_log(remote_path)

sha1_key = subprocess.check_output(

'ssh-keyscan -t rsa 127.0.0.1 2>/dev/null | grep -v "\\^#" | ' +

@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \

'size': 4194304 })

vm.shutdown()

- iotests.img_info_log(remote_path, filter_path=disk_path)

+ iotests.img_info_log(remote_path)

#

# Invalid path and user

diff --git a/tests/qemu-iotests/207.out b/tests/qemu-iotests/207.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/207.out

+++ b/tests/qemu-iotests/207.out

@@ -XXX,XX +XXX,XX @@

{"execute": "job-dismiss", "arguments": {"id": "job0"}}

{"return": {}}

-image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}}

+image: TEST_IMG

file format: IMGFMT

virtual size: 4 MiB (4194304 bytes)

@@ -XXX,XX +XXX,XX @@ virtual size: 4 MiB (4194304 bytes)

{"execute": "job-dismiss", "arguments": {"id": "job0"}}

{"return": {}}

-image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}}

+image: TEST_IMG

file format: IMGFMT

virtual size: 8 MiB (8388608 bytes)

@@ -XXX,XX +XXX,XX @@ virtual size: 8 MiB (8388608 bytes)

{"execute": "job-dismiss", "arguments": {"id": "job0"}}

{"return": {}}

-image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}}

+image: TEST_IMG

file format: IMGFMT

virtual size: 4 MiB (4194304 bytes)

@@ -XXX,XX +XXX,XX @@ Job failed: remote host key does not match host_key_check 'wrong'

{"execute": "job-dismiss", "arguments": {"id": "job0"}}

{"return": {}}

-image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}}

+image: TEST_IMG

file format: IMGFMT

virtual size: 8 MiB (8388608 bytes)

@@ -XXX,XX +XXX,XX @@ Job failed: remote host key does not match host_key_check 'wrong'

{"execute": "job-dismiss", "arguments": {"id": "job0"}}

{"return": {}}

-image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}}

+image: TEST_IMG

file format: IMGFMT

virtual size: 4 MiB (4194304 bytes)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/common.rc

+++ b/tests/qemu-iotests/common.rc

@@ -XXX,XX +XXX,XX @@ else

TEST_IMG="nbd:127.0.0.1:10810"

elif [ "$IMGPROTO" = "ssh" ]; then

TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT

- REMOTE_TEST_DIR="ssh://127.0.0.1$TEST_DIR"

+ REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR"

TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE"

elif [ "$IMGPROTO" = "nfs" ]; then

TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ def remote_filename(path):

if imgproto == 'file':

return path

elif imgproto == 'ssh':

- return "ssh://127.0.0.1%s" % (path)

+ return "ssh://%s@127.0.0.1:22%s" % (os.environ.get('USER'), path)

else:

raise Exception("Protocol %s not supported" % (imgproto))

--

2.20.1

ssh_bdrv_dirname() is basically the generic bdrv_dirname(), except it

takes care not to silently chop off any query string (i.e.,

host_key_check).

Signed-off-by: Max Reitz <mreitz@redhat.com>

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Message-id: 20190225190828.17726-3-mreitz@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/ssh.c | 21 +++++++++++++++++++++

1 file changed, 21 insertions(+)

diff --git a/block/ssh.c b/block/ssh.c

index XXXXXXX..XXXXXXX 100644

--- a/block/ssh.c

+++ b/block/ssh.c

@@ -XXX,XX +XXX,XX @@ static void ssh_refresh_filename(BlockDriverState *bs)

}

}

+static char *ssh_bdrv_dirname(BlockDriverState *bs, Error **errp)

+{

+ if (qdict_haskey(bs->full_open_options, "host_key_check")) {

+ /*

+ * We cannot generate a simple prefix if we would have to

+ * append a query string.

+ */

+ error_setg(errp,

+ "Cannot generate a base directory with host_key_check set");

+ return NULL;

+ }

+

+ if (bs->exact_filename[0] == '\0') {

+ error_setg(errp, "Cannot generate a base directory for this ssh node");

+ return NULL;

+ }

+

+ return path_combine(bs->exact_filename, "");

+}

+

static const char *const ssh_strong_runtime_opts[] = {

"host",

"port",

@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_ssh = {

.bdrv_co_truncate = ssh_co_truncate,

.bdrv_co_flush_to_disk = ssh_co_flush,

.bdrv_refresh_filename = ssh_refresh_filename,

+ .bdrv_dirname = ssh_bdrv_dirname,

.create_opts = &ssh_create_opts,

.strong_runtime_opts = ssh_strong_runtime_opts,

};

--

2.20.1