public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 2/2] migrate: send updated tpmstate volid to target node
Date: Mon, 22 Nov 2021 11:30:11 +0100	[thread overview]
Message-ID: <20211122103011.2374843-2-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20211122103011.2374843-1-f.gruenbichler@proxmox.com>

and update the in-memory config for starting the target VM accordingly.
this possibly breaks migration new -> old iff
- spice is not used (else the explicit ticket wins because it comes
  later)
- a local TPM state volume is used
- that local TPM state volume has a different volume id on the target
  node (switched storage, volname already taken, ..)

because the target node will then mis-interpret the tpmstate0 line as
spice ticket and set it accordingly. if the old tpm state volume ID does
not exist on the target node, migration will fail. if it exists by
chance, it might work albeit with a wrong spice ticket (new because of
this patch) and tpm state volume (pre-existing breakage).

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    https://forum.proxmox.com/threads/error-live-migrate-a-windows-vm-with-tpm.99906/#post-431345
    
    IMHO this is okay since new->old is always best effort and might fail for
    $reasons. we could extend this to check the broadcasted version on the source
    node once pve-manager requires a new enough qemu-server, and die early there to
    avoid running into the compat issue..
    
    the original compat logic was unfortunate - an "old" source node would only
    ever send an unprefixed spice ticket as first line on STDIN, if the compat
    logic had included a check for that this would be a bit easier..

 PVE/API2/Qemu.pm   |  4 ++++
 PVE/QemuMigrate.pm |  8 ++++++++
 PVE/QemuServer.pm  | 10 +++++++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8b834bb..6992f6f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2319,6 +2319,7 @@ __PACKAGE__->register_method({
 	my $spice_ticket;
 	my $nbd_protocol_version = 0;
 	my $replicated_volumes = {};
+	my $tpmstate_vol;
 	if ($stateuri && ($stateuri eq 'tcp' || $stateuri eq 'unix') && $migratedfrom && ($rpcenv->{type} eq 'cli')) {
 	    while (defined(my $line = <STDIN>)) {
 		chomp $line;
@@ -2328,6 +2329,8 @@ __PACKAGE__->register_method({
 		    $nbd_protocol_version = $1;
 		} elsif ($line =~ m/^replicated_volume: (.*)$/) {
 		    $replicated_volumes->{$1} = 1;
+		} elsif ($line =~ m/^tpmstate0: (.*)$/) {
+		    $tpmstate_vol = $1;
 		} elsif (!$spice_ticket) {
 		    # fallback for old source node
 		    $spice_ticket = $line;
@@ -2369,6 +2372,7 @@ __PACKAGE__->register_method({
 		    storagemap => $storagemap,
 		    nbd_proto_version => $nbd_protocol_version,
 		    replicated_volumes => $replicated_volumes,
+		    tpmstate_vol => $tpmstate_vol,
 		};
 
 		my $params = {
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index ae3eaf1..c9bc39d 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -870,6 +870,14 @@ sub phase2 {
     # version > 0 for unix socket support
     my $nbd_protocol_version = 1;
     my $input = "nbd_protocol_version: $nbd_protocol_version\n";
+
+    if ($conf->{tpmstate0}) {
+	my $tpmdrive = PVE::QemuServer::parse_drive('tpmstate0', $conf->{tpmstate0});
+	my $tpmvol = $tpmdrive->{file};
+	$input .= "tpmstate0: $self->{volume_map}->{$tpmvol}"
+	    if $self->{volume_map}->{$tpmvol} && $tpmvol ne $self->{volume_map}->{$tpmvol};
+    }
+
     $input .= "spice_ticket: $spice_ticket\n" if $spice_ticket;
 
     my @online_replicated_volumes = $self->filter_local_volumes('online', 1);
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 76d45a2..45b704d 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5370,7 +5370,8 @@ sub vm_start {
 #   network => CIDR of migration network
 #   type => secure/insecure - tunnel over encrypted connection or plain-text
 #   nbd_proto_version => int, 0 for TCP, 1 for UNIX
-#   replicated_volumes = which volids should be re-used with bitmaps for nbd migration
+#   replicated_volumes => which volids should be re-used with bitmaps for nbd migration
+#   tpmstate_vol => new volid of tpmstate0, not yet contained in config
 sub vm_start_nolock {
     my ($storecfg, $vmid, $conf, $params, $migrate_opts) = @_;
 
@@ -5395,6 +5396,13 @@ sub vm_start_nolock {
     # this way we can reuse the old ISO with the correct config
     PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid) if !$migratedfrom;
 
+    # override TPM state vol if migrated, conf is out of date still
+    if (my $tpmvol = $migrate_opts->{tpmstate_vol}) {
+        my $parsed = parse_drive("tpmstate0", $conf->{tpmstate0});
+        $parsed->{file} = $tpmvol;
+        $conf->{tpmstate0} = print_drive($parsed);
+    }
+
     my $defaults = load_defaults();
 
     # set environment variable useful inside network script
-- 
2.30.2





  reply	other threads:[~2021-11-22 10:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-22 10:30 [pve-devel] [PATCH qemu-server 1/2] migrate: improve start STDIN-parameter parsing Fabian Grünbichler
2021-11-22 10:30 ` Fabian Grünbichler [this message]
2021-11-22 16:09 ` [pve-devel] applied-series: " 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=20211122103011.2374843-2-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@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