Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-20 Thread Vladimir Sementsov-Ogievskiy 
17.08.2019 17:30, Eric Blake wrote:
> On 8/16/19 5:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
 +++ b/blockdev-nbd.c
 @@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool 
 has_name, const char *name,
    }

    exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
 - writable ? 0 : NBD_FLAG_READ_ONLY,
 + writable ? 0 : NBD_FLAG_READ_ONLY, true,
>>>
>>> s/true/!writable ?
>>
>> Oh, I see, John already noticed this, it's checked in nbd_export_new anyway..
> 
> Still, since two reviewers have caught it, I'm fixing it :)

With it or without:

Reviewed-by: Vladimir Sementsov-Ogievskiy 

> 
> 
 @@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, 
 uint64_t dev_offset,
    perm = BLK_PERM_CONSISTENT_READ;
    if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
    perm |= BLK_PERM_WRITE;
 +    } else if (shared) {
 +    nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
>>
>> For me it looks a bit strange: we already have nbdflags parameter for 
>> nbd_export_new(), why
>> to add a separate boolean to pass one of nbdflags flags?
> 
> Because I want to get rid of the nbdflags in my next patch.
> 
>>
>> Also, for qemu-nbd, shouldn't we allow -e only together with -r ?
> 
> I'm reluctant to; it might break whatever existing user is okay exposing
> it (although such users are questionable, so maybe we can argue they
> were already broken).  Maybe it's time to start a deprecation cycle?
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-17 Thread Eric Blake 
On 8/16/19 5:47 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> +++ b/blockdev-nbd.c
>>> @@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool 
>>> has_name, const char *name,
>>>   }
>>>
>>>   exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
>>> - writable ? 0 : NBD_FLAG_READ_ONLY,
>>> + writable ? 0 : NBD_FLAG_READ_ONLY, true,
>>
>> s/true/!writable ?
> 
> Oh, I see, John already noticed this, it's checked in nbd_export_new anyway..

Still, since two reviewers have caught it, I'm fixing it :)


>>> @@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, 
>>> uint64_t dev_offset,
>>>   perm = BLK_PERM_CONSISTENT_READ;
>>>   if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
>>>   perm |= BLK_PERM_WRITE;
>>> +    } else if (shared) {
>>> +    nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
> 
> For me it looks a bit strange: we already have nbdflags parameter for 
> nbd_export_new(), why
> to add a separate boolean to pass one of nbdflags flags?

Because I want to get rid of the nbdflags in my next patch.

> 
> Also, for qemu-nbd, shouldn't we allow -e only together with -r ?

I'm reluctant to; it might break whatever existing user is okay exposing
it (although such users are questionable, so maybe we can argue they
were already broken).  Maybe it's time to start a deprecation cycle?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-16 Thread Vladimir Sementsov-Ogievskiy 
16.08.2019 13:23, Vladimir Sementsov-Ogievskiy wrote:
> 15.08.2019 21:50, Eric Blake wrote:
>> The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
>> advertised when the server promises cache consistency between
>> simultaneous clients (basically, rules that determine what FUA and
>> flush from one client are able to guarantee for reads from another
>> client).  When we don't permit simultaneous clients (such as qemu-nbd
>> without -e), the bit makes no sense; and for writable images, we
>> probably have a lot more work before we can declare that actions from
>> one client are cache-consistent with actions from another.  But for
>> read-only images, where flush isn't changing any data, we might as
>> well advertise multi-conn support.  What's more, advertisement of the
>> bit makes it easier for clients to determine if 'qemu-nbd -e' was in
>> use, where a second connection will succeed rather than hang until the
>> first client goes away.
>>
>> This patch affects qemu as server in advertising the bit.  We may want
>> to consider patches to qemu as client to attempt parallel connections
>> for higher throughput by spreading the load over those connections
>> when a server advertises multi-conn, but for now sticking to one
>> connection per nbd:// BDS is okay.
>>
>> See also: https://bugzilla.redhat.com/1708300
>> Signed-off-by: Eric Blake 
>> ---
>>   docs/interop/nbd.txt | 1 +
>>   include/block/nbd.h  | 2 +-
>>   blockdev-nbd.c   | 2 +-
>>   nbd/server.c | 4 +++-
>>   qemu-nbd.c   | 2 +-
>>   5 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
>> index fc64473e02b2..6dfec7f47647 100644
>> --- a/docs/interop/nbd.txt
>> +++ b/docs/interop/nbd.txt
>> @@ -53,3 +53,4 @@ the operation of that feature.
>>   * 2.12: NBD_CMD_BLOCK_STATUS for "base:allocation"
>>   * 3.0: NBD_OPT_STARTTLS with TLS Pre-Shared Keys (PSK),
>>   NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
>> +* 4.2: NBD_FLAG_CAN_MULTI_CONN for sharable read-only exports
>> diff --git a/include/block/nbd.h b/include/block/nbd.h
>> index 7b36d672f046..991fd52a5134 100644
>> --- a/include/block/nbd.h
>> +++ b/include/block/nbd.h
>> @@ -326,7 +326,7 @@ 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, uint16_t nbdflags,
>> +  const char *bitmap, uint16_t nbdflags, bool 
>> shared,
>>     void (*close)(NBDExport *), bool writethrough,
>>     BlockBackend *on_eject_blk, Error **errp);
>>   void nbd_export_close(NBDExport *exp);
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 66eebab31875..e5d228771292 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool 
>> has_name, const char *name,
>>   }
>>
>>   exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
>> - writable ? 0 : NBD_FLAG_READ_ONLY,
>> + writable ? 0 : NBD_FLAG_READ_ONLY, true,
> 
> s/true/!writable ?

Oh, I see, John already noticed this, it's checked in nbd_export_new anyway..

> 
>>    NULL, false, on_eject_blk, errp);
>>   if (!exp) {
>>   return;
>> diff --git a/nbd/server.c b/nbd/server.c
>> index a2cf085f7635..a602d85070ff 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -1460,7 +1460,7 @@ 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, uint16_t nbdflags,
>> +  const char *bitmap, uint16_t nbdflags, bool 
>> shared,
>>     void (*close)(NBDExport *), bool writethrough,
>>     BlockBackend *on_eject_blk, Error **errp)
>>   {
>> @@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, 
>> uint64_t dev_offset,
>>   perm = BLK_PERM_CONSISTENT_READ;
>>   if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
>>   perm |= BLK_PERM_WRITE;
>> +    } else if (shared) {
>> +    nbdflags |= NBD_FLAG_CAN_MULTI_CONN;

For me it looks a bit strange: we already have nbdflags parameter for 
nbd_export_new(), why
to add a separate boolean to pass one of nbdflags flags?

Also, for qemu-nbd, shouldn't we allow -e only together with -r ?

>>   }
>>   blk = blk_new(bdrv_get_aio_context(bs), perm,
>>     BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 049645491dab..55f5ceaf5c92 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -1173,7 +1173,7 @@ int main(int argc, char

Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-16 Thread Vladimir Sementsov-Ogievskiy 
15.08.2019 21:50, Eric Blake wrote:
> The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
> advertised when the server promises cache consistency between
> simultaneous clients (basically, rules that determine what FUA and
> flush from one client are able to guarantee for reads from another
> client).  When we don't permit simultaneous clients (such as qemu-nbd
> without -e), the bit makes no sense; and for writable images, we
> probably have a lot more work before we can declare that actions from
> one client are cache-consistent with actions from another.  But for
> read-only images, where flush isn't changing any data, we might as
> well advertise multi-conn support.  What's more, advertisement of the
> bit makes it easier for clients to determine if 'qemu-nbd -e' was in
> use, where a second connection will succeed rather than hang until the
> first client goes away.
> 
> This patch affects qemu as server in advertising the bit.  We may want
> to consider patches to qemu as client to attempt parallel connections
> for higher throughput by spreading the load over those connections
> when a server advertises multi-conn, but for now sticking to one
> connection per nbd:// BDS is okay.
> 
> See also: https://bugzilla.redhat.com/1708300
> Signed-off-by: Eric Blake 
> ---
>   docs/interop/nbd.txt | 1 +
>   include/block/nbd.h  | 2 +-
>   blockdev-nbd.c   | 2 +-
>   nbd/server.c | 4 +++-
>   qemu-nbd.c   | 2 +-
>   5 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
> index fc64473e02b2..6dfec7f47647 100644
> --- a/docs/interop/nbd.txt
> +++ b/docs/interop/nbd.txt
> @@ -53,3 +53,4 @@ the operation of that feature.
>   * 2.12: NBD_CMD_BLOCK_STATUS for "base:allocation"
>   * 3.0: NBD_OPT_STARTTLS with TLS Pre-Shared Keys (PSK),
>   NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
> +* 4.2: NBD_FLAG_CAN_MULTI_CONN for sharable read-only exports
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 7b36d672f046..991fd52a5134 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -326,7 +326,7 @@ 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, uint16_t nbdflags,
> +  const char *bitmap, uint16_t nbdflags, bool shared,
> void (*close)(NBDExport *), bool writethrough,
> BlockBackend *on_eject_blk, Error **errp);
>   void nbd_export_close(NBDExport *exp);
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 66eebab31875..e5d228771292 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>   }
> 
>   exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
> - writable ? 0 : NBD_FLAG_READ_ONLY,
> + writable ? 0 : NBD_FLAG_READ_ONLY, true,

s/true/!writable ?

>NULL, false, on_eject_blk, errp);
>   if (!exp) {
>   return;
> diff --git a/nbd/server.c b/nbd/server.c
> index a2cf085f7635..a602d85070ff 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1460,7 +1460,7 @@ 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, uint16_t nbdflags,
> +  const char *bitmap, uint16_t nbdflags, bool shared,
> void (*close)(NBDExport *), bool writethrough,
> BlockBackend *on_eject_blk, Error **errp)
>   {
> @@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, 
> uint64_t dev_offset,
>   perm = BLK_PERM_CONSISTENT_READ;
>   if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
>   perm |= BLK_PERM_WRITE;
> +} else if (shared) {
> +nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
>   }
>   blk = blk_new(bdrv_get_aio_context(bs), perm,
> BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 049645491dab..55f5ceaf5c92 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -1173,7 +1173,7 @@ int main(int argc, char **argv)
>   }
> 
>   export = nbd_export_new(bs, dev_offset, fd_size, export_name,
> -export_description, bitmap, nbdflags,
> +export_description, bitmap, nbdflags, shared > 1,
>   nbd_export_closed, writethrough, NULL,
>   _fatal);
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-15 Thread no-reply 
Patchew URL: https://patchew.org/QEMU/20190815185024.7010-1-ebl...@redhat.com/



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa

echo
echo "=== UNAME ==="
uname -a

CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

  CC  mips64-softmmu/trace/control-target.o
  CC  mips64-softmmu/trace/generated-helpers.o
  LINKmips64-softmmu/qemu-system-mips64
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:209: qemu-system-mips64] Error 1
make: *** [Makefile:472: mips64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs


The full log is available at
http://patchew.org/logs/20190815185024.7010-1-ebl...@redhat.com/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-15 Thread Richard W.M. Jones 
On Thu, Aug 15, 2019 at 01:50:24PM -0500, Eric Blake wrote:
> The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
> advertised when the server promises cache consistency between
> simultaneous clients (basically, rules that determine what FUA and
> flush from one client are able to guarantee for reads from another
> client).  When we don't permit simultaneous clients (such as qemu-nbd
> without -e), the bit makes no sense; and for writable images, we
> probably have a lot more work before we can declare that actions from
> one client are cache-consistent with actions from another.  But for
> read-only images, where flush isn't changing any data, we might as
> well advertise multi-conn support.  What's more, advertisement of the
> bit makes it easier for clients to determine if 'qemu-nbd -e' was in
> use, where a second connection will succeed rather than hang until the
> first client goes away.
> 
> This patch affects qemu as server in advertising the bit.  We may want
> to consider patches to qemu as client to attempt parallel connections
> for higher throughput by spreading the load over those connections
> when a server advertises multi-conn, but for now sticking to one
> connection per nbd:// BDS is okay.
> 
> See also: https://bugzilla.redhat.com/1708300
> Signed-off-by: Eric Blake 
> ---
>  docs/interop/nbd.txt | 1 +
>  include/block/nbd.h  | 2 +-
>  blockdev-nbd.c   | 2 +-
>  nbd/server.c | 4 +++-
>  qemu-nbd.c   | 2 +-
>  5 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
> index fc64473e02b2..6dfec7f47647 100644
> --- a/docs/interop/nbd.txt
> +++ b/docs/interop/nbd.txt
> @@ -53,3 +53,4 @@ the operation of that feature.
>  * 2.12: NBD_CMD_BLOCK_STATUS for "base:allocation"
>  * 3.0: NBD_OPT_STARTTLS with TLS Pre-Shared Keys (PSK),
>  NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
> +* 4.2: NBD_FLAG_CAN_MULTI_CONN for sharable read-only exports
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 7b36d672f046..991fd52a5134 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -326,7 +326,7 @@ 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, uint16_t nbdflags,
> +  const char *bitmap, uint16_t nbdflags, bool shared,
>void (*close)(NBDExport *), bool writethrough,
>BlockBackend *on_eject_blk, Error **errp);
>  void nbd_export_close(NBDExport *exp);
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 66eebab31875..e5d228771292 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>  }
> 
>  exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
> - writable ? 0 : NBD_FLAG_READ_ONLY,
> + writable ? 0 : NBD_FLAG_READ_ONLY, true,
>   NULL, false, on_eject_blk, errp);
>  if (!exp) {
>  return;
> diff --git a/nbd/server.c b/nbd/server.c
> index a2cf085f7635..a602d85070ff 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1460,7 +1460,7 @@ 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, uint16_t nbdflags,
> +  const char *bitmap, uint16_t nbdflags, bool shared,
>void (*close)(NBDExport *), bool writethrough,
>BlockBackend *on_eject_blk, Error **errp)
>  {
> @@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, 
> uint64_t dev_offset,
>  perm = BLK_PERM_CONSISTENT_READ;
>  if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
>  perm |= BLK_PERM_WRITE;
> +} else if (shared) {
> +nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
>  }
>  blk = blk_new(bdrv_get_aio_context(bs), perm,
>BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 049645491dab..55f5ceaf5c92 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -1173,7 +1173,7 @@ int main(int argc, char **argv)
>  }
> 
>  export = nbd_export_new(bs, dev_offset, fd_size, export_name,
> -export_description, bitmap, nbdflags,
> +export_description, bitmap, nbdflags, shared > 1,
>  nbd_export_closed, writethrough, NULL,
>  _fatal);
> 

Multi-conn is a no-brainer.  For nbdkit it more than doubled
throughput:

[Qemu-devel] [PATCH] nbd: Advertise multi-conn for shared read-only connections

2019-08-15 Thread Eric Blake 
The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
advertised when the server promises cache consistency between
simultaneous clients (basically, rules that determine what FUA and
flush from one client are able to guarantee for reads from another
client).  When we don't permit simultaneous clients (such as qemu-nbd
without -e), the bit makes no sense; and for writable images, we
probably have a lot more work before we can declare that actions from
one client are cache-consistent with actions from another.  But for
read-only images, where flush isn't changing any data, we might as
well advertise multi-conn support.  What's more, advertisement of the
bit makes it easier for clients to determine if 'qemu-nbd -e' was in
use, where a second connection will succeed rather than hang until the
first client goes away.

This patch affects qemu as server in advertising the bit.  We may want
to consider patches to qemu as client to attempt parallel connections
for higher throughput by spreading the load over those connections
when a server advertises multi-conn, but for now sticking to one
connection per nbd:// BDS is okay.

See also: https://bugzilla.redhat.com/1708300
Signed-off-by: Eric Blake 
---
 docs/interop/nbd.txt | 1 +
 include/block/nbd.h  | 2 +-
 blockdev-nbd.c   | 2 +-
 nbd/server.c | 4 +++-
 qemu-nbd.c   | 2 +-
 5 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
index fc64473e02b2..6dfec7f47647 100644
--- a/docs/interop/nbd.txt
+++ b/docs/interop/nbd.txt
@@ -53,3 +53,4 @@ the operation of that feature.
 * 2.12: NBD_CMD_BLOCK_STATUS for "base:allocation"
 * 3.0: NBD_OPT_STARTTLS with TLS Pre-Shared Keys (PSK),
 NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE
+* 4.2: NBD_FLAG_CAN_MULTI_CONN for sharable read-only exports
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 7b36d672f046..991fd52a5134 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -326,7 +326,7 @@ 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, uint16_t nbdflags,
+  const char *bitmap, uint16_t nbdflags, bool shared,
   void (*close)(NBDExport *), bool writethrough,
   BlockBackend *on_eject_blk, Error **errp);
 void nbd_export_close(NBDExport *exp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 66eebab31875..e5d228771292 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -189,7 +189,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, 
const char *name,
 }

 exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
- writable ? 0 : NBD_FLAG_READ_ONLY,
+ writable ? 0 : NBD_FLAG_READ_ONLY, true,
  NULL, false, on_eject_blk, errp);
 if (!exp) {
 return;
diff --git a/nbd/server.c b/nbd/server.c
index a2cf085f7635..a602d85070ff 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1460,7 +1460,7 @@ 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, uint16_t nbdflags,
+  const char *bitmap, uint16_t nbdflags, bool shared,
   void (*close)(NBDExport *), bool writethrough,
   BlockBackend *on_eject_blk, Error **errp)
 {
@@ -1486,6 +1486,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t 
dev_offset,
 perm = BLK_PERM_CONSISTENT_READ;
 if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
 perm |= BLK_PERM_WRITE;
+} else if (shared) {
+nbdflags |= NBD_FLAG_CAN_MULTI_CONN;
 }
 blk = blk_new(bdrv_get_aio_context(bs), perm,
   BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 049645491dab..55f5ceaf5c92 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1173,7 +1173,7 @@ int main(int argc, char **argv)
 }

 export = nbd_export_new(bs, dev_offset, fd_size, export_name,
-export_description, bitmap, nbdflags,
+export_description, bitmap, nbdflags, shared > 1,
 nbd_export_closed, writethrough, NULL,
 _fatal);

-- 
2.20.1