From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Roman Kagan <rkagan@virtuozzo.com>,
Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Cc: "kwolf@redhat.com" <kwolf@redhat.com>,
"fam@euphon.net" <fam@euphon.net>,
Denis Lunev <den@virtuozzo.com>,
"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"armbru@redhat.com" <armbru@redhat.com>,
"stefanha@redhat.com" <stefanha@redhat.com>,
"mreitz@redhat.com" <mreitz@redhat.com>,
"jsnow@redhat.com" <jsnow@redhat.com>,
"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: [PATCH v2 3/6] qemu-nbd: add compression flag support
Date: Mon, 7 Oct 2019 11:52:17 +0000 [thread overview]
Message-ID: <e84705fb-1c32-1042-d2e2-cf831fa97eae@virtuozzo.com> (raw)
In-Reply-To: <20191004101903.GA4478@rkaganb.sw.ru>
04.10.2019 13:19, Roman Kagan wrote:
> On Wed, Oct 02, 2019 at 05:22:43PM +0300, Andrey Shinkevich wrote:
>> Added possibility to write compressed data by using the
>> blk_write_compressed. This action has the limitations of the format
>> driver. For example we can't write compressed data over other.
>>
>> $ ./qemu-img create -f qcow2 -o size=10G ./image.qcow2
>> $ sudo ./qemu-nbd -c /dev/nbd0 ./image.qcow2
>> $ sudo dd if=/dev/sda1 of=/dev/nbd0 bs=10M count=10
>> 10+0 records in
>> 10+0 records out
>> 104857600 bytes (105 MB) copied, 0,0890581 s, 1,2 GB/s
>> $ sudo ./qemu-nbd -d /dev/nbd0
>> $ du -sh ./image.qcow2
>> 101M ./image.qcow2
>>
>> $ ./qemu-img create -f qcow2 -o size=10G ./image.qcow2
>> $ sudo ./qemu-nbd -C -c /dev/nbd0 ./image.qcow2
>> $ sudo dd if=/dev/sda1 of=/dev/nbd0 bs=10M count=10
>> 10+0 records in
>> 10+0 records out
>> 104857600 bytes (105 MB) copied, 0,076046 s, 1,4 GB/s
>> $ sudo ./qemu-nbd -d /dev/nbd0
>> $ du -sh ./image.qcow2
>> 5,3M ./image.qcow2
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>> blockdev-nbd.c | 8 ++++++--
>> include/block/nbd.h | 11 +++++++++--
>> nbd/server.c | 14 ++++++++++----
>> qemu-nbd.c | 30 ++++++++++++++++++++++++++----
>> qemu-nbd.texi | 2 ++
>> 5 files changed, 53 insertions(+), 12 deletions(-)
>>
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 6a8b206..e83036b 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -151,6 +151,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
>> BlockBackend *on_eject_blk;
>> NBDExport *exp;
>> int64_t len;
>> + uint32_t iflags = 0;
>> AioContext *aio_context;
>>
>> if (!nbd_server) {
>> @@ -189,9 +190,12 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
>> if (bdrv_is_read_only(bs)) {
>> writable = false;
>> }
>> + if (!writable) {
>> + iflags = NBD_INTERNAL_FLAG_READONLY | NBD_INTERNAL_FLAG_SHARED;
>> + }
>>
>> - exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,
>> - NULL, false, on_eject_blk, errp);
>> + exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, iflags, NULL,
>> + on_eject_blk, errp);
>> if (!exp) {
>> goto out;
>> }
>> diff --git a/include/block/nbd.h b/include/block/nbd.h
>> index 316fd70..80be9d5 100644
>> --- a/include/block/nbd.h
>> +++ b/include/block/nbd.h
>> @@ -25,6 +25,13 @@
>> #include "crypto/tlscreds.h"
>> #include "qapi/error.h"
>>
>> +enum {
>> + NBD_INTERNAL_FLAG_READONLY = 1 << 0, /* Device is read-only */
>> + NBD_INTERNAL_FLAG_SHARED = 1 << 1, /* Device can be shared */
>> + NBD_INTERNAL_FLAG_WRITETHROUGH = 1 << 2, /* Enable write cache */
>> + NBD_INTERNAL_FLAG_COMPRESS = 1 << 3, /* Use compressed write */
>> +};
>> +
>> /* Handshake phase structs - this struct is passed on the wire */
>>
>> struct NBDOption {
>> @@ -330,8 +337,8 @@ typedef struct NBDClient NBDClient;
>>
>> NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>> uint64_t size, const char *name, const char *desc,
>> - const char *bitmap, bool readonly, bool shared,
>> - void (*close)(NBDExport *), bool writethrough,
>> + const char *bitmap, uint32_t iflags,
>> + void (*close)(NBDExport *),
>> BlockBackend *on_eject_blk, Error **errp);
>> void nbd_export_close(NBDExport *exp);
>> void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
>> diff --git a/nbd/server.c b/nbd/server.c
>> index d8d1e62..cf3b75d 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -91,6 +91,7 @@ struct NBDExport {
>> uint16_t nbdflags;
>> QTAILQ_HEAD(, NBDClient) clients;
>> QTAILQ_ENTRY(NBDExport) next;
>> + uint32_t iflags;
>>
>> AioContext *ctx;
>>
>> @@ -1471,13 +1472,14 @@ static void nbd_eject_notifier(Notifier *n, void *data)
>>
>> NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>> uint64_t size, const char *name, const char *desc,
>> - const char *bitmap, bool readonly, bool shared,
>> - void (*close)(NBDExport *), bool writethrough,
>> + const char *bitmap, uint32_t iflags,
>> + void (*close)(NBDExport *),
>> BlockBackend *on_eject_blk, Error **errp)
>> {
>> AioContext *ctx;
>> BlockBackend *blk;
>> NBDExport *exp = g_new0(NBDExport, 1);
>> + bool readonly = iflags & NBD_INTERNAL_FLAG_READONLY;
>> uint64_t perm;
>> int ret;
>>
>> @@ -1504,7 +1506,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>> if (ret < 0) {
>> goto fail;
>> }
>> - blk_set_enable_write_cache(blk, !writethrough);
>> + blk_set_enable_write_cache(blk, !(iflags & NBD_INTERNAL_FLAG_WRITETHROUGH));
>> blk_set_allow_aio_context_change(blk, true);
>>
>> exp->refcount = 1;
>> @@ -1518,13 +1520,14 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>> NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_CACHE);
>> if (readonly) {
>> exp->nbdflags |= NBD_FLAG_READ_ONLY;
>> - if (shared) {
>> + if (iflags & NBD_INTERNAL_FLAG_SHARED) {
>> exp->nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
>> }
>> } else {
>> exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
>> NBD_FLAG_SEND_FAST_ZERO);
>> }
>> + exp->iflags = iflags;
>> assert(size <= INT64_MAX - dev_offset);
>> exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
>>
>> @@ -2312,6 +2315,9 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>> if (request->flags & NBD_CMD_FLAG_FUA) {
>> flags |= BDRV_REQ_FUA;
>> }
>> + if (exp->iflags & NBD_INTERNAL_FLAG_COMPRESS) {
>> + flags |= BDRV_REQ_WRITE_COMPRESSED;
>> + }
>> ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
>> data, request->len, flags);
>> return nbd_send_generic_reply(client, request->handle, ret,
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 9032b6d..4163135 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -139,6 +139,7 @@ static void usage(const char *name)
>> " --discard=MODE set discard mode (ignore, unmap)\n"
>> " --detect-zeroes=MODE set detect-zeroes mode (off, on, unmap)\n"
>> " --image-opts treat FILE as a full set of image options\n"
>> +" -C, --compress use data compression (if the target format supports it)\n"
>> "\n"
>> QEMU_HELP_BOTTOM "\n"
>> , name, name, NBD_DEFAULT_PORT, "DEVICE");
>> @@ -602,6 +603,7 @@ int main(int argc, char **argv)
>> BlockDriverState *bs;
>> uint64_t dev_offset = 0;
>> bool readonly = false;
>> + uint32_t iflags = 0;
>> bool disconnect = false;
>> const char *bindto = NULL;
>> const char *port = NULL;
>> @@ -610,7 +612,7 @@ int main(int argc, char **argv)
>> int64_t fd_size;
>> QemuOpts *sn_opts = NULL;
>> const char *sn_id_or_name = NULL;
>> - const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:L";
>> + const char *sopt = "hVb:o:p:rsnCP:c:dvk:e:f:tl:x:T:D:B:L";
>> struct option lopt[] = {
>> { "help", no_argument, NULL, 'h' },
>> { "version", no_argument, NULL, 'V' },
>> @@ -619,6 +621,7 @@ int main(int argc, char **argv)
>> { "socket", required_argument, NULL, 'k' },
>> { "offset", required_argument, NULL, 'o' },
>> { "read-only", no_argument, NULL, 'r' },
>> + { "compress", no_argument, NULL, 'C'},
>> { "partition", required_argument, NULL, 'P' },
>> { "bitmap", required_argument, NULL, 'B' },
>> { "connect", required_argument, NULL, 'c' },
>> @@ -786,6 +789,9 @@ int main(int argc, char **argv)
>> readonly = true;
>> flags &= ~BDRV_O_RDWR;
>> break;
>> + case 'C':
>> + iflags |= NBD_INTERNAL_FLAG_COMPRESS;
>> + break;
>> case 'P':
>> warn_report("The '-P' option is deprecated; use --image-opts with "
>> "a raw device wrapper for subset exports instead");
>> @@ -1117,6 +1123,14 @@ int main(int argc, char **argv)
>>
>> blk_set_enable_write_cache(blk, !writethrough);
>>
>> + if ((iflags & NBD_INTERNAL_FLAG_COMPRESS) &&
>> + !(bs->drv && bs->drv->bdrv_co_pwritev_compressed_part))
>> + {
>> + error_report("Compression not supported for this file format %s",
>> + argv[optind]);
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> if (sn_opts) {
>> ret = bdrv_snapshot_load_tmp(bs,
>> qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
>> @@ -1173,10 +1187,18 @@ int main(int argc, char **argv)
>> fd_size = limit;
>> }
>>
>> + if (writethrough) {
>> + iflags |= NBD_INTERNAL_FLAG_WRITETHROUGH;
>> + }
>> + if (readonly) {
>> + iflags |= NBD_INTERNAL_FLAG_READONLY;
>> + }
>> + if (shared > 1) {
>> + iflags |= NBD_INTERNAL_FLAG_SHARED;
>> + }
>> export = nbd_export_new(bs, dev_offset, fd_size, export_name,
>> - export_description, bitmap, readonly, shared > 1,
>> - nbd_export_closed, writethrough, NULL,
>> - &error_fatal);
>> + export_description, bitmap, iflags,
>> + nbd_export_closed, NULL, &error_fatal);
>>
>> if (device) {
>> #if HAVE_NBD_DEVICE
>> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
>> index 7f55657..26ea1ec 100644
>> --- a/qemu-nbd.texi
>> +++ b/qemu-nbd.texi
>> @@ -55,6 +55,8 @@ Force the use of the block driver for format @var{fmt} instead of
>> auto-detecting.
>> @item -r, --read-only
>> Export the disk as read-only.
>> +@item -C, --compress
>> +Use data compression (if the target format supports it)
>> @item -P, --partition=@var{num}
>> Deprecated: Only expose MBR partition @var{num}. Understands physical
>> partitions 1-4 and logical partition 5. New code should instead use
>
> I wonder if we're better off adding a generic blockdev option instead,
> so that all tools can benefit from it?
>
I like the idea. And then we'll deprecate compression option in backup job.
--
Best regards,
Vladimir
next prev parent reply other threads:[~2019-10-07 11:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-02 14:22 [PATCH v2 0/6] qcow2: advanced compression options Andrey Shinkevich
2019-10-02 14:22 ` [PATCH v2 1/6] qcow2: Allow writing compressed data to multiple clusters Andrey Shinkevich
2019-10-03 14:16 ` Vladimir Sementsov-Ogievskiy
2019-10-02 14:22 ` [PATCH v2 2/6] tests/qemu-iotests: add case of " Andrey Shinkevich
2019-10-03 14:21 ` Vladimir Sementsov-Ogievskiy
2019-10-02 14:22 ` [PATCH v2 3/6] qemu-nbd: add compression flag support Andrey Shinkevich
2019-10-03 14:32 ` Vladimir Sementsov-Ogievskiy
2019-10-04 10:19 ` Roman Kagan
2019-10-07 11:52 ` Vladimir Sementsov-Ogievskiy [this message]
2019-10-02 14:22 ` [PATCH v2 4/6] block: support compressed write for copy-on-read Andrey Shinkevich
2019-10-03 14:37 ` Vladimir Sementsov-Ogievskiy
2019-10-02 14:22 ` [PATCH v2 5/6] block-stream: add compress option Andrey Shinkevich
2019-10-03 14:48 ` Vladimir Sementsov-Ogievskiy
2019-10-02 14:22 ` [PATCH v2 6/6] tests/qemu-iotests: add case for block-stream compress Andrey Shinkevich
2019-10-03 14:58 ` Vladimir Sementsov-Ogievskiy
2019-10-15 17:57 ` Andrey Shinkevich
2019-10-02 15:07 ` [PATCH v2 0/6] qcow2: advanced compression options no-reply
2019-10-02 15:35 ` Vladimir Sementsov-Ogievskiy
2019-10-02 15:58 ` Max Reitz
2019-10-02 19:04 ` Markus Armbruster
2019-10-02 15:07 ` no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e84705fb-1c32-1042-d2e2-cf831fa97eae@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=andrey.shinkevich@virtuozzo.com \
--cc=armbru@redhat.com \
--cc=den@virtuozzo.com \
--cc=dgilbert@redhat.com \
--cc=fam@euphon.net \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rkagan@virtuozzo.com \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.