From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 68F3FD980
 for <pve-devel@lists.proxmox.com>; Fri,  2 Dec 2022 13:54:57 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 4A042349E2
 for <pve-devel@lists.proxmox.com>; Fri,  2 Dec 2022 13:54:57 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Fri,  2 Dec 2022 13:54:56 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 96B424476D
 for <pve-devel@lists.proxmox.com>; Fri,  2 Dec 2022 13:54:55 +0100 (CET)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Fri,  2 Dec 2022 13:54:52 +0100
Message-Id: <20221202125452.54621-1-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.027 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [qmmock.pm, qemuserver.pm]
Subject: [pve-devel] [RFC qemu-server] migration: nbd export: switch away
 from deprecated QMP command
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Fri, 02 Dec 2022 12:54:57 -0000

The 'nbd-server-add' QMP command has been deprecated since QEMU 5.2 in
favor of a more general 'block-export-add'.

When using 'nbd-server-add', QEMU internally converts the parameters
and calls blk_exp_add() which is also used by 'block-export-add'. It
does one more thing, namely calling nbd_export_set_on_eject_blk() to
auto-remove the export from the server when the backing drive goes
away. But that behavior is not needed in our case, stopping the NBD
server removes the exports anyways.

It was checked with a debugger that the parameters to blk_exp_add()
are still the same after this change. Well, the block node names are
autogenerated and not consistent across invocations.

The alternative to using 'query-block' would be specifying a
predictable 'node-name' for our '-drive' commandline. It's not that
difficult for this use case, but in general one needs to be careful
(e.g. it can't be specified for an empty CD drive, but would need to
be set when inserting a CD later). Querying the actual 'node-name'
seemed a bit more future-proof.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

RFC, because I'm not sure which approach is better.

 PVE/QemuServer.pm            | 17 ++++++++++++++++-
 test/MigrationTest/QmMock.pm |  4 +++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a52a883e..07ee2d67 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5969,10 +5969,25 @@ sub vm_start_nolock {
 	    $migrate_storage_uri = "nbd:${localip}:${storage_migrate_port}";
 	}
 
+	my $block_info = mon_cmd($vmid, "query-block");
+	$block_info = { map { $_->{device} => $_ } $block_info->@* };
+
 	foreach my $opt (sort keys %$nbd) {
 	    my $drivestr = $nbd->{$opt}->{drivestr};
 	    my $volid = $nbd->{$opt}->{volid};
-	    mon_cmd($vmid, "nbd-server-add", device => "drive-$opt", writable => JSON::true );
+
+	    my $block_node = $block_info->{"drive-$opt"}->{inserted}->{'node-name'};
+
+	    mon_cmd(
+		$vmid,
+		"block-export-add",
+		id => "drive-$opt",
+		'node-name' => $block_node,
+		writable => JSON::true,
+		type => "nbd",
+		name => "drive-$opt", # NBD export name
+	    );
+
 	    my $nbd_uri = "$migrate_storage_uri:exportname=drive-$opt";
 	    print "storage migration listens on $nbd_uri volume:$drivestr\n";
 	    print "re-using replicated volume: $opt - $volid\n"
diff --git a/test/MigrationTest/QmMock.pm b/test/MigrationTest/QmMock.pm
index 2d5d5c6c..e34686e3 100644
--- a/test/MigrationTest/QmMock.pm
+++ b/test/MigrationTest/QmMock.pm
@@ -103,8 +103,10 @@ $MigrationTest::Shared::qemu_server_module->mock(
 
 	if ($command eq 'nbd-server-start') {
 	    return;
-	} elsif ($command eq 'nbd-server-add') {
+	} elsif ($command eq 'block-export-add') {
 	    return;
+	} elsif ($command eq 'query-block') {
+	    return [];
 	} elsif ($command eq 'qom-set') {
 	    return;
 	}
-- 
2.30.2