Discussion:
[sheepdog] [Qemu-devel] [PATCH 1/7] gluster: Move glfs_close() to create's clean-up
Eric Blake
2018-02-13 14:54:57 UTC
Permalink
glfs_close() is a classical clean-up operation, as can be seen by the
fact that it is executed even if the truncation before it failed.
Also, moving it to clean-up makes it more clear that if it fails, we do
not want it to overwrite the current ret value if that signifies an
error already.
---
block/gluster.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 14:56:27 UTC
Permalink
Pull out the truncation code from the qemu_cluster_create() function so
we can later reuse it in qemu_gluster_truncate().
---
block/gluster.c | 74 +++++++++++++++++++++++++++++++--------------------------
1 file changed, 40 insertions(+), 34 deletions(-)
Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 14:57:50 UTC
Permalink
Instead of expecting the current size to be 0, query it and allocate
only the area [current_size, offset) if preallocation is requested.
---
block/gluster.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
I don't have a gluster setup handy to test this, but the concept matches
file-posix and the change looks reasonable.

Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 14:58:29 UTC
Permalink
By using qemu_do_cluster_truncate() in qemu_cluster_truncate(), we now
automatically have preallocated truncation.
---
block/gluster.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 15:02:40 UTC
Permalink
We want to use this function in sd_truncate() later on, so taking a
filename is not exactly ideal.
---
block/sheepdog.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
@@ -1837,10 +1837,11 @@ static int sd_prealloc(const char *filename, Error **errp)
void *buf = NULL;
int ret;
- blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
- if (blk == NULL) {
- ret = -EIO;
+ blk = blk_new(BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | BLK_PERM_RESIZE,
+ BLK_PERM_ALL);
+
+ ret = blk_insert_bs(blk, bs, errp);
+ if (ret < 0) {
if (prealloc) {
- ret = sd_prealloc(filename, errp);
+ BlockDriverState *bs;
+ QDict *opts;
+
+ opts = qdict_new();
+ qdict_put_str(opts, "driver", "sheepdog");
+ bs = bdrv_open(filename, NULL, opts, BDRV_O_PROTOCOL | BDRV_O_RDWR,
+ errp);
I'm a bit weak on ensuring the same ending flags result after the new
bdrv_open()/blk_new() as what you used to get on blk_new_open(), so
someone that can actually test this is appreciated.

Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 15:04:12 UTC
Permalink
sd_prealloc() will now preallocate the area [old_size, new_size). As
before, it rounds to buf_size and may thus overshoot and preallocate
areas that were not requested to be preallocated. For image creation,
this is no change in behavior. For truncation, this is in accordance
with the documentation for preallocated truncation.
---
block/sheepdog.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
@@ -1847,19 +1847,13 @@ static int sd_prealloc(BlockDriverState *bs, Error **errp)
blk_set_allow_write_beyond_eof(blk, true);
- vdi_size = blk_getlength(blk);
- if (vdi_size < 0) {
- ret = vdi_size;
- goto out;
- }
-
@@ -2119,7 +2113,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
goto out;
}
- ret = sd_prealloc(bs, errp);
+ ret = sd_prealloc(bs, 0, s->inode.vdi_size, errp);
Nice - you also got rid of a potential failure in blk_getlength().

Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Eric Blake
2018-02-13 15:05:20 UTC
Permalink
---
block/sheepdog.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Eric Blake <***@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Kevin Wolf
2018-02-13 15:19:47 UTC
Permalink
As far as I can see, these are the only protocols beside file-posix that
support preallocated creation. In contrast to file-posix, however, they
have not supported preallocated truncation so far. This series brings
their truncation code to feature parity with their creation code in this
regard.
Note that I do not have a test setup for either of the two drivers, so I
do not actually know whether this works. Anyone with a setup is more
than welcome to test this series.
Thanks, applied to the block branch.

We'll see if someone screams once this hits master.

Kevin
--
sheepdog mailing list
***@lists.wpkg.org
https://lists.wpkg.org/mailman/listinfo/sheepdog
Loading...