[Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

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(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

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

[Qemu-devel] [PATCH v2 2/2] block/ssh: Implement .bdrv_dirname()

[Max Reitz]

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

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Richard W.M. Jones]

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

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

[-- 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 --]

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Richard W.M. Jones]

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

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

[-- 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 --]

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Richard W.M. Jones]

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

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

[-- 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 --]

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

[-- 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 --]

Re: [Qemu-devel] [PATCH v2 0/2] block/ssh: Implement .bdrv_refresh_filename()

[Max Reitz]

[-- 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 --]