* [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate()
@ 2018-02-14 20:49 Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create() Max Reitz
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Max Reitz @ 2018-02-14 20:49 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Richard W . M . Jones, Jeff Cody
For (x-)blockdev-create, all protocol drivers that support image
creation also need to offer a .bdrv_truncate() implementation that
matches in features. A previous series of mine brought gluster's and
sheepdog's implementation up to par regarding preallocated truncation;
but I forgot about drivers that have a .bdrv_create() but no
.bdrv_truncate() at all.
There is only one of these, and that is ssh. Since libssh2 does not
seem to know any way of truncating files, we can only support growing
files -- but that is what we need for (x-)blockdev-create.
Note that there are also drivers which do not support growing files,
namely iscsi and file-posix for host devices (maybe more? I hope not).
But these also pretty much ignore the specified size on .bdrv_create()
and just use the size of the existing device. They just check that the
specified size does not exceed the actual size, so that pretty much
matches what their .bdrv_truncate() supports, and we should be fine
there.
Max Reitz (3):
block/ssh: Pull ssh_grow_file() from ssh_create()
block/ssh: Make ssh_grow_file() blocking
block/ssh: Add basic .bdrv_truncate()
block/ssh.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 53 insertions(+), 8 deletions(-)
--
2.14.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create()
2018-02-14 20:49 [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
@ 2018-02-14 20:49 ` Max Reitz
2018-02-14 21:08 ` Eric Blake
2018-02-14 20:49 ` [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking Max Reitz
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Max Reitz @ 2018-02-14 20:49 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Richard W . M . Jones, Jeff Cody
If we ever want to offer even rudimentary truncation functionality for
ssh, we should put the respective code into a reusable function.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/ssh.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index b63addcf94..964e55f7fe 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -803,6 +803,26 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
return ret;
}
+static int ssh_grow_file(BDRVSSHState *s, int64_t offset, Error **errp)
+{
+ ssize_t ret;
+ char c[1] = { '\0' };
+
+ /* offset must be strictly greater than the current size so we do
+ * not overwrite anything */
+ assert(offset > 0 && offset > s->attrs.filesize);
+
+ libssh2_sftp_seek64(s->sftp_handle, offset - 1);
+ ret = libssh2_sftp_write(s->sftp_handle, c, 1);
+ if (ret < 0) {
+ sftp_error_setg(errp, s, "Failed to grow file");
+ return -EIO;
+ }
+
+ s->attrs.filesize = offset;
+ return 0;
+}
+
static QemuOptsList ssh_create_opts = {
.name = "ssh-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(ssh_create_opts.head),
@@ -822,8 +842,6 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
int64_t total_size = 0;
QDict *uri_options = NULL;
BDRVSSHState s;
- ssize_t r2;
- char c[1] = { '\0' };
ssh_state_init(&s);
@@ -849,14 +867,10 @@ static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
}
if (total_size > 0) {
- libssh2_sftp_seek64(s.sftp_handle, total_size-1);
- r2 = libssh2_sftp_write(s.sftp_handle, c, 1);
- if (r2 < 0) {
- sftp_error_setg(errp, &s, "truncate failed");
- ret = -EINVAL;
+ ret = ssh_grow_file(&s, total_size, errp);
+ if (ret < 0) {
goto out;
}
- s.attrs.filesize = total_size;
}
ret = 0;
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking
2018-02-14 20:49 [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create() Max Reitz
@ 2018-02-14 20:49 ` Max Reitz
2018-02-14 21:11 ` Eric Blake
2018-02-14 20:49 ` [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-23 13:51 ` [Qemu-devel] [PATCH 0/3] " Max Reitz
3 siblings, 1 reply; 10+ messages in thread
From: Max Reitz @ 2018-02-14 20:49 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Richard W . M . Jones, Jeff Cody
At runtime (that is, during a future ssh_truncate()), the SSH session is
non-blocking. However, ssh_truncate() (or rather, bdrv_truncate() in
general) is not a coroutine, so this resize operation needs to block.
For ssh_create(), that is fine, too; the session is never set to
non-blocking anyway.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/ssh.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block/ssh.c b/block/ssh.c
index 964e55f7fe..ff8576f21e 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -803,17 +803,24 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
return ret;
}
+/* Note: This is a blocking operation */
static int ssh_grow_file(BDRVSSHState *s, int64_t offset, Error **errp)
{
ssize_t ret;
char c[1] = { '\0' };
+ int was_blocking = libssh2_session_get_blocking(s->session);
/* offset must be strictly greater than the current size so we do
* not overwrite anything */
assert(offset > 0 && offset > s->attrs.filesize);
+ libssh2_session_set_blocking(s->session, 1);
+
libssh2_sftp_seek64(s->sftp_handle, offset - 1);
ret = libssh2_sftp_write(s->sftp_handle, c, 1);
+
+ libssh2_session_set_blocking(s->session, was_blocking);
+
if (ret < 0) {
sftp_error_setg(errp, s, "Failed to grow file");
return -EIO;
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate()
2018-02-14 20:49 [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create() Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking Max Reitz
@ 2018-02-14 20:49 ` Max Reitz
2018-02-14 21:12 ` Eric Blake
2018-02-15 11:12 ` Richard W.M. Jones
2018-02-23 13:51 ` [Qemu-devel] [PATCH 0/3] " Max Reitz
3 siblings, 2 replies; 10+ messages in thread
From: Max Reitz @ 2018-02-14 20:49 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Richard W . M . Jones, Jeff Cody
libssh2 does not seem to offer real truncation support, so we can only
grow files -- but that is better than nothing.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/ssh.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/block/ssh.c b/block/ssh.c
index ff8576f21e..c235eec255 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1219,6 +1219,29 @@ static int64_t ssh_getlength(BlockDriverState *bs)
return length;
}
+static int ssh_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
+{
+ BDRVSSHState *s = bs->opaque;
+
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_str(prealloc));
+ return -ENOTSUP;
+ }
+
+ if (offset < s->attrs.filesize) {
+ error_setg(errp, "ssh driver does not support shrinking files");
+ return -ENOTSUP;
+ }
+
+ if (offset == s->attrs.filesize) {
+ return 0;
+ }
+
+ return ssh_grow_file(s, offset, errp);
+}
+
static BlockDriver bdrv_ssh = {
.format_name = "ssh",
.protocol_name = "ssh",
@@ -1231,6 +1254,7 @@ static BlockDriver bdrv_ssh = {
.bdrv_co_readv = ssh_co_readv,
.bdrv_co_writev = ssh_co_writev,
.bdrv_getlength = ssh_getlength,
+ .bdrv_truncate = ssh_truncate,
.bdrv_co_flush_to_disk = ssh_co_flush,
.create_opts = &ssh_create_opts,
};
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create()
2018-02-14 20:49 ` [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create() Max Reitz
@ 2018-02-14 21:08 ` Eric Blake
0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2018-02-14 21:08 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Richard W . M . Jones, Jeff Cody, qemu-devel
On 02/14/2018 02:49 PM, Max Reitz wrote:
> If we ever want to offer even rudimentary truncation functionality for
> ssh, we should put the respective code into a reusable function.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/ssh.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> +++ b/block/ssh.c
> @@ -803,6 +803,26 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> return ret;
> }
>
> +static int ssh_grow_file(BDRVSSHState *s, int64_t offset, Error **errp)
> +{
> + ssize_t ret;
> + char c[1] = { '\0' };
Could spell this 'char c[1] = "";', but you just did code motion.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking
2018-02-14 20:49 ` [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking Max Reitz
@ 2018-02-14 21:11 ` Eric Blake
2018-02-14 21:28 ` Max Reitz
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2018-02-14 21:11 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Richard W . M . Jones, Jeff Cody, qemu-devel
On 02/14/2018 02:49 PM, Max Reitz wrote:
> At runtime (that is, during a future ssh_truncate()), the SSH session is
> non-blocking. However, ssh_truncate() (or rather, bdrv_truncate() in
> general) is not a coroutine, so this resize operation needs to block.
>
> For ssh_create(), that is fine, too; the session is never set to
> non-blocking anyway.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/ssh.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index 964e55f7fe..ff8576f21e 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -803,17 +803,24 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
> return ret;
> }
>
> +/* Note: This is a blocking operation */
> static int ssh_grow_file(BDRVSSHState *s, int64_t offset, Error **errp)
> {
> ssize_t ret;
> char c[1] = { '\0' };
> + int was_blocking = libssh2_session_get_blocking(s->session);
>
> /* offset must be strictly greater than the current size so we do
> * not overwrite anything */
> assert(offset > 0 && offset > s->attrs.filesize);
>
> + libssh2_session_set_blocking(s->session, 1);
> +
> libssh2_sftp_seek64(s->sftp_handle, offset - 1);
> ret = libssh2_sftp_write(s->sftp_handle, c, 1);
> +
> + libssh2_session_set_blocking(s->session, was_blocking);
Is it worth skipping the two libssh2_session_set_blocking() calls if
was_blocking is 1? But that's a micro-optimization that probably won't
be noticeable, so I'm also fine with unconditional.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate()
2018-02-14 20:49 ` [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
@ 2018-02-14 21:12 ` Eric Blake
2018-02-15 11:12 ` Richard W.M. Jones
1 sibling, 0 replies; 10+ messages in thread
From: Eric Blake @ 2018-02-14 21:12 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Richard W . M . Jones, Jeff Cody, qemu-devel
On 02/14/2018 02:49 PM, Max Reitz wrote:
> libssh2 does not seem to offer real truncation support, so we can only
> grow files -- but that is better than nothing.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/ssh.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking
2018-02-14 21:11 ` Eric Blake
@ 2018-02-14 21:28 ` Max Reitz
0 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2018-02-14 21:28 UTC (permalink / raw)
To: Eric Blake, qemu-block
Cc: Kevin Wolf, Richard W . M . Jones, Jeff Cody, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]
On 2018-02-14 22:11, Eric Blake wrote:
> On 02/14/2018 02:49 PM, Max Reitz wrote:
>> At runtime (that is, during a future ssh_truncate()), the SSH session is
>> non-blocking. However, ssh_truncate() (or rather, bdrv_truncate() in
>> general) is not a coroutine, so this resize operation needs to block.
>>
>> For ssh_create(), that is fine, too; the session is never set to
>> non-blocking anyway.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/ssh.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/block/ssh.c b/block/ssh.c
>> index 964e55f7fe..ff8576f21e 100644
>> --- a/block/ssh.c
>> +++ b/block/ssh.c
>> @@ -803,17 +803,24 @@ static int ssh_file_open(BlockDriverState *bs,
>> QDict *options, int bdrv_flags,
>> return ret;
>> }
>> +/* Note: This is a blocking operation */
>> static int ssh_grow_file(BDRVSSHState *s, int64_t offset, Error **errp)
>> {
>> ssize_t ret;
>> char c[1] = { '\0' };
>> + int was_blocking = libssh2_session_get_blocking(s->session);
>> /* offset must be strictly greater than the current size so we do
>> * not overwrite anything */
>> assert(offset > 0 && offset > s->attrs.filesize);
>> + libssh2_session_set_blocking(s->session, 1);
>> +
>> libssh2_sftp_seek64(s->sftp_handle, offset - 1);
>> ret = libssh2_sftp_write(s->sftp_handle, c, 1);
>> +
>> + libssh2_session_set_blocking(s->session, was_blocking);
>
> Is it worth skipping the two libssh2_session_set_blocking() calls if
> was_blocking is 1? But that's a micro-optimization that probably won't
> be noticeable, so I'm also fine with unconditional.
I was hoping libssh2 is clever enough for that itself. :-)
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate()
2018-02-14 20:49 ` [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-14 21:12 ` Eric Blake
@ 2018-02-15 11:12 ` Richard W.M. Jones
1 sibling, 0 replies; 10+ messages in thread
From: Richard W.M. Jones @ 2018-02-15 11:12 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf, Jeff Cody
The series looks fine to me:
Reviewed-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
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate()
2018-02-14 20:49 [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
` (2 preceding siblings ...)
2018-02-14 20:49 ` [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
@ 2018-02-23 13:51 ` Max Reitz
3 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2018-02-23 13:51 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Kevin Wolf, Richard W . M . Jones, Jeff Cody
[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]
On 2018-02-14 21:49, Max Reitz wrote:
> For (x-)blockdev-create, all protocol drivers that support image
> creation also need to offer a .bdrv_truncate() implementation that
> matches in features. A previous series of mine brought gluster's and
> sheepdog's implementation up to par regarding preallocated truncation;
> but I forgot about drivers that have a .bdrv_create() but no
> .bdrv_truncate() at all.
>
> There is only one of these, and that is ssh. Since libssh2 does not
> seem to know any way of truncating files, we can only support growing
> files -- but that is what we need for (x-)blockdev-create.
>
> Note that there are also drivers which do not support growing files,
> namely iscsi and file-posix for host devices (maybe more? I hope not).
> But these also pretty much ignore the specified size on .bdrv_create()
> and just use the size of the existing device. They just check that the
> specified size does not exceed the actual size, so that pretty much
> matches what their .bdrv_truncate() supports, and we should be fine
> there.
>
>
> Max Reitz (3):
> block/ssh: Pull ssh_grow_file() from ssh_create()
> block/ssh: Make ssh_grow_file() blocking
> block/ssh: Add basic .bdrv_truncate()
>
> block/ssh.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 53 insertions(+), 8 deletions(-)
Applied to my block branch.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-02-23 13:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-14 20:49 [Qemu-devel] [PATCH 0/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 1/3] block/ssh: Pull ssh_grow_file() from ssh_create() Max Reitz
2018-02-14 21:08 ` Eric Blake
2018-02-14 20:49 ` [Qemu-devel] [PATCH 2/3] block/ssh: Make ssh_grow_file() blocking Max Reitz
2018-02-14 21:11 ` Eric Blake
2018-02-14 21:28 ` Max Reitz
2018-02-14 20:49 ` [Qemu-devel] [PATCH 3/3] block/ssh: Add basic .bdrv_truncate() Max Reitz
2018-02-14 21:12 ` Eric Blake
2018-02-15 11:12 ` Richard W.M. Jones
2018-02-23 13:51 ` [Qemu-devel] [PATCH 0/3] " Max Reitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.