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 <vsement...@virtuozzo.com> > > >>>> @@ -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