public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC qemu-server] migration: nbd export: switch away from deprecated QMP command
Date: Fri,  2 Dec 2022 13:54:52 +0100	[thread overview]
Message-ID: <20221202125452.54621-1-f.ebner@proxmox.com> (raw)

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





             reply	other threads:[~2022-12-02 12:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02 12:54 Fiona Ebner [this message]
2022-12-16 11:52 ` Thomas Lamprecht
2022-12-16 12:03   ` Fiona Ebner
2023-01-16 13:02 ` [pve-devel] applied: " Thomas Lamprecht

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=20221202125452.54621-1-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal