* [Qemu-devel] [PATCH v2 1/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 15:29 [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Max Reitz
@ 2019-02-06 15:29 ` Max Reitz
2019-02-06 15:29 ` [Qemu-devel] [PATCH v2 2/2] block/ssh: Implement .bdrv_dirname() Max Reitz
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-02-06 15:29 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Richard W . M . Jones, Kevin Wolf
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/ssh.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 5 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 190ef95300..88401bbd31 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -75,6 +75,14 @@ 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)
@@ -87,6 +95,8 @@ 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);
}
@@ -640,14 +650,13 @@ 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;
@@ -697,7 +706,7 @@ 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;
}
@@ -1254,6 +1263,38 @@ 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",
@@ -1280,6 +1321,7 @@ 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,
};
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] block/ssh: Implement .bdrv_dirname()
2019-02-06 15:29 [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Max Reitz
2019-02-06 15:29 ` [Qemu-devel] [PATCH v2 1/2] " Max Reitz
@ 2019-02-06 15:29 ` Max Reitz
2019-02-06 15:45 ` [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Richard W.M. Jones
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-02-06 15:29 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Richard W . M . Jones, Kevin Wolf
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>
---
block/ssh.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/block/ssh.c b/block/ssh.c
index 88401bbd31..b519c00869 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1295,6 +1295,26 @@ 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",
@@ -1322,6 +1342,7 @@ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 15:29 [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Max Reitz
2019-02-06 15:29 ` [Qemu-devel] [PATCH v2 1/2] " Max Reitz
2019-02-06 15:29 ` [Qemu-devel] [PATCH v2 2/2] block/ssh: Implement .bdrv_dirname() Max Reitz
@ 2019-02-06 15:45 ` Richard W.M. Jones
2019-02-06 15:51 ` Max Reitz
2019-02-06 16:37 ` Richard W.M. Jones
2019-02-25 13:39 ` Max Reitz
4 siblings, 1 reply; 11+ messages in thread
From: Richard W.M. Jones @ 2019-02-06 15:45 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf
On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
> This series implements .bdrv_refresh_filename() for the ssh block
> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> query strings for backing files with relative filenames.
>
> This series depends on my "block: Fix some filename generation issues"
> series.
Hey Max, I couldn't find a version of "block: Fix some filename
generation issues" that applies to qemu master. Do you have a git
repo I could pull it from?
Rich.
> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>
>
> v2:
> - No longer based on the libssh2 -> libssh patches
> - Put /* and */ on their own lines to make checkpatch happy
>
>
> Max Reitz (2):
> block/ssh: Implement .bdrv_refresh_filename()
> block/ssh: Implement .bdrv_dirname()
>
> block/ssh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 68 insertions(+), 5 deletions(-)
>
> --
> 2.20.1
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 15:45 ` [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Richard W.M. Jones
@ 2019-02-06 15:51 ` Max Reitz
0 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-02-06 15:51 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: qemu-block, qemu-devel, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]
On 06.02.19 16:45, Richard W.M. Jones wrote:
> On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
>> This series implements .bdrv_refresh_filename() for the ssh block
>> driver, along with an appropriate .bdrv_dirname() so we don't chop off
>> query strings for backing files with relative filenames.
>>
>> This series depends on my "block: Fix some filename generation issues"
>> series.
>
> Hey Max, I couldn't find a version of "block: Fix some filename
> generation issues" that applies to qemu master. Do you have a git
> repo I could pull it from?
Ah, great, and I only sent it on Friday...
I pushed it here:
https://git.xanclic.moe/XanClic/qemu bds-base-filename-v13
Thanks,
Max
>> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>>
>>
>> v2:
>> - No longer based on the libssh2 -> libssh patches
>> - Put /* and */ on their own lines to make checkpatch happy
>>
>>
>> Max Reitz (2):
>> block/ssh: Implement .bdrv_refresh_filename()
>> block/ssh: Implement .bdrv_dirname()
>>
>> block/ssh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 68 insertions(+), 5 deletions(-)
>>
>> --
>> 2.20.1
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 15:29 [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Max Reitz
` (2 preceding siblings ...)
2019-02-06 15:45 ` [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Richard W.M. Jones
@ 2019-02-06 16:37 ` Richard W.M. Jones
2019-02-06 16:42 ` Max Reitz
2019-02-25 13:39 ` Max Reitz
4 siblings, 1 reply; 11+ messages in thread
From: Richard W.M. Jones @ 2019-02-06 16:37 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf
On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
> This series implements .bdrv_refresh_filename() for the ssh block
> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> query strings for backing files with relative filenames.
>
> This series depends on my "block: Fix some filename generation issues"
> series.
>
> Based-on: 20190201192935.18394-1-mreitz@redhat.com
I have verified that this doesn't appear to break the existing driver:
ssh connections to block devices still work as well as they did before
(which is to say, not very well, I wish we would replace this driver
with Pino Toscano's reimplementation that uses libssh1).
However I wasn't sure how I could trigger the bdrv_refresh_filename
code path, so I don't think I tested that.
Rich.
>
> v2:
> - No longer based on the libssh2 -> libssh patches
> - Put /* and */ on their own lines to make checkpatch happy
>
>
> Max Reitz (2):
> block/ssh: Implement .bdrv_refresh_filename()
> block/ssh: Implement .bdrv_dirname()
>
> block/ssh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 68 insertions(+), 5 deletions(-)
>
> --
> 2.20.1
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 16:37 ` Richard W.M. Jones
@ 2019-02-06 16:42 ` Max Reitz
2019-02-06 17:00 ` Richard W.M. Jones
0 siblings, 1 reply; 11+ messages in thread
From: Max Reitz @ 2019-02-06 16:42 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: qemu-block, qemu-devel, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]
On 06.02.19 17:37, Richard W.M. Jones wrote:
> On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
>> This series implements .bdrv_refresh_filename() for the ssh block
>> driver, along with an appropriate .bdrv_dirname() so we don't chop off
>> query strings for backing files with relative filenames.
>>
>> This series depends on my "block: Fix some filename generation issues"
>> series.
>>
>> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>
> I have verified that this doesn't appear to break the existing driver:
> ssh connections to block devices still work as well as they did before
> (which is to say, not very well, I wish we would replace this driver
> with Pino Toscano's reimplementation that uses libssh1).
>
> However I wasn't sure how I could trigger the bdrv_refresh_filename
> code path, so I don't think I tested that.
One test case goes like this:
Before this series:
$ ./qemu-img create -f qcow2 /tmp/base.qcow2 64M
$ ./qemu-img create -f qcow2 -b base.qcow2 /tmp/top.qcow2
$ ./qemu-img info ssh://localhost/tmp/top.qcow2
image: json:{"driver": "qcow2", "file": {"server.host": "localhost",
"server.port": "22", "driver": "ssh", "path": "/tmp/top.qcow2"}}
[...]
backing file: base.qcow2 (cannot determine actual path)
[...]
$ ./qemu-io ssh://localhost/tmp/top.qcow2
can't open device ssh://localhost/tmp/top.qcow2: Cannot generate a base
directory for ssh nodes
So the filename is weird and you cannot open overlays with relative
backing files.
After this series:
$ ./qemu-img info ssh://localhost/tmp/top.qcow2
image: ssh://maxx@localhost:22/tmp/top.qcow2
[...]
backing file: base.qcow2 (actual path:
ssh://maxx@localhost:22/tmp/base.qcow2)
$ ./qemu-io ssh://localhost/tmp/top.qcow2
qemu-io> quit
The filename looks better and the image is usable.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 16:42 ` Max Reitz
@ 2019-02-06 17:00 ` Richard W.M. Jones
2019-02-06 17:16 ` Max Reitz
0 siblings, 1 reply; 11+ messages in thread
From: Richard W.M. Jones @ 2019-02-06 17:00 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf
On Wed, Feb 06, 2019 at 05:42:15PM +0100, Max Reitz wrote:
> On 06.02.19 17:37, Richard W.M. Jones wrote:
> > On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
> >> This series implements .bdrv_refresh_filename() for the ssh block
> >> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> >> query strings for backing files with relative filenames.
> >>
> >> This series depends on my "block: Fix some filename generation issues"
> >> series.
> >>
> >> Based-on: 20190201192935.18394-1-mreitz@redhat.com
> >
> > I have verified that this doesn't appear to break the existing driver:
> > ssh connections to block devices still work as well as they did before
> > (which is to say, not very well, I wish we would replace this driver
> > with Pino Toscano's reimplementation that uses libssh1).
> >
> > However I wasn't sure how I could trigger the bdrv_refresh_filename
> > code path, so I don't think I tested that.
>
> One test case goes like this:
>
> Before this series:
>
> $ ./qemu-img create -f qcow2 /tmp/base.qcow2 64M
> $ ./qemu-img create -f qcow2 -b base.qcow2 /tmp/top.qcow2
> $ ./qemu-img info ssh://localhost/tmp/top.qcow2
> image: json:{"driver": "qcow2", "file": {"server.host": "localhost",
> "server.port": "22", "driver": "ssh", "path": "/tmp/top.qcow2"}}
> [...]
> backing file: base.qcow2 (cannot determine actual path)
> [...]
> $ ./qemu-io ssh://localhost/tmp/top.qcow2
> can't open device ssh://localhost/tmp/top.qcow2: Cannot generate a base
> directory for ssh nodes
>
>
> So the filename is weird and you cannot open overlays with relative
> backing files.
>
> After this series:
>
> $ ./qemu-img info ssh://localhost/tmp/top.qcow2
> image: ssh://maxx@localhost:22/tmp/top.qcow2
> [...]
> backing file: base.qcow2 (actual path:
> ssh://maxx@localhost:22/tmp/base.qcow2)
> $ ./qemu-io ssh://localhost/tmp/top.qcow2
> qemu-io> quit
>
> The filename looks better and the image is usable.
I have to use ?host_key_check=no because of my modern ssh server. I
see this error (with your patch series applied):
$ ./qemu-img info 'ssh://localhost/tmp/top.qcow2?host_key_check=no'
image: ssh://rjones@localhost:22/tmp/top.qcow2?host_key_check=no
...
backing file: base.qcow2 (cannot determine actual path)
$ ./qemu-io 'ssh://localhost/tmp/top.qcow2?host_key_check=no'
can't open device ssh://localhost/tmp/top.qcow2?host_key_check=no: Cannot generate a base directory with host_key_check set
I can see the error message in block/ssh.c:
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");
It seems as if bdrv_dirname is mis-designed? Either it shouldn't
assume dirname is always a strict prefix of a path, or g_strconcatdir
[from bdrv_make_absolute_filename] should really be a bdrv_* function
so that block devices can override it.
In any case I don't think this should hold up the patch, so:
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 17:00 ` Richard W.M. Jones
@ 2019-02-06 17:16 ` Max Reitz
0 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-02-06 17:16 UTC (permalink / raw)
To: Richard W.M. Jones; +Cc: qemu-block, qemu-devel, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 3504 bytes --]
On 06.02.19 18:00, Richard W.M. Jones wrote:
> On Wed, Feb 06, 2019 at 05:42:15PM +0100, Max Reitz wrote:
>> On 06.02.19 17:37, Richard W.M. Jones wrote:
>>> On Wed, Feb 06, 2019 at 04:29:17PM +0100, Max Reitz wrote:
>>>> This series implements .bdrv_refresh_filename() for the ssh block
>>>> driver, along with an appropriate .bdrv_dirname() so we don't chop off
>>>> query strings for backing files with relative filenames.
>>>>
>>>> This series depends on my "block: Fix some filename generation issues"
>>>> series.
>>>>
>>>> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>>>
>>> I have verified that this doesn't appear to break the existing driver:
>>> ssh connections to block devices still work as well as they did before
>>> (which is to say, not very well, I wish we would replace this driver
>>> with Pino Toscano's reimplementation that uses libssh1).
>>>
>>> However I wasn't sure how I could trigger the bdrv_refresh_filename
>>> code path, so I don't think I tested that.
>>
>> One test case goes like this:
>>
>> Before this series:
>>
>> $ ./qemu-img create -f qcow2 /tmp/base.qcow2 64M
>> $ ./qemu-img create -f qcow2 -b base.qcow2 /tmp/top.qcow2
>> $ ./qemu-img info ssh://localhost/tmp/top.qcow2
>> image: json:{"driver": "qcow2", "file": {"server.host": "localhost",
>> "server.port": "22", "driver": "ssh", "path": "/tmp/top.qcow2"}}
>> [...]
>> backing file: base.qcow2 (cannot determine actual path)
>> [...]
>> $ ./qemu-io ssh://localhost/tmp/top.qcow2
>> can't open device ssh://localhost/tmp/top.qcow2: Cannot generate a base
>> directory for ssh nodes
>>
>>
>> So the filename is weird and you cannot open overlays with relative
>> backing files.
>>
>> After this series:
>>
>> $ ./qemu-img info ssh://localhost/tmp/top.qcow2
>> image: ssh://maxx@localhost:22/tmp/top.qcow2
>> [...]
>> backing file: base.qcow2 (actual path:
>> ssh://maxx@localhost:22/tmp/base.qcow2)
>> $ ./qemu-io ssh://localhost/tmp/top.qcow2
>> qemu-io> quit
>>
>> The filename looks better and the image is usable.
>
> I have to use ?host_key_check=no because of my modern ssh server. I
> see this error (with your patch series applied):
>
> $ ./qemu-img info 'ssh://localhost/tmp/top.qcow2?host_key_check=no'
> image: ssh://rjones@localhost:22/tmp/top.qcow2?host_key_check=no
> ...
> backing file: base.qcow2 (cannot determine actual path)
>
> $ ./qemu-io 'ssh://localhost/tmp/top.qcow2?host_key_check=no'
> can't open device ssh://localhost/tmp/top.qcow2?host_key_check=no: Cannot generate a base directory with host_key_check set
>
> I can see the error message in block/ssh.c:
>
> 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");
>
> It seems as if bdrv_dirname is mis-designed? Either it shouldn't
> assume dirname is always a strict prefix of a path, or g_strconcatdir
> [from bdrv_make_absolute_filename] should really be a bdrv_* function
> so that block devices can override it.
We can always make it more complicated, yes. :-)
> In any case I don't think this should hold up the patch, so:
>
> Tested-by: Richard W.M. Jones <rjones@redhat.com>
Thanks!
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-06 15:29 [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename() Max Reitz
` (3 preceding siblings ...)
2019-02-06 16:37 ` Richard W.M. Jones
@ 2019-02-25 13:39 ` Max Reitz
2019-02-25 14:10 ` Max Reitz
4 siblings, 1 reply; 11+ messages in thread
From: Max Reitz @ 2019-02-25 13:39 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Richard W . M . Jones, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
On 06.02.19 16:29, Max Reitz wrote:
> This series implements .bdrv_refresh_filename() for the ssh block
> driver, along with an appropriate .bdrv_dirname() so we don't chop off
> query strings for backing files with relative filenames.
>
> This series depends on my "block: Fix some filename generation issues"
> series.
>
> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>
>
> v2:
> - No longer based on the libssh2 -> libssh patches
> - Put /* and */ on their own lines to make checkpatch happy
>
>
> Max Reitz (2):
> block/ssh: Implement .bdrv_refresh_filename()
> block/ssh: Implement .bdrv_dirname()
>
> block/ssh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 68 insertions(+), 5 deletions(-)
Thanks for having had a look at this series, Rich; I've applied it to my
block branch.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()
2019-02-25 13:39 ` Max Reitz
@ 2019-02-25 14:10 ` Max Reitz
0 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-02-25 14:10 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Richard W . M . Jones, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]
On 25.02.19 14:39, Max Reitz wrote:
> On 06.02.19 16:29, Max Reitz wrote:
>> This series implements .bdrv_refresh_filename() for the ssh block
>> driver, along with an appropriate .bdrv_dirname() so we don't chop off
>> query strings for backing files with relative filenames.
>>
>> This series depends on my "block: Fix some filename generation issues"
>> series.
>>
>> Based-on: 20190201192935.18394-1-mreitz@redhat.com
>>
>>
>> v2:
>> - No longer based on the libssh2 -> libssh patches
>> - Put /* and */ on their own lines to make checkpatch happy
>>
>>
>> Max Reitz (2):
>> block/ssh: Implement .bdrv_refresh_filename()
>> block/ssh: Implement .bdrv_dirname()
>>
>> block/ssh.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 68 insertions(+), 5 deletions(-)
>
> Thanks for having had a look at this series, Rich; I've applied it to my
> block branch.
Or maybe not. It breaks iotests 104 (because the filename now includes
your username) and 207 (because the json:{} filename is a nice filename
now). So I'll drop it and send a v3.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread