Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-01-26'…

… into staging

nbd patches for 2018-01-26

- Vladimir Sementsov-Ogievskiy - nbd export qmp interface
- Eric Blake - hmp: Add nbd_server_remove to mirror QMP command
- Edgar Kaziakhmedov - nbd: implement bdrv_get_info callback

# gpg: Signature made Fri 26 Jan 2018 16:02:34 GMT
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2018-01-26:
  nbd: implement bdrv_get_info callback
  hmp: Add nbd_server_remove to mirror QMP command
  iotest 205: new test for qmp nbd-server-remove
  iotests: implement QemuIoInteractive class
  iotest 147: add cases to test new @name parameter of nbd-server-add
  qapi: add nbd-server-remove
  hmp: Add name parameter to nbd_server_add
  qapi: add name parameter to nbd-server-add

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

block/nbd.c

@@ -566,6 +566,14 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
     bs->full_open_options = opts;
 }
 
+static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+    if (bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP) {
+        bdi->can_write_zeroes_with_unmap = true;
+    }
+    return 0;
+}
+
 static BlockDriver bdrv_nbd = {
     .format_name                = "nbd",
     .protocol_name              = "nbd",
@@ -583,6 +591,7 @@ static BlockDriver bdrv_nbd = {
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
     .bdrv_refresh_filename      = nbd_refresh_filename,
+    .bdrv_get_info              = nbd_get_info,
 };
 
 static BlockDriver bdrv_nbd_tcp = {
@@ -602,6 +611,7 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
     .bdrv_refresh_filename      = nbd_refresh_filename,
+    .bdrv_get_info              = nbd_get_info,
 };
 
 static BlockDriver bdrv_nbd_unix = {
@@ -621,6 +631,7 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
     .bdrv_refresh_filename      = nbd_refresh_filename,
+    .bdrv_get_info              = nbd_get_info,
 };
 
 static void bdrv_nbd_init(void)

blockdev-nbd.c

@@ -140,8 +140,8 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
     qapi_free_SocketAddress(addr_flat);
 }
 
-void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
-                        Error **errp)
+void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
+                        bool has_writable, bool writable, Error **errp)
 {
     BlockDriverState *bs = NULL;
     BlockBackend *on_eject_blk;
@@ -152,8 +152,12 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
         return;
     }
 
-    if (nbd_export_find(device)) {
-        error_setg(errp, "NBD server already exporting device '%s'", device);
+    if (!has_name) {
+        name = device;
+    }
+
+    if (nbd_export_find(name)) {
+        error_setg(errp, "NBD server already has export named '%s'", name);
         return;
     }
 
@@ -177,14 +181,38 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
         return;
     }
 
-    nbd_export_set_name(exp, device);
+    nbd_export_set_name(exp, name);
 
     /* The list of named exports has a strong reference to this export now and
      * our only way of accessing it is through nbd_export_find(), so we can drop
      * the strong reference that is @exp. */
     nbd_export_put(exp);
 }
 
+void qmp_nbd_server_remove(const char *name,
+                           bool has_mode, NbdServerRemoveMode mode,
+                           Error **errp)
+{
+    NBDExport *exp;
+
+    if (!nbd_server) {
+        error_setg(errp, "NBD server not running");
+        return;
+    }
+
+    exp = nbd_export_find(name);
+    if (exp == NULL) {
+        error_setg(errp, "Export '%s' is not found", name);
+        return;
+    }
+
+    if (!has_mode) {
+        mode = NBD_SERVER_REMOVE_MODE_SAFE;
+    }
+
+    nbd_export_remove(exp, mode, errp);
+}
+
 void qmp_nbd_server_stop(Error **errp)
 {
     nbd_export_close_all();

hmp-commands.hx

@@ -1553,17 +1553,35 @@ ETEXI
 
     {
         .name       = "nbd_server_add",
-        .args_type  = "writable:-w,device:B",
-        .params     = "nbd_server_add [-w] device",
+        .args_type  = "writable:-w,device:B,name:s?",
+        .params     = "nbd_server_add [-w] device [name]",
         .help       = "export a block device via NBD",
         .cmd        = hmp_nbd_server_add,
     },
 STEXI
-@item nbd_server_add @var{device}
+@item nbd_server_add @var{device} [ @var{name} ]
 @findex nbd_server_add
 Export a block device through QEMU's NBD server, which must be started
 beforehand with @command{nbd_server_start}.  The @option{-w} option makes the
-exported device writable too.
+exported device writable too.  The export name is controlled by @var{name},
+defaulting to @var{device}.
+ETEXI
+
+    {
+        .name       = "nbd_server_remove",
+        .args_type  = "force:-f,name:s",
+        .params     = "nbd_server_remove [-f] name",
+        .help       = "remove an export previously exposed via NBD",
+        .cmd        = hmp_nbd_server_remove,
+    },
+STEXI
+@item nbd_server_remove [-f] @var{name}
+@findex nbd_server_remove
+Stop exporting a block device through QEMU's NBD server, which was
+previously started with @command{nbd_server_add}.  The @option{-f}
+option forces the server to drop the export immediately even if
+clients are connected; otherwise the command fails unless there are no
+clients.
 ETEXI
 
     {

hmp.c

@@ -2203,7 +2203,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
             continue;
         }
 
-        qmp_nbd_server_add(info->value->device, true, writable, &local_err);
+        qmp_nbd_server_add(info->value->device, false, NULL,
+                           true, writable, &local_err);
 
         if (local_err != NULL) {
             qmp_nbd_server_stop(NULL);
@@ -2220,14 +2221,23 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
+    const char *name = qdict_get_try_str(qdict, "name");
     bool writable = qdict_get_try_bool(qdict, "writable", false);
     Error *local_err = NULL;
 
-    qmp_nbd_server_add(device, true, writable, &local_err);
+    qmp_nbd_server_add(device, !!name, name, true, writable, &local_err);
+    hmp_handle_error(mon, &local_err);
+}
 
-    if (local_err != NULL) {
-        hmp_handle_error(mon, &local_err);
-    }
+void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
+{
+    const char *name = qdict_get_str(qdict, "name");
+    bool force = qdict_get_try_bool(qdict, "force", false);
+    Error *err = NULL;
+
+    /* Rely on NBD_SERVER_REMOVE_MODE_SAFE being the default */
+    qmp_nbd_server_remove(name, force, NBD_SERVER_REMOVE_MODE_HARD, &err);
+    hmp_handle_error(mon, &err);
 }
 
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)

hmp.h

@@ -101,6 +101,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void hmp_screendump(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 void hmp_chardev_add(Monitor *mon, const QDict *qdict);
 void hmp_chardev_change(Monitor *mon, const QDict *qdict);

include/block/nbd.h

@@ -261,6 +261,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
                           bool writethrough, BlockBackend *on_eject_blk,
                           Error **errp);
 void nbd_export_close(NBDExport *exp);
+void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
 

nbd/server.c

@@ -1177,6 +1177,19 @@ void nbd_export_close(NBDExport *exp)
     nbd_export_put(exp);
 }
 
+void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp)
+{
+    if (mode == NBD_SERVER_REMOVE_MODE_HARD || QTAILQ_EMPTY(&exp->clients)) {
+        nbd_export_close(exp);
+        return;
+    }
+
+    assert(mode == NBD_SERVER_REMOVE_MODE_SAFE);
+
+    error_setg(errp, "export '%s' still in use", exp->name);
+    error_append_hint(errp, "Use mode='hard' to force client disconnect\n");
+}
+
 void nbd_export_get(NBDExport *exp)
 {
     assert(exp->refcount > 0);

qapi/block.json

@@ -213,14 +213,60 @@
 #
 # @device: The device name or node name of the node to be exported
 #
+# @name: Export name. If unspecified, the @device parameter is used as the
+#        export name. (Since 2.12)
+#
 # @writable: Whether clients should be able to write to the device via the
 #     NBD connection (default false).
 #
-# Returns: error if the device is already marked for export.
+# Returns: error if the server is not running, or export with the same name
+#          already exists.
 #
 # Since: 1.3.0
 ##
-{ 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
+{ 'command': 'nbd-server-add',
+  'data': {'device': 'str', '*name': 'str', '*writable': 'bool'} }
+
+##
+# @NbdServerRemoveMode:
+#
+# Mode for removing an NBD export.
+#
+# @safe: Remove export if there are no existing connections, fail otherwise.
+#
+# @hard: Drop all connections immediately and remove export.
+#
+# Potential additional modes to be added in the future:
+#
+# hide: Just hide export from new clients, leave existing connections as is.
+#       Remove export after all clients are disconnected.
+#
+# soft: Hide export from new clients, answer with ESHUTDOWN for all further
+#       requests from existing clients.
+#
+# Since: 2.12
+##
+{'enum': 'NbdServerRemoveMode', 'data': ['safe', 'hard']}
+
+##
+# @nbd-server-remove:
+#
+# Remove NBD export by name.
+#
+# @name: Export name.
+#
+# @mode: Mode of command operation. See @NbdServerRemoveMode description.
+#        Default is 'safe'.
+#
+# Returns: error if
+#            - the server is not running
+#            - export is not found
+#            - mode is 'safe' and there are existing connections
+#
+# Since: 2.12
+##
+{ 'command': 'nbd-server-remove',
+  'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
 
 ##
 # @nbd-server-stop:

tests/qemu-iotests/147

@@ -38,8 +38,8 @@ def flatten_sock_addr(crumpled_address):
 
 
 class NBDBlockdevAddBase(iotests.QMPTestCase):
-    def blockdev_add_options(self, address, export=None):
-        options = { 'node-name': 'nbd-blockdev',
+    def blockdev_add_options(self, address, export, node_name):
+        options = { 'node-name': node_name,
                     'driver': 'raw',
                     'file': {
                         'driver': 'nbd',
@@ -50,23 +50,28 @@ class NBDBlockdevAddBase(iotests.QMPTestCase):
             options['file']['export'] = export
         return options
 
-    def client_test(self, filename, address, export=None):
-        bao = self.blockdev_add_options(address, export)
+    def client_test(self, filename, address, export=None,
+                    node_name='nbd-blockdev', delete=True):
+        bao = self.blockdev_add_options(address, export, node_name)
         result = self.vm.qmp('blockdev-add', **bao)
         self.assert_qmp(result, 'return', {})
 
+        found = False
         result = self.vm.qmp('query-named-block-nodes')
         for node in result['return']:
-            if node['node-name'] == 'nbd-blockdev':
+            if node['node-name'] == node_name:
+                found = True
                 if isinstance(filename, str):
                     self.assert_qmp(node, 'image/filename', filename)
                 else:
                     self.assert_json_filename_equal(node['image']['filename'],
                                                     filename)
                 break
+        self.assertTrue(found)
 
-        result = self.vm.qmp('blockdev-del', node_name='nbd-blockdev')
-        self.assert_qmp(result, 'return', {})
+        if delete:
+            result = self.vm.qmp('blockdev-del', node_name=node_name)
+            self.assert_qmp(result, 'return', {})
 
 
 class QemuNBD(NBDBlockdevAddBase):
@@ -125,26 +130,63 @@ class BuiltinNBD(NBDBlockdevAddBase):
         except OSError:
             pass
 
-    def _server_up(self, address):
+    def _server_up(self, address, export_name=None, export_name2=None):
         result = self.server.qmp('nbd-server-start', addr=address)
         self.assert_qmp(result, 'return', {})
 
-        result = self.server.qmp('nbd-server-add', device='nbd-export')
+        if export_name is None:
+            result = self.server.qmp('nbd-server-add', device='nbd-export')
+        else:
+            result = self.server.qmp('nbd-server-add', device='nbd-export',
+                                     name=export_name)
         self.assert_qmp(result, 'return', {})
 
+        if export_name2 is not None:
+            result = self.server.qmp('nbd-server-add', device='nbd-export',
+                                     name=export_name2)
+            self.assert_qmp(result, 'return', {})
+
+
     def _server_down(self):
         result = self.server.qmp('nbd-server-stop')
         self.assert_qmp(result, 'return', {})
 
-    def test_inet(self):
+    def do_test_inet(self, export_name=None):
         address = { 'type': 'inet',
                     'data': {
                         'host': 'localhost',
                         'port': str(NBD_PORT)
                     } }
-        self._server_up(address)
-        self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT,
-                         flatten_sock_addr(address), 'nbd-export')
+        self._server_up(address, export_name)
+        export_name = export_name or 'nbd-export'
+        self.client_test('nbd://localhost:%i/%s' % (NBD_PORT, export_name),
+                         flatten_sock_addr(address), export_name)
+        self._server_down()
+
+    def test_inet_default_export_name(self):
+        self.do_test_inet()
+
+    def test_inet_same_export_name(self):
+        self.do_test_inet('nbd-export')
+
+    def test_inet_different_export_name(self):
+        self.do_test_inet('shadow')
+
+    def test_inet_two_exports(self):
+        address = { 'type': 'inet',
+                    'data': {
+                        'host': 'localhost',
+                        'port': str(NBD_PORT)
+                    } }
+        self._server_up(address, 'exp1', 'exp2')
+        self.client_test('nbd://localhost:%i/%s' % (NBD_PORT, 'exp1'),
+                         flatten_sock_addr(address), 'exp1', 'node1', False)
+        self.client_test('nbd://localhost:%i/%s' % (NBD_PORT, 'exp2'),
+                         flatten_sock_addr(address), 'exp2', 'node2', False)
+        result = self.vm.qmp('blockdev-del', node_name='node1')
+        self.assert_qmp(result, 'return', {})
+        result = self.vm.qmp('blockdev-del', node_name='node2')
+        self.assert_qmp(result, 'return', {})
         self._server_down()
 
     def test_inet6(self):